Disallow read-only classes as target

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

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
             ));
         }
 

src/Component/AutoMapper/Exception/ReadOnlyTargetException.php

@@ -0,0 +1,19 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Jane\Component\AutoMapper\Exception;
+
+/**
+ * @author Baptiste Leduc <baptiste.leduc@gmail.com>
+ */
+class ReadOnlyTargetException extends RuntimeException
+{
+}

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,
         ]);

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.
      */

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;
+    }
 }

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;
+    }
 }

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.
      *

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);
     }
 }

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;
@@ -1052,6 +1053,8 @@ public function testEnum(): void
      */
     public function testReadonly(string $addressWithReadonlyClass): void
     {
+        $this->buildAutoMapper(true);
+
         $address = new Address();
         $address->setCity('city');
 
@@ -1096,8 +1099,22 @@ public static function provideReadonly(): iterable
     /**
      * @requires PHP 8.2
      */
-    public function testReadonlyClass(): void
+    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 testTargetReadonlyClassAllowed(): void
     {
+        $this->buildAutoMapper(true);
+
         $data = ['city' => 'Nantes'];
         $toPopulate = new AddressDTOReadonlyClass('city');
 

src/Component/AutoMapper/Tests/Extractor/FromSourceMappingExtractorTest.php

@@ -77,7 +77,7 @@ private function fromSourceMappingExtractorBootstrap(bool $private = true): void
     public function testWithTargetAsArray(): void
     {
         $userReflection = new \ReflectionClass(Fixtures\User::class);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\User::class, 'array');
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\User::class, 'array', false);
         $sourcePropertiesMapping = $this->fromSourceMappingExtractor->getPropertiesMapping($mapperMetadata);
 
         self::assertCount(\count($userReflection->getProperties()), $sourcePropertiesMapping);
@@ -90,7 +90,7 @@ public function testWithTargetAsArray(): void
     public function testWithTargetAsStdClass(): void
     {
         $userReflection = new \ReflectionClass(Fixtures\User::class);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\User::class, 'stdClass');
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\User::class, 'stdClass', false);
         $sourcePropertiesMapping = $this->fromSourceMappingExtractor->getPropertiesMapping($mapperMetadata);
 
         self::assertCount(\count($userReflection->getProperties()), $sourcePropertiesMapping);
@@ -102,7 +102,7 @@ public function testWithTargetAsStdClass(): void
 
     public function testWithSourceAsEmpty(): void
     {
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\Empty_::class, 'array');
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\Empty_::class, 'array', false);
         $sourcePropertiesMapping = $this->fromSourceMappingExtractor->getPropertiesMapping($mapperMetadata);
 
         self::assertCount(0, $sourcePropertiesMapping);
@@ -111,12 +111,12 @@ public function testWithSourceAsEmpty(): void
     public function testWithSourceAsPrivate(): void
     {
         $privateReflection = new \ReflectionClass(Fixtures\Private_::class);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\Private_::class, 'array');
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\Private_::class, 'array', false);
         $sourcePropertiesMapping = $this->fromSourceMappingExtractor->getPropertiesMapping($mapperMetadata);
         self::assertCount(\count($privateReflection->getProperties()), $sourcePropertiesMapping);
 
         $this->fromSourceMappingExtractorBootstrap(false);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\Private_::class, 'array');
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, Fixtures\Private_::class, 'array', false);
         $sourcePropertiesMapping = $this->fromSourceMappingExtractor->getPropertiesMapping($mapperMetadata);
         self::assertCount(0, $sourcePropertiesMapping);
     }
@@ -126,7 +126,7 @@ public function testWithSourceAsArray(): void
         self::expectException(InvalidMappingException::class);
         self::expectExceptionMessage('Only array or stdClass are accepted as a target');
 
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, 'array', Fixtures\User::class);
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, 'array', Fixtures\User::class, false);
         $this->fromSourceMappingExtractor->getPropertiesMapping($mapperMetadata);
     }
 
@@ -135,7 +135,7 @@ public function testWithSourceAsStdClass(): void
         self::expectException(InvalidMappingException::class);
         self::expectExceptionMessage('Only array or stdClass are accepted as a target');
 
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, 'stdClass', Fixtures\User::class);
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromSourceMappingExtractor, 'stdClass', Fixtures\User::class, false);
         $this->fromSourceMappingExtractor->getPropertiesMapping($mapperMetadata);
     }
 }

src/Component/AutoMapper/Tests/Extractor/FromTargetMappingExtractorTest.php

@@ -77,7 +77,7 @@ private function fromTargetMappingExtractorBootstrap(bool $private = true): void
     public function testWithSourceAsArray(): void
     {
         $userReflection = new \ReflectionClass(Fixtures\User::class);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\User::class);
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\User::class, false);
         $targetPropertiesMapping = $this->fromTargetMappingExtractor->getPropertiesMapping($mapperMetadata);
 
         self::assertCount(\count($userReflection->getProperties()), $targetPropertiesMapping);
@@ -89,7 +89,7 @@ public function testWithSourceAsArray(): void
     public function testWithSourceAsStdClass(): void
     {
         $userReflection = new \ReflectionClass(Fixtures\User::class);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'stdClass', Fixtures\User::class);
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'stdClass', Fixtures\User::class, false);
         $targetPropertiesMapping = $this->fromTargetMappingExtractor->getPropertiesMapping($mapperMetadata);
 
         self::assertCount(\count($userReflection->getProperties()), $targetPropertiesMapping);
@@ -100,7 +100,7 @@ public function testWithSourceAsStdClass(): void
 
     public function testWithTargetAsEmpty(): void
     {
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\Empty_::class);
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\Empty_::class, false);
         $targetPropertiesMapping = $this->fromTargetMappingExtractor->getPropertiesMapping($mapperMetadata);
 
         self::assertCount(0, $targetPropertiesMapping);
@@ -109,12 +109,12 @@ public function testWithTargetAsEmpty(): void
     public function testWithTargetAsPrivate(): void
     {
         $privateReflection = new \ReflectionClass(Fixtures\Private_::class);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\Private_::class);
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\Private_::class, false);
         $targetPropertiesMapping = $this->fromTargetMappingExtractor->getPropertiesMapping($mapperMetadata);
         self::assertCount(\count($privateReflection->getProperties()), $targetPropertiesMapping);
 
         $this->fromTargetMappingExtractorBootstrap(false);
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\Private_::class);
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, 'array', Fixtures\Private_::class, false);
         $targetPropertiesMapping = $this->fromTargetMappingExtractor->getPropertiesMapping($mapperMetadata);
         self::assertCount(0, $targetPropertiesMapping);
     }
@@ -124,7 +124,7 @@ public function testWithTargetAsArray(): void
         self::expectException(InvalidMappingException::class);
         self::expectExceptionMessage('Only array or stdClass are accepted as a source');
 
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, Fixtures\User::class, 'array');
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, Fixtures\User::class, 'array', false);
         $this->fromTargetMappingExtractor->getPropertiesMapping($mapperMetadata);
     }
 
@@ -133,7 +133,7 @@ public function testWithTargetAsStdClass(): void
         self::expectException(InvalidMappingException::class);
         self::expectExceptionMessage('Only array or stdClass are accepted as a source');
 
-        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, Fixtures\User::class, 'stdClass');
+        $mapperMetadata = new MapperMetadata($this->autoMapper, $this->fromTargetMappingExtractor, Fixtures\User::class, 'stdClass', false);
         $this->fromTargetMappingExtractor->getPropertiesMapping($mapperMetadata);
     }
 }