As designed setParam vs. setParams behaves very differently

[TickTackk]

Affected version

2.1.7

Behind the scene, XF:FileCheck entity is extended and it's results getter as to mark certain files as "whitelisted".

Using setParams

                $fileCheck->clearCache('results');
                $results = $fileCheck->results;
                $reply->setParams([
                    'tab' => $tab,
                    'results' => $results
                ]);

and dumping $results in controller shows this:

array(5) {
  ["missing"] => array(1) {
    ["Thanos/DidNothingWrong"] => array(0) {
    }
  }
  ["inconsistent"] => array(2) {
    ["XF"] => array(4) {
      [0] => string(47) "js/vendor/codemirror/addon/edit/closetag.min.js"
      [1] => string(52) "js/vendor/codemirror/mode/htmlmixed/htmlmixed.min.js"
      [2] => string(26) "src/XF/Entity/Template.php"
      [3] => string(42) "src/XF/Repository/TemplateModification.php"
    }
    ["Thanos/DidNothingWrong"] => array(3) {
      [0] => string(63) "src/addons/Thanos/DidNothingWrong/Admin/Controller/Example.php"
      [1] => string(53) "src/addons/Thanos/DidNothingWrong/Entity/Example.php"
      [2] => string(61) "src/addons/Thanos/DidNothingWrong/Pub/Controller/Example.php"
    }
  }
  ["total_missing"] => int(0)
  ["total_inconsistent"] => int(7)
  ["total_checked"] => int(6996)
}

and dumping $results in template it shows"

array(5) {
  ["missing"] => array(1) {
    ["Thanos/DidNothingWrong"] => array(1) {
      [0] => string(64) "src/addons/Thanos/DidNothingWrong/_data/widget_positions.xml"
    }
  }
  ["inconsistent"] => array(2) {
    ["XF"] => array(4) {
      [0] => string(47) "js/vendor/codemirror/addon/edit/closetag.min.js"
      [1] => string(52) "js/vendor/codemirror/mode/htmlmixed/htmlmixed.min.js"
      [2] => string(26) "src/XF/Entity/Template.php"
      [3] => string(42) "src/XF/Repository/TemplateModification.php"
    }
    ["Thanos/DidNothingWrong"] => array(3) {
      [0] => string(63) "src/addons/Thanos/DidNothingWrong/Admin/Controller/Example.php"
      [1] => string(53) "src/addons/Thanos/DidNothingWrong/Entity/Example.php"
      [2] => string(61) "src/addons/Thanos/DidNothingWrong/Pub/Controller/Example.php"
    }
  }
  ["total_missing"] => int(0)
  ["total_inconsistent"] => int(7)
  ["total_checked"] => int(6996)
}

When using setParam

                $reply->setParam('tab', $tab);

                $fileCheck->clearCache('results');
                $results = $fileCheck->results;
                $reply->setParam('results', $results);

and dumping $results in controller shows this:

array(5) {
  ["missing"] => array(1) {
    ["Thanos/DidNothingWrong"] => array(0) {
    }
  }
  ["inconsistent"] => array(2) {
    ["XF"] => array(4) {
      [0] => string(47) "js/vendor/codemirror/addon/edit/closetag.min.js"
      [1] => string(52) "js/vendor/codemirror/mode/htmlmixed/htmlmixed.min.js"
      [2] => string(26) "src/XF/Entity/Template.php"
      [3] => string(42) "src/XF/Repository/TemplateModification.php"
    }
    ["Thanos/DidNothingWrong"] => array(3) {
      [0] => string(63) "src/addons/Thanos/DidNothingWrong/Admin/Controller/Example.php"
      [1] => string(53) "src/addons/Thanos/DidNothingWrong/Entity/Example.php"
      [2] => string(61) "src/addons/Thanos/DidNothingWrong/Pub/Controller/Example.php"
    }
  }
  ["total_missing"] => int(0)
  ["total_inconsistent"] => int(7)
  ["total_checked"] => int(6996)
}

and dumping $results in template shows this:

array(5) {
  ["missing"] => array(1) {
    ["Thanos/DidNothingWrong"] => array(0) {
    }
  }
  ["inconsistent"] => array(2) {
    ["XF"] => array(4) {
      [0] => string(47) "js/vendor/codemirror/addon/edit/closetag.min.js"
      [1] => string(52) "js/vendor/codemirror/mode/htmlmixed/htmlmixed.min.js"
      [2] => string(26) "src/XF/Entity/Template.php"
      [3] => string(42) "src/XF/Repository/TemplateModification.php"
    }
    ["Thanos/DidNothingWrong"] => array(3) {
      [0] => string(63) "src/addons/Thanos/DidNothingWrong/Admin/Controller/Example.php"
      [1] => string(53) "src/addons/Thanos/DidNothingWrong/Entity/Example.php"
      [2] => string(61) "src/addons/Thanos/DidNothingWrong/Pub/Controller/Example.php"
    }
  }
  ["total_missing"] => int(0)
  ["total_inconsistent"] => int(7)
  ["total_checked"] => int(6996)
}

The difference is big but I'm not entirely sure if this is even a bug or a feature? Shouldn't the setParams just call setParam in a loop or something similar instead of trying to merge?

 

[Chris D]

I'm going to go ahead and call this as designed.

Our current approach, while in some cases unexpected, is arguably safer as it attempts where possible to merge the existing params with the new params so it isn't possible to unintentionally overwrite the existing params.

Your own example demonstrates it - if you do setParam on the results key only, then this entry is entirely missing from the resulting array, even though it was in the params originally.

 [0] => string(64) "src/addons/Thanos/DidNothingWrong/_data/widget_positions.xml"

Depending on your exact use case, that may be desirable or undesirable.

But I feel it's not exactly an approach we can change now as existing usage of that function may require it to merge rather than overwrite.

So in this case you have to adjust your code to produce the result you require.

There are a few approaches if you don't want the merge behaviour including explicitly overwriting the params by setting the $merge param to false, but just overwriting the params you need to overwrite using setParam (if that is required) is what we'd recommend.