From bca2b0edf3ddbf1f4afc6903c2682d2f0ce67b3e Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 16 Feb 2012 13:55:25 +0100 Subject: [PATCH 1/6] [Config] Improve PrototypedArrayNode default value management --- .../Builder/ArrayNodeDefinition.php | 33 ++++++++- .../Config/Definition/PrototypedArrayNode.php | 24 +++++++ .../Builder/ArrayNodeDefinitionTest.php | 25 +++++++ .../Definition/PrototypedArrayNodeTest.php | 70 ++++++++++++++++++- 4 files changed, 148 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php index 669ced43a6fd9..d979a8fe407e5 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -31,6 +31,7 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition protected $key; protected $removeKeyItem; protected $addDefaults; + protected $addDefaultChildren; protected $nodeBuilder; /** @@ -42,6 +43,7 @@ public function __construct($name, NodeParentInterface $parent = null) $this->children = array(); $this->addDefaults = false; + $this->addDefaultChildren = false; $this->allowNewKeys = true; $this->atLeastOne = false; $this->allowEmptyValue = true; @@ -98,6 +100,22 @@ public function addDefaultsIfNotSet() return $this; } + /** + * Adds children with a default value when none are defined. + * + * @param integer|string|array $children The number of children|The child name|The children names to be added + * + * This method is applicable to prototype nodes only. + * + * @return ArrayNodeDefinition + */ + public function addDefaultChildrenWhenNoneSet($children = null) + { + $this->addDefaultChildren = null === $children ? 'defaults' : $children; + + return $this; + } + /** * Requires the node to have at least one element. * @@ -280,7 +298,7 @@ protected function createNode() $node = new ArrayNode($this->name, $this->parent); $node->setAddIfNotSet($this->addDefaults); - + foreach ($this->children as $child) { $child->parent = $node; $node->addChild($child->getNode()); @@ -292,6 +310,11 @@ protected function createNode() ); } + if ($this->default && false !== $this->addDefaultChildren) { + throw new InvalidDefinitionException('A default value and default children might not be used together.'); + + } + $node = new PrototypedArrayNode($this->name, $this->parent); if (null !== $this->key) { @@ -306,8 +329,16 @@ protected function createNode() $node->setDefaultValue($this->defaultValue); } + if (false !== $this->addDefaultChildren) { + $node->setAddChildrenIfNoneSet($this->addDefaultChildren); + if ($this->prototype instanceof static) { + $this->prototype->addDefaultsIfNotSet(); + } + } + $this->prototype->parent = $node; $node->setPrototype($this->prototype->getNode()); + } $node->setAllowNewKeys($this->allowNewKeys); diff --git a/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php b/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php index e49d0d49f0ce8..e8a52376ea9ba 100644 --- a/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php +++ b/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php @@ -28,6 +28,7 @@ class PrototypedArrayNode extends ArrayNode protected $removeKeyAttribute; protected $minNumberOfElements; protected $defaultValue; + protected $defaultChildren; /** * Constructor. @@ -120,13 +121,36 @@ public function hasDefaultValue() return true; } + /** + * Adds default children when none are set. + * + * @param integer|string|array $children The number of children|The child name|The children names to be added + */ + public function setAddChildrenIfNoneSet($children = array('defaults')) + { + $this->defaultChildren = is_integer($children) && $children > 0 ? range(1, $children) : (array) $children; + } + /** * Retrieves the default value. * + * The default value could be either explicited or derived from the prototype + * default value. + * * @return array The default value */ public function getDefaultValue() { + if (null !== $this->defaultChildren) { + $default = $this->prototype->hasDefaultValue() ? $this->prototype->getDefaultValue() : array(); + $defaults = array(); + foreach (array_values($this->defaultChildren) as $i => $name) { + $defaults[null === $this->keyAttribute ? $i : $name] = $default; + } + + return $defaults; + } + return $this->defaultValue; } diff --git a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php index 71ae7c865f4ba..9639b94a2f3e4 100644 --- a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php +++ b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php @@ -64,6 +64,31 @@ public function testConcreteNodeSpecificOption() $node->getNode(); } + /** + * @expectedException Symfony\Component\Config\Definition\Exception\InvalidDefinitionException + */ + public function testPrototypeNodesCantHaveADefaultValueWhenUsingDefaulChildren() + { + $node = new ArrayNodeDefinition('root'); + $node + ->defaultValue(array()) + ->addDefaultChildrenWhenNoneSet('foo') + ->prototype('array') + ; + $node->getNode(); + } + + public function testArrayNodeDefaultWhenUsingDefaultChildren() + { + $node = new ArrayNodeDefinition('root'); + $node + ->addDefaultChildrenWhenNoneSet() + ->prototype('array') + ; + $tree = $node->getNode(); + $this->assertEquals(array(array()), $tree->getDefaultValue()); + } + protected function getField($object, $field) { $reflection = new \ReflectionProperty($object, $field); diff --git a/tests/Symfony/Tests/Component/Config/Definition/PrototypedArrayNodeTest.php b/tests/Symfony/Tests/Component/Config/Definition/PrototypedArrayNodeTest.php index 042e6671ac3f0..4045b63b7824e 100644 --- a/tests/Symfony/Tests/Component/Config/Definition/PrototypedArrayNodeTest.php +++ b/tests/Symfony/Tests/Component/Config/Definition/PrototypedArrayNodeTest.php @@ -43,7 +43,7 @@ public function testRemappedKeysAreUnset() $node->addChild($mappingsNode); // each item under mappings is just a scalar - $prototype= new ScalarNode(null, $mappingsNode); + $prototype = new ScalarNode(null, $mappingsNode); $mappingsNode->setPrototype($prototype); $remappings = array(); @@ -78,7 +78,7 @@ public function testMappedAttributeKeyIsRemoved() $node->setKeyAttribute('id', true); // each item under the root is an array, with one scalar item - $prototype= new ArrayNode(null, $node); + $prototype = new ArrayNode(null, $node); $prototype->addChild(new ScalarNode('foo')); $node->setPrototype($prototype); @@ -101,7 +101,7 @@ public function testMappedAttributeKeyNotRemoved() $node->setKeyAttribute('id', false); // each item under the root is an array, with two scalar items - $prototype= new ArrayNode(null, $node); + $prototype = new ArrayNode(null, $node); $prototype->addChild(new ScalarNode('foo')); $prototype->addChild(new ScalarNode('id')); // the key attribute will remain $node->setPrototype($prototype); @@ -114,4 +114,68 @@ public function testMappedAttributeKeyNotRemoved() $expected['item_name'] = array('id' => 'item_name', 'foo' => 'bar'); $this->assertEquals($expected, $normalized); } + + public function testAddDefaultChildren() + { + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setAddChildrenIfNoneSet(); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array(array('foo' => 'bar')), $node->getDefaultValue()); + + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setKeyAttribute('foobar'); + $node->setAddChildrenIfNoneSet(); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array('defaults' => array('foo' => 'bar')), $node->getDefaultValue()); + + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setKeyAttribute('foobar'); + $node->setAddChildrenIfNoneSet('defaultkey'); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array('defaultkey' => array('foo' => 'bar')), $node->getDefaultValue()); + + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setKeyAttribute('foobar'); + $node->setAddChildrenIfNoneSet(array('defaultkey')); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array('defaultkey' => array('foo' => 'bar')), $node->getDefaultValue()); + + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setKeyAttribute('foobar'); + $node->setAddChildrenIfNoneSet(array('dk1', 'dk2')); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array('dk1' => array('foo' => 'bar'), 'dk2' => array('foo' => 'bar')), $node->getDefaultValue()); + + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setAddChildrenIfNoneSet(array(5, 6)); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array(0 => array('foo' => 'bar'), 1 => array('foo' => 'bar')), $node->getDefaultValue()); + + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setAddChildrenIfNoneSet(2); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array(array('foo' => 'bar'), array('foo' => 'bar')), $node->getDefaultValue()); + } + + public function testDefaultChildrenWinsOverDefaultValue() + { + $node = $this->getPrototypeNodeWithDefaultChildren(); + $node->setAddChildrenIfNoneSet(); + $node->setDefaultValue(array('bar' => 'foo')); + $this->assertTrue($node->hasDefaultValue()); + $this->assertEquals(array(array('foo' => 'bar')), $node->getDefaultValue()); + } + + protected function getPrototypeNodeWithDefaultChildren() + { + $node = new PrototypedArrayNode('root'); + $prototype = new ArrayNode(null, $node); + $child = new ScalarNode('foo'); + $child->setDefaultValue('bar'); + $prototype->addChild($child); + $prototype->setAddIfNotSet(true); + $node->setPrototype($prototype); + + return $node; + } } From cba2c332adb11f6f8e782440a7a5f84c6a10afea Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 20 Feb 2012 09:02:45 +0100 Subject: [PATCH 2/6] [Config] Improve error messages & extensibility --- .../Builder/ArrayNodeDefinition.php | 86 ++++++++++++------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php index d979a8fe407e5..44033849090c9 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -278,25 +278,10 @@ protected function getNodeBuilder() protected function createNode() { if (null === $this->prototype) { - if (null !== $this->key) { - throw new InvalidDefinitionException( - sprintf('%s::useAttributeAsKey() is not applicable to concrete nodes.', __CLASS__) - ); - } - - if (true === $this->atLeastOne) { - throw new InvalidDefinitionException( - sprintf('%s::requiresAtLeastOneElement() is not applicable to concrete nodes.', __CLASS__) - ); - } + $node = new ArrayNode($this->name, $this->parent); - if ($this->default) { - throw new InvalidDefinitionException( - sprintf('%s::defaultValue() is not applicable to concrete nodes.', __CLASS__) - ); - } + $this->validateConcreteNode($node); - $node = new ArrayNode($this->name, $this->parent); $node->setAddIfNotSet($this->addDefaults); foreach ($this->children as $child) { @@ -304,19 +289,10 @@ protected function createNode() $node->addChild($child->getNode()); } } else { - if ($this->addDefaults) { - throw new InvalidDefinitionException( - sprintf('%s::addDefaultsIfNotSet() is not applicable to prototype nodes.', __CLASS__) - ); - } - - if ($this->default && false !== $this->addDefaultChildren) { - throw new InvalidDefinitionException('A default value and default children might not be used together.'); - - } - $node = new PrototypedArrayNode($this->name, $this->parent); + $this->validatePrototypeNode($node); + if (null !== $this->key) { $node->setKeyAttribute($this->key, $this->removeKeyItem); } @@ -338,7 +314,6 @@ protected function createNode() $this->prototype->parent = $node; $node->setPrototype($this->prototype->getNode()); - } $node->setAllowNewKeys($this->allowNewKeys); @@ -366,4 +341,57 @@ protected function createNode() return $node; } + /** + * Validate the confifuration of a concrete node. + * + * @param NodeInterface $node The related node + * + * @throws InvalidDefinitionException When an error is detected in the configuration + */ + protected function validateConcreteNode(ArrayNode $node) + { + $path = $node->getPath(); + + if (null !== $this->key) { + throw new InvalidDefinitionException( + sprintf('->useAttributeAsKey() is not applicable to concrete nodes at path "%s"', $path) + ); + } + + if (true === $this->atLeastOne) { + throw new InvalidDefinitionException( + sprintf('->requiresAtLeastOneElement() is not applicable to concrete nodes at path "%s"', $path) + ); + } + + if ($this->default) { + throw new InvalidDefinitionException( + sprintf('->defaultValue() is not applicable to concrete nodes at path "%s"', $path) + ); + } + } + + /** + * Validate the configuration of a prototype node. + * + * @param NodeInterface $node The related node + * + * @throws InvalidDefinitionException When an error is detected in the configuration + */ + protected function validatePrototypeNode(PrototypedArrayNode $node) + { + $path = $node->getPath(); + + if ($this->addDefaults) { + throw new InvalidDefinitionException( + sprintf('->addDefaultsIfNotSet() is not applicable to prototype nodes at path "%s"', $path) + ); + } + + if ($this->default && false !== $this->addDefaultChildren) { + throw new InvalidDefinitionException( + sprintf('A default value and default children might not be used together at path "%s"', $path) + ); + } + } } From 675e5eb6e0a53706c3782390c22edc03d166f74c Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 20 Feb 2012 11:20:39 +0100 Subject: [PATCH 3/6] [Config] Take advantage of the new PrototypedArrayNode API in the core bundles --- .../FrameworkBundle/DependencyInjection/Configuration.php | 4 ++-- .../Bundle/TwigBundle/DependencyInjection/Configuration.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index d96705d1ff37f..855bf6f81fbaa 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -217,8 +217,8 @@ private function addTemplatingSection(ArrayNodeDefinition $rootNode) ->fixXmlConfig('resource') ->children() ->arrayNode('resources') - ->defaultValue(array('FrameworkBundle:Form')) - ->prototype('scalar')->end() + ->addDefaultChildrenWhenNoneSet() + ->prototype('scalar')->defaultValue('FrameworkBundle:Form')->end() ->validate() ->ifTrue(function($v) {return !in_array('FrameworkBundle:Form', $v); }) ->then(function($v){ diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php index 4e5fe7ce6b01c..9829fad629b65 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php @@ -54,8 +54,8 @@ private function addFormSection(ArrayNodeDefinition $rootNode) ->fixXmlConfig('resource') ->children() ->arrayNode('resources') - ->defaultValue(array('form_div_layout.html.twig')) - ->prototype('scalar')->end() + ->addDefaultChildrenWhenNoneSet() + ->prototype('scalar')->defaultValue('form_div_layout.html.twig')->end() ->setExample(array('MyBundle::form.html.twig')) ->validate() ->ifTrue(function($v) { return !in_array('form_div_layout.html.twig', $v); }) From bc122bdb2d90932ac59e1cfc4245bef2180d1c94 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 20 Feb 2012 18:01:14 +0100 Subject: [PATCH 4/6] [Config] Fix nested prototyped array nodes --- .../Config/Definition/Builder/ArrayNodeDefinition.php | 2 +- .../Definition/Builder/ArrayNodeDefinitionTest.php | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php index 44033849090c9..4ac4b188954d8 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -307,7 +307,7 @@ protected function createNode() if (false !== $this->addDefaultChildren) { $node->setAddChildrenIfNoneSet($this->addDefaultChildren); - if ($this->prototype instanceof static) { + if ($this->prototype instanceof static && null === $this->prototype->prototype) { $this->prototype->addDefaultsIfNotSet(); } } diff --git a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php index 9639b94a2f3e4..35d62a8b3e891 100644 --- a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php +++ b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php @@ -89,6 +89,17 @@ public function testArrayNodeDefaultWhenUsingDefaultChildren() $this->assertEquals(array(array()), $tree->getDefaultValue()); } + public function testNestedPrototypedArrayNodes() + { + $node = new ArrayNodeDefinition('root'); + $node + ->addDefaultChildrenWhenNoneSet() + ->prototype('array') + ->prototype('array') + ; + $tree = $node->getNode(); + } + protected function getField($object, $field) { $reflection = new \ReflectionProperty($object, $field); From 4feba09aa97ff89e74ecdabf185994abd9558c27 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 20 Feb 2012 19:15:01 +0100 Subject: [PATCH 5/6] [Config] implements feedback --- .../FrameworkBundle/DependencyInjection/Configuration.php | 2 +- .../TwigBundle/DependencyInjection/Configuration.php | 2 +- .../Config/Definition/Builder/ArrayNodeDefinition.php | 8 +++++++- .../Config/Definition/Builder/ArrayNodeDefinitionTest.php | 7 ++++--- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 855bf6f81fbaa..38eac5879a4af 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -217,7 +217,7 @@ private function addTemplatingSection(ArrayNodeDefinition $rootNode) ->fixXmlConfig('resource') ->children() ->arrayNode('resources') - ->addDefaultChildrenWhenNoneSet() + ->addDefaultChildrenIfNoneSet() ->prototype('scalar')->defaultValue('FrameworkBundle:Form')->end() ->validate() ->ifTrue(function($v) {return !in_array('FrameworkBundle:Form', $v); }) diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php index 9829fad629b65..9802094b70c0c 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php @@ -54,7 +54,7 @@ private function addFormSection(ArrayNodeDefinition $rootNode) ->fixXmlConfig('resource') ->children() ->arrayNode('resources') - ->addDefaultChildrenWhenNoneSet() + ->addDefaultChildrenIfNoneSet() ->prototype('scalar')->defaultValue('form_div_layout.html.twig')->end() ->setExample(array('MyBundle::form.html.twig')) ->validate() diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php index 4ac4b188954d8..4717e5e2dd827 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -109,7 +109,7 @@ public function addDefaultsIfNotSet() * * @return ArrayNodeDefinition */ - public function addDefaultChildrenWhenNoneSet($children = null) + public function addDefaultChildrenIfNoneSet($children = null) { $this->addDefaultChildren = null === $children ? 'defaults' : $children; @@ -369,6 +369,12 @@ protected function validateConcreteNode(ArrayNode $node) sprintf('->defaultValue() is not applicable to concrete nodes at path "%s"', $path) ); } + + if (false !== $this->addDefaultChildren) { + throw new InvalidDefinitionException( + sprintf('->addDefaultChildrenIfNoneSet() is not applicable to concrete nodes at path "%s"', $path) + ); + } } /** diff --git a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php index 35d62a8b3e891..93903c6302758 100644 --- a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php +++ b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php @@ -49,6 +49,7 @@ public function providePrototypeNodeSpecificCalls() { return array( array('defaultValue', array(array())), + array('addDefaultChildrenIfNoneSet', array()), array('requiresAtLeastOneElement', array()), array('useAttributeAsKey', array('foo')) ); @@ -72,7 +73,7 @@ public function testPrototypeNodesCantHaveADefaultValueWhenUsingDefaulChildren() $node = new ArrayNodeDefinition('root'); $node ->defaultValue(array()) - ->addDefaultChildrenWhenNoneSet('foo') + ->addDefaultChildrenIfNoneSet('foo') ->prototype('array') ; $node->getNode(); @@ -82,7 +83,7 @@ public function testArrayNodeDefaultWhenUsingDefaultChildren() { $node = new ArrayNodeDefinition('root'); $node - ->addDefaultChildrenWhenNoneSet() + ->addDefaultChildrenIfNoneSet() ->prototype('array') ; $tree = $node->getNode(); @@ -93,7 +94,7 @@ public function testNestedPrototypedArrayNodes() { $node = new ArrayNodeDefinition('root'); $node - ->addDefaultChildrenWhenNoneSet() + ->addDefaultChildrenIfNoneSet() ->prototype('array') ->prototype('array') ; From b269e271910190ee02ccb7ed12fb4b6542a1cd5a Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 20 Feb 2012 23:07:03 +0100 Subject: [PATCH 6/6] [Config] Improve handling of PrototypedArrayNode defaults --- .../Builder/ArrayNodeDefinition.php | 26 ++++++--- .../Config/Definition/PrototypedArrayNode.php | 8 ++- .../Builder/ArrayNodeDefinitionTest.php | 54 +++++++++++++++++-- 3 files changed, 77 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php index 4717e5e2dd827..28c93d49cec64 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -103,7 +103,7 @@ public function addDefaultsIfNotSet() /** * Adds children with a default value when none are defined. * - * @param integer|string|array $children The number of children|The child name|The children names to be added + * @param integer|string|array|null $children The number of children|The child name|The children names to be added * * This method is applicable to prototype nodes only. * @@ -111,7 +111,7 @@ public function addDefaultsIfNotSet() */ public function addDefaultChildrenIfNoneSet($children = null) { - $this->addDefaultChildren = null === $children ? 'defaults' : $children; + $this->addDefaultChildren = $children; return $this; } @@ -394,10 +394,24 @@ protected function validatePrototypeNode(PrototypedArrayNode $node) ); } - if ($this->default && false !== $this->addDefaultChildren) { - throw new InvalidDefinitionException( - sprintf('A default value and default children might not be used together at path "%s"', $path) - ); + if (false !== $this->addDefaultChildren) { + if ($this->default) { + throw new InvalidDefinitionException( + sprintf('A default value and default children might not be used together at path "%s"', $path) + ); + } + + if (null !== $this->key && (null === $this->addDefaultChildren || is_integer($this->addDefaultChildren) && $this->addDefaultChildren > 0)) { + throw new InvalidDefinitionException( + sprintf('->addDefaultChildrenIfNoneSet() should set default children names as ->useAttributeAsKey() is used at path "%s"', $path) + ); + } + + if (null === $this->key && (is_string($this->addDefaultChildren) || is_array($this->addDefaultChildren))) { + throw new InvalidDefinitionException( + sprintf('->addDefaultChildrenIfNoneSet() might not set default children names as ->useAttributeAsKey() is not used at path "%s"', $path) + ); + } } } } diff --git a/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php b/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php index e8a52376ea9ba..f800a3f8f5415 100644 --- a/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php +++ b/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php @@ -124,11 +124,15 @@ public function hasDefaultValue() /** * Adds default children when none are set. * - * @param integer|string|array $children The number of children|The child name|The children names to be added + * @param integer|string|array|null $children The number of children|The child name|The children names to be added */ public function setAddChildrenIfNoneSet($children = array('defaults')) { - $this->defaultChildren = is_integer($children) && $children > 0 ? range(1, $children) : (array) $children; + if (null === $children) { + $this->defaultChildren = array('defaults'); + } else { + $this->defaultChildren = is_integer($children) && $children > 0 ? range(1, $children) : (array) $children; + } } /** diff --git a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php index 93903c6302758..b3495d4e44a4a 100644 --- a/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php +++ b/tests/Symfony/Tests/Component/Config/Definition/Builder/ArrayNodeDefinitionTest.php @@ -13,6 +13,7 @@ use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\ScalarNodeDefinition; +use Symfony\Component\Config\Definition\Exception\InvalidDefinitionException; class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase { @@ -21,7 +22,7 @@ public function testAppendingSomeNode() $parent = new ArrayNodeDefinition('root'); $child = new ScalarNodeDefinition('child'); - $node = $parent + $parent ->children() ->scalarNode('foo')->end() ->scalarNode('bar')->end() @@ -79,7 +80,7 @@ public function testPrototypeNodesCantHaveADefaultValueWhenUsingDefaulChildren() $node->getNode(); } - public function testArrayNodeDefaultWhenUsingDefaultChildren() + public function testPrototypedArrayNodeDefaultWhenUsingDefaultChildren() { $node = new ArrayNodeDefinition('root'); $node @@ -90,6 +91,53 @@ public function testArrayNodeDefaultWhenUsingDefaultChildren() $this->assertEquals(array(array()), $tree->getDefaultValue()); } + /** + * @dataProvider providePrototypedArrayNodeDefaults + */ + public function testPrototypedArrayNodeDefault($args, $shouldThrowWhenUsingAttrAsKey, $shouldThrowWhenNotUsingAttrAsKey, $defaults) + { + $node = new ArrayNodeDefinition('root'); + $node + ->addDefaultChildrenIfNoneSet($args) + ->prototype('array') + ; + + try { + $tree = $node->getNode(); + $this->assertFalse($shouldThrowWhenNotUsingAttrAsKey); + $this->assertEquals($defaults, $tree->getDefaultValue()); + } catch (InvalidDefinitionException $e) { + $this->assertTrue($shouldThrowWhenNotUsingAttrAsKey); + } + + $node = new ArrayNodeDefinition('root'); + $node + ->useAttributeAsKey('attr') + ->addDefaultChildrenIfNoneSet($args) + ->prototype('array') + ; + + try { + $tree = $node->getNode(); + $this->assertFalse($shouldThrowWhenUsingAttrAsKey); + $this->assertEquals($defaults, $tree->getDefaultValue()); + } catch (InvalidDefinitionException $e) { + $this->assertTrue($shouldThrowWhenUsingAttrAsKey); + } + } + + public function providePrototypedArrayNodeDefaults() + { + return array( + array(null, true, false, array(array())), + array(2, true, false, array(array(), array())), + array('2', false, true, array('2' => array())), + array('foo', false, true, array('foo' => array())), + array(array('foo'), false, true, array('foo' => array())), + array(array('foo', 'bar'), false, true, array('foo' => array(), 'bar' => array())), + ); + } + public function testNestedPrototypedArrayNodes() { $node = new ArrayNodeDefinition('root'); @@ -98,7 +146,7 @@ public function testNestedPrototypedArrayNodes() ->prototype('array') ->prototype('array') ; - $tree = $node->getNode(); + $node->getNode(); } protected function getField($object, $field)