FLUID-6191: Improved diagnostic for failed indirect reference from im…

…plicit model relay
fluid-project · Sep 19, 2017 · 32f7d55 · 32f7d55
1 parent 89c0a0b
commit 32f7d55
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 14 deletions.
diff --git a/src/framework/core/js/DataBinding.js b/src/framework/core/js/DataBinding.js
@@ -413,8 +413,12 @@ var fluid_3_0_0 = fluid_3_0_0 || {};
      *    segs {Array of String} Holds the full array of path segments found by parsing the original reference - only useful in <code>nonModel</code> case
      */
     fluid.parseValidModelReference = function (that, name, ref, implicitRelay) {
-        var reject = function (message) {
-            fluid.fail("Error in " + name + ": ", ref, message);
+        var reject = function () {
+            var failArgs = ["Error in " + name + ": ", ref].concat(fluid.makeArray(arguments));
+            fluid.fail.apply(null, failArgs);
+        };
+        var rejectNonModel = function (value) {
+            reject(" must be a reference to a component with a ChangeApplier (descended from fluid.modelComponent), instead got ", value);
         };
         var parsed; // resolve ref into context and modelSegs
         if (typeof(ref) === "string") {
@@ -432,7 +436,6 @@ var fluid_3_0_0 = fluid_3_0_0 || {};
                     parsed.contextSegs = parsed.segs.slice(0, modelPoint);
                     delete parsed.path;
                 }
-
             } else {
                 parsed = {
                     path: ref,
@@ -448,30 +451,31 @@ var fluid_3_0_0 = fluid_3_0_0 || {};
                 modelSegs: fluid.expandOptions(ref.segs, that)
             };
         }
-        var target; // resolve target component, which defaults to "that"
+        var contextTarget, target; // resolve target component, which defaults to "that"
         if (parsed.context) {
-            target = fluid.resolveContext(parsed.context, that);
-            if (!target) {
-                reject(" must be a reference to an existing component");
-            }
-            if (parsed.contextSegs) {
-                target = fluid.getForComponent(target, parsed.contextSegs);
+            contextTarget = fluid.resolveContext(parsed.context, that);
+            if (!contextTarget) {
+                reject(" context must be a reference to an existing component");
             }
+            target = parsed.contextSegs ? fluid.getForComponent(contextTarget, parsed.contextSegs) : contextTarget;
         } else {
             target = that;
         }
         if (!parsed.nonModel) {
+            if (!fluid.isComponent(target)) {
+                rejectNonModel(target);
+            }
             if (!target.applier) {
                 fluid.getForComponent(target, ["applier"]);
             }
             if (!target.applier) {
-                reject(" must be a reference to a component with a ChangeApplier (descended from fluid.modelComponent)");
+                rejectNonModel(target);
             }
         }
         parsed.that = target;
-        parsed.applier = target.applier;
+        parsed.applier = target && target.applier;
         if (!parsed.path) { // ChangeToApplicable amongst others rely on this
-            parsed.path = target.applier.composeSegments.apply(null, parsed.modelSegs);
+            parsed.path = target && target.applier.composeSegments.apply(null, parsed.modelSegs);
         }
         return parsed;
     };

diff --git a/src/framework/core/js/Fluid.js b/src/framework/core/js/Fluid.js
@@ -189,7 +189,7 @@ var fluid = fluid || fluid_3_0_0;
     };
 
     fluid.renderLoggingArg = function (arg) {
-        return fluid.isPrimitive(arg) || !fluid.isPlainObject(arg) ? arg : JSON.stringify(arg);
+        return arg === undefined ? "undefined" : fluid.isPrimitive(arg) || !fluid.isPlainObject(arg) ? arg : JSON.stringify(arg);
     };
 
     // The framework's built-in "fail" failure handler - this throws an exception of type <code>fluid.FluidError</code>

diff --git a/tests/framework-tests/core/js/DataBindingTests.js b/tests/framework-tests/core/js/DataBindingTests.js
@@ -1273,6 +1273,26 @@ https://github.com/fluid-project/infusion/raw/master/Infusion-LICENSE.txt
         }, "source");
     });
 
+    /** FLUID-6191: Proper diagnostic on indirect model reference which fails to resolve **/
+
+    fluid.defaults("fluid.tests.fluid6191root", {
+        gradeNames: "fluid.modelComponent",
+        components: {
+            badRefHolder: {
+                type: "fluid.modelComponent",
+                options: {
+                    model: "{fluid6191root}.nonexistent.model.path"
+                }
+            }
+        }
+    });
+
+    jqUnit.test("FLUID-6191: Framework diagnostic on bad indirect implicit model relay reference", function () {
+        jqUnit.expectFrameworkDiagnostic("Framework diagnostic on bad indirect implicit model relay reference", function () {
+            fluid.tests.fluid6191root();
+        }, ["reference", "nonexistent", "implicit", "undefined"]);
+    });
+
     /** Demonstrate resolving a set of model references which is cyclic in components (although not in values), as well as
      * double relay and longer "transform" form of relay specification */