From 2b650bf31c08a40339a5b3c1744c91b4353d4e7d Mon Sep 17 00:00:00 2001 From: iamluc Date: Fri, 13 Dec 2013 11:52:13 +0100 Subject: [PATCH 1/4] [OptionsResolver] Implemented policy for unknown options --- .../OptionsResolver/OptionsResolver.php | 29 +++++++++-- .../OptionsResolverInterface.php | 15 +++++- .../Tests/OptionsResolverTest.php | 49 +++++++++++++++++++ 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/OptionsResolver/OptionsResolver.php b/src/Symfony/Component/OptionsResolver/OptionsResolver.php index 237ab8135f503..af4ae1cdc779e 100644 --- a/src/Symfony/Component/OptionsResolver/OptionsResolver.php +++ b/src/Symfony/Component/OptionsResolver/OptionsResolver.php @@ -214,9 +214,18 @@ public function isRequired($option) /** * {@inheritdoc} */ - public function resolve(array $options = array()) + public function resolve(array $options = array(), $flags = 0) { - $this->validateOptionsExistence($options); + if (($flags & OptionsResolverInterface::REMOVE_UNKNOWN) && ($flags & OptionsResolverInterface::IGNORE_UNKNOWN)) { + throw new \InvalidArgumentException('OptionsResolverInterface::REMOVE_UNKNOWN and OptionsResolverInterface::IGNORE_UNKNOWN are mutually exclusive'); + } + + if ($flags & OptionsResolverInterface::REMOVE_UNKNOWN) { + $options = $this->removeUnknownOptions($options); + } elseif (!($flags & OptionsResolverInterface::IGNORE_UNKNOWN)) { + $this->validateOptionsExistence($options); + } + $this->validateOptionsCompleteness($options); // Make sure this method can be called multiple times @@ -236,11 +245,23 @@ public function resolve(array $options = array()) return $resolvedOptions; } + /** + * Remove unknown options + * + * @param array $options A list of option names as keys. + * + * @return array Options with all unknown options removed + */ + private function removeUnknownOptions(array $options) + { + return array_intersect_key($options, $this->knownOptions); + } + /** * Validates that the given option names exist and throws an exception * otherwise. * - * @param array $options An list of option names as keys. + * @param array $options A list of option names as keys. * * @throws InvalidOptionsException If any of the options has not been defined. */ @@ -264,7 +285,7 @@ private function validateOptionsExistence(array $options) * Validates that all required options are given and throws an exception * otherwise. * - * @param array $options An list of option names as keys. + * @param array $options A list of option names as keys. * * @throws MissingOptionsException If a required option is missing. */ diff --git a/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php b/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php index 8474c4bcd793d..e5a1f1e591c89 100644 --- a/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php +++ b/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php @@ -16,6 +16,18 @@ */ interface OptionsResolverInterface { + /** + * Remove unknown options passed to the resolve() method + * Is mutually exclusive with IGNORE_UNKNOWN + */ + const REMOVE_UNKNOWN = 1; + + /** + * Ignore unknown options passed to the resolve() method + * Is mutually exclusive with REMOVE_UNKNOWN + */ + const IGNORE_UNKNOWN = 2; + /** * Sets default option values. * @@ -196,6 +208,7 @@ public function isRequired($option); * Returns the combination of the default and the passed options. * * @param array $options The custom option values. + * @param int $flags Can be REMOVE_UNKNOWN or IGNORE_UNKNOWN * * @return array A list of options and their values. * @@ -206,5 +219,5 @@ public function isRequired($option); * @throws Exception\OptionDefinitionException If a cyclic dependency is detected * between two lazy options. */ - public function resolve(array $options = array()); + public function resolve(array $options = array(), $flags = 0); } diff --git a/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php b/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php index fc3b3fc5d38e3..f4f7558ee815f 100644 --- a/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php +++ b/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\OptionsResolver\Tests; +use Symfony\Component\OptionsResolver\OptionsResolverInterface; use Symfony\Component\OptionsResolver\OptionsResolver; use Symfony\Component\OptionsResolver\Options; @@ -717,4 +718,52 @@ public function testClone() 'three' => '3', ), $clone->resolve()); } + + /** + * @expectedException \InvalidArgumentException + */ + public function testRemoveUnknownAndIgnoreUnknownAreMutuallyExclusive() + { + $this->resolver->resolve(array( + 'one' => 'one', + 'two' => 'two' + ), OptionsResolverInterface::REMOVE_UNKNOWN | OptionsResolverInterface::IGNORE_UNKNOWN); + } + + public function testRemoveUnknownOption() + { + $this->resolver->setDefaults(array( + 'one' => 'default', + 'two' => 'default' + )); + + $options = $this->resolver->resolve(array( + 'one' => 'keep', + 'three' => 'remove' + ), OptionsResolverInterface::REMOVE_UNKNOWN); + + $this->assertEquals($options, array( + 'one' => 'keep', + 'two' => 'default' + )); + } + + public function testIgnoreUnknownOption() + { + $this->resolver->setDefaults(array( + 'one' => 'default', + 'two' => 'default' + )); + + $options = $this->resolver->resolve(array( + 'one' => 'keep', + 'three' => 'ignored' + ), OptionsResolverInterface::IGNORE_UNKNOWN); + + $this->assertEquals($options, array( + 'one' => 'keep', + 'two' => 'default', + 'three' => 'ignored' + )); + } } From 4b440f16f5e80bd51db24da2c138834188088326 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 28 Mar 2014 18:50:36 +0100 Subject: [PATCH 2/4] [OptionsResolver] Completed PR adding a policy for unknown options --- .../Component/OptionsResolver/CHANGELOG.md | 8 +++ .../OptionsResolver/OptionsResolver.php | 34 +++++---- .../OptionsResolverInterface.php | 30 +++++--- .../Tests/OptionsResolverTest.php | 72 ++++++++++++++++++- 4 files changed, 117 insertions(+), 27 deletions(-) create mode 100644 src/Symfony/Component/OptionsResolver/CHANGELOG.md diff --git a/src/Symfony/Component/OptionsResolver/CHANGELOG.md b/src/Symfony/Component/OptionsResolver/CHANGELOG.md new file mode 100644 index 0000000000000..a3a44679289c9 --- /dev/null +++ b/src/Symfony/Component/OptionsResolver/CHANGELOG.md @@ -0,0 +1,8 @@ +CHANGELOG +========= + +2.5.0 +----- + + * added flags FORBID_UNKNOWN, REMOVE_UNKNOWN, IGNORE_UNKNOWN, FORBID_MISSING + and IGNORE_MISSING to OptionsResolverInterface diff --git a/src/Symfony/Component/OptionsResolver/OptionsResolver.php b/src/Symfony/Component/OptionsResolver/OptionsResolver.php index af4ae1cdc779e..4cbd6482dd633 100644 --- a/src/Symfony/Component/OptionsResolver/OptionsResolver.php +++ b/src/Symfony/Component/OptionsResolver/OptionsResolver.php @@ -216,17 +216,27 @@ public function isRequired($option) */ public function resolve(array $options = array(), $flags = 0) { - if (($flags & OptionsResolverInterface::REMOVE_UNKNOWN) && ($flags & OptionsResolverInterface::IGNORE_UNKNOWN)) { - throw new \InvalidArgumentException('OptionsResolverInterface::REMOVE_UNKNOWN and OptionsResolverInterface::IGNORE_UNKNOWN are mutually exclusive'); + if (!($flags & (self::FORBID_UNKNOWN | self::REMOVE_UNKNOWN | self::IGNORE_UNKNOWN))) { + $flags |= self::FORBID_UNKNOWN; } - if ($flags & OptionsResolverInterface::REMOVE_UNKNOWN) { - $options = $this->removeUnknownOptions($options); - } elseif (!($flags & OptionsResolverInterface::IGNORE_UNKNOWN)) { + if (!($flags & (self::FORBID_MISSING | self::IGNORE_MISSING))) { + $flags |= self::FORBID_MISSING; + } + + if (($flags & self::REMOVE_UNKNOWN) && ($flags & self::IGNORE_UNKNOWN)) { + throw new \InvalidArgumentException('self::REMOVE_UNKNOWN and self::IGNORE_UNKNOWN are mutually exclusive'); + } + + if ($flags & self::FORBID_UNKNOWN) { $this->validateOptionsExistence($options); + } elseif ($flags & self::REMOVE_UNKNOWN) { + $options = array_intersect_key($options, $this->knownOptions); } - $this->validateOptionsCompleteness($options); + if ($flags & self::FORBID_MISSING) { + $this->validateOptionsCompleteness($options); + } // Make sure this method can be called multiple times $combinedOptions = clone $this->defaultOptions; @@ -245,18 +255,6 @@ public function resolve(array $options = array(), $flags = 0) return $resolvedOptions; } - /** - * Remove unknown options - * - * @param array $options A list of option names as keys. - * - * @return array Options with all unknown options removed - */ - private function removeUnknownOptions(array $options) - { - return array_intersect_key($options, $this->knownOptions); - } - /** * Validates that the given option names exist and throws an exception * otherwise. diff --git a/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php b/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php index e5a1f1e591c89..29088a532eaec 100644 --- a/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php +++ b/src/Symfony/Component/OptionsResolver/OptionsResolverInterface.php @@ -17,16 +17,29 @@ interface OptionsResolverInterface { /** - * Remove unknown options passed to the resolve() method - * Is mutually exclusive with IGNORE_UNKNOWN + * Throw an exception if unknown options are passed to {@link resolve()}. */ - const REMOVE_UNKNOWN = 1; + const FORBID_UNKNOWN = 1; /** - * Ignore unknown options passed to the resolve() method - * Is mutually exclusive with REMOVE_UNKNOWN + * Remove unknown options passed to {@link resolve()}. */ - const IGNORE_UNKNOWN = 2; + const REMOVE_UNKNOWN = 2; + + /** + * Ignore unknown options passed to {@link resolve()}. + */ + const IGNORE_UNKNOWN = 4; + + /** + * Throw an exception if a required option is missing in {@link resolve()}. + */ + const FORBID_MISSING = 32; + + /** + * Ignore missing required options in {@link resolve()}. + */ + const IGNORE_MISSING = 64; /** * Sets default option values. @@ -207,8 +220,9 @@ public function isRequired($option); /** * Returns the combination of the default and the passed options. * - * @param array $options The custom option values. - * @param int $flags Can be REMOVE_UNKNOWN or IGNORE_UNKNOWN + * @param array $options The custom option values. + * @param integer $flags A combination of the flag constants in this + * interface. * * @return array A list of options and their values. * diff --git a/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php b/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php index f4f7558ee815f..7e3f439d9379c 100644 --- a/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php +++ b/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php @@ -176,7 +176,7 @@ public function testResolveLazyReplaceDefaults() /** * @expectedException \Symfony\Component\OptionsResolver\Exception\InvalidOptionsException */ - public function testResolveFailsIfNonExistingOption() + public function testResolveFailsIfUnknownOption() { $this->resolver->setDefaults(array( 'one' => '1', @@ -195,6 +195,57 @@ public function testResolveFailsIfNonExistingOption() )); } + public function testResolveRemovesUnknownOptionIfFlagIsSet() + { + $this->resolver->setDefaults(array( + 'one' => '1', + )); + + $this->resolver->setRequired(array( + 'two', + )); + + $this->resolver->setOptional(array( + 'three', + )); + + $options = $this->resolver->resolve(array( + 'foo' => 'bar', + 'two' => '20', + ), OptionsResolverInterface::REMOVE_UNKNOWN); + + $this->assertEquals(array( + 'one' => '1', + 'two' => '20' + ), $options); + } + + public function testResolveIgnoresUnknownOptionsIfFlagIsSet() + { + $this->resolver->setDefaults(array( + 'one' => '1', + )); + + $this->resolver->setRequired(array( + 'two', + )); + + $this->resolver->setOptional(array( + 'three', + )); + + $options = $this->resolver->resolve(array( + 'foo' => 'bar', + 'two' => '20', + ), OptionsResolverInterface::IGNORE_UNKNOWN); + + $this->assertEquals(array( + 'one' => '1', + 'foo' => 'bar', + 'two' => '20', + ), $options); + } + /** * @expectedException \Symfony\Component\OptionsResolver\Exception\MissingOptionsException */ @@ -213,6 +264,25 @@ public function testResolveFailsIfMissingRequiredOption() )); } + public function testResolveIgnoresMissingRequiredOptionIfFlagIsSet() + { + $this->resolver->setRequired(array( + 'one', + )); + + $this->resolver->setDefaults(array( + 'two' => '2', + )); + + $options = $this->resolver->resolve(array( + 'two' => '20', + ), OptionsResolverInterface::IGNORE_MISSING); + + $this->assertEquals(array( + 'two' => '20', + ), $options); + } + public function testResolveSucceedsIfOptionValueAllowed() { $this->resolver->setDefaults(array( From 93cc43586d0fa638311943ad9cd51e1407addbbe Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 28 Mar 2014 19:10:47 +0100 Subject: [PATCH 3/4] [OptionsResolver] Fixed excess space --- src/Symfony/Component/OptionsResolver/OptionsResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/OptionsResolver/OptionsResolver.php b/src/Symfony/Component/OptionsResolver/OptionsResolver.php index 4cbd6482dd633..0ed57970e9021 100644 --- a/src/Symfony/Component/OptionsResolver/OptionsResolver.php +++ b/src/Symfony/Component/OptionsResolver/OptionsResolver.php @@ -234,7 +234,7 @@ public function resolve(array $options = array(), $flags = 0) $options = array_intersect_key($options, $this->knownOptions); } - if ($flags & self::FORBID_MISSING) { + if ($flags & self::FORBID_MISSING) { $this->validateOptionsCompleteness($options); } From 3eb8c476db91fda0748b34026d001832fe1896d3 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Sun, 30 Mar 2014 17:37:32 +0200 Subject: [PATCH 4/4] [OptionsResolver] Removed flag check Either we need to check all flags for mutual exclusivity, or none. Since checking all flags is expensive, I'm removing this check altogether. --- .../Component/OptionsResolver/OptionsResolver.php | 4 ---- .../OptionsResolver/Tests/OptionsResolverTest.php | 11 ----------- 2 files changed, 15 deletions(-) diff --git a/src/Symfony/Component/OptionsResolver/OptionsResolver.php b/src/Symfony/Component/OptionsResolver/OptionsResolver.php index 0ed57970e9021..6e9f69cdecc4e 100644 --- a/src/Symfony/Component/OptionsResolver/OptionsResolver.php +++ b/src/Symfony/Component/OptionsResolver/OptionsResolver.php @@ -224,10 +224,6 @@ public function resolve(array $options = array(), $flags = 0) $flags |= self::FORBID_MISSING; } - if (($flags & self::REMOVE_UNKNOWN) && ($flags & self::IGNORE_UNKNOWN)) { - throw new \InvalidArgumentException('self::REMOVE_UNKNOWN and self::IGNORE_UNKNOWN are mutually exclusive'); - } - if ($flags & self::FORBID_UNKNOWN) { $this->validateOptionsExistence($options); } elseif ($flags & self::REMOVE_UNKNOWN) { diff --git a/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php b/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php index 7e3f439d9379c..3060ddc5b7f6b 100644 --- a/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php +++ b/src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php @@ -789,17 +789,6 @@ public function testClone() ), $clone->resolve()); } - /** - * @expectedException \InvalidArgumentException - */ - public function testRemoveUnknownAndIgnoreUnknownAreMutuallyExclusive() - { - $this->resolver->resolve(array( - 'one' => 'one', - 'two' => 'two' - ), OptionsResolverInterface::REMOVE_UNKNOWN | OptionsResolverInterface::IGNORE_UNKNOWN); - } - public function testRemoveUnknownOption() { $this->resolver->setDefaults(array(