Symfony Bundle configuration tweaks (#641)

- Added more type validation - Harden the tests
nelmio · Nov 22, 2016 · 5ef572c · 5ef572c
1 parent 288b977
commit 5ef572c
Show file tree

Hide file tree

Showing 8 changed files with 266 additions and 16 deletions.
diff --git a/doc/getting-started.md b/doc/getting-started.md
@@ -161,7 +161,7 @@ nelmio_alice:
     loading_limit: 5 # Alice may do some recursion to resolve certain values.
                      # This parameter defines a limit which will stop the
                      # resolution once reached.
-    max_unique_values_retries: 150 # Maximum number of time Alice can try to
+    max_unique_values_retry: 150 # Maximum number of time Alice can try to
                                    # generate a unique value before stopping and
                                    # failing.
 ```

diff --git a/fixtures/Bridge/Symfony/Application/config_custom.yml b/fixtures/Bridge/Symfony/Application/config_custom.yml
@@ -21,4 +21,4 @@ nelmio_alice:
     locale: 'fr_FR'
     seed: 100
     loading_limit: 50
-    max_unique_values_retries: 15
+    max_unique_values_retry: 15
diff --git a/src/Bridge/Symfony/DependencyInjection/Configuration.php b/src/Bridge/Symfony/DependencyInjection/Configuration.php
@@ -34,6 +34,9 @@ public function getConfigTreeBuilder()
                 ->scalarNode('locale')
                     ->defaultValue('en_US')
                     ->info('Default locale for the Faker Generator')
+                    ->validate()
+                        ->always($this->createStringValidatorClosure('nelmio_alice.locale'))
+                    ->end()
                 ->end()
                 ->scalarNode('seed')
                     ->defaultValue(1)
@@ -58,19 +61,60 @@ function ($seed) {
                         .'functions are used over Faker formatters. If you want to change that, simply blacklist the '
                         .'PHP function.'
                     )
+                    ->validate()
+                        ->always(
+                            function (array $value) {
+                                foreach ($value as $item) {
+                                    if (false === is_string($item)) {
+                                        throw InvalidArgumentExceptionFactory::createForExpectedConfigurationArrayOfStringValue($item);
+                                    }
+                                }
+
+                                return $value;
+                            }
+                        )
+                    ->end()
                 ->end()
-                ->scalarNode('loading_limit')
+                ->integerNode('loading_limit')
                     ->defaultValue(5)
                     ->info('Alice may do some recursion to resolve certain values. This parameter defines a limit which '
                         .'will stop the resolution once reached.'
                     )
+                    ->validate()
+                        ->always($this->createPositiveIntegerValidatorClosure())
+                    ->end()
                 ->end()
-                ->scalarNode('max_unique_values_retries')
+                ->integerNode('max_unique_values_retry')
                     ->defaultValue(150)
                     ->info('Maximum number of time Alice can try to generate a unique value before stopping and failing.')
+                    ->validate()
+                        ->always($this->createPositiveIntegerValidatorClosure())
+                    ->end()
                 ->end()
             ->end()
         ;
         return $treeBuilder;
     }
+
+    private function createStringValidatorClosure(): \Closure
+    {
+        return function ($value) {
+            if (is_string($value)) {
+                return $value;
+            }
+
+            throw InvalidArgumentExceptionFactory::createForExpectedConfigurationStringValue($value);
+        };
+    }
+
+    private function createPositiveIntegerValidatorClosure(): \Closure
+    {
+        return function ($value) {
+            if (is_int($value) && 0 < $value) {
+                return $value;
+            }
+
+            throw InvalidArgumentExceptionFactory::createForExpectedConfigurationPositiveIntegerValue($value);
+        };
+    }
 }
diff --git a/src/Bridge/Symfony/Resources/config/generator/resolver/value.xml b/src/Bridge/Symfony/Resources/config/generator/resolver/value.xml
@@ -111,7 +111,7 @@
                  class="Nelmio\Alice\Generator\Resolver\Value\Chainable\UniqueValueResolver">
             <argument type="service" id="nelmio_alice.generator.resolver.value.unique_values_pool" />
             <argument>null</argument>
-            <argument>%nelmio_alice.max_unique_values_retries%</argument>
+            <argument>%nelmio_alice.max_unique_values_retry%</argument>
 
             <tag name="nelmio_alice.generator.resolver.value.chainable_resolver" />
         </service>

diff --git a/src/Throwable/Exception/InvalidArgumentExceptionFactory.php b/src/Throwable/Exception/InvalidArgumentExceptionFactory.php
@@ -64,13 +64,43 @@ public static function createForInvalidSeedConfigurationValue($seed): \InvalidAr
     {
         return new \InvalidArgumentException(
             sprintf(
-                'Expected value "nelmio_alice.seed" to be either null or a strictly positive integer but got "%s" '
+                'Expected value to be either null or a strictly positive integer but got "%s" '
                 .'instead.',
                 $seed
             )
         );
     }
 
+    public static function createForExpectedConfigurationStringValue($value): \InvalidArgumentException
+    {
+        return new \InvalidArgumentException(
+            sprintf(
+                'Expected a string value but got "%s" instead.',
+                gettype($value)
+            )
+        );
+    }
+
+    public static function createForExpectedConfigurationPositiveIntegerValue(int $value): \InvalidArgumentException
+    {
+        return new \InvalidArgumentException(
+            sprintf(
+                'Expected a strictly positive integer but got "%s" instead.',
+                $value
+            )
+        );
+    }
+
+    public static function createForExpectedConfigurationArrayOfStringValue($value): \InvalidArgumentException
+    {
+        return new \InvalidArgumentException(
+            sprintf(
+                'Expected an array of strings but got "%s" element in the array instead.',
+                gettype($value)
+            )
+        );
+    }
+
     public static function createForRedundantUniqueValue(string $id): \InvalidArgumentException
     {
         return new \InvalidArgumentException(

diff --git a/tests/Bridge/Symfony/DependencyInjection/ConfigurationTest.php b/tests/Bridge/Symfony/DependencyInjection/ConfigurationTest.php
@@ -32,13 +32,60 @@ public function testDefaultValues()
             'seed' => 1,
             'functions_blacklist' => ['current'],
             'loading_limit' => 5,
-            'max_unique_values_retries' => 150,
+            'max_unique_values_retry' => 150,
         ];
         $actual = $processor->processConfiguration($configuration, []);
 
         $this->assertEquals($expected, $actual);
     }
 
+    public function testOverriddeValues()
+    {
+        $configuration = new Configuration();
+        $processor = new Processor();
+
+        $expected = [
+            'locale' => 'fr_FR',
+            'seed' => 10,
+            'functions_blacklist' => ['fake'],
+            'loading_limit' => 50,
+            'max_unique_values_retry' => 15,
+        ];
+        $actual = $processor->processConfiguration(
+            $configuration,
+            [
+                'nelmio_alice' => [
+                    'locale' => 'fr_FR',
+                    'seed' => 10,
+                    'functions_blacklist' => ['fake'],
+                    'loading_limit' => 50,
+                    'max_unique_values_retry' => 15,
+                ],
+            ]
+        );
+
+        $this->assertEquals($expected, $actual);
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage Invalid configuration for path "nelmio_alice.locale": Expected a string value but got "boolean" instead.
+     */
+    public function testLocaleMustBeAStringValues()
+    {
+        $configuration = new Configuration();
+        $processor = new Processor();
+
+        $processor->processConfiguration(
+            $configuration,
+            [
+                'nelmio_alice' => [
+                    'locale' => false,
+                ]
+            ]
+        );
+    }
+
     public function testSeedCanBeNull()
     {
         $configuration = new Configuration();
@@ -49,7 +96,7 @@ public function testSeedCanBeNull()
             'seed' => null,
             'functions_blacklist' => ['current'],
             'loading_limit' => 5,
-            'max_unique_values_retries' => 150,
+            'max_unique_values_retry' => 150,
         ];
         $actual = $processor->processConfiguration(
             $configuration,
@@ -64,12 +111,105 @@ public function testSeedCanBeNull()
     }
 
     /**
-     * @dataProvider provideInvalidSeedValues
-     *
      * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
-     * @expectedExceptionMessageRegExp /^Invalid configuration for path "nelmio_alice\.seed": Expected value "nelmio_alice\.seed" to be either null or a strictly positive integer but got ".*?" instead\.$/
+     * @expectedExceptionMessageRegExp /^Invalid type for path "nelmio_alice.functions_blacklist"\. Expected array, but got string/
+     */
+    public function testFunctionsBlacklistMustAnArray()
+    {
+        $configuration = new Configuration();
+        $processor = new Processor();
+
+        $processor->processConfiguration(
+            $configuration,
+            [
+                'nelmio_alice' => [
+                    'functions_blacklist' => 'string',
+                ]
+            ]
+        );
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage Invalid configuration for path "nelmio_alice.functions_blacklist": Expected an array of strings but got "boolean" element in the array instead.
+     */
+    public function testFunctionsBlacklistMustBeStrings()
+    {
+        $configuration = new Configuration();
+        $processor = new Processor();
+
+        $processor->processConfiguration(
+            $configuration,
+            [
+                'nelmio_alice' => [
+                    'functions_blacklist' => [true],
+                ]
+            ]
+        );
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage Invalid configuration for path "nelmio_alice.max_unique_values_retry": Expected a strictly positive integer but got "0" instead.
+     */
+    public function testMaxUniqueValuesRetryMustBeAStrictlyPositiveValues()
+    {
+        $configuration = new Configuration();
+        $processor = new Processor();
+
+        $processor->processConfiguration(
+            $configuration,
+            [
+                'nelmio_alice' => [
+                    'max_unique_values_retry' => 0,
+                ]
+            ]
+        );
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessageRegExp /^Invalid type for path "nelmio_alice.loading_limit"\. Expected int, but got boolean\./
+     */
+    public function testLoadingLimitMustBeAnInteger()
+    {
+        $configuration = new Configuration();
+        $processor = new Processor();
+
+        $processor->processConfiguration(
+            $configuration,
+            [
+                'nelmio_alice' => [
+                    'loading_limit' => false,
+                ]
+            ]
+        );
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage Invalid configuration for path "nelmio_alice.loading_limit": Expected a strictly positive integer but got "0" instead.
+     */
+    public function testLoadingLimitMustBeAStrictlyPositiveValues()
+    {
+        $configuration = new Configuration();
+        $processor = new Processor();
+
+        $processor->processConfiguration(
+            $configuration,
+            [
+                'nelmio_alice' => [
+                    'loading_limit' => 0,
+                ]
+            ]
+        );
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessageRegExp /^Invalid type for path "nelmio_alice.max_unique_values_retry"\. Expected int, but got boolean\./
      */
-    public function testIfNotNullThenSeedMustBeAStrictlyPositiveInteger($seed)
+    public function testMaxUniqueValuesRetryMustBeAnInteger()
     {
         $configuration = new Configuration();
         $processor = new Processor();
@@ -78,7 +218,7 @@ public function testIfNotNullThenSeedMustBeAStrictlyPositiveInteger($seed)
             $configuration,
             [
                 'nelmio_alice' => [
-                    'seed' => $seed,
+                    'max_unique_values_retry' => false,
                 ]
             ]
         );
@@ -94,7 +234,7 @@ public function testConfigurationParametersAreInjectedAsParameters()
             'nelmio_alice.functions_blacklist' => ['current'],
             'nelmio_alice.seed' => 1,
             'nelmio_alice.loading_limit' => 5,
-            'nelmio_alice.max_unique_values_retries' => 150,
+            'nelmio_alice.max_unique_values_retry' => 150,
         ];
 
         foreach ($expected as $parameterName => $value) {

diff --git a/tests/Generator/Resolver/Value/Chainable/FunctionCallArgumentResolverTest.php b/tests/Generator/Resolver/Value/Chainable/FunctionCallArgumentResolverTest.php
@@ -50,7 +50,7 @@ public function testCanResolvePropertyReferenceValues()
         $this->assertTrue($resolver->canResolve(new FunctionCallValue('')));
         $this->assertFalse($resolver->canResolve(new FakeValue()));
     }
-    
+
     public function testIsResolverAware()
     {
         $decoratedResolverConstructor = function () {

diff --git a/tests/Throwable/Exception/InvalidArgumentExceptionFactoryTest.php b/tests/Throwable/Exception/InvalidArgumentExceptionFactoryTest.php
@@ -66,7 +66,43 @@ public function testTestCreateForInvalidSeedConfigurationValue()
         $exception = InvalidArgumentExceptionFactory::createForInvalidSeedConfigurationValue(10);
 
         $this->assertEquals(
-            'Expected value "nelmio_alice.seed" to be either null or a strictly positive integer but got "10" instead.',
+            'Expected value to be either null or a strictly positive integer but got "10" instead.',
+            $exception->getMessage()
+        );
+        $this->assertEquals(0, $exception->getCode());
+        $this->assertNull($exception->getPrevious());
+    }
+
+    public function testTestCreateForExpectedConfigurationStringValue()
+    {
+        $exception = InvalidArgumentExceptionFactory::createForExpectedConfigurationStringValue(10);
+
+        $this->assertEquals(
+            'Expected a string value but got "integer" instead.',
+            $exception->getMessage()
+        );
+        $this->assertEquals(0, $exception->getCode());
+        $this->assertNull($exception->getPrevious());
+    }
+
+    public function testTestCreateForExpectedConfigurationPositiveIntegerValue()
+    {
+        $exception = InvalidArgumentExceptionFactory::createForExpectedConfigurationPositiveIntegerValue(-1);
+
+        $this->assertEquals(
+            'Expected a strictly positive integer but got "-1" instead.',
+            $exception->getMessage()
+        );
+        $this->assertEquals(0, $exception->getCode());
+        $this->assertNull($exception->getPrevious());
+    }
+
+    public function testTestCreateForExpectedConfigurationArrayOfStringValue()
+    {
+        $exception = InvalidArgumentExceptionFactory::createForExpectedConfigurationArrayOfStringValue(10);
+
+        $this->assertEquals(
+            'Expected an array of strings but got "integer" element in the array instead.',
             $exception->getMessage()
         );
         $this->assertEquals(0, $exception->getCode());