Merge pull request #148 from asm89/ProxyIdentifer

Do not load entity on retrieving identifier from a proxy
doctrine · Oct 15, 2011 · ba38f3e · ba38f3e
2 parents 4654175 + d46352d
commit ba38f3e
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 6 deletions.
diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php
@@ -209,6 +209,11 @@ private function _generateMethods(ClassMetadata $class)
 
                 $methods .= $parameterString . ')';
                 $methods .= "\n" . '    {' . "\n";
+                if ($this->isShortIdentifierGetter($method, $class)) {
+                    $methods .= '        if ($this->__isInitialized__ === false) {' . "\n";
+                    $methods .= '            return $this->_identifier["' . lcfirst(substr($method->getName(), 3)) . '"];' . "\n";
+                    $methods .= '        }' . "\n";
+                }
                 $methods .= '        $this->__load();' . "\n";
                 $methods .= '        return parent::' . $method->getName() . '(' . $argumentString . ');';
                 $methods .= "\n" . '    }' . "\n";
@@ -218,6 +223,21 @@ private function _generateMethods(ClassMetadata $class)
         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
@@ -78,7 +78,7 @@ public function testGetReferenceWithPostLoadEventIsDelayedUntilProxyTrigger()
         $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 @@ public function getId() {
         return $this->id;
     }
 
+    public function getValue() {
+        return $this->value;
+    }
+
     /** @PrePersist */
     public function doStuffOnPrePersist() {
         $this->prePersistCallbackInvoked = true;
@@ -274,4 +278,4 @@ public function preUpdate(PreUpdateEventArgs $eventArgs)
     {
         $eventArgs->setNewValue('name', 'Bob');
     }
-}
+}
diff --git a/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php b/tests/Doctrine/Tests/ORM/Functional/ReferenceProxyTest.php
@@ -147,4 +147,28 @@ public function testWakeupCalledOnProxy()
 
         $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/Functional/Ticket/DDC1238Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php
@@ -52,6 +52,8 @@ public function testIssueProxyClear()
         $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 @@ public function testIssueProxyClear()
 
         $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");
     }
 }

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC381Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC381Test.php
@@ -31,8 +31,8 @@ public function testCallUnserializedProxyMethods()
 
         $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 @@ public function getId()
     {
         return $this->id;
     }
+
+    public function getOtherMethod()
+    {
+
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyClassGeneratorTest.php
@@ -73,12 +73,13 @@ public function testReferenceProxyExecutesLoadingOnlyOnce()
         $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 @@ public function __sleep()
     {
         return array('id');
     }
-}
+}