From eee87c376db3874bf941ae0e9f0c226beba11b33 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas <nicolas.grekas@gmail.com> Date: Thu, 6 Jul 2023 14:28:12 +0200 Subject: [PATCH] Fix cloning entities when using lazy-ghost proxies --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 60 +++++++++++-------- phpstan-baseline.neon | 2 +- psalm-baseline.xml | 7 ++- .../Tests/ORM/Proxy/ProxyFactoryTest.php | 20 ++----- 4 files changed, 47 insertions(+), 42 deletions(-) diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 2ba41caba72..e9992f3ca22 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -48,13 +48,12 @@ class <proxyShortClassName> extends \<className> implements \<baseProxyInterface { <useLazyGhostTrait> - /** - * @internal - */ - public bool $__isCloning = false; - - public function __construct(?\Closure $initializer = null) + public function __construct(?\Closure $initializer = null, ?\Closure $cloner = null) { + if ($cloner !== null) { + return; + } + self::createLazyGhost($initializer, <skippedProperties>, $this); } @@ -63,17 +62,6 @@ public function __isInitialized(): bool return isset($this->lazyObjectState) && $this->isLazyObjectInitialized(); } - public function __clone() - { - $this->__isCloning = true; - - try { - $this->__doClone(); - } finally { - $this->__isCloning = false; - } - } - public function __serialize(): array { <serializeImpl> @@ -98,6 +86,9 @@ public function __serialize(): array */ private $identifierFlattener; + /** @var ProxyDefinition[] */ + private $definitions = []; + /** * Initializes a new instance of the <tt>ProxyFactory</tt> class that is * connected to the given <tt>EntityManager</tt>. @@ -131,6 +122,26 @@ public function __construct(EntityManagerInterface $em, $proxyDir, $proxyNs, $au $this->identifierFlattener = new IdentifierFlattener($this->uow, $em->getMetadataFactory()); } + /** + * {@inheritDoc} + */ + public function getProxy($className, array $identifier) + { + $proxy = parent::getProxy($className, $identifier); + + if (! $this->em->getConfiguration()->isLazyGhostObjectEnabled()) { + return $proxy; + } + + $initializer = $this->definitions[$className]->initializer; + + $proxy->__construct(static function (Proxy $object) use ($initializer, $proxy): void { + $initializer($object, $proxy); + }); + + return $proxy; + } + /** * {@inheritDoc} */ @@ -158,7 +169,7 @@ protected function createProxyDefinition($className) $cloner = $this->createCloner($classMetadata, $entityPersister); } - return new ProxyDefinition( + return $this->definitions[$className] = new ProxyDefinition( ClassUtils::generateProxyClassName($className, $this->proxyNs), $classMetadata->getIdentifierFieldNames(), $classMetadata->getReflectionProperties(), @@ -231,15 +242,15 @@ private function createInitializer(ClassMetadata $classMetadata, EntityPersister /** * Creates a closure capable of initializing a proxy * - * @return Closure(Proxy):void + * @return Closure(Proxy, Proxy):void * * @throws EntityNotFoundException */ private function createLazyInitializer(ClassMetadata $classMetadata, EntityPersister $entityPersister): Closure { - return function (Proxy $proxy) use ($entityPersister, $classMetadata): void { - $identifier = $classMetadata->getIdentifierValues($proxy); - $entity = $entityPersister->loadById($identifier, $proxy->__isCloning ? null : $proxy); + return function (Proxy $proxy, Proxy $original) use ($entityPersister, $classMetadata): void { + $identifier = $classMetadata->getIdentifierValues($original); + $entity = $entityPersister->loadById($identifier, $original); if ($entity === null) { throw EntityNotFoundException::fromClassNameAndIdentifier( @@ -248,7 +259,7 @@ private function createLazyInitializer(ClassMetadata $classMetadata, EntityPersi ); } - if (! $proxy->__isCloning) { + if ($proxy === $original) { return; } @@ -315,7 +326,6 @@ private function generateUseLazyGhostTrait(ClassMetadata $class): string isLazyObjectInitialized as private; createLazyGhost as private; resetLazyObject as private; - __clone as private __doClone; }'), $code); return $code; @@ -323,7 +333,7 @@ private function generateUseLazyGhostTrait(ClassMetadata $class): string private function generateSkippedProperties(ClassMetadata $class): string { - $skippedProperties = ['__isCloning' => true]; + $skippedProperties = []; $identifiers = array_flip($class->getIdentifierFieldNames()); $filter = ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED | ReflectionProperty::IS_PRIVATE; $reflector = $class->getReflectionClass(); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 42b40438d7d..87d9dc9837c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -286,7 +286,7 @@ parameters: path: lib/Doctrine/ORM/Proxy/ProxyFactory.php - - message: "#^Access to an undefined property Doctrine\\\\Persistence\\\\Proxy\\:\\:\\$__isCloning\\.$#" + message: "#^Call to an undefined method Doctrine\\\\Common\\\\Proxy\\\\Proxy\\:\\:__construct\\(\\)\\.$#" count: 1 path: lib/Doctrine/ORM/Proxy/ProxyFactory.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index e24083e4046..37cd0acd5eb 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1392,6 +1392,11 @@ <code>$classMetadata</code> <code>$classMetadata</code> </ArgumentTypeCoercion> + <DirectConstructorCall> + <code><![CDATA[$proxy->__construct(static function (Proxy $object) use ($initializer, $proxy): void { + $initializer($object, $proxy); + })]]></code> + </DirectConstructorCall> <InvalidArgument> <code><![CDATA[$classMetadata->getReflectionProperties()]]></code> <code><![CDATA[$em->getMetadataFactory()]]></code> @@ -1400,7 +1405,6 @@ <NoInterfaceProperties> <code><![CDATA[$metadata->isEmbeddedClass]]></code> <code><![CDATA[$metadata->isMappedSuperclass]]></code> - <code><![CDATA[$proxy->__isCloning]]></code> </NoInterfaceProperties> <PossiblyNullPropertyFetch> <code><![CDATA[$property->name]]></code> @@ -1411,6 +1415,7 @@ <code>setAccessible</code> </PossiblyNullReference> <UndefinedInterfaceMethod> + <code>__construct</code> <code>__wakeup</code> </UndefinedInterfaceMethod> </file> diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php index 804cbbe040e..3739aaf4dd5 100644 --- a/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -5,7 +5,6 @@ namespace Doctrine\Tests\ORM\Proxy; use Doctrine\Common\EventManager; -use Doctrine\Common\Proxy\Proxy as CommonProxy; use Doctrine\DBAL\Connection; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\ORM\EntityNotFoundException; @@ -227,21 +226,12 @@ public function testProxyClonesParentFields(): void ->expects(self::atLeastOnce()) ->method('loadById'); - if ($proxy instanceof CommonProxy) { - $loadByIdMock->willReturn($companyEmployee); + $loadByIdMock->willReturn($companyEmployee); - $persister - ->expects(self::atLeastOnce()) - ->method('getClassMetadata') - ->willReturn($classMetaData); - } else { - $loadByIdMock->willReturnCallback(static function (array $id, CompanyEmployee $companyEmployee) { - $companyEmployee->setSalary(1000); // A property on the CompanyEmployee - $companyEmployee->setName('Bob'); // A property on the parent class, CompanyPerson - - return $companyEmployee; - }); - } + $persister + ->expects(self::atLeastOnce()) + ->method('getClassMetadata') + ->willReturn($classMetaData); $cloned = clone $proxy; assert($cloned instanceof CompanyEmployee);