Merge pull request #11 from NicolasGuilloux/fix-is-optional-issues

Version 2.1.2
NicolasGuilloux · Oct 9, 2019 · a43f177 · a43f177
2 parents d3be39b + b741219
commit a43f177
Show file tree

Hide file tree

Showing 9 changed files with 149 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,15 +1,24 @@
 # Changelog
 
+## 2.1.2
+
+Bug fix:
+ * The `@Assert\DateTime` and `@Assert\Date` will now transform the property type in `DateTime` if no type is set.
+ * You can now add a type to bypass the exception thrown by a `ParamConverter`. Check the [documentation](Doc/ParamConverter.md#bypass-paramconverter-exception-for-specific-classes).
+
+
 ## 2.1.1
 
 Behaviour Change:
  * The DTO handler can now also bind data from the cookie of the `Request`. It now uses the following priority: `Request > Attributes > Query > Cookies`.
 
+
 ## 2.1.0
 
 New features:
  * The DTO handler can now also bind data from the attributes and query of the `Request` object. It loads the content with the following priority: `Request > Attributes > Query`.
 
+
 ## 2.0.0
 
 New features:

diff --git a/ConfigurationExtractor/PropertyConfigurationExtractor.php b/ConfigurationExtractor/PropertyConfigurationExtractor.php
@@ -16,6 +16,8 @@
 use Doctrine\Common\Annotations\AnnotationReader;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\Validator\Constraints\All;
+use Symfony\Component\Validator\Constraints\Date;
+use Symfony\Component\Validator\Constraints\DateTime;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Constraints\NotNull;
 use Symfony\Component\Validator\Constraints\Type;
@@ -76,6 +78,10 @@ public function __construct(\ReflectionProperty $property)
         $arrayAnnotation = $annotationReader->getPropertyAnnotation($property, All::class);
         /** @var Type $typeAnnotation */
         $typeAnnotation = $annotationReader->getPropertyAnnotation($property, Type::class);
+        /** @var DateTime $dateTimeAnnotation */
+        $dateTimeAnnotation = $annotationReader->getPropertyAnnotation($property, DateTime::class);
+        /** @var Date $dateAnnotation */
+        $dateAnnotation = $annotationReader->getPropertyAnnotation($property, Date::class);
         /** @var MapTo $mapToAnnotation */
         $mapToAnnotation = $annotationReader->getPropertyAnnotation($property, MapTo::class);
         /** @var NotNull $notNullAnnotation */
@@ -93,6 +99,10 @@ public function __construct(\ReflectionProperty $property)
              $typeAnnotation = $this->findTypeConstraint($arrayAnnotation) ?? $typeAnnotation;
         }
 
+        if ($dateTimeAnnotation !== null || $dateAnnotation !== null) {
+            $this->type = \DateTime::class;
+        }
+
         if ($typeAnnotation !== null && \class_exists($typeAnnotation->type)) {
             $this->type = $typeAnnotation->type;
         }

diff --git a/DependencyInjection/ChapleanDtoHandlerExtension.php b/DependencyInjection/ChapleanDtoHandlerExtension.php
@@ -28,9 +28,30 @@ class ChapleanDtoHandlerExtension extends Extension
     public function load(array $configs, ContainerBuilder $container): void
     {
         $configuration = new Configuration();
-        $this->processConfiguration($configuration, $configs);
+        $config = $this->processConfiguration($configuration, $configs);
 
         $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
         $loader->load('services.xml');
+
+        $container->setParameter('chaplean_dto_handler', $config);
+        $this->setParameters($container, 'chaplean_dto_handler', $config);
+    }
+
+    /**
+     * @param ContainerBuilder $container
+     * @param string           $name
+     * @param array            $configs
+     *
+     * @return void
+     */
+    public function setParameters(ContainerBuilder $container, $name, array $configs): void
+    {
+        foreach ($configs as $key => $parameter) {
+            $container->setParameter($name . '.' . $key, $parameter);
+
+            if (is_array($parameter)) {
+                $this->setParameters($container, $name . '.' . $key, $parameter);
+            }
+        }
     }
 }
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -25,7 +25,19 @@ final class Configuration implements ConfigurationInterface
     public function getConfigTreeBuilder(): TreeBuilder
     {
         $treeBuilder = new TreeBuilder();
-        $treeBuilder->root('chaplean_dto_handler');
+        $rootNode = $treeBuilder->root('chaplean_dto_handler');
+
+        $rootNode
+            ->children()
+                ->arrayNode('bypass_param_converter_exception')
+                    ->info('Bypass the ParamConverter exception for specified classes')
+                    ->defaultValue([
+                        \DateTime::class
+                    ])
+                    ->prototype('scalar')->end()
+                ->end()
+            ->end()
+        ->end();
 
         return $treeBuilder;
     }

diff --git a/Doc/ParamConverter.md b/Doc/ParamConverter.md
@@ -73,3 +73,16 @@ public function postAction(
     // ...
 }
 ```
+
+### Bypass `ParamConverter` exception for specific classes
+
+Some `ParamConverter` will throw an exception in case of a bad input. This is the case of the `DateTimeParamConverter` which will throw a 404 Not Found error if it fails to transform the input in date, when you give no value for instance. A 404 is not appropriated in most cases, especially if you have the `NotBlank` or `NotNull` assertion which will better handle the error.
+
+To bypass it, you can set the following options. This is the default value.
+
+```yaml
+chaplean_dto_handler:
+    bypass_param_converter_exception:
+        - 'DateTime'
+```
+
diff --git a/ParamConverter/DataTransferObjectParamConverter.php b/ParamConverter/DataTransferObjectParamConverter.php
@@ -19,10 +19,9 @@
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Sensio\Bundle\FrameworkExtraBundle\Request\ParamConverter\ParamConverterInterface;
 use Sensio\Bundle\FrameworkExtraBundle\Request\ParamConverter\ParamConverterManager;
-use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
-use Symfony\Component\Validator\ConstraintViolation;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 
 /**
@@ -37,13 +36,18 @@ class DataTransferObjectParamConverter implements ParamConverterInterface
     /**
      * @var array
      */
-    protected $taggedDtoClasses;
+    protected $bypassParamConverterExceptionClasses;
 
     /**
      * @var ParamConverterManager
      */
     protected $manager;
 
+    /**
+     * @var array
+     */
+    protected $taggedDtoClasses;
+
     /**
      * @var ValidatorInterface
      */
@@ -52,13 +56,16 @@ class DataTransferObjectParamConverter implements ParamConverterInterface
     /**
      * DataTransferObjectParamConverter constructor.
      *
+     * @param ContainerInterface      $container
      * @param ParamConverterManager   $paramConverterManager
      * @param ValidatorInterface|null $validator
      */
     public function __construct(
+        ContainerInterface $container,
         ParamConverterManager $paramConverterManager,
         ValidatorInterface $validator = null
     ) {
+        $this->bypassParamConverterExceptionClasses = $container->getParameter('chaplean_dto_handler.bypass_param_converter_exception') ?? [];
         $this->manager = $paramConverterManager;
         $this->validator = $validator;
         $this->taggedDtoClasses = [];
@@ -245,7 +252,11 @@ protected function autoConfigureOne(
         $config = new ParamConverter([]);
         $config->setName($name);
         $config->setClass($propertyConfigurationModel->getType());
-        $config->setIsOptional($propertyConfigurationModel->isOptional());
+        $config->setIsOptional(true);
+
+        if (!\in_array($propertyConfigurationModel->getType(), $this->bypassParamConverterExceptionClasses, true)) {
+            $config->setIsOptional($propertyConfigurationModel->isOptional());
+        }
 
         if ($propertyConfigurationModel->getMapTo() !== null) {
             $config->setOptions(

diff --git a/Resources/config/services.xml b/Resources/config/services.xml
@@ -8,6 +8,7 @@
         <defaults autowire="true" autoconfigure="true" public="false"/>
 
         <service id="Chaplean\Bundle\DtoHandlerBundle\ParamConverter\DataTransferObjectParamConverter">
+            <argument type="service" id="service_container" />
             <argument type="service" id="sensio_framework_extra.converter.manager" />
             <argument type="service" id="validator" />
             <tag name="request.param_converter" converter="data_transfer_object_converter" />

diff --git a/Tests/ConfigurationExtractor/PropertyConfigurationExtractorTest.php b/Tests/ConfigurationExtractor/PropertyConfigurationExtractorTest.php
@@ -200,4 +200,60 @@ public function testDummyEntityWithCollectionConstraintWithoutEntity(): void
         self::assertTrue($propertyConfigurationModel->isOptional());
         self::assertTrue($propertyConfigurationModel->isCollection());
     }
+
+    /**
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::__construct()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getName()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getMapTo()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getType()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getParamConverterAnnotation()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::isOptional()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::isCollection()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::findTypeConstraint()
+     *
+     * @return void
+     *
+     * @throws AnnotationException
+     * @throws \ReflectionException
+     */
+    public function testDateTimeType(): void
+    {
+        $property = $this->dtoReflectionClass->getProperty('property8');
+        $propertyConfigurationModel = new PropertyConfigurationExtractor($property);
+
+        self::assertSame('property8', $propertyConfigurationModel->getName());
+        self::assertNull($propertyConfigurationModel->getMapTo());
+        self::assertSame(\DateTime::class, $propertyConfigurationModel->getType());
+        self::assertNull($propertyConfigurationModel->getParamConverterAnnotation());
+        self::assertFalse($propertyConfigurationModel->isOptional());
+        self::assertFalse($propertyConfigurationModel->isCollection());
+    }
+
+    /**
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::__construct()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getName()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getMapTo()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getType()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::getParamConverterAnnotation()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::isOptional()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::isCollection()
+     * @covers \Chaplean\Bundle\DtoHandlerBundle\ConfigurationExtractor\PropertyConfigurationExtractor::findTypeConstraint()
+     *
+     * @return void
+     *
+     * @throws AnnotationException
+     * @throws \ReflectionException
+     */
+    public function testDateType(): void
+    {
+        $property = $this->dtoReflectionClass->getProperty('property9');
+        $propertyConfigurationModel = new PropertyConfigurationExtractor($property);
+
+        self::assertSame('property9', $propertyConfigurationModel->getName());
+        self::assertNull($propertyConfigurationModel->getMapTo());
+        self::assertSame(\DateTime::class, $propertyConfigurationModel->getType());
+        self::assertNull($propertyConfigurationModel->getParamConverterAnnotation());
+        self::assertTrue($propertyConfigurationModel->isOptional());
+        self::assertFalse($propertyConfigurationModel->isCollection());
+    }
 }
diff --git a/Tests/ParamConverter/DataTransferObjectParamConverterTest.php b/Tests/ParamConverter/DataTransferObjectParamConverterTest.php
@@ -18,7 +18,7 @@
 use phpmock\mockery\PHPMockery;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Sensio\Bundle\FrameworkExtraBundle\Request\ParamConverter\ParamConverterManager;
-use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Validator\ConstraintViolation;
 use Symfony\Component\Validator\ConstraintViolationList;
@@ -62,7 +62,16 @@ public function setUp(): void
 
         PHPMockery::mock('Chaplean\Bundle\DtoHandlerBundle\ParamConverter', 'uniqid')->andReturn('hash');
 
+        $container = \Mockery::mock(ContainerInterface::class);
+        $container->shouldReceive('getParameter')
+            ->once()
+            ->with('chaplean_dto_handler.bypass_param_converter_exception')
+            ->andReturn([
+                \DateTime::class
+            ]);
+
         $this->dataTransferObjectParamConverter = new DataTransferObjectParamConverter(
+            $container,
             $this->manager,
             $this->validator
         );