Disallow read-only classes as target

janephp · Apr 12, 2023 · 227fe2b · 227fe2b
1 parent 6017762
commit 227fe2b
Show file tree

Hide file tree

Showing 17 changed files with 176 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [AutoMapper] [GH#713](https://github.com/janephp/janephp/pull/713) Use serializer's "ignore" attribute
 - [AutoMapper] [GH#714](https://github.com/janephp/janephp/pull/714) Allow custom context in AutomapperNormalizer
 - [AutoMapper] [GH#716](https://github.com/janephp/janephp/pull/716) Add readonly properties support
+- [AutoMapper] [GH#718](https://github.com/janephp/janephp/pull/718) Disallow readonly target when using object to populate
 
 ## [7.4.3] - 2023-03-23
 ### Added

diff --git a/src/Bundle/AutoMapperBundle/DependencyInjection/Configuration.php b/src/Bundle/AutoMapperBundle/DependencyInjection/Configuration.php
@@ -29,6 +29,7 @@ public function getConfigTreeBuilder(): TreeBuilder
                 ->scalarNode('cache_dir')->defaultValue('%kernel.cache_dir%/automapper')->end()
                 ->scalarNode('date_time_format')->defaultValue(\DateTimeInterface::RFC3339)->end()
                 ->booleanNode('hot_reload')->defaultValue($this->debug)->end()
+                ->booleanNode('allow_readonly_target_to_populate')->defaultFalse()->end()
             ->end()
         ;
 

diff --git a/src/Bundle/AutoMapperBundle/DependencyInjection/JaneAutoMapperExtension.php b/src/Bundle/AutoMapperBundle/DependencyInjection/JaneAutoMapperExtension.php
@@ -5,6 +5,7 @@
 use Jane\Bundle\AutoMapperBundle\Configuration\MapperConfigurationInterface;
 use Jane\Component\AutoMapper\Extractor\FromSourceMappingExtractor;
 use Jane\Component\AutoMapper\Extractor\FromTargetMappingExtractor;
+use Jane\Component\AutoMapper\Generator\Generator;
 use Jane\Component\AutoMapper\Loader\FileLoader;
 use Jane\Component\AutoMapper\MapperGeneratorMetadataFactory;
 use Jane\Component\AutoMapper\MapperGeneratorMetadataInterface;
@@ -69,6 +70,12 @@ public function load(array $configs, ContainerBuilder $container)
                 ->addArgument(new Reference($config['name_converter']));
         }
 
+        if ($config['allow_readonly_target_to_populate']) {
+            $container
+                ->getDefinition(Generator::class)
+                ->replaceArgument(2, $config['allow_readonly_target_to_populate']);
+        }
+
         $container->setParameter('automapper.cache_dir', $config['cache_dir']);
     }
 }
diff --git a/src/Bundle/AutoMapperBundle/README.md b/src/Bundle/AutoMapperBundle/README.md
@@ -24,6 +24,7 @@ Possible properties:
 - `cache_dir` (default: `%kernel.cache_dir%/automapper`): This settings allows you to customize the output directory for generated mappers;
 - `mappings`: This option allows you to customize Mapper metadata, you have to specify `source` & `target` data types and related configuration using `pass` field.
 This configuration should implements `Jane\Bundle\AutoMapper\Configuration\ConfigurationPassInterface`.
+- `allow_readonly_target_to_populate` (default: `false`): Will throw an exception if you use a readonly class as target to populate if set to `false`.
 
 ## Normalizer Bridge
 A Normalizer Bridge is available, aiming to be 100% feature compatible with the ObjectNormalizer of the ``symfony/serializer`` component. The goal of this bridge **is not to replace the ObjectNormalizer** but rather providing a very fast alternative.

diff --git a/src/Bundle/AutoMapperBundle/Resources/config/services.xml b/src/Bundle/AutoMapperBundle/Resources/config/services.xml
@@ -55,6 +55,7 @@
         <service id="Jane\Component\AutoMapper\Generator\Generator">
             <argument>null</argument>
             <argument type="service" id="jane.mapping.class_discriminator_from_class_metadata" />
+            <argument>false</argument>
         </service>
 
         <service id="jane.mapping.class_discriminator_from_class_metadata" class="Symfony\Component\Serializer\Mapping\ClassDiscriminatorFromClassMetadata">

diff --git a/src/Component/AutoMapper/AutoMapper.php b/src/Component/AutoMapper/AutoMapper.php
@@ -186,14 +186,16 @@ public static function create(
         string $classPrefix = 'Mapper_',
         bool $attributeChecking = true,
         bool $autoRegister = true,
-        string $dateTimeFormat = \DateTime::RFC3339
+        string $dateTimeFormat = \DateTime::RFC3339,
+        bool $allowReadOnlyTargetToPopulate = false
     ): self {
         $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
 
         if (null === $loader) {
             $loader = new EvalLoader(new Generator(
                 (new ParserFactory())->create(ParserFactory::PREFER_PHP7),
-                new ClassDiscriminatorFromClassMetadata($classMetadataFactory)
+                new ClassDiscriminatorFromClassMetadata($classMetadataFactory),
+                $allowReadOnlyTargetToPopulate
             ));
         }
 

diff --git a/src/Component/AutoMapper/Exception/ReadOnlyTargetException.php b/src/Component/AutoMapper/Exception/ReadOnlyTargetException.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace Jane\Component\AutoMapper\Exception;
+
+use Jane\Component\AutoMapper\MapperContext;
+
+/**
+ * @author Baptiste Leduc <baptiste.leduc@gmail.com>
+ */
+class ReadOnlyTargetException extends RuntimeException
+{
+    public function __construct(int $code = 0, ?Throwable $previous = null)
+    {
+        parent::__construct(sprintf('Cannot use readonly class as an object to populate. You can opt-out this behavior by using the context "%s"', MapperContext::ALLOW_READONLY_TARGET_TO_POPULATE), $code, $previous);
+    }
+}
diff --git a/src/Component/AutoMapper/Generator/Generator.php b/src/Component/AutoMapper/Generator/Generator.php
@@ -4,6 +4,7 @@
 
 use Jane\Component\AutoMapper\AutoMapperRegistryInterface;
 use Jane\Component\AutoMapper\Exception\CompileException;
+use Jane\Component\AutoMapper\Exception\ReadOnlyTargetException;
 use Jane\Component\AutoMapper\Extractor\WriteMutator;
 use Jane\Component\AutoMapper\GeneratedMapper;
 use Jane\Component\AutoMapper\MapperContext;
@@ -31,10 +32,13 @@ final class Generator
 
     private $classDiscriminator;
 
-    public function __construct(Parser $parser = null, ClassDiscriminatorResolverInterface $classDiscriminator = null)
+    private $allowReadOnlyTargetToPopulate;
+
+    public function __construct(Parser $parser = null, ClassDiscriminatorResolverInterface $classDiscriminator = null, bool $allowReadOnlyTargetToPopulate = false)
     {
         $this->parser = $parser ?? (new ParserFactory())->create(ParserFactory::PREFER_PHP7);
         $this->classDiscriminator = $classDiscriminator;
+        $this->allowReadOnlyTargetToPopulate = $allowReadOnlyTargetToPopulate;
     }
 
     /**
@@ -87,10 +91,21 @@ public function generate(MapperGeneratorMetadataInterface $mapperGeneratorMetada
         [$createObjectStmts, $inConstructor, $constructStatementsForCreateObjects, $injectMapperStatements] = $this->getCreateObjectStatements($mapperGeneratorMetadata, $result, $contextVariable, $sourceInput, $uniqueVariableScope);
         $constructStatements = array_merge($constructStatements, $constructStatementsForCreateObjects);
 
+        $targetToPopulate = new Expr\ArrayDimFetch($contextVariable, new Scalar\String_(MapperContext::TARGET_TO_POPULATE));
         $statements[] = new Stmt\Expression(new Expr\Assign($result, new Expr\BinaryOp\Coalesce(
-            new Expr\ArrayDimFetch($contextVariable, new Scalar\String_(MapperContext::TARGET_TO_POPULATE)),
+            $targetToPopulate,
             new Expr\ConstFetch(new Name('null'))
         )));
+        if (!$this->allowReadOnlyTargetToPopulate && $mapperGeneratorMetadata->isTargetReadOnlyClass()) {
+            $statements[] = new Stmt\If_(
+                new Expr\BinaryOp\BooleanAnd(
+                    new Expr\BooleanNot(new Expr\BinaryOp\Coalesce(new Expr\ArrayDimFetch($contextVariable, new Scalar\String_(MapperContext::ALLOW_READONLY_TARGET_TO_POPULATE)), new Expr\ConstFetch(new Name('false')))),
+                    new Expr\FuncCall(new Name('is_object'), [new Arg(new Expr\BinaryOp\Coalesce($targetToPopulate, new Expr\ConstFetch(new Name('null'))))])
+                ), [
+                'stmts' => [new Stmt\Expression(new Expr\Throw_(new Expr\New_(new Name(ReadOnlyTargetException::class))))],
+            ]);
+        }
+
         $statements[] = new Stmt\If_(new Expr\BinaryOp\Identical(new Expr\ConstFetch(new Name('null')), $result), [
             'stmts' => $createObjectStmts,
         ]);

diff --git a/src/Component/AutoMapper/MapperContext.php b/src/Component/AutoMapper/MapperContext.php
@@ -24,6 +24,7 @@ class MapperContext
     public const TARGET_TO_POPULATE = 'target_to_populate';
     public const CONSTRUCTOR_ARGUMENTS = 'constructor_arguments';
     public const SKIP_NULL_VALUES = 'skip_null_values';
+    public const ALLOW_READONLY_TARGET_TO_POPULATE = 'allow_readonly_target_to_populate';
 
     private $context = [
         self::DEPTH => 0,
@@ -86,6 +87,20 @@ public function setConstructorArgument(string $class, string $key, $value): self
         return $this;
     }
 
+    public function setSkipNullValues(bool $skipNullValues): self
+    {
+        $this->context[self::SKIP_NULL_VALUES] = $skipNullValues;
+
+        return $this;
+    }
+
+    public function setAllowReadOnlyTargetToPopulate(bool $allowReadOnlyTargetToPopulate): self
+    {
+        $this->context[self::ALLOW_READONLY_TARGET_TO_POPULATE] = $allowReadOnlyTargetToPopulate;
+
+        return $this;
+    }
+
     /**
      * Whether a reference has reached it's limit.
      */

diff --git a/src/Component/AutoMapper/MapperGeneratorMetadataFactory.php b/src/Component/AutoMapper/MapperGeneratorMetadataFactory.php
@@ -51,10 +51,24 @@ public function create(MapperGeneratorMetadataRegistryInterface $autoMapperRegis
             $extractor = $this->fromSourcePropertiesMappingExtractor;
         }
 
-        $mapperMetadata = new MapperMetadata($autoMapperRegister, $extractor, $source, $target, $this->classPrefix);
+        $mapperMetadata = new MapperMetadata($autoMapperRegister, $extractor, $source, $target, $this->isReadOnly($target), $this->classPrefix);
         $mapperMetadata->setAttributeChecking($this->attributeChecking);
         $mapperMetadata->setDateTimeFormat($this->dateTimeFormat);
 
         return $mapperMetadata;
     }
+
+    private function isReadOnly(string $mappedType): bool
+    {
+        try {
+            $reflClass = new \ReflectionClass($mappedType);
+        } catch (\ReflectionException $e) {
+            $reflClass = null;
+        }
+        if (\PHP_VERSION_ID >= 80200 && null !== $reflClass && $reflClass->isReadOnly()) {
+            return true;
+        }
+
+        return false;
+    }
 }
diff --git a/src/Component/AutoMapper/MapperMetadata.php b/src/Component/AutoMapper/MapperMetadata.php
@@ -31,6 +31,8 @@ class MapperMetadata implements MapperGeneratorMetadataInterface
 
     private $isConstructorAllowed;
 
+    private $isTargetReadOnlyClass;
+
     private $dateTimeFormat;
 
     private $classPrefix;
@@ -39,12 +41,13 @@ class MapperMetadata implements MapperGeneratorMetadataInterface
 
     private $targetReflectionClass = null;
 
-    public function __construct(MapperGeneratorMetadataRegistryInterface $metadataRegistry, MappingExtractorInterface $mappingExtractor, string $source, string $target, string $classPrefix = 'Mapper_')
+    public function __construct(MapperGeneratorMetadataRegistryInterface $metadataRegistry, MappingExtractorInterface $mappingExtractor, string $source, string $target, bool $isTargetReadOnlyClass, string $classPrefix = 'Mapper_')
     {
         $this->mappingExtractor = $mappingExtractor;
         $this->metadataRegistry = $metadataRegistry;
         $this->source = $source;
         $this->target = $target;
+        $this->isTargetReadOnlyClass = $isTargetReadOnlyClass;
         $this->isConstructorAllowed = true;
         $this->dateTimeFormat = \DateTime::RFC3339;
         $this->classPrefix = $classPrefix;
@@ -309,4 +312,9 @@ private function checkCircularMapperConfiguration(MapperGeneratorMetadataInterfa
 
         return false;
     }
+
+    public function isTargetReadOnlyClass(): bool
+    {
+        return $this->isTargetReadOnlyClass;
+    }
 }
diff --git a/src/Component/AutoMapper/MapperMetadataInterface.php b/src/Component/AutoMapper/MapperMetadataInterface.php
@@ -21,6 +21,11 @@ public function getSource(): string;
      */
     public function getTarget(): string;
 
+    /**
+     * Check if the target is a read-only class.
+     */
+    public function isTargetReadOnlyClass(): bool;
+
     /**
      * Get properties to map between source and target.
      *

diff --git a/src/Component/AutoMapper/Tests/AutoMapperBaseTest.php b/src/Component/AutoMapper/Tests/AutoMapperBaseTest.php
@@ -9,6 +9,7 @@
 use Jane\Component\AutoMapper\Loader\FileLoader;
 use PhpParser\ParserFactory;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\Serializer\Mapping\ClassDiscriminatorFromClassMetadata;
 use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory;
 use Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader;
@@ -26,14 +27,22 @@ abstract class AutoMapperBaseTest extends TestCase
 
     protected function setUp(): void
     {
-        @unlink(__DIR__ . '/cache/registry.php');
+        unset($this->autoMapper, $this->loader);
+        $this->buildAutoMapper();
+    }
+
+    protected function buildAutoMapper(bool $allowReadOnlyTargetToPopulate = false): AutoMapper
+    {
+        $fs = new Filesystem();
+        $fs->remove(__DIR__ . '/cache/');
         $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
 
         $this->loader = new FileLoader(new Generator(
             (new ParserFactory())->create(ParserFactory::PREFER_PHP7),
-            new ClassDiscriminatorFromClassMetadata($classMetadataFactory)
+            new ClassDiscriminatorFromClassMetadata($classMetadataFactory),
+            $allowReadOnlyTargetToPopulate
         ), __DIR__ . '/cache');
 
-        $this->autoMapper = AutoMapper::create(true, $this->loader);
+        return $this->autoMapper = AutoMapper::create(true, $this->loader);
     }
 }
diff --git a/src/Component/AutoMapper/Tests/AutoMapperTest.php b/src/Component/AutoMapper/Tests/AutoMapperTest.php
@@ -5,6 +5,7 @@
 use Jane\Component\AutoMapper\AutoMapper;
 use Jane\Component\AutoMapper\Exception\CircularReferenceException;
 use Jane\Component\AutoMapper\Exception\NoMappingFoundException;
+use Jane\Component\AutoMapper\Exception\ReadOnlyTargetException;
 use Jane\Component\AutoMapper\MapperContext;
 use Jane\Component\AutoMapper\Tests\Fixtures\Address;
 use Jane\Component\AutoMapper\Tests\Fixtures\AddressDTOReadonlyClass;
@@ -1046,12 +1047,57 @@ public function testEnum(): void
         self::assertEquals($address->getType(), $copyAddress->getType());
     }
 
+    /**
+     * @requires PHP 8.2
+     */
+    public function testTargetReadonlyClass(): void
+    {
+        $data = ['city' => 'Nantes'];
+        $toPopulate = new AddressDTOReadonlyClass('city');
+
+        self::expectException(ReadOnlyTargetException::class);
+        $this->autoMapper->map($data, $toPopulate);
+    }
+
+    /**
+     * @requires PHP 8.2
+     */
+    public function testTargetReadonlyClassSkippedContext(): void
+    {
+        $data = ['city' => 'Nantes'];
+        $toPopulate = new AddressDTOReadonlyClass('city');
+
+        self::expectException(ReadOnlyTargetException::class);
+        $this->autoMapper->map($data, $toPopulate, [MapperContext::ALLOW_READONLY_TARGET_TO_POPULATE => true]);
+
+        // value didn't changed because the object class is readonly, we can't change the value there
+        self::assertEquals('city', $toPopulate->city);
+    }
+
+    /**
+     * @requires PHP 8.2
+     */
+    public function testTargetReadonlyClassAllowed(): void
+    {
+        $this->buildAutoMapper(true);
+
+        $data = ['city' => 'Nantes'];
+        $toPopulate = new AddressDTOReadonlyClass('city');
+
+        $this->autoMapper->map($data, $toPopulate);
+
+        // value didn't changed because the object class is readonly, we can't change the value there
+        self::assertEquals('city', $toPopulate->city);
+    }
+
     /**
      * @requires PHP 8.1
      * @dataProvider provideReadonly
      */
     public function testReadonly(string $addressWithReadonlyClass): void
     {
+        $this->buildAutoMapper(true);
+
         $address = new Address();
         $address->setCity('city');
 
@@ -1092,18 +1138,4 @@ public static function provideReadonly(): iterable
             yield [AddressDTOReadonlyClass::class];
         }
     }
-
-    /**
-     * @requires PHP 8.2
-     */
-    public function testReadonlyClass(): void
-    {
-        $data = ['city' => 'Nantes'];
-        $toPopulate = new AddressDTOReadonlyClass('city');
-
-        $this->autoMapper->map($data, $toPopulate);
-
-        // value didn't changed because the object class is readonly, we can't change the value there
-        self::assertEquals('city', $toPopulate->city);
-    }
 }