Fix one to one inverse side cached entity association key generation

doctrine · Nov 13, 2015 · dc9f31d · dc9f31d
1 parent 3a44a3d
commit dc9f31d
Show file tree

Hide file tree

Showing 8 changed files with 265 additions and 13 deletions.
diff --git a/lib/Doctrine/ORM/Cache/DefaultEntityHydrator.php b/lib/Doctrine/ORM/Cache/DefaultEntityHydrator.php
@@ -80,27 +80,39 @@ public function buildCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, $e
                 continue;
             }
 
-            if (! ($assoc['type'] & ClassMetadata::TO_ONE)) {
+            if ( ! ($assoc['type'] & ClassMetadata::TO_ONE)) {
                 unset($data[$name]);
+
                 continue;
             }
 
             if ( ! isset($assoc['cache'])) {
                 $targetClassMetadata = $this->em->getClassMetadata($assoc['targetEntity']);
-                $associationIds = $this->identifierFlattener->flattenIdentifier($targetClassMetadata, $targetClassMetadata->getIdentifierValues($data[$name]));
+                $owningAssociation   = ( ! $assoc['isOwningSide'])
+                    ? $targetClassMetadata->associationMappings[$assoc['mappedBy']]
+                    : $assoc;
+                $associationIds      = $this->identifierFlattener->flattenIdentifier(
+                    $targetClassMetadata,
+                    $targetClassMetadata->getIdentifierValues($data[$name])
+                );
+
                 unset($data[$name]);
 
                 foreach ($associationIds as $fieldName => $fieldValue) {
-                    if (isset($targetClassMetadata->associationMappings[$fieldName])){
-                        $targetAssoc = $targetClassMetadata->associationMappings[$fieldName];
+                    if (isset($targetClassMetadata->fieldMappings[$fieldName])) {
+                        $fieldMapping = $targetClassMetadata->fieldMappings[$fieldName];
+
+                        $data[$owningAssociation['targetToSourceKeyColumns'][$fieldMapping['columnName']]] = $fieldValue;
+
+                        continue;
+                    }
+
+                    $targetAssoc = $targetClassMetadata->associationMappings[$fieldName];
 
-                        foreach($assoc['targetToSourceKeyColumns'] as $referencedColumn => $localColumn) {
-                            if (isset($targetAssoc['sourceToTargetKeyColumns'][$referencedColumn])) {
-                                $data[$localColumn] = $fieldValue;
-                            }
+                    foreach($assoc['targetToSourceKeyColumns'] as $referencedColumn => $localColumn) {
+                        if (isset($targetAssoc['sourceToTargetKeyColumns'][$referencedColumn])) {
+                            $data[$localColumn] = $fieldValue;
                         }
-                    } else {
-                        $data[$assoc['targetToSourceKeyColumns'][$targetClassMetadata->fieldMappings[$fieldName]['columnName']]] = $fieldValue;
                     }
                 }
 

diff --git a/tests/Doctrine/Tests/Models/Cache/Address.php b/tests/Doctrine/Tests/Models/Cache/Address.php
@@ -0,0 +1,65 @@
+<?php
+
+namespace Doctrine\Tests\Models\Cache;
+
+/**
+ * @Entity
+ * @Table("cache_client_address")
+ */
+class Address
+{
+    const CLASSNAME = __CLASS__;
+
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     */
+    protected $id;
+
+    /**
+     * @JoinColumn(name="person_id", referencedColumnName="id")
+     * @OneToOne(targetEntity="Person", inversedBy="address")
+     */
+    protected $person;
+
+    /**
+     * @Column
+     */
+    protected $location;
+
+    public function __construct($location)
+    {
+        $this->location = $location;
+    }
+
+    public function getId()
+    {
+        return $this->id;
+    }
+
+    public function setId($id)
+    {
+        $this->id = $id;
+    }
+
+    public function getPerson()
+    {
+        return $this->person;
+    }
+
+    public function setPerson(Person $person)
+    {
+        $this->person = $person;
+    }
+
+    public function getLocation()
+    {
+        return $this->location;
+    }
+
+    public function setLocation($location)
+    {
+        $this->location = $location;
+    }
+}
diff --git a/tests/Doctrine/Tests/Models/Cache/Person.php b/tests/Doctrine/Tests/Models/Cache/Person.php
@@ -0,0 +1,71 @@
+<?php
+
+namespace Doctrine\Tests\Models\Cache;
+
+/**
+ * @Entity
+ * @Table("cache_person")
+ * @Cache("NONSTRICT_READ_WRITE")
+ */
+class Person
+{
+    const CLASSNAME = __CLASS__;
+
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     */
+    public $id;
+
+    /**
+     * @Column(unique=true)
+     */
+    public $name;
+
+    /**
+     * @OneToOne(targetEntity="Address", mappedBy="person")
+     */
+    private $address;
+
+    public function __construct($name)
+    {
+        $this->name = $name;
+    }
+
+    public function getId()
+    {
+        return $this->id;
+    }
+
+    public function setId($id)
+    {
+        $this->id = $id;
+    }
+
+    public function getName()
+    {
+        return $this->name;
+    }
+
+    public function setName($name)
+    {
+        $this->name = $name;
+    }
+
+    /**
+     * @return Address
+     */
+    public function getAddress()
+    {
+        return $this->address;
+    }
+
+    /**
+     * @param Address $address
+     */
+    public function setAddress(Address $address)
+    {
+        $this->address = $address;
+    }
+}
diff --git a/tests/Doctrine/Tests/Models/Cache/TravelerProfile.php b/tests/Doctrine/Tests/Models/Cache/TravelerProfile.php
@@ -24,8 +24,8 @@ class TravelerProfile
     private $name;
 
     /**
-     * @OneToOne(targetEntity="TravelerProfileInfo", mappedBy="profile")
      * @Cache()
+     * @OneToOne(targetEntity="TravelerProfileInfo", mappedBy="profile")
      */
     private $info;
 
@@ -49,9 +49,9 @@ public function getName()
         return $this->name;
     }
 
-    public function setName($nae)
+    public function setName($name)
     {
-        $this->name = $nae;
+        $this->name = $name;
     }
 
     public function getInfo()

diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheAbstractTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheAbstractTest.php
@@ -2,6 +2,8 @@
 
 namespace Doctrine\Tests\ORM\Functional;
 
+use Doctrine\Tests\Models\Cache\Address;
+use Doctrine\Tests\Models\Cache\Person;
 use Doctrine\Tests\OrmFunctionalTestCase;
 
 use Doctrine\Tests\Models\Cache\Country;
@@ -25,6 +27,8 @@
  */
 abstract class SecondLevelCacheAbstractTest extends OrmFunctionalTestCase
 {
+    protected $people               = array();
+    protected $addresses            = array();
     protected $countries            = array();
     protected $states               = array();
     protected $cities               = array();
@@ -224,6 +228,37 @@ protected function loadFixturesAttractionsInfo()
         $this->_em->flush();
     }
 
+    protected function loadFixturesPersonWithAddress()
+    {
+        $person1  = new Person('Guilherme Blanco');
+        $address1 = new Address('Canada');
+
+        $person1->setAddress($address1);
+        $address1->setPerson($person1);
+
+        $person2  = new Person('Marco Pivetta');
+        $address2 = new Address('Germany');
+
+        $person2->setAddress($address2);
+        $address2->setPerson($person2);
+
+        $this->people[] = $person1;
+        $this->people[] = $person2;
+
+        $this->addresses[] = $address1;
+        $this->addresses[] = $address2;
+
+        foreach ($this->people as $person) {
+            $this->_em->persist($person);
+        }
+
+        foreach ($this->addresses as $address) {
+            $this->_em->persist($address);
+        }
+
+        $this->_em->flush();
+    }
+
     protected function getEntityRegion($className)
     {
         return $this->cache->getEntityCacheRegion($className)->getName();

diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToManyTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToManyTest.php
@@ -33,6 +33,7 @@ public function testPutAndLoadOneToManyRelation()
         $this->loadFixturesCountries();
         $this->loadFixturesStates();
         $this->loadFixturesCities();
+
         $this->_em->clear();
         $this->secondLevelCacheLogger->clearStats();
 
@@ -131,6 +132,7 @@ public function testLoadOneToManyCollectionFromDatabaseWhenEntityMissing()
         $this->loadFixturesCountries();
         $this->loadFixturesStates();
         $this->loadFixturesCities();
+
         $this->_em->clear();
 
         //trigger lazy load from database

diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToOneTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToOneTest.php
@@ -2,7 +2,11 @@
 
 namespace Doctrine\Tests\ORM\Functional;
 
+use Doctrine\Tests\Models\Cache\Action;
+use Doctrine\Tests\Models\Cache\Address;
 use Doctrine\Tests\Models\Cache\Client;
+use Doctrine\Tests\Models\Cache\ComplexAction;
+use Doctrine\Tests\Models\Cache\Person;
 use Doctrine\Tests\Models\Cache\Token;
 use Doctrine\Tests\Models\Cache\Traveler;
 use Doctrine\Tests\Models\Cache\TravelerProfile;
@@ -189,6 +193,67 @@ public function testPutAndLoadOneToOneBidirectionalRelation()
         $this->assertEquals($queryCount, $this->getCurrentQueryCount());
     }
 
+    public function testInverseSidePutAndLoadOneToOneBidirectionalRelation()
+    {
+        $this->loadFixturesPersonWithAddress();
+
+        $this->_em->clear();
+
+        $this->cache->evictEntityRegion(Person::CLASSNAME);
+        $this->cache->evictEntityRegion(Address::CLASSNAME);
+
+        $entity1 = $this->addresses[0]->getPerson();
+        $entity2 = $this->addresses[1]->getPerson();
+
+        $this->assertFalse($this->cache->containsEntity(Person::CLASSNAME, $entity1->getId()));
+        $this->assertFalse($this->cache->containsEntity(Person::CLASSNAME, $entity2->getId()));
+        $this->assertFalse($this->cache->containsEntity(Address::CLASSNAME, $entity1->getAddress()->getId()));
+        $this->assertFalse($this->cache->containsEntity(Address::CLASSNAME, $entity2->getAddress()->getId()));
+
+        $p1 = $this->_em->find(Person::CLASSNAME, $entity1->getId());
+        $p2 = $this->_em->find(Person::CLASSNAME, $entity2->getId());
+
+        $this->assertEquals($entity1->getId(), $p1->getId());
+        $this->assertEquals($entity1->getName(), $p1->getName());
+        $this->assertEquals($entity1->getAddress()->getId(), $p1->getAddress()->getId());
+        $this->assertEquals($entity1->getAddress()->getLocation(), $p1->getAddress()->getLocation());
+
+        $this->assertEquals($entity2->getId(), $p2->getId());
+        $this->assertEquals($entity2->getName(), $p2->getName());
+        $this->assertEquals($entity2->getAddress()->getId(), $p2->getAddress()->getId());
+        $this->assertEquals($entity2->getAddress()->getLocation(), $p2->getAddress()->getLocation());
+
+        $this->assertTrue($this->cache->containsEntity(Person::CLASSNAME, $entity1->getId()));
+        $this->assertTrue($this->cache->containsEntity(Person::CLASSNAME, $entity2->getId()));
+        // The inverse side its not cached
+        $this->assertFalse($this->cache->containsEntity(Address::CLASSNAME, $entity1->getAddress()->getId()));
+        $this->assertFalse($this->cache->containsEntity(Address::CLASSNAME, $entity2->getAddress()->getId()));
+
+        $this->_em->clear();
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $p3 = $this->_em->find(Person::CLASSNAME, $entity1->getId());
+        $p4 = $this->_em->find(Person::CLASSNAME, $entity2->getId());
+
+        $this->assertInstanceOf(Person::CLASSNAME, $p3);
+        $this->assertInstanceOf(Person::CLASSNAME, $p4);
+        $this->assertInstanceOf(Address::CLASSNAME, $p3->getAddress());
+        $this->assertInstanceOf(Address::CLASSNAME, $p4->getAddress());
+
+        $this->assertEquals($entity1->getId(), $p3->getId());
+        $this->assertEquals($entity1->getName(), $p3->getName());
+        $this->assertEquals($entity1->getAddress()->getId(), $p3->getAddress()->getId());
+        $this->assertEquals($entity1->getAddress()->getLocation(), $p3->getAddress()->getLocation());
+
+        $this->assertEquals($entity2->getId(), $p4->getId());
+        $this->assertEquals($entity2->getName(), $p4->getName());
+        $this->assertEquals($entity2->getAddress()->getId(), $p4->getAddress()->getId());
+        $this->assertEquals($entity2->getAddress()->getLocation(), $p4->getAddress()->getLocation());
+
+        $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount());
+    }
+
     public function testPutAndLoadNonCacheableOneToOne()
     {
         $this->assertNull($this->cache->getEntityCacheRegion(Client::CLASSNAME));

diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php
@@ -189,6 +189,8 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
             'Doctrine\Tests\Models\Cache\Token',
             'Doctrine\Tests\Models\Cache\Login',
             'Doctrine\Tests\Models\Cache\Client',
+            'Doctrine\Tests\Models\Cache\Person',
+            'Doctrine\Tests\Models\Cache\Address',
             'Doctrine\Tests\Models\Cache\Action',
             'Doctrine\Tests\Models\Cache\ComplexAction',
             'Doctrine\Tests\Models\Cache\AttractionInfo',