Uploaded image for project: 'Fluid Infusion'
  1. Fluid Infusion
  2. FLUID-5668

Corruption when material written for "members" requires merging

    Details

    • Type: Bug
    • Status: Reopened
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 3.0
    • Component/s: Framework
    • Labels:
      None

      Description

      This was discovered when updating the prefs framework test cases written for FLUID-5599 to function with the modern FLUID-5249 framework branch. These had originally been written in a "demands block" style - but the test definition had been updated to use a distributeOptions directive:

          fluid.defaults("fluid.tests.prefs.diffInit", {
              distributeOptions: {
                  record: {
                      theme: "wb",
                      textFont: "times"
                  },
                  target: "{fluid.prefs.prefsEditor}.options.members.initialModel"
              }
          });
      

      and the prefs editorLoader definition uses the following subcomponent definition to inject the initialModel down:

              components: {
                  prefsEditor: {
                      type: "fluid.prefs.prefsEditor",
                      options: {
                          members: {
                              initialModel: "{prefsEditorLoader}.initialModel"
                          },
      

      The test cases adds a mixin grade to the loader to adjust its initial model:

          fluid.defaults("fluid.prefs.initialModel.localeStarter", {
              members: {
                  initialModel: {
                      locale: "fr"
                  }
              }
          });
      

      These two definitions targetted at the "initialModel" member end up creating "mouse droppings" in the final member value, where the reference

      "{prefsEditorLoader}.initialModel"
      

      rather than being resolved, is "exploded" into an array of characters before being merged in.

      Interestingly, the test cases for this issue were too weak to detect the problem, and it was only noticed while stepping through the implementation in the debugger. Given that the algorithm for evaluating "members" is the same as in the old framework, it is very likely that this bug has always been present.

      The evaluation of "members" is unfortunately awkwardly linked with many sensitive parts of the framework's workflow, given that all three of the crucial definitions for a "modelComponent" (model, applier, and modelRelay) are cast as "members". The aim was to enable all of the implementation for this grade to be done using standard facilities of core IoC and not to pollute it with special cases. The current implementation of "members" indeed does not supply the standard framework workflow for the underlying component options - instead, they are marked as "noexpand" along with all of the other framework builtins. The resulting record is then slung directly into fluid.expandOptions via the "fluid.memberFromRecord" strategy point:

              var value = fluid.expandOptions(memberrec, that, null, null, {freeRoot: true});
      

      This touches on several other sensitive framework issues. For example, there is the relation to FLUID-5208, "Root reference to events and listeners blocks should be specifiable using IoC reference" - this is a directly analogous issue, since the main loss through the simpleminded approach of performing manual merging and expansion is the loss of the ability to customise the mergePolicy for nested material. However, the current framework implementation has never clearly supported the ability to specify nested mergePolicies in any case (see FLUID-5381, FLUID-4932) so this doesn't represent a current loss.

      Secondly there is the relation to FLUID-5226 and our ability to detect "material exotic by virtue of a custom constructor". As part of this work, we've succeeded in understanding the strategy of the standard jQuery code and can fold this into the framework - this is related to the strange "special branch" of fluid.expandOptions used by fluid.memberFromRecord using the field "freeRoot. The first effect seen once we started to move the options backing "members" to a standard workflow was the corruption of the "thisist thing" defined in an IoC test case:

              members: {
                  "thisistThing": {
                      expander: { funcName: "fluid.tests.makeThisistThing" }
                  }
              },
      

      Finally, it appears that the relations of those three core members of "modelComponent",

              members: {
                  model: "@expand:fluid.initRelayModel({that}, {that}.modelRelay)",
                  applier: "@expand:fluid.makeHolderChangeApplier({that}, {that}.options.changeApplierOptions)",
                  modelRelay: "@expand:fluid.establishModelRelay({that}, {that}.options.model, {that}.options.modelListeners, {that}.options.modelRelay, {that}.applier)"
              },
      

      can no longer be disentangled if "members" was moved to the standard workflow. The result is that the attempted evaluation of "members.modelRelay" then attempts to trigger the valuation of "applier" which then triggers evaluation of "members.applier" - which then corrupts the "in creation marker" (!!) assigned to top-level "members" by the merging workflow. This relates to FLUID-4930 and implies that this issue is alive and well despite not being observed in practice for over 2 years. The fact that we can re-trigger this issue by change in status of "members" implies that it is still only a matter of time before some suitably complex series of user definitions ends up triggering it too. There is no way to resolve this set of linked issues without the wholesale reform of the framework planned in FLUID-4925 and FLUID-4982.

      In the meantime, this has shed some clearer light on the way in which the current order of evaluation of options in the framework is problematic. We will write up observations on the wiki at http://wiki.fluidproject.org/display/fluid/Framework+Evaluation+Order%2C+old+and+new - in particular, it seems that the current overall workflow of evaluating all overall options FIRST (and also triggering an eager, aggressive expansion of all options VERY FIRST), and then only operating mounted material (via "records) NEXT is entirely wrong - we need a data-driven approach whereby we first trigger a SHALLOW evaluation of options to the top-level required in order to instantiate, say, "members and invokers", and THEN trigger a data-driven evaluation and expansion of their underlying options, and only THEN FINALLY evaluate all currently unevaluated options. This implies moving the framework over to an explicit "worklist discovery model" whereby the "fringe" of unexplored options has an explicit, flat representation. We should combine this at the same time with the "monomorphising" scheme that was found to significantly improve performance in the FLUID-5249 work on expanders.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                antranig Antranig Basman
                Reporter:
                antranig Antranig Basman
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: