From bcbeaad96746982b6155d2bef756ba863130dfb4 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Fri, 20 Sep 2024 09:58:20 +0700 Subject: [PATCH 1/7] fix: non managed properties should trigger proxy load --- src/Proxy/ProxyFactory.php | 2 +- tests/Tests/Models/Product/Product.php | 57 ++++++++++++++++++++++ tests/Tests/ORM/Proxy/ProxyFactoryTest.php | 36 ++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 tests/Tests/Models/Product/Product.php diff --git a/src/Proxy/ProxyFactory.php b/src/Proxy/ProxyFactory.php index dc8a72bfcea..12023176106 100644 --- a/src/Proxy/ProxyFactory.php +++ b/src/Proxy/ProxyFactory.php @@ -447,7 +447,7 @@ private function getProxyFactory(string $className): Closure foreach ($reflector->getProperties($filter) as $property) { $name = $property->name; - if ($property->isStatic() || (($class->hasField($name) || $class->hasAssociation($name)) && ! isset($identifiers[$name]))) { + if ($property->isStatic() || ! isset($identifiers[$name])) { continue; } diff --git a/tests/Tests/Models/Product/Product.php b/tests/Tests/Models/Product/Product.php new file mode 100644 index 00000000000..5e101a2dc69 --- /dev/null +++ b/tests/Tests/Models/Product/Product.php @@ -0,0 +1,57 @@ +name = $name; + } + + public function getId(): int + { + return $this->id; + } + + public function getName(): string + { + return $this->name; + } + + public function getImage(): ?string + { + return $this->image; + } + + public function setImage(string $image): void + { + $this->image = $image; + } +} diff --git a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php index e6335c46c8f..293dd9bc636 100644 --- a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -18,6 +18,7 @@ use Doctrine\Tests\Models\Company\CompanyEmployee; use Doctrine\Tests\Models\Company\CompanyPerson; use Doctrine\Tests\Models\ECommerce\ECommerceFeature; +use Doctrine\Tests\Models\Product\Product; use Doctrine\Tests\OrmTestCase; use Doctrine\Tests\PHPUnitCompatibility\MockBuilderCompatibilityTools; use Exception; @@ -239,6 +240,41 @@ public function testProxyClonesParentFields(): void self::assertSame(1000, $cloned->getSalary(), 'Expect properties on the CompanyEmployee class to be cloned'); self::assertSame('Bob', $cloned->getName(), 'Expect properties on the CompanyPerson class to be cloned'); } + + public function testUnmanagedPropertyTriggerProxyLoad(): void + { + $product = new Product('someName'); + $product->setImage('someImage'); + $identifier = ['id' => 42]; + + $classMetaData = $this->emMock->getClassMetadata(Product::class); + + $persister = $this + ->getMockBuilder(BasicEntityPersister::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->uowMock->setEntityPersister(Product::class, $persister); + + $persister + ->expects(self::atLeastOnce()) + ->method('loadById') + ->with(self::equalTo($identifier)) + ->will(self::returnValue($product)); + + $persister + ->expects(self::atLeastOnce()) + ->method('getClassMetadata') + ->willReturn($classMetaData); + + $proxy = $this->proxyFactory->getProxy(Product::class, $identifier); + + self::assertFalse($proxy->__isInitialized()); + $proxy->getId(); + self::assertFalse($proxy->__isInitialized()); + $proxy->getImage(); + self::assertTrue($proxy->__isInitialized()); + } } abstract class AbstractClass From 273b07aedfb61d82930ed7c59e51d3bd1f603626 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Fri, 20 Sep 2024 12:53:24 +0700 Subject: [PATCH 2/7] cs --- tests/Tests/ORM/Proxy/ProxyFactoryTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php index 293dd9bc636..995ccb44d93 100644 --- a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -249,7 +249,7 @@ public function testUnmanagedPropertyTriggerProxyLoad(): void $classMetaData = $this->emMock->getClassMetadata(Product::class); - $persister = $this + $persister = $this ->getMockBuilder(BasicEntityPersister::class) ->disableOriginalConstructor() ->getMock(); From 9d5106c5ec60fc3ede38021ed570a470aaa667db Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:18:42 +0700 Subject: [PATCH 3/7] use static variable for testing to avoid triggering proxy load --- tests/Tests/ORM/Functional/LifecycleCallbackTest.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/Tests/ORM/Functional/LifecycleCallbackTest.php b/tests/Tests/ORM/Functional/LifecycleCallbackTest.php index 2317363b114..9709d8b112c 100644 --- a/tests/Tests/ORM/Functional/LifecycleCallbackTest.php +++ b/tests/Tests/ORM/Functional/LifecycleCallbackTest.php @@ -132,10 +132,10 @@ public function testGetReferenceWithPostLoadEventIsDelayedUntilProxyTrigger(): v $this->_em->clear(); $reference = $this->_em->getReference(LifecycleCallbackTestEntity::class, $id); - self::assertFalse($reference->postLoadCallbackInvoked); + self::assertFalse(LifecycleCallbackTestEntity::$staticPostLoadCallbackInvoked); $reference->getValue(); // trigger proxy load - self::assertTrue($reference->postLoadCallbackInvoked); + self::assertTrue(LifecycleCallbackTestEntity::$staticPostLoadCallbackInvoked); } /** @group DDC-958 */ @@ -494,6 +494,9 @@ class LifecycleCallbackTestEntity /** @var bool */ public $postLoadCallbackInvoked = false; + /** @var bool */ + public static $staticPostLoadCallbackInvoked = false; + /** @var bool */ public $postLoadCascaderNotNull = false; @@ -546,8 +549,9 @@ public function doStuffOnPostPersist(): void /** @PostLoad */ public function doStuffOnPostLoad(): void { - $this->postLoadCallbackInvoked = true; - $this->postLoadCascaderNotNull = isset($this->cascader); + $this->postLoadCallbackInvoked = true; + self::$staticPostLoadCallbackInvoked = true; + $this->postLoadCascaderNotNull = isset($this->cascader); } /** @PreUpdate */ From bba8f93cd6af3960ed81cf002a868c94ab378978 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:20:09 +0700 Subject: [PATCH 4/7] reset $staticPostLoadCallbackInvoked on setUp --- tests/Tests/ORM/Functional/LifecycleCallbackTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Tests/ORM/Functional/LifecycleCallbackTest.php b/tests/Tests/ORM/Functional/LifecycleCallbackTest.php index 9709d8b112c..14934298445 100644 --- a/tests/Tests/ORM/Functional/LifecycleCallbackTest.php +++ b/tests/Tests/ORM/Functional/LifecycleCallbackTest.php @@ -47,6 +47,8 @@ protected function setUp(): void { parent::setUp(); + LifecycleCallbackTestEntity::$staticPostLoadCallbackInvoked = false; + $this->createSchemaForModels( LifecycleCallbackEventArgEntity::class, LifecycleCallbackTestEntity::class, From a71539fc7299057e59a8ded90486546ff2278334 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Fri, 20 Sep 2024 13:46:48 +0700 Subject: [PATCH 5/7] remove typed properties --- tests/Tests/Models/Product/Product.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Tests/Models/Product/Product.php b/tests/Tests/Models/Product/Product.php index 5e101a2dc69..7316a2b463c 100644 --- a/tests/Tests/Models/Product/Product.php +++ b/tests/Tests/Models/Product/Product.php @@ -13,6 +13,7 @@ class Product { /** + * @var int * @ORM\Id * @ORM\Column() * @ORM\GeneratedValue @@ -20,7 +21,7 @@ class Product #[ORM\Id] #[ORM\Column()] #[ORM\GeneratedValue] - private int $id = 42; + private $id = 42; /** * @ORM\Column() From e9fb6a1b548800ceb0e90d6ee4a3d6b62d8f5b9d Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:07:19 +0700 Subject: [PATCH 6/7] remove type hint --- tests/Tests/Models/Product/Product.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/Tests/Models/Product/Product.php b/tests/Tests/Models/Product/Product.php index 7316a2b463c..d2db0d8928f 100644 --- a/tests/Tests/Models/Product/Product.php +++ b/tests/Tests/Models/Product/Product.php @@ -24,12 +24,16 @@ class Product private $id = 42; /** + * @var string * @ORM\Column() */ #[ORM\Column()] - private string $name; + private $name; - private ?string $image = null; + /** + * @var string|null + */ + private $image = null; public function __construct(string $name) { From 8aec2d6a688e85af68ac29fc3e7cf56f91d4e0c1 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:09:59 +0700 Subject: [PATCH 7/7] change will(returnValue) to willReturn --- tests/Tests/ORM/Proxy/ProxyFactoryTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php index 995ccb44d93..ff70d2d38d4 100644 --- a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -260,7 +260,7 @@ public function testUnmanagedPropertyTriggerProxyLoad(): void ->expects(self::atLeastOnce()) ->method('loadById') ->with(self::equalTo($identifier)) - ->will(self::returnValue($product)); + ->willReturn($product); $persister ->expects(self::atLeastOnce())