Skip to content
Permalink
Browse files

Do not merge the contents of merged properties.

Merging the contents/configuration in merged properties often results in
the wrong answer as non-associative values will be duplicated N times
where N is the number of ancestors with the same value. Instead, only
merge missing top level items. The config for each item will be taken
from the top most class it is defined in.

Refs #2914
  • Loading branch information...
markstory committed Mar 1, 2014
1 parent d2cde87 commit 1a9a8fb777ff924275ab661054b4bdd692776d39
Showing with 26 additions and 10 deletions.
  1. +24 −6 src/Utility/MergeVariablesTrait.php
  2. +2 −4 tests/TestCase/Utility/MergeVariablesTraitTest.php
@@ -80,19 +80,37 @@ protected function _mergeProperty($property, $parentClasses, $options) {
}
foreach ($parentClasses as $class) {
$parentProperties = get_class_vars($class);
if (!isset($parentProperties[$property])) {
if (empty($parentProperties[$property])) {
continue;
}
$parentProperty = $parentProperties[$property];
if (empty($parentProperty) || $parentProperty === true) {
if (!is_array($parentProperty)) {
continue;
}
if ($isAssoc) {
$parentProperty = Hash::normalize($parentProperty);
}
$thisValue = Hash::merge($parentProperty, $thisValue);
$thisValue = $this->_mergePropertyData($thisValue, $parentProperty, $isAssoc);
}
$this->{$property} = $thisValue;
}
/**
* Merge each of the keys in a property together.
*
* @param array $current The current merged value.
* @param array $parent The parent class' value.
* @param boolean $isAssoc Whether or not the merging should be done in associative mode.
* @return mixed The updated value.
*/
protected function _mergePropertyData($current, $parent, $isAssoc) {
if (!$isAssoc) {
return array_merge($parent, $current);
}
$parent = Hash::normalize($parent);
foreach ($parent as $key => $value) {
if (!isset($current[$key])) {
$current[$key] = $value;
}
}
return $current;
}
}
@@ -66,7 +66,6 @@ class Grandchild extends Child {
public $nestedProperty = [
'Red' => [
'apple' => 'mcintosh',
'citrus' => 'blood orange',
],
'Green' => [
@@ -105,7 +104,7 @@ public function testMergeVarsAsAssoc() {
$expected = [
'Red' => null,
'Orange' => null,
'Green' => ['lime', 'apple'],
'Green' => ['apple'],
'Yellow' => ['banana'],
];
$this->assertEquals($expected, $object->assocProperty);
@@ -123,7 +122,6 @@ public function testMergeVarsAsAssocWithKeyValues() {
$expected = [
'Red' => [
'apple' => 'mcintosh',
'citrus' => 'blood orange',
],
'Green' => [
@@ -144,7 +142,7 @@ public function testMergeVarsMixedModes() {
$expected = [
'Red' => null,
'Orange' => null,
'Green' => ['lime', 'apple'],
'Green' => ['apple'],
'Yellow' => ['banana'],
];
$this->assertEquals($expected, $object->assocProperty);

0 comments on commit 1a9a8fb

Please sign in to comment.
You can’t perform that action at this time.