FLUID-5512: Allow valueMapper to handle the defaultOutputValue when the transformation is based upon compound input values

Description

When valueMapper is used to transform compound input values, "defaultOutputValue" is not properly handled.

Example:

var rules = {
    type: "fluid.transforms.valueMapper",
    inputPath: "",
    defaultOutputValue: false,
    options: [{
        inputValue: {
            "isTooltipOpen": true,
            "isDialogOpen": true
        },
        outputValue: true,
    }]
};

In cases that the input value is NOT

{
    "isTooltipOpen": true,
    "isDialogOpen": true
}

We expect the output value is false.

However, what happens is, the output value is always true regardless of value combinations of "isTooltipOpen" + "isDialogOpen".

Comments

• Cindy Li commented 2014-09-19T16:12:16.510-0400

  Issued a pull request (https://github.com/fluid-project/infusion/pull/561) with a unit test to demonstrate this issue.

• Cindy Li commented 2014-09-22T16:33:40.982-0400

  The work round for handling cases that the source model values are booleans is to use a combination of "fluid.transforms.condition" and "fluid.transforms.binaryOp":

  type: "fluid.transforms.condition",
                  condition: {
                      transform: {
                          type: "fluid.transforms.binaryOp",
                          leftPath: "inTransit.bothOpen",
                          rightPath: "isShareTarget",
                          operator: "&&"
                      }
                  },
                  "true": true,
                  "false": false
  

  The shortcoming of the work around is,

  1. Only applicable for boolean source values;
  2. "fluid.transforms.condition" is not invertible and cannot be used for model relay that requires inversion;
  3. When there are more conditions checking input value combinations, the transform options in the work around would be nested and complicated.

• Antranig Basman commented 2014-11-18T15:17:36.206-0500

  This behaviour is by the design of the valueMapper - if any branch matches even partially, this is considered a better match than "no match at all" which triggers the default case. It appears that your case could be handled by the alternative definition

  { 
      type: "fluid.transforms.valueMapper", 
      inputPath: "", 
      defaultOutputValue: false, 
      options: [{ 
          inputValue: { 
              "isTooltipOpen": true 
          }, 
          outputValue: true, 
      }, { 
          inputValue: { 
              "isDialogOpen": true 
          }, 
          outputValue: true, 
      }] 
  };
  

  Would this work for your case? If not, could you describe the wider problem that leads to it and what behaviour it requires?

  Thanks

• Cindy Li commented 2014-11-18T15:57:42.160-0500

  I understand your design goal of "better than nothing". However, this use case is on the picky side with an attempt to match on the exact compound value, otherwise the default output value is expected to be returned.

  I tried the alternative approach, the result is, in case 2 & 3 (https://github.com/cindyli/infusion/blob/FLUID-5512/tests/framework-tests/core/js/ModelTransformationTests.js#L1679-L1696) where either "isTooltipOpen" or "isDialogOpen" has a true value, the returned result is {value: true}. In other cases, an empty object is returned, even with the case 1 where both "isTooltipOpen" and "isDialogOpen" have true values.

• Antranig Basman commented 2014-11-18T16:05:36.063-0500

  Ok, in that case I suggest the following representation, and avoid the use of the "defaultValue" infrastructure entirely:

  { 
      type: "fluid.transforms.valueMapper", 
      inputPath: "", 
      options: [{ 
          inputValue: { 
              "isTooltipOpen": true, 
              "isDialogOpen": true 
          }, 
          outputValue: true, 
      }, { 
          inputValue: { 
              "isTooltipOpen": false
          }, 
          outputValue: false,
      }, { 
          inputValue: { 
              "isDialogOpen": false
          }, 
          outputValue: false, 
      }] 
  };
  

  do you think this will meet the case?

• Cindy Li commented 2014-11-18T16:23:29.740-0500

  This approach looks really promising, except we may have found a real bug in valueMapper. My test report:

  1. Using the rule in your suggestion:
  Result: Case 1, 5 pass. Case 2, 3, 4 return empty objects

  2. I altered the rule a bit to retry your previous suggestion by removing the first option block:

  {
      type: "fluid.transforms.valueMapper",
      inputPath: "",
      options: [{
          inputValue: {
              "isTooltipOpen": false
          },
          outputValue: false,
      }, {
          inputValue: {
              "isDialogOpen": false
          },
          outputValue: false,
      }]
  }
  

  Result: Case 2, 3, 5, pass. Case 1, 4 return empty objects.

  It seems having the same key checked by multiple option blocks confuses valueMapper.

• Cindy Li commented 2014-11-19T09:09:59.202-0500

  Antranig issued a pull request: https://github.com/fluid-project/infusion/pull/572

• Antranig Basman commented 2014-12-11T14:57:58.931-0500

  Recall again that the meaning of "defaultOutputValue" is not "the output in case no case matches" but "the outputValue that should be used in a case where it has not been explicitly written". It looks like this option remains confusing (see FLUID-5473 for another issue in which it wasn't helpful) and it would be nice to withdraw it in favour automatically applying the new-style "funky partial nothing block" that appears in the pull request for this issue -

  { // a "match always" rule - this has the effect which is frequently imagined for "defaultOutputValue: false"
  undefinedInputValue: true,
  partialMatches: true,
  outputValue: false
  }

  however this would presumably mean withdrawing "defaultInputValue" (although perhaps not necessarily?) which would be quite a disruptive change. Note that there are no transforms within the GPII which make use of defaultOutputValue. Remember notes on the valueMapper at http://wiki.fluidproject.org/display/fluid/Notes+on+valueMapper+Requirements

• Cindy Li commented 2015-01-19T10:01:28.980-0500

  Merged into the master branch of the project repo @ c860fe56239a70aa057532426a36457af11279e3

• Kasper Galschiot Markus commented 2016-10-26T09:23:13.513-0400

  Antranig Basman Cindy Li: I think this is covered by my valuemapper overhaul: https://github.com/fluid-project/infusion/pull/710 ... if you agree, this can be closed

• Justin Obara commented 2017-09-26T10:49:39.553-0400

  Antranig Basman and Cindy Li can this be closed?