From 2b650bf31c08a40339a5b3c1744c91b4353d4e7d Mon Sep 17 00:00:00 2001 From: iamluc Date: Fri, 13 Dec 2013 11:52:13 +0100 Subject: [PATCH 1/5] [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/5] [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/5] [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/5] [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( From bf281522ad2d6d2667ed2868670f6d9e8ddb8f4c Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Wed, 2 Apr 2014 10:38:05 +0200 Subject: [PATCH 5/5] [OptionsResolver] Documented BC break in resolve() --- UPGRADE-2.5.md | 34 ++++++++++++++++++- .../Component/OptionsResolver/CHANGELOG.md | 1 + 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/UPGRADE-2.5.md b/UPGRADE-2.5.md index e3b581b5b9dfd..111b4bddecab7 100644 --- a/UPGRADE-2.5.md +++ b/UPGRADE-2.5.md @@ -45,6 +45,38 @@ Form { ``` +OptionsResolver +--------------- + + * A new parameter `$flags` was added to `OptionsResolverInterface::resolve()`. + The parameter takes a bitwise combination of the new flag constants in that + interface. + + If you implement the interface in your code, you should add the parameter as + well. + + Before: + + ``` + public function resolve(array $options = array()) + { + // ... + } + ``` + + After: + + ``` + public function resolve(array $options = array(), $flags = 0) + { + if ($flags & self::FORBID_UNKNOWN) { + // ... + } + + // ... + } + ``` + Validator --------- @@ -56,7 +88,7 @@ Validator After: - Default email validation is now done via a simple regex which may cause invalid emails (not RFC compilant) to be + Default email validation is now done via a simple regex which may cause invalid emails (not RFC compilant) to be valid. This is the default behaviour. Strict email validation has to be explicitly activated in the configuration file by adding diff --git a/src/Symfony/Component/OptionsResolver/CHANGELOG.md b/src/Symfony/Component/OptionsResolver/CHANGELOG.md index a3a44679289c9..e7518adf7132e 100644 --- a/src/Symfony/Component/OptionsResolver/CHANGELOG.md +++ b/src/Symfony/Component/OptionsResolver/CHANGELOG.md @@ -4,5 +4,6 @@ CHANGELOG 2.5.0 ----- + * [BC BREAK] added parameter $flags to OptionsResolverInterface::resolve() * added flags FORBID_UNKNOWN, REMOVE_UNKNOWN, IGNORE_UNKNOWN, FORBID_MISSING and IGNORE_MISSING to OptionsResolverInterface