FLUID-5517: Error in batching composite changes in model relay system

Metadata

Source
FLUID-5517
Type
Bug
Priority
Major
Status
Open
Resolution
N/A
Assignee
Antranig Basman
Reporter
Antranig Basman
Created
2014-09-25T10:43:24.212-0400
Updated
2017-10-04T18:36:31.272-0400
Versions
N/A
Fixed Versions
N/A
Component
  1. Data Binder
  2. Model Transformation System

Description

The model relay system should be batching up composite changes which arise from a model transformation, to ensure that it doesn't try to trigger a fresh relay which might read an inconsistent model state partway through an update. This system appears to be failing in some cases -

This can be demonstrated by checking out the following branch of the feedback component - and applying the following steps:
https://github.com/jobara/metadata/tree/FLUID-5517

i) Load up the demo held in demos/feedback/index.html
ii) Click the "mismatch" button to bring up the mismatch dialog
iii) Whilst it is open, then check the "match" button

The expected result is that {match: true, mismatch: false} is relayed out of the arrayToSetMembership transform. Unfortunately the system appears to try to react to the relay output before it is complete, and after a complex relay sequence involving 4 steps, eventually stabilises at {match: false, mismatch: false}.

Comments

  • Antranig Basman commented 2015-01-28T14:55:48.551-0500

    Initial investigation is starting to suggest that the set of relay rules is at fault, rather than necessarily the ModelRelay implementation (which does indeed have known faults, but they may not be serious enough to require the rewrite straight away). Here is the minimal configuration which demonstrates the same behaviour as in the FLOE-230 fault report:

    fluid.defaults("fluid.tests.fluid5517root", {
            gradeNames: ["fluid.standardRelayComponent", "autoInit"],
            model: {
                userData: {
                    opinion: {
                        mismatch: true
                    }
                },
                inTransit: {
                    opinion: ["dislike"]   // Possible values: like, dislike
                }
            },
            modelRelay: [{
                source: "{that}.model.inTransit.opinion",
                target: "{that}.model.userData.opinion",
                forward: "liveOnly",
                singleTransform: {
                    type: "fluid.transforms.arrayToSetMembership",
                    options: {
                        "like": "match",
                        "dislike": "mismatch"
                    }
                }
            }],
            events: {
                onFeedbackMarkupReady: null
            },
            components: {
                bindMatchConfirmation: {
                    type: "fluid.standardRelayComponent",
                    createOnEvent: "onFeedbackMarkupReady",
                    options: {
                        modelRelay: {
                            source: "{fluid5517root}.model.inTransit",
                            target: "{that}.model.isActive",
                            backward: "liveOnly",
                            transform: {
                                transform: {
                                    type: "fluid.transforms.arrayToSetMembership",
                                    inputPath: "opinion",
                                    options: {
                                        "like": ""
                                    }
                                }
                            }
                        }
                    }
                },
                bindMismatchDetails: {
                    type: "fluid.standardRelayComponent",
                    createOnEvent: "onFeedbackMarkupReady",
                    options: {
                        modelRelay: {
                            source: "{fluid5517root}.model.inTransit",
                            target: "{that}.model.isActive",
                            backward: "liveOnly",
                            transform: {
                                transform: {
                                    type: "fluid.transforms.arrayToSetMembership",
                                    inputPath: "opinion",
                                    options: {
                                        "dislike": ""
                                    }
                                }
                            }
                        }
                    }
                }
            }
        });
        
        fluid.tests.dumpModel = function (that) {
            console.log("Root model: ", JSON.stringify(that.model, null, 2));
            if (that.bindMatchConfirmation) {
                console.log("Match model: ", JSON.stringify(that.bindMatchConfirmation.model, null, 2));
                console.log("Mismatch model: ", JSON.stringify(that.bindMismatchDetails.model, null, 2));
            }
        };
        jqUnit.test("FLUID-5517: Batching failure in model relay", function () {
            var that = fluid.tests.fluid5517root();
            jqUnit.assertDeepEq("Initial model synchronised", ["dislike"], that.model.inTransit.opinion);
            fluid.tests.dumpModel(that);
            console.log("=============================== FIRING ATTACHMENT EVENT ================");
            that.events.onFeedbackMarkupReady.fire();
            fluid.tests.dumpModel(that);
     
            that.applier.change("userData.opinion.match", true);
            jqUnit.assertDeepEq("Stabilised model synchronised", ["like"], that.model.inTransit.opinion);
            
            fluid.tests.dumpModel(that);
        });
    

    This results in the following (partial) output:

    OUTPUT AFTER firing change from line 89:

    Root model:

    {
      "userData": {
        "opinion": {
          "mismatch": true,
          "match": false
        }
      },
      "inTransit": {
        "opinion": [
          "dislike"
        ]
      }
    }
    Match model:
    {
      "isActive": false
    }
    Mismatch model:
    {
      "isActive": true
    }
     
    setMembershipToArray making output ["like","dislike"] given options {"match":"like","mismatch":"dislike"}
    arrayToSetMembership with options {"like":"match","dislike":"mismatch"} and input value ["like","dislike"]
    arrayToSetMembership setting path match to true
    arrayToSetMembership setting path mismatch to true
    arrayToSetMembership with options {"like":""} and input value ["like","dislike"]
    arrayToSetMembership setting path  to true
    setMembershipToArray making output ["like"] given options {"":"like"}  <------ PROBLEM HERE! target array has been overwritten with this incomplete relay result
    

    The relay rule with arrayToSetMembership configuration "like": "" has ended up corrupting the source array held in "inTransit.opinion" - shortly afterwards, it is destroyed completely by the other rule. This may be more readable in the following gist: https://gist.github.com/amb26/38cf42a2c57224c01a1c

    The remainder of the relay cycle appears here:

    arrayToSetMembership with options {"like":"match","dislike":"mismatch"} and input value ["like"]
    arrayToSetMembership setting path match to true
    arrayToSetMembership setting path mismatch to false
    setMembershipToArray making output ["like"] given options {"match":"like","mismatch":"dislike"}
    arrayToSetMembership with options {"like":""} and input value ["like"]
    arrayToSetMembership setting path  to true
    arrayToSetMembership with options {"dislike":""} and input value ["like"]
    arrayToSetMembership setting path  to false
    setMembershipToArray making output [] given options {"":"dislike"}
    arrayToSetMembership with options {"like":"match","dislike":"mismatch"} and input value []
    arrayToSetMembership setting path match to false
    arrayToSetMembership setting path mismatch to false
    setMembershipToArray making output [] given options {"match":"like","mismatch":"dislike"}
    arrayToSetMembership with options {"like":""} and input value []
    arrayToSetMembership setting path  to false
    setMembershipToArray making output [] given options {"":"like"}
    arrayToSetMembership with options {"dislike":""} and input value []
    arrayToSetMembership setting path  to false
    arrayToSetMembership with options {"dislike":""} and input value []
    arrayToSetMembership setting path  to false
    FIRING PRECOMMIT from transaction undefined: original model {"userData":{"opinion":{"mismatch":true,"match":false}},"inTransit":{"opinion":["dislike"]}} current model {"userData":{"opinion":{"mismatch":false,"match":false}},"inTransit":{"opinion":[]}}
    

    The intention here is clear - we required to operate mutual exclusion as was the original requirement prompting the FLUID-5498 report. We can't do it like this because the inverse path of both of the arrayToSetMembership relays to the subcomponents are destructive and will always eventually result in a transition to the empty array as the only fixed point of the mapping.

    We need much better diagnostics (hopefully automated) of these kinds of relay rules, but as ever these will not arise for years. It's worth noticing that relay rules of these kinds can be treated as an "FSM" (Finite State Machine) which is what gives rise to the correspondence between what we call "model-based development" and what goes by that name in the rest of the industry. In the meantime we should fix up these rules and be vigilant in future to avoid writing rules which are self-destabilising in this way.

  • Antranig Basman commented 2015-01-28T15:24:00.412-0500

    Note - the "proper" solution to this problem (or rather, the only one we can implement without stateful relay rules) is simple-seeming but rather obscure - it was explained in IRC at https://botbot.me/freenode/fluid-work/2014-08-25/?msg=20363141&page=2

    The idea is to tackle the issue at the point of initial data binding - the UI action of selecting, say, the "match" button is not bound to the boolean end of the relay, but the array and - and the binding has the effect, say, of writing the entire array ["match"] to the so-called "inTransit.opinion" (this needs to be renamed) - rather than writing to the boolean end of the relay.