Avoid checking circular reference if there is no possibilities of it

janephp · Jan 29, 2019 · 3753afc · 3753afc
1 parent 534636f
commit 3753afc
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 7 deletions.
diff --git a/src/AutoMapper/.gitignore b/src/AutoMapper/.gitignore
@@ -4,3 +4,4 @@ composer.lock
 Tests/generated/*
 !Tests/generated/.gitkeep
 Tests/fixtures/*/generated
+Bundle/Tests/Resources/var
diff --git a/src/AutoMapper/AbstractAutoMapper.php b/src/AutoMapper/AbstractAutoMapper.php
@@ -93,7 +93,7 @@ public function getConfiguration(string $source, string $target): ?MapperConfigu
                 return null;
             }
 
-            $this->register($this->mapperConfigurationFactory->create($source, $target));
+            $this->register($this->mapperConfigurationFactory->create($this, $source, $target));
         }
 
         return $this->configurations[$source][$target];

diff --git a/src/AutoMapper/AutoMapperRegisterInterface.php b/src/AutoMapper/AutoMapperRegisterInterface.php
@@ -5,4 +5,6 @@
 interface AutoMapperRegisterInterface
 {
     public function register(MapperConfigurationInterface $configuration): void;
+
+    public function getConfiguration(string $source, string $target): ?MapperConfigurationInterface;
 }
diff --git a/src/AutoMapper/Bundle/DependencyInjection/JaneAutoMapperExtension.php b/src/AutoMapper/Bundle/DependencyInjection/JaneAutoMapperExtension.php
@@ -64,6 +64,7 @@ private function createMapperConfigurationDefinition(ContainerBuilder $container
         $serviceName = 'Mapping_' . $config['source'] . '_' . $config['target'];
         $definition = $container->register($serviceName, MapperConfiguration::class);
         $definition->setFactory([new Reference(MapperConfigurationFactory::class), 'create']);
+        $definition->addArgument(new Reference(AutoMapper::class));
         $definition->addArgument($config['source']);
         $definition->addArgument($config['target']);
         $definition->addTag('jane_auto_mapper.mapper_configuration');

diff --git a/src/AutoMapper/Bundle/Resources/config/services.xml b/src/AutoMapper/Bundle/Resources/config/services.xml
@@ -34,7 +34,7 @@
             <argument type="service" id="Jane\AutoMapper\Compiler\SourceTargetPropertiesMappingExtractor" />
             <argument type="service" id="Jane\AutoMapper\Compiler\FromSourcePropertiesMappingExtractor" />
             <argument type="service" id="Jane\AutoMapper\Compiler\FromTargetPropertiesMappingExtractor" />
-            <argument type="service" id="serializer.mapping.class_metadata_factory" />
+            <argument>Symfony_Mapper_</argument>
         </service>
 
         <service id="Jane\AutoMapper\Compiler\FileLoader">

diff --git a/src/AutoMapper/Bundle/Tests/Resources/app/AppKernel.php b/src/AutoMapper/Bundle/Tests/Resources/app/AppKernel.php
@@ -49,6 +49,11 @@ public function indexAction()
     {
         return new Response();
     }
+
+    public function getProjectDir()
+    {
+        return __DIR__ . '/..';
+    }
 }
 
 class UserConfigurationPass implements ConfigurationPassInterface

diff --git a/src/AutoMapper/Compiler/Compiler.php b/src/AutoMapper/Compiler/Compiler.php
@@ -40,14 +40,15 @@ public function compile(MapperConfigurationInterface $mapperConfiguration)
         $constructStatements = [];
         $injectMapperStatements = [];
         $addedDependencies = [];
+        $canHaveCircularDependency = $mapperConfiguration->canHaveCircularDependency() && $mapperConfiguration->getSource() !== 'array';
 
         $statements = [
             new Stmt\If_(new Expr\BinaryOp\Identical(new Expr\ConstFetch(new Name('null')), $sourceInput), [
                 'stmts' => [new Stmt\Return_($sourceInput)],
             ]),
         ];
 
-        if ($mapperConfiguration->getSource() !== 'array') {
+        if ($canHaveCircularDependency) {
             $statements[] = new Stmt\Expression(new Expr\Assign($hashVariable, new Expr\BinaryOp\Concat(new Expr\FuncCall(new Name('spl_object_hash'), [
                 new Arg($sourceInput),
             ]),
@@ -96,7 +97,7 @@ public function compile(MapperConfigurationInterface $mapperConfiguration)
         }
 
         if (\count($addedDependencies) > 0) {
-            if ($mapperConfiguration->getSource() !== 'array') {
+            if ($canHaveCircularDependency) {
                 $statements[] = new Stmt\Expression(new Expr\Assign(
                     $contextVariable,
                     new Expr\MethodCall($contextVariable, 'withReference', [

diff --git a/src/AutoMapper/MapperConfiguration.php b/src/AutoMapper/MapperConfiguration.php
@@ -13,9 +13,12 @@ class MapperConfiguration extends AbstractMapperConfiguration
 
     private $customMapping = [];
 
-    public function __construct(PropertiesMappingExtractorInterface $mappingExtractor, string $source, string $target, string $classPrefix = 'Mapper_')
+    private $autoMapperRegister;
+
+    public function __construct(AutoMapperRegisterInterface $autoMapperRegister, PropertiesMappingExtractorInterface $mappingExtractor, string $source, string $target, string $classPrefix = 'Mapper_')
     {
         $this->mappingExtractor = $mappingExtractor;
+        $this->autoMapperRegister = $autoMapperRegister;
 
         parent::__construct($source, $target, $classPrefix);
     }
@@ -103,4 +106,36 @@ public function isTargetCloneable(): bool
 
         return $reflection->isCloneable() && !$reflection->hasMethod('__clone');
     }
+
+    public function canHaveCircularDependency(): bool
+    {
+        $checked = [];
+
+        return $this->checkCircularMapperConfiguration($this, $checked);
+    }
+
+    protected function checkCircularMapperConfiguration(MapperConfigurationInterface $configuration, &$checked)
+    {
+        foreach ($configuration->getPropertiesMapping() as $propertyMapping) {
+            foreach ($propertyMapping->getTransformer()->getDependencies() as $dependency) {
+                if (isset($checked[$dependency->getName()])) {
+                    continue;
+                }
+
+                $checked[$dependency->getName()] = true;
+
+                if ($dependency->getSource() === $this->getSource() && $dependency->getTarget() === $this->getTarget()) {
+                    return true;
+                }
+
+                $subConfiguration = $this->autoMapperRegister->getConfiguration($dependency->getSource(), $dependency->getTarget());
+
+                if (null !== $subConfiguration && true === $this->checkCircularMapperConfiguration($subConfiguration, $checked)) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
 }
diff --git a/src/AutoMapper/MapperConfigurationFactory.php b/src/AutoMapper/MapperConfigurationFactory.php
@@ -25,7 +25,7 @@ public function __construct(
         $this->classPrefix = $classPrefix;
     }
 
-    public function create($source, $target): MapperConfigurationInterface
+    public function create(AutoMapperRegisterInterface $autoMapperRegister, $source, $target): MapperConfiguration
     {
         $extractor = $this->sourceTargetPropertiesMappingExtractor;
 
@@ -37,6 +37,6 @@ public function create($source, $target): MapperConfigurationInterface
             $extractor = $this->fromSourcePropertiesMappingExtractor;
         }
 
-        return new MapperConfiguration($extractor, $source, $target, $this->classPrefix);
+        return new MapperConfiguration($autoMapperRegister, $extractor, $source, $target, $this->classPrefix);
     }
 }
diff --git a/src/AutoMapper/MapperConfigurationInterface.php b/src/AutoMapper/MapperConfigurationInterface.php
@@ -28,4 +28,6 @@ public function isConstructorAllowed(): bool;
     public function isTargetCloneable(): bool;
 
     public function getDateTimeFormat(): string;
+
+    public function canHaveCircularDependency(): bool;
 }