Making sure the ProxyFactory#createProxy() is always given a `Class…

…Metadata` instance Ref: doctrine#6719 (comment)
lcobucci · Dec 31, 2017 · a25fdb5 · a25fdb5
1 parent 36e6526
commit a25fdb5
Show file tree

Hide file tree

Showing 11 changed files with 185 additions and 146 deletions.
diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php
@@ -530,7 +530,7 @@ public function getReference($entityName, $id)
             return $this->find($entityName, $sortedId);
         }
 
-        $entity = $this->proxyFactory->getProxy($className, $id);
+        $entity = $this->proxyFactory->getProxy($class, $id);
 
         $this->unitOfWork->registerManaged($entity, $sortedId, []);
 

diff --git a/lib/Doctrine/ORM/Proxy/Factory/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/Factory/ProxyFactory.php
@@ -5,6 +5,7 @@
 
 namespace Doctrine\ORM\Proxy\Factory;
 
+use Doctrine\ORM\Mapping\ClassMetadata;
 use ProxyManager\Proxy\GhostObjectInterface;
 
 interface ProxyFactory
@@ -47,15 +48,15 @@ interface ProxyFactory
     const AUTOGENERATE_EVAL = 3;
 
     /**
-     * @param \Doctrine\ORM\Mapping\ClassMetadata[] $classMetadataList
+     * @param \Doctrine\ORM\Mapping\ClassMetadata[] $classes
      *
      * @return int
      */
-    public function generateProxyClasses(array $classMetadataList) : int;
+    public function generateProxyClasses(array $classes) : int;
 
     /**
      * Gets a reference proxy instance for the entity of the given type and identified by
      * the given identifier.
      */
-    public function getProxy(string $className, array $identifier) : GhostObjectInterface;
+    public function getProxy(ClassMetadata $metadata, array $identifier) : GhostObjectInterface;
 }
diff --git a/lib/Doctrine/ORM/Proxy/Factory/StaticProxyFactory.php b/lib/Doctrine/ORM/Proxy/Factory/StaticProxyFactory.php
@@ -39,11 +39,6 @@ final class StaticProxyFactory implements ProxyFactory
      */
     private $proxyFactory;
 
-    /**
-     * @var ClassMetadata[] indexed by metadata class name
-     */
-    private $cachedMetadata = [];
-
     /**
      * @var \Closure[] indexed by metadata class name
      */
@@ -70,28 +65,23 @@ public function __construct(
     /**
      * {@inheritdoc}
      *
-     * @param ClassMetadata[] $classMetadataList
+     * @param ClassMetadata[] $classes
      */
-    public function generateProxyClasses(array $classMetadataList) : int
+    public function generateProxyClasses(array $classes) : int
     {
-        $concreteClasses = \array_filter($classMetadataList, function (ClassMetadata $metadata) : bool {
+        $concreteClasses = \array_filter($classes, function (ClassMetadata $metadata) : bool {
             return ! ($metadata->isMappedSuperclass || $metadata->getReflectionClass()->isAbstract());
         });
 
         foreach ($concreteClasses as $metadata) {
-            $className = $metadata->getClassName();
-
             $this
                 ->proxyFactory
                 ->createProxy(
-                    $className,
+                    $metadata->getClassName(),
                     function () {
                         // empty closure, serves its purpose, for now
                     },
-                    $this->cachedSkippedProperties[$className]
-                        ?? $this->cachedSkippedProperties[$className] = [
-                            self::SKIPPED_PROPERTIES => $this->skippedFieldsFqns($metadata)
-                        ]
+                    $this->skippedFieldsFqns($metadata)
                 );
         }
 
@@ -103,10 +93,9 @@ function () {
      *
      * @throws \Doctrine\ORM\EntityNotFoundException
      */
-    public function getProxy(string $className, array $identifier) : GhostObjectInterface
+    public function getProxy(ClassMetadata $metadata, array $identifier) : GhostObjectInterface
     {
-        $metadata  = $this->cachedMetadata[$className]
-            ?? $this->cachedMetadata[$className] = $this->entityManager->getClassMetadata($className);
+        $className = $metadata->getClassName();
         $persister = $this->cachedPersisters[$className]
             ?? $this->cachedPersisters[$className] = $this
                 ->entityManager

diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
@@ -2349,15 +2349,15 @@ public function createEntity($className, array $data, &$hints = [])
                     switch (true) {
                         // We are negating the condition here. Other cases will assume it is valid!
                         case ($hints['fetchMode'][$class->getClassName()][$field] !== FetchMode::EAGER):
-                            $newValue = $this->em->getProxyFactory()->getProxy($targetEntity, $normalizedAssociatedId);
+                            $newValue = $this->em->getProxyFactory()->getProxy($targetClass, $normalizedAssociatedId);
                             break;
 
                         // Deferred eager load only works for single identifier classes
                         case (isset($hints[self::HINT_DEFEREAGERLOAD]) && !$targetClass->isIdentifierComposite()):
                             // TODO: Is there a faster approach?
                             $this->eagerLoadingEntities[$targetClass->getRootClassName()][$relatedIdHash] = current($normalizedAssociatedId);
 
-                            $newValue = $this->em->getProxyFactory()->getProxy($targetEntity, $normalizedAssociatedId);
+                            $newValue = $this->em->getProxyFactory()->getProxy($targetClass, $normalizedAssociatedId);
                             break;
 
                         default:
@@ -2571,7 +2571,7 @@ private function tryGetByIdOrLoadProxy(array $id, string $rootClassName)
             $sortedId[$idField] = $id[$idField];
         }
 
-        $proxy = $this->em->getProxyFactory()->getProxy($rootClassName, $sortedId);
+        $proxy = $this->em->getProxyFactory()->getProxy($class, $sortedId);
 
         $this->registerManaged($proxy, $sortedId, []);
 

diff --git a/tests/Doctrine/Performance/LazyLoading/ProxyInitializationTimeBench.php b/tests/Doctrine/Performance/LazyLoading/ProxyInitializationTimeBench.php
@@ -38,14 +38,18 @@ final class ProxyInitializationTimeBench
 
     public function init() : void
     {
-        $proxyFactory = (new NonProxyLoadingEntityManager(EntityManagerFactory::getEntityManager([])))
+        $entityManager = EntityManagerFactory::getEntityManager([]);
+        $proxyFactory  = (new NonProxyLoadingEntityManager($entityManager))
             ->getProxyFactory();
 
+        $cmsUser     = $entityManager->getClassMetadata(CmsUser::class);
+        $cmsEmployee = $entityManager->getClassMetadata(CmsEmployee::class);
+
         for ($i = 0; $i < 10000; ++$i) {
-            $this->cmsUsers[$i]             = $proxyFactory->getProxy(CmsUser::class, ['id' => $i]);
-            $this->cmsEmployees[$i]         = $proxyFactory->getProxy(CmsEmployee::class, ['id' => $i]);
-            $this->initializedUsers[$i]     = $proxyFactory->getProxy(CmsUser::class, ['id' => $i]);
-            $this->initializedEmployees[$i] = $proxyFactory->getProxy(CmsEmployee::class, ['id' => $i]);
+            $this->cmsUsers[$i]             = $proxyFactory->getProxy($cmsUser, ['id' => $i]);
+            $this->cmsEmployees[$i]         = $proxyFactory->getProxy($cmsEmployee, ['id' => $i]);
+            $this->initializedUsers[$i]     = $proxyFactory->getProxy($cmsUser, ['id' => $i]);
+            $this->initializedEmployees[$i] = $proxyFactory->getProxy($cmsEmployee, ['id' => $i]);
 
             $this->initializedUsers[$i]->initializeProxy();
             $this->initializedEmployees[$i]->initializeProxy();

diff --git a/tests/Doctrine/Performance/LazyLoading/ProxyInstantiationTimeBench.php b/tests/Doctrine/Performance/LazyLoading/ProxyInstantiationTimeBench.php
@@ -4,6 +4,7 @@
 
 namespace Doctrine\Performance\LazyLoading;
 
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Proxy\Factory\ProxyFactory;
 use Doctrine\Performance\EntityManagerFactory;
 use Doctrine\Tests\Models\CMS\CmsEmployee;
@@ -20,22 +21,35 @@ final class ProxyInstantiationTimeBench
      */
     private $proxyFactory;
 
+    /**
+     * @var ClassMetadata
+     */
+    private $cmsUserMetadata;
+
+    /**
+     * @var ClassMetadata
+     */
+    private $cmsEmployeeMetadata;
+
     public function init() : void
     {
-        $this->proxyFactory = EntityManagerFactory::getEntityManager([])->getProxyFactory();
+        $entityManager             = EntityManagerFactory::getEntityManager([]);
+        $this->proxyFactory        = $entityManager->getProxyFactory();
+        $this->cmsUserMetadata     = $entityManager->getClassMetadata(CmsUser::class);
+        $this->cmsEmployeeMetadata = $entityManager->getClassMetadata(CmsEmployee::class);
     }
 
     public function benchCmsUserInstantiation() : void
     {
         for ($i = 0; $i < 100000; ++$i) {
-            $this->proxyFactory->getProxy(CmsUser::class, ['id' => $i]);
+            $this->proxyFactory->getProxy($this->cmsUserMetadata, ['id' => $i]);
         }
     }
 
     public function benchCmsEmployeeInstantiation() : void
     {
         for ($i = 0; $i < 100000; ++$i) {
-            $this->proxyFactory->getProxy(CmsEmployee::class, ['id' => $i]);
+            $this->proxyFactory->getProxy($this->cmsEmployeeMetadata, ['id' => $i]);
         }
     }
 }

diff --git a/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php b/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php
@@ -69,7 +69,8 @@ protected function setUp()
     public function testPersistUpdate()
     {
         // Considering case (a)
-        $proxy = $this->em->getProxyFactory()->getProxy(CmsUser::class, ['id' => 123]);
+        $metadata = $this->em->getClassMetadata(CmsUser::class);
+        $proxy    = $this->em->getProxyFactory()->getProxy($metadata, ['id' => 123]);
 
         $proxy->setProxyInitializer(null);
         $proxy->id = null;
@@ -83,8 +84,7 @@ public function testPersistUpdate()
 
         $proxy->name = 'Marco Pivetta';
 
-        $this->em->getUnitOfWork()
-            ->computeChangeSet($this->em->getClassMetadata(CmsUser::class), $proxy);
+        $this->em->getUnitOfWork()->computeChangeSet($metadata, $proxy);
         self::assertNotEmpty($this->em->getUnitOfWork()->getEntityChangeSet($proxy));
         self::assertEquals('Marco Pivetta', $this->em->find(CmsUser::class, $proxy->getId())->name);
 
@@ -114,7 +114,10 @@ public function testEntityWithIdentifier()
      */
     public function testProxyAsDqlParameterPersist()
     {
-        $proxy = $this->em->getProxyFactory()->getProxy(CmsUser::class, ['id' => $this->user->getId()]);
+        $proxy = $this->em->getProxyFactory()->getProxy(
+            $this->em->getClassMetadata(CmsUser::class),
+            ['id' => $this->user->getId()]
+        );
 
         $proxy->id = $this->user->getId();
 

diff --git a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php
@@ -257,7 +257,10 @@ public function testCommonPersistenceProxy()
             'Proxies are being written to disk in this test'
         );
 
-        $proxy = $this->factory->getProxy(ECommerceProduct::class, ['id' => 123]);
+        $proxy = $this->factory->getProxy(
+            $this->em->getClassMetadata(ECommerceProduct::class),
+            ['id' => 123]
+        );
 
         $proxyClass = new \ReflectionClass($proxy);
 

diff --git a/tests/Doctrine/Tests/ORM/Hydration/HydrationTestCase.php b/tests/Doctrine/Tests/ORM/Hydration/HydrationTestCase.php
@@ -4,10 +4,14 @@
 
 namespace Doctrine\Tests\ORM\Hydration;
 
+use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Query\ParserResult;
 
 class HydrationTestCase extends \Doctrine\Tests\OrmTestCase
 {
+    /**
+     * @var EntityManagerInterface
+     */
     protected $em;
 
     protected function setUp()