Skip to content

Commit

Permalink
feature #20361 [Config] Enable cannotBeEmpty along with requiresAtLea…
Browse files Browse the repository at this point in the history
…stOneElement (ro0NL)

This PR was squashed before being merged into the 3.4 branch (closes #20361).

Discussion
----------

[Config] Enable cannotBeEmpty along with requiresAtLeastOneElement

| Q | A |
| --- | --- |
| Branch? | "master" |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes
| Fixed tickets | #20356 |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |

As @vudaltsov mentioned, we ignore any calls to `ArrayNodeDefinition::cannotBeEmpty`, which can lead to unexpected behavior. Imo. all subclasses should follow the base API.

Commits
-------

d40e7e4 [Config] Enable cannotBeEmpty along with requiresAtLeastOneElement
  • Loading branch information
fabpot committed Aug 5, 2017
2 parents 3b54ce8 + d40e7e4 commit 73d3d5a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
Expand Up @@ -428,7 +428,7 @@ protected function createNode()
$node->setKeyAttribute($this->key, $this->removeKeyItem);
}

if (true === $this->atLeastOne) {
if (true === $this->atLeastOne || false === $this->allowEmptyValue) {
$node->setMinNumberOfElements(1);
}

Expand Down Expand Up @@ -490,6 +490,12 @@ protected function validateConcreteNode(ArrayNode $node)
);
}

if (false === $this->allowEmptyValue) {
throw new InvalidDefinitionException(
sprintf('->cannotBeEmpty() 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)
Expand Down
Expand Up @@ -54,6 +54,7 @@ public function providePrototypeNodeSpecificCalls()
array('defaultValue', array(array())),
array('addDefaultChildrenIfNoneSet', array()),
array('requiresAtLeastOneElement', array()),
array('cannotBeEmpty', array()),
array('useAttributeAsKey', array('foo')),
);
}
Expand Down Expand Up @@ -285,6 +286,32 @@ public function getEnableableNodeFixtures()
);
}

public function testRequiresAtLeastOneElement()
{
$node = new ArrayNodeDefinition('root');
$node
->requiresAtLeastOneElement()
->integerPrototype();

$node->getNode()->finalize(array(1));

$this->addToAssertionCount(1);
}

/**
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
* @expectedExceptionMessage The path "root" should have at least 1 element(s) defined.
*/
public function testCannotBeEmpty()
{
$node = new ArrayNodeDefinition('root');
$node
->cannotBeEmpty()
->integerPrototype();

$node->getNode()->finalize(array());
}

protected function getField($object, $field)
{
$reflection = new \ReflectionProperty($object, $field);
Expand Down

0 comments on commit 73d3d5a

Please sign in to comment.