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 669ced43a6..d979a8fe40 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 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition $this->children = array(); $this->addDefaults = false; + $this->addDefaultChildren = false; $this->allowNewKeys = true; $this->atLeastOne = false; $this->allowEmptyValue = true; @@ -98,6 +100,22 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition 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 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition $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 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition ); } + 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 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition $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 e49d0d49f0..e8a52376ea 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 @@ class PrototypedArrayNode extends ArrayNode 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 71ae7c865f..9639b94a2f 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 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase $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 042e6671ac..4045b63b78 100644 --- a/tests/Symfony/Tests/Component/Config/Definition/PrototypedArrayNodeTest.php +++ b/tests/Symfony/Tests/Component/Config/Definition/PrototypedArrayNodeTest.php @@ -43,7 +43,7 @@ class PrototypedArrayNodeTest extends \PHPUnit_Framework_TestCase $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 @@ class PrototypedArrayNodeTest extends \PHPUnit_Framework_TestCase $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 @@ class PrototypedArrayNodeTest extends \PHPUnit_Framework_TestCase $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 @@ class PrototypedArrayNodeTest extends \PHPUnit_Framework_TestCase $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 | 88 ++++++++++++------- 1 file changed, 58 insertions(+), 30 deletions(-) diff --git a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php index d979a8fe40..4403384909 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -278,25 +278,10 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition 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__) - ); - } - - if ($this->default) { - throw new InvalidDefinitionException( - sprintf('%s::defaultValue() is not applicable to concrete nodes.', __CLASS__) - ); - } - $node = new ArrayNode($this->name, $this->parent); + + $this->validateConcreteNode($node); + $node->setAddIfNotSet($this->addDefaults); foreach ($this->children as $child) { @@ -304,19 +289,10 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition $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 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition $this->prototype->parent = $node; $node->setPrototype($this->prototype->getNode()); - } $node->setAllowNewKeys($this->allowNewKeys); @@ -366,4 +341,57 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition 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 d96705d1ff..855bf6f81f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -217,8 +217,8 @@ class Configuration implements ConfigurationInterface ->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 4e5fe7ce6b..9829fad629 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php @@ -54,8 +54,8 @@ class Configuration implements ConfigurationInterface ->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 4403384909..4ac4b18895 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -307,7 +307,7 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition 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 9639b94a2f..35d62a8b3e 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 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase $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 855bf6f81f..38eac5879a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -217,7 +217,7 @@ class Configuration implements ConfigurationInterface ->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 9829fad629..9802094b70 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/Configuration.php @@ -54,7 +54,7 @@ class Configuration implements ConfigurationInterface ->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 4ac4b18895..4717e5e2dd 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -109,7 +109,7 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition * * @return ArrayNodeDefinition */ - public function addDefaultChildrenWhenNoneSet($children = null) + public function addDefaultChildrenIfNoneSet($children = null) { $this->addDefaultChildren = null === $children ? 'defaults' : $children; @@ -369,6 +369,12 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition 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 35d62a8b3e..93903c6302 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 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase { return array( array('defaultValue', array(array())), + array('addDefaultChildrenIfNoneSet', array()), array('requiresAtLeastOneElement', array()), array('useAttributeAsKey', array('foo')) ); @@ -72,7 +73,7 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase $node = new ArrayNodeDefinition('root'); $node ->defaultValue(array()) - ->addDefaultChildrenWhenNoneSet('foo') + ->addDefaultChildrenIfNoneSet('foo') ->prototype('array') ; $node->getNode(); @@ -82,7 +83,7 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase { $node = new ArrayNodeDefinition('root'); $node - ->addDefaultChildrenWhenNoneSet() + ->addDefaultChildrenIfNoneSet() ->prototype('array') ; $tree = $node->getNode(); @@ -93,7 +94,7 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase { $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 4717e5e2dd..28c93d49ce 100644 --- a/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php +++ b/src/Symfony/Component/Config/Definition/Builder/ArrayNodeDefinition.php @@ -103,7 +103,7 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition /** * 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 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition */ public function addDefaultChildrenIfNoneSet($children = null) { - $this->addDefaultChildren = null === $children ? 'defaults' : $children; + $this->addDefaultChildren = $children; return $this; } @@ -394,10 +394,24 @@ class ArrayNodeDefinition extends NodeDefinition implements ParentNodeDefinition ); } - 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 e8a52376ea..f800a3f8f5 100644 --- a/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php +++ b/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php @@ -124,11 +124,15 @@ class PrototypedArrayNode extends ArrayNode /** * 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 93903c6302..b3495d4e44 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 @@ namespace Symfony\Tests\Component\Config\Definition\Builder; 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 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase $parent = new ArrayNodeDefinition('root'); $child = new ScalarNodeDefinition('child'); - $node = $parent + $parent ->children() ->scalarNode('foo')->end() ->scalarNode('bar')->end() @@ -79,7 +80,7 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase $node->getNode(); } - public function testArrayNodeDefaultWhenUsingDefaultChildren() + public function testPrototypedArrayNodeDefaultWhenUsingDefaultChildren() { $node = new ArrayNodeDefinition('root'); $node @@ -90,6 +91,53 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase $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 @@ class ArrayNodeDefinitionTest extends \PHPUnit_Framework_TestCase ->prototype('array') ->prototype('array') ; - $tree = $node->getNode(); + $node->getNode(); } protected function getField($object, $field)