Skip to content

Commit

Permalink
bug #14082 [config] Fix issue when key removed and left value only (z…
Browse files Browse the repository at this point in the history 
…erustech)

This PR was submitted for the 2.3 branch but it was merged into the 2.7 branch instead (closes #14082).

Discussion
----------

[config] Fix issue when key removed and left value only

| Q | A |
| --- | --- |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #15270 |
| License | MIT |
| Doc PR | n/a |

When a key attribute is mapped and the key is removed from the value array, if
only 'value' element is left in the array, it should replace its wrapper
array.

Assume the original value array is as follows (key attribute is 'id').

``` php
array(
    'things' => array(
        array('id' => 'option1', 'value' => 'value1'),
        array('id' => 'option2', 'value' => 'value2')
    )
)
```

After normalized, the above shall be converted to the following array.

``` php
array(
    'things' => array(
        'option1' => 'value1',
        'option2' => 'value2'
    )
)
```

It's also possible to mix 'value-only' and 'none-value-only' elements in
the array:

``` php
array(
    'things' => array(
        array('id' => 'option1', 'value' => 'value1'),
        array('id' => 'option2', 'value' => 'value2', 'foo' => 'foo2')
    )
)
```

The above shall be converted to the following array.

``` php
array(
    'things' => array(
        'option1' => 'value1',
        'option2' => array('value' => 'value2','foo' => 'foo2')
    )
)
```

The 'value' element can also be array:

``` php
array(
    'things' => array(
        array(
            'id' => 'option1',
            'value' => array('foo'=>'foo1', 'bar' => 'bar1')
        )
    )
)
```

The above shall be converted to the following array.

``` php
array(
    'things' => array(
        'option1' => array('foo' => 'foo1', 'bar' => 'bar1')
    )
)
```

When using VariableNode for value element, it's also possible to mix
different types of value elements:

``` php
array(
    'things' => array(
        array('id' => 'option1', 'value' => array('foo'=>'foo1', 'bar' => 'bar1')),
        array('id' => 'option2', 'value' => 'value2')
    )
)
```

The above shall be converted to the following array.

``` php
array(
    'things' => array(
        'option1' => array('foo'=>'foo1', 'bar' => 'bar1'),
        'option2' => 'value2'
    )
)
```

Commits
-------

b587a72 [config] Fix issue when key removed and left value only
  • Loading branch information
@fabpot
fabpot committed Dec 14, 2016
2 parents 79e6896 + b587a72 commit a7fb8c4
Show file tree
Hide file tree
Showing 2 changed files with 227 additions and 8 deletions.
74 changes: 66 additions & 8 deletions src/Symfony/Component/Config/Definition/PrototypedArrayNode.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ class PrototypedArrayNode extends ArrayNode
protected $minNumberOfElements = 0;
protected $defaultValue = array();
protected $defaultChildren;
/**
* @var NodeInterface[] An array of the prototypes of the simplified value children
*/
private $valuePrototypes = array();

/**
* Sets the minimum number of elements that a prototype based node must
Expand Down Expand Up @@ -194,9 +198,9 @@ protected function finalizeValue($value)
}

foreach ($value as $k => $v) {
$this->prototype->setName($k);
$prototype = $this->getPrototypeForChild($k);
try {
$value[$k] = $this->prototype->finalize($v);
$value[$k] = $prototype->finalize($v);
} catch (UnsetKeyException $e) {
unset($value[$k]);
}
Expand Down Expand Up @@ -250,8 +254,18 @@ protected function normalizeValue($value)
}

// if only "value" is left
if (1 == count($v) && isset($v['value'])) {
if (array_keys($v) === array('value')) {
$v = $v['value'];
if ($this->prototype instanceof ArrayNode && ($children = $this->prototype->getChildren()) && array_key_exists('value', $children)) {
$valuePrototype = current($this->valuePrototypes) ?: clone($children['value']);
$valuePrototype->parent = $this;
$originalClosures = $this->prototype->normalizationClosures;
if (is_array($originalClosures)) {
$valuePrototypeClosures = $valuePrototype->normalizationClosures;
$valuePrototype->normalizationClosures = is_array($valuePrototypeClosures) ? array_merge($originalClosures, $valuePrototypeClosures) : $originalClosures;
}
$this->valuePrototypes[$k] = $valuePrototype;
}
}
}

Expand All @@ -264,11 +278,11 @@ protected function normalizeValue($value)
}
}

$this->prototype->setName($k);
$prototype = $this->getPrototypeForChild($k);
if (null !== $this->keyAttribute || $isAssoc) {
$normalized[$k] = $this->prototype->normalize($v);
$normalized[$k] = $prototype->normalize($v);
} else {
$normalized[] = $this->prototype->normalize($v);
$normalized[] = $prototype->normalize($v);
}
}

Expand Down Expand Up @@ -322,10 +336,54 @@ protected function mergeValues($leftSide, $rightSide)
continue;
}

$this->prototype->setName($k);
$leftSide[$k] = $this->prototype->merge($leftSide[$k], $v);
$prototype = $this->getPrototypeForChild($k);
$leftSide[$k] = $prototype->merge($leftSide[$k], $v);
}

return $leftSide;
}

/**
* Returns a prototype for the child node that is associated to $key in the value array.
* For general child nodes, this will be $this->prototype.
* But if $this->removeKeyAttribute is true and there are only two keys in the child node:
* one is same as this->keyAttribute and the other is 'value', then the prototype will be different.
*
* For example, assume $this->keyAttribute is 'name' and the value array is as follows:
* array(
* array(
* 'name' => 'name001',
* 'value' => 'value001'
* )
* )
*
* Now, the key is 0 and the child node is:
* array(
* 'name' => 'name001',
* 'value' => 'value001'
* )
*
* When normalizing the value array, the 'name' element will removed from the child node
* and its value becomes the new key of the child node:
* array(
* 'name001' => array('value' => 'value001')
* )
*
* Now only 'value' element is left in the child node which can be further simplified into a string:
* array('name001' => 'value001')
*
* Now, the key becomes 'name001' and the child node becomes 'value001' and
* the prototype of child node 'name001' should be a ScalarNode instead of an ArrayNode instance.
*
* @param string $key The key of the child node
*
* @return mixed The prototype instance
*/
private function getPrototypeForChild($key)
{
$prototype = isset($this->valuePrototypes[$key]) ? $this->valuePrototypes[$key] : $this->prototype;
$prototype->setName($key);

return $prototype;
}
}
161 changes: 161 additions & 0 deletions src/Symfony/Component/Config/Tests/Definition/PrototypedArrayNodeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Config\Definition\PrototypedArrayNode;
use Symfony\Component\Config\Definition\ArrayNode;
use Symfony\Component\Config\Definition\ScalarNode;
use Symfony\Component\Config\Definition\VariableNode;

class PrototypedArrayNodeTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -177,4 +178,164 @@ protected function getPrototypeNodeWithDefaultChildren()

return $node;
}

/**
* Tests that when a key attribute is mapped, that key is removed from the array.
* And if only 'value' element is left in the array, it will replace its wrapper array.
*
* <things>
* <option id="option1" value="value1">
* </things>
*
* The above should finally be mapped to an array that looks like this
* (because "id" is the key attribute).
*
* array(
* 'things' => array(
* 'option1' => 'value1'
* )
* )
*
* It's also possible to mix 'value-only' and 'non-value-only' elements in the array.
*
* <things>
* <option id="option1" value="value1">
* <option id="option2" value="value2" foo="foo2">
* </things>
*
* The above should finally be mapped to an array as follows
*
* array(
* 'things' => array(
* 'option1' => 'value1',
* 'option2' => array(
* 'value' => 'value2',
* 'foo' => 'foo2'
* )
* )
* )
*
* The 'value' element can also be ArrayNode:
*
* <things>
* <option id="option1">
* <value>
* <foo>foo1</foo>
* <bar>bar1</bar>
* </value>
* </option>
* </things>
*
* The above should be finally be mapped to an array as follows
*
* array(
* 'things' => array(
* 'option1' => array(
* 'foo' => 'foo1',
* 'bar' => 'bar1'
* )
* )
* )
*
* If using VariableNode for value node, it's also possible to mix different types of value nodes:
*
* <things>
* <option id="option1">
* <value>
* <foo>foo1</foo>
* <bar>bar1</bar>
* </value>
* </option>
* <option id="option2" value="value2">
* </things>
*
* The above should be finally mapped to an array as follows
*
* array(
* 'things' => array(
* 'option1' => array(
* 'foo' => 'foo1',
* 'bar' => 'bar1'
* ),
* 'option2' => 'value2'
* )
* )
*
*
* @dataProvider getDataForKeyRemovedLeftValueOnly
*/
public function testMappedAttributeKeyIsRemovedLeftValueOnly($value, $children, $expected)
{
$node = new PrototypedArrayNode('root');
$node->setKeyAttribute('id', true);

// each item under the root is an array, with one scalar item
$prototype = new ArrayNode(null, $node);
$prototype->addChild(new ScalarNode('id'));
$prototype->addChild(new ScalarNode('foo'));
$prototype->addChild($value);
$node->setPrototype($prototype);

$normalized = $node->normalize($children);
$this->assertEquals($expected, $normalized);
}

public function getDataForKeyRemovedLeftValueOnly()
{
$scalarValue = new ScalarNode('value');

$arrayValue = new ArrayNode('value');
$arrayValue->addChild(new ScalarNode('foo'));
$arrayValue->addChild(new ScalarNode('bar'));

$variableValue = new VariableNode('value');

return array(
array(
$scalarValue,
array(
array('id' => 'option1', 'value' => 'value1'),
),
array('option1' => 'value1'),
),

array(
$scalarValue,
array(
array('id' => 'option1', 'value' => 'value1'),
array('id' => 'option2', 'value' => 'value2', 'foo' => 'foo2'),
),
array(
'option1' => 'value1',
'option2' => array('value' => 'value2', 'foo' => 'foo2'),
),
),

array(
$arrayValue,
array(
array(
'id' => 'option1',
'value' => array('foo' => 'foo1', 'bar' => 'bar1'),
),
),
array(
'option1' => array('foo' => 'foo1', 'bar' => 'bar1'),
),
),

array($variableValue,
array(
array(
'id' => 'option1', 'value' => array('foo' => 'foo1', 'bar' => 'bar1'),
),
array('id' => 'option2', 'value' => 'value2'),
),
array(
'option1' => array('foo' => 'foo1', 'bar' => 'bar1'),
'option2' => 'value2',
),
),
);
}
}

0 comments on commit a7fb8c4

Please sign in to comment.