From bc4e14a99f26338e83c1e643293074c064bc5bdd Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 20 May 2011 20:50:03 +0200 Subject: [PATCH 1/3] Prototype for a proxy extension that avoids loads when calling for a getter that is named after an identifier. --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 20 +++++++++++++++++++ .../ORM/Functional/LifecycleCallbackTest.php | 6 +++++- .../ORM/Functional/Ticket/DDC381Test.php | 9 +++++++-- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 23186d7aa..5794e26f9 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -209,6 +209,11 @@ class ProxyFactory $methods .= $parameterString . ')'; $methods .= PHP_EOL . ' {' . PHP_EOL; + if ($this->isShortIdentifierGetter($method, $class)) { + $methods .= ' if ($this->__isInitialized__ === false) {' . PHP_EOL; + $methods .= ' return $this->_identifier["' . lcfirst(substr($method->getName(), 3)) . '"];' . PHP_EOL; + $methods .= ' }' . PHP_EOL; + } $methods .= ' $this->__load();' . PHP_EOL; $methods .= ' return parent::' . $method->getName() . '(' . $argumentString . ');'; $methods .= PHP_EOL . ' }' . PHP_EOL; @@ -218,6 +223,21 @@ class ProxyFactory return $methods; } + /** + * @param ReflectionMethod $method + * @param ClassMetadata $class + * @return bool + */ + private function isShortIdentifierGetter($method, $class) + { + return ( + $method->getNumberOfParameters() == 0 && + substr($method->getName(), 0, 3) == "get" && + in_array(lcfirst(substr($method->getName(), 3)), $class->identifier, true) && + (($method->getEndLine() - $method->getStartLine()) <= 4) + ); + } + /** * Generates the code for the __sleep method for a proxy class. * diff --git a/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php b/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php index 1ddd7a8ff..a860ecd62 100644 --- a/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php @@ -78,7 +78,7 @@ class LifecycleCallbackTest extends \Doctrine\Tests\OrmFunctionalTestCase $reference = $this->_em->getReference('Doctrine\Tests\ORM\Functional\LifecycleCallbackTestEntity', $id); $this->assertFalse($reference->postLoadCallbackInvoked); - $reference->getId(); // trigger proxy load + $reference->getValue(); // trigger proxy load $this->assertTrue($reference->postLoadCallbackInvoked); } @@ -210,6 +210,10 @@ class LifecycleCallbackTestEntity return $this->id; } + public function getValue() { + return $this->value; + } + /** @PrePersist */ public function doStuffOnPrePersist() { $this->prePersistCallbackInvoked = true; diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC381Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC381Test.php index c25a8aa75..678135daf 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC381Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC381Test.php @@ -31,8 +31,8 @@ class DDC381Test extends \Doctrine\Tests\OrmFunctionalTestCase $entity = $this->_em->getReference('Doctrine\Tests\ORM\Functional\Ticket\DDC381Entity', $persistedId); - // explicitly load proxy - $id = $entity->getId(); + // explicitly load proxy (getId() does not trigger reload of proxy) + $id = $entity->getOtherMethod(); $data = serialize($entity); $entity = unserialize($data); @@ -55,4 +55,9 @@ class DDC381Entity { return $this->id; } + + public function getOtherMethod() + { + + } } \ No newline at end of file From 8d1b852aa22569fc62c0b9a3ea9b13713b08de84 Mon Sep 17 00:00:00 2001 From: Alexander Date: Sat, 15 Oct 2011 17:31:09 +0200 Subject: [PATCH 2/3] Added tests for not loading the entity + fixed a test --- .../Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php | 5 ++++- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php b/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php index a860ecd62..35eb5443c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php @@ -78,6 +78,9 @@ class LifecycleCallbackTest extends \Doctrine\Tests\OrmFunctionalTestCase $reference = $this->_em->getReference('Doctrine\Tests\ORM\Functional\LifecycleCallbackTestEntity', $id); $this->assertFalse($reference->postLoadCallbackInvoked); + $reference->getId(); // doesn't trigger proxy load + $this->assertFalse($reference->postLoadCallbackInvoked); + $reference->getValue(); // trigger proxy load $this->assertTrue($reference->postLoadCallbackInvoked); } @@ -278,4 +281,4 @@ class LifecycleListenerPreUpdate { $eventArgs->setNewValue('name', 'Bob'); } -} \ No newline at end of file +} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php index 93a89818e..467577a43 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php @@ -52,6 +52,8 @@ class DDC1238Test extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $this->_em->clear(); + // force proxy load, getId() doesn't work anymore + $user->getName(); $userId = $user->getId(); $this->_em->clear(); @@ -60,6 +62,8 @@ class DDC1238Test extends \Doctrine\Tests\OrmFunctionalTestCase $user2 = $this->_em->getReference(__NAMESPACE__ . '\\DDC1238User', $userId); + // force proxy load, getId() doesn't work anymore + $user->getName(); $this->assertNull($user->getId(), "Now this is null, we already have a user instance of that type"); } } From d46352da0106278ce3b271bca907a381254c42d8 Mon Sep 17 00:00:00 2001 From: Alexander Date: Sat, 15 Oct 2011 17:58:00 +0200 Subject: [PATCH 3/3] Fixed tests + added dedicated tests for proxy loading and identifiers --- .../ORM/Functional/LifecycleCallbackTest.php | 3 --- .../ORM/Functional/ReferenceProxyTest.php | 24 +++++++++++++++++++ .../ORM/Proxy/ProxyClassGeneratorTest.php | 5 ++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php b/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php index 35eb5443c..2d71541d2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/LifecycleCallbackTest.php @@ -78,9 +78,6 @@ class LifecycleCallbackTest extends \Doctrine\Tests\OrmFunctionalTestCase $reference = $this->_em->getReference('Doctrine\Tests\ORM\Functional\LifecycleCallbackTestEntity', $id); $this->assertFalse($reference->postLoadCallbackInvoked); - $reference->getId(); // doesn't trigger proxy load - $this->assertFalse($reference->postLoadCallbackInvoked); - $reference->getValue(); // trigger proxy load $this->assertTrue($reference->postLoadCallbackInvoked); } diff --git a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php index 8ecb389af..b89f3d04e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php @@ -147,4 +147,28 @@ class ReferenceProxyTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertTrue($entity->wakeUp, "Loading the proxy should call __wakeup()."); } + + public function testDoNotInitializeProxyOnGettingTheIdentifier() + { + $id = $this->createProduct(); + + /* @var $entity Doctrine\Tests\Models\ECommerce\ECommerceProduct */ + $entity = $this->_em->getReference('Doctrine\Tests\Models\ECommerce\ECommerceProduct' , $id); + + $this->assertFalse($entity->__isInitialized__, "Pre-Condition: Object is unitialized proxy."); + $this->assertEquals($id, $entity->getId()); + $this->assertFalse($entity->__isInitialized__, "Getting the identifier doesn't initialize the proxy."); + } + + public function testInitializeProxyOnGettingSomethingOtherThanTheIdentifier() + { + $id = $this->createProduct(); + + /* @var $entity Doctrine\Tests\Models\ECommerce\ECommerceProduct */ + $entity = $this->_em->getReference('Doctrine\Tests\Models\ECommerce\ECommerceProduct' , $id); + + $this->assertFalse($entity->__isInitialized__, "Pre-Condition: Object is unitialized proxy."); + $this->assertEquals('Doctrine Cookbook', $entity->getName()); + $this->assertTrue($entity->__isInitialized__, "Getting something other than the identifier initializes the proxy."); + } } diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php index 6257dbe70..39bdcfe5a 100644 --- a/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php +++ b/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php @@ -73,12 +73,13 @@ class ProxyClassGeneratorTest extends \Doctrine\Tests\OrmTestCase $persister = $this->_getMockPersister(); $this->_uowMock->setEntityPersister('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $persister); $proxy = $this->_proxyFactory->getProxy('Doctrine\Tests\Models\ECommerce\ECommerceFeature', $identifier); + $persister->expects($this->atLeastOnce()) ->method('load') ->with($this->equalTo($identifier), $this->isInstanceOf($proxyClass)) ->will($this->returnValue(new \stdClass())); // fake return of entity instance - $proxy->getId(); $proxy->getDescription(); + $proxy->getProduct(); } public function testReferenceProxyRespectsMethodsParametersTypeHinting() @@ -179,4 +180,4 @@ class SleepClass { return array('id'); } -} \ No newline at end of file +}