Skip to content

Commit edb5b35

Browse files
zerustechfabpot
zerustech
authored and
fabpot
committed
[config] Fix issue when key removed and left value only
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' ) ) ``` | 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
1 parent 613ea3b commit edb5b35

File tree

2 files changed

+227
-8
lines changed

2 files changed

+227
-8
lines changed

‎Definition/PrototypedArrayNode.php‎

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ class PrototypedArrayNode extends ArrayNode
2929
protected $minNumberOfElements = 0;
3030
protected $defaultValue = array();
3131
protected $defaultChildren;
32+
/**
33+
* @var NodeInterface[] An array of the prototypes of the simplified value children
34+
*/
35+
private $valuePrototypes = array();
3236

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

196200
foreach ($value as $k => $v) {
197-
$this->prototype->setName($k);
201+
$prototype = $this->getPrototypeForChild($k);
198202
try {
199-
$value[$k] = $this->prototype->finalize($v);
203+
$value[$k] = $prototype->finalize($v);
200204
} catch (UnsetKeyException $e) {
201205
unset($value[$k]);
202206
}
@@ -250,8 +254,18 @@ protected function normalizeValue($value)
250254
}
251255

252256
// if only "value" is left
253-
if (1 == count($v) && isset($v['value'])) {
257+
if (array_keys($v) === array('value')) {
254258
$v = $v['value'];
259+
if ($this->prototype instanceof ArrayNode && ($children = $this->prototype->getChildren()) && array_key_exists('value', $children)) {
260+
$valuePrototype = current($this->valuePrototypes) ?: clone($children['value']);
261+
$valuePrototype->parent = $this;
262+
$originalClosures = $this->prototype->normalizationClosures;
263+
if (is_array($originalClosures)) {
264+
$valuePrototypeClosures = $valuePrototype->normalizationClosures;
265+
$valuePrototype->normalizationClosures = is_array($valuePrototypeClosures) ? array_merge($originalClosures, $valuePrototypeClosures) : $originalClosures;
266+
}
267+
$this->valuePrototypes[$k] = $valuePrototype;
268+
}
255269
}
256270
}
257271

@@ -264,11 +278,11 @@ protected function normalizeValue($value)
264278
}
265279
}
266280

267-
$this->prototype->setName($k);
281+
$prototype = $this->getPrototypeForChild($k);
268282
if (null !== $this->keyAttribute || $isAssoc) {
269-
$normalized[$k] = $this->prototype->normalize($v);
283+
$normalized[$k] = $prototype->normalize($v);
270284
} else {
271-
$normalized[] = $this->prototype->normalize($v);
285+
$normalized[] = $prototype->normalize($v);
272286
}
273287
}
274288

@@ -322,10 +336,54 @@ protected function mergeValues($leftSide, $rightSide)
322336
continue;
323337
}
324338

325-
$this->prototype->setName($k);
326-
$leftSide[$k] = $this->prototype->merge($leftSide[$k], $v);
339+
$prototype = $this->getPrototypeForChild($k);
340+
$leftSide[$k] = $prototype->merge($leftSide[$k], $v);
327341
}
328342

329343
return $leftSide;
330344
}
345+
346+
/**
347+
* Returns a prototype for the child node that is associated to $key in the value array.
348+
* For general child nodes, this will be $this->prototype.
349+
* But if $this->removeKeyAttribute is true and there are only two keys in the child node:
350+
* one is same as this->keyAttribute and the other is 'value', then the prototype will be different.
351+
*
352+
* For example, assume $this->keyAttribute is 'name' and the value array is as follows:
353+
* array(
354+
* array(
355+
* 'name' => 'name001',
356+
* 'value' => 'value001'
357+
* )
358+
* )
359+
*
360+
* Now, the key is 0 and the child node is:
361+
* array(
362+
* 'name' => 'name001',
363+
* 'value' => 'value001'
364+
* )
365+
*
366+
* When normalizing the value array, the 'name' element will removed from the child node
367+
* and its value becomes the new key of the child node:
368+
* array(
369+
* 'name001' => array('value' => 'value001')
370+
* )
371+
*
372+
* Now only 'value' element is left in the child node which can be further simplified into a string:
373+
* array('name001' => 'value001')
374+
*
375+
* Now, the key becomes 'name001' and the child node becomes 'value001' and
376+
* the prototype of child node 'name001' should be a ScalarNode instead of an ArrayNode instance.
377+
*
378+
* @param string $key The key of the child node
379+
*
380+
* @return mixed The prototype instance
381+
*/
382+
private function getPrototypeForChild($key)
383+
{
384+
$prototype = isset($this->valuePrototypes[$key]) ? $this->valuePrototypes[$key] : $this->prototype;
385+
$prototype->setName($key);
386+
387+
return $prototype;
388+
}
331389
}

‎Tests/Definition/PrototypedArrayNodeTest.php‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\Config\Definition\PrototypedArrayNode;
1515
use Symfony\Component\Config\Definition\ArrayNode;
1616
use Symfony\Component\Config\Definition\ScalarNode;
17+
use Symfony\Component\Config\Definition\VariableNode;
1718

1819
class PrototypedArrayNodeTest extends \PHPUnit_Framework_TestCase
1920
{
@@ -177,4 +178,164 @@ protected function getPrototypeNodeWithDefaultChildren()
177178

178179
return $node;
179180
}
181+
182+
/**
183+
* Tests that when a key attribute is mapped, that key is removed from the array.
184+
* And if only 'value' element is left in the array, it will replace its wrapper array.
185+
*
186+
* <things>
187+
* <option id="option1" value="value1">
188+
* </things>
189+
*
190+
* The above should finally be mapped to an array that looks like this
191+
* (because "id" is the key attribute).
192+
*
193+
* array(
194+
* 'things' => array(
195+
* 'option1' => 'value1'
196+
* )
197+
* )
198+
*
199+
* It's also possible to mix 'value-only' and 'non-value-only' elements in the array.
200+
*
201+
* <things>
202+
* <option id="option1" value="value1">
203+
* <option id="option2" value="value2" foo="foo2">
204+
* </things>
205+
*
206+
* The above should finally be mapped to an array as follows
207+
*
208+
* array(
209+
* 'things' => array(
210+
* 'option1' => 'value1',
211+
* 'option2' => array(
212+
* 'value' => 'value2',
213+
* 'foo' => 'foo2'
214+
* )
215+
* )
216+
* )
217+
*
218+
* The 'value' element can also be ArrayNode:
219+
*
220+
* <things>
221+
* <option id="option1">
222+
* <value>
223+
* <foo>foo1</foo>
224+
* <bar>bar1</bar>
225+
* </value>
226+
* </option>
227+
* </things>
228+
*
229+
* The above should be finally be mapped to an array as follows
230+
*
231+
* array(
232+
* 'things' => array(
233+
* 'option1' => array(
234+
* 'foo' => 'foo1',
235+
* 'bar' => 'bar1'
236+
* )
237+
* )
238+
* )
239+
*
240+
* If using VariableNode for value node, it's also possible to mix different types of value nodes:
241+
*
242+
* <things>
243+
* <option id="option1">
244+
* <value>
245+
* <foo>foo1</foo>
246+
* <bar>bar1</bar>
247+
* </value>
248+
* </option>
249+
* <option id="option2" value="value2">
250+
* </things>
251+
*
252+
* The above should be finally mapped to an array as follows
253+
*
254+
* array(
255+
* 'things' => array(
256+
* 'option1' => array(
257+
* 'foo' => 'foo1',
258+
* 'bar' => 'bar1'
259+
* ),
260+
* 'option2' => 'value2'
261+
* )
262+
* )
263+
*
264+
*
265+
* @dataProvider getDataForKeyRemovedLeftValueOnly
266+
*/
267+
public function testMappedAttributeKeyIsRemovedLeftValueOnly($value, $children, $expected)
268+
{
269+
$node = new PrototypedArrayNode('root');
270+
$node->setKeyAttribute('id', true);
271+
272+
// each item under the root is an array, with one scalar item
273+
$prototype = new ArrayNode(null, $node);
274+
$prototype->addChild(new ScalarNode('id'));
275+
$prototype->addChild(new ScalarNode('foo'));
276+
$prototype->addChild($value);
277+
$node->setPrototype($prototype);
278+
279+
$normalized = $node->normalize($children);
280+
$this->assertEquals($expected, $normalized);
281+
}
282+
283+
public function getDataForKeyRemovedLeftValueOnly()
284+
{
285+
$scalarValue = new ScalarNode('value');
286+
287+
$arrayValue = new ArrayNode('value');
288+
$arrayValue->addChild(new ScalarNode('foo'));
289+
$arrayValue->addChild(new ScalarNode('bar'));
290+
291+
$variableValue = new VariableNode('value');
292+
293+
return array(
294+
array(
295+
$scalarValue,
296+
array(
297+
array('id' => 'option1', 'value' => 'value1'),
298+
),
299+
array('option1' => 'value1'),
300+
),
301+
302+
array(
303+
$scalarValue,
304+
array(
305+
array('id' => 'option1', 'value' => 'value1'),
306+
array('id' => 'option2', 'value' => 'value2', 'foo' => 'foo2'),
307+
),
308+
array(
309+
'option1' => 'value1',
310+
'option2' => array('value' => 'value2', 'foo' => 'foo2'),
311+
),
312+
),
313+
314+
array(
315+
$arrayValue,
316+
array(
317+
array(
318+
'id' => 'option1',
319+
'value' => array('foo' => 'foo1', 'bar' => 'bar1'),
320+
),
321+
),
322+
array(
323+
'option1' => array('foo' => 'foo1', 'bar' => 'bar1'),
324+
),
325+
),
326+
327+
array($variableValue,
328+
array(
329+
array(
330+
'id' => 'option1', 'value' => array('foo' => 'foo1', 'bar' => 'bar1'),
331+
),
332+
array('id' => 'option2', 'value' => 'value2'),
333+
),
334+
array(
335+
'option1' => array('foo' => 'foo1', 'bar' => 'bar1'),
336+
'option2' => 'value2',
337+
),
338+
),
339+
);
340+
}
180341
}

0 commit comments

Comments
 (0)