diff --git a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php index 3172fe9994b..abaef17a680 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php @@ -32,6 +32,7 @@ /** * @author Fabio B. Silva + * @author Guilherme Blanco * @since 2.5 */ abstract class AbstractCollectionPersister implements CachedCollectionPersister @@ -164,10 +165,18 @@ public function loadCollectionCache(PersistentCollection $collection, Collection public function storeCollectionCache(CollectionCacheKey $key, $elements) { /* @var $targetPersister CachedEntityPersister */ + $associationMapping = $this->sourceEntity->associationMappings[$key->association]; $targetPersister = $this->uow->getEntityPersister($this->targetEntity->rootEntityName); $targetRegion = $targetPersister->getCacheRegion(); $targetHydrator = $targetPersister->getEntityHydrator(); - $entry = $this->hydrator->buildCacheEntry($this->targetEntity, $key, $elements); + + // Only preserve ordering if association configured it + if ( ! (isset($associationMapping['indexBy']) && $associationMapping['indexBy'])) { + // Elements may be an array or a Collection + $elements = array_values(is_array($elements) ? $elements : $elements->getValues()); + } + + $entry = $this->hydrator->buildCacheEntry($this->targetEntity, $key, $elements); foreach ($entry->identifiers as $index => $entityKey) { if ($targetRegion->contains($entityKey)) { diff --git a/lib/Doctrine/ORM/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php b/lib/Doctrine/ORM/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php index fd296c63716..54c814a9a22 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php @@ -26,6 +26,7 @@ * Specific non-strict read/write cached entity persister * * @author Fabio B. Silva + * @author Guilherme Blanco * @since 2.5 */ class NonStrictReadWriteCachedEntityPersister extends AbstractEntityPersister @@ -77,13 +78,16 @@ public function afterTransactionRolledBack() */ public function delete($entity) { - $key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity)); + $key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity)); + $deleted = $this->persister->delete($entity); - if ($this->persister->delete($entity)) { + if ($deleted) { $this->region->evict($key); } $this->queuedCache['delete'][] = $key; + + return $deleted; } /** diff --git a/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php b/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php index 8413082deac..07cac510d4c 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersister.php @@ -30,6 +30,7 @@ * Specific read-write entity persister * * @author Fabio B. Silva + * @author Guilherme Blanco * @since 2.5 */ class ReadWriteCachedEntityPersister extends AbstractEntityPersister @@ -100,21 +101,24 @@ public function afterTransactionRolledBack() */ public function delete($entity) { - $key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity)); - $lock = $this->region->lock($key); + $key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity)); + $lock = $this->region->lock($key); + $deleted = $this->persister->delete($entity); - if ($this->persister->delete($entity)) { + if ($deleted) { $this->region->evict($key); } if ($lock === null) { - return; + return $deleted; } $this->queuedCache['delete'][] = array( 'lock' => $lock, 'key' => $key ); + + return $deleted; } /** diff --git a/lib/Doctrine/ORM/Persisters/Collection/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Persisters/Collection/AbstractCollectionPersister.php index 094895671c5..2e85b67f3b2 100644 --- a/lib/Doctrine/ORM/Persisters/Collection/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Persisters/Collection/AbstractCollectionPersister.php @@ -90,10 +90,6 @@ protected function isValidEntityState($entity) // If Entity is scheduled for inclusion, it is not in this collection. // We can assure that because it would have return true before on array check - if ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($entity)) { - return false; - } - - return true; + return ! ($entityState === UnitOfWork::STATE_MANAGED && $this->uow->isScheduledForInsert($entity)); } } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index a58e687eff6..3ba860b8917 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -733,7 +733,6 @@ public function computeChangeSet(ClassMetadata $class, $entity) // Look for changes in associations of the entity foreach ($class->associationMappings as $field => $assoc) { - if (($val = $class->reflFields[$field]->getValue($entity)) === null) { continue; } @@ -799,7 +798,7 @@ public function computeChangeSets() // Only MANAGED entities that are NOT SCHEDULED FOR INSERTION OR DELETION are processed here. $oid = spl_object_hash($entity); - if ( ! isset($this->entityInsertions[$oid]) && ! isset($this->entityDeletions[$oid]) && isset($this->entityStates[$oid])) { + if ( ! isset($this->entityInsertions[$oid]) && ! isset($this->entityDeletions[$oid]) && isset($this->entityStates[$oid])) { $this->computeChangeSet($class, $entity); } } @@ -826,10 +825,7 @@ private function computeAssociationChanges($assoc, $value) if ($value instanceof PersistentCollection && $value->isDirty()) { $coid = spl_object_hash($value); - if ($assoc['isOwningSide']) { - $this->collectionUpdates[$coid] = $value; - } - + $this->collectionUpdates[$coid] = $value; $this->visitedCollections[$coid] = $value; } diff --git a/tests/Doctrine/Tests/Models/Cache/State.php b/tests/Doctrine/Tests/Models/Cache/State.php index 64812869dd6..0d37e4d8faa 100644 --- a/tests/Doctrine/Tests/Models/Cache/State.php +++ b/tests/Doctrine/Tests/Models/Cache/State.php @@ -33,7 +33,7 @@ class State protected $country; /** - * @Cache + * @Cache("NONSTRICT_READ_WRITE") * @OneToMany(targetEntity="City", mappedBy="state") */ protected $cities; diff --git a/tests/Doctrine/Tests/Models/Cache/Traveler.php b/tests/Doctrine/Tests/Models/Cache/Traveler.php index 32d7cf9361a..3f720b46595 100644 --- a/tests/Doctrine/Tests/Models/Cache/Traveler.php +++ b/tests/Doctrine/Tests/Models/Cache/Traveler.php @@ -26,7 +26,7 @@ class Traveler protected $name; /** - * @Cache() + * @Cache("NONSTRICT_READ_WRITE") * @OneToMany(targetEntity="Travel", mappedBy="traveler", cascade={"persist", "remove"}, orphanRemoval=true) * * @var \Doctrine\Common\Collections\Collection diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php index 8c0225e7f0d..00fa409736f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php @@ -2,10 +2,10 @@ namespace Doctrine\Tests\ORM\Functional; +use Doctrine\Tests\Models\Cache\City; use Doctrine\Tests\Models\Cache\ComplexAction; use Doctrine\Tests\Models\Cache\Country; use Doctrine\Tests\Models\Cache\State; -use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\Tests\Models\Cache\Token; use Doctrine\Tests\Models\Cache\Action; @@ -98,6 +98,40 @@ public function testPutAndLoadManyToOneRelation() $this->assertEquals($this->states[1]->getCountry()->getName(), $c4->getCountry()->getName()); } + public function testInverseSidePutShouldEvictCollection() + { + $this->loadFixturesCountries(); + $this->loadFixturesStates(); + + $this->_em->clear(); + + $this->cache->evictEntityRegion(State::CLASSNAME); + $this->cache->evictEntityRegion(Country::CLASSNAME); + + //evict collection on add + $c3 = $this->_em->find(State::CLASSNAME, $this->states[0]->getId()); + $prev = $c3->getCities(); + $count = $prev->count(); + $city = new City("Buenos Aires", $c3); + + $c3->addCity($city); + + $this->_em->persist($city); + $this->_em->persist($c3); + $this->_em->flush(); + $this->_em->clear(); + + $state = $this->_em->find(State::CLASSNAME, $c3->getId()); + $queryCount = $this->getCurrentQueryCount(); + + // Association was cleared from EM + $this->assertNotEquals($prev, $state->getCities()); + + // New association has one more item (cache was evicted) + $this->assertEquals($count + 1, $state->getCities()->count()); + $this->assertEquals($queryCount, $this->getCurrentQueryCount()); + } + public function testShouldNotReloadWhenAssociationIsMissing() { $this->loadFixturesCountries(); diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToManyTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToManyTest.php index 3e4833dc826..829cc4056b6 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToManyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheOneToManyTest.php @@ -14,18 +14,18 @@ */ class SecondLevelCacheOneToManyTest extends SecondLevelCacheAbstractTest { - public function testShouldNotPutCollectionInverseSideOnPersist() + public function testShouldPutCollectionInverseSideOnPersist() { $this->loadFixturesCountries(); $this->loadFixturesStates(); $this->loadFixturesCities(); + $this->_em->clear(); $this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $this->states[0]->getId())); $this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $this->states[1]->getId())); - - $this->assertFalse($this->cache->containsCollection(State::CLASSNAME, 'cities', $this->states[0]->getId())); - $this->assertFalse($this->cache->containsCollection(State::CLASSNAME, 'cities', $this->states[1]->getId())); + $this->assertTrue($this->cache->containsCollection(State::CLASSNAME, 'cities', $this->states[0]->getId())); + $this->assertTrue($this->cache->containsCollection(State::CLASSNAME, 'cities', $this->states[1]->getId())); } public function testPutAndLoadOneToManyRelation() @@ -187,6 +187,7 @@ public function testOneToManyRemove() $this->loadFixturesCountries(); $this->loadFixturesStates(); $this->loadFixturesCities(); + $this->_em->clear(); $this->secondLevelCacheLogger->clearStats(); @@ -247,8 +248,8 @@ public function testOneToManyRemove() $this->_em->remove($city0); $this->_em->persist($state); $this->_em->flush(); - $this->_em->clear(); + $this->secondLevelCacheLogger->clearStats(); $queryCount = $this->getCurrentQueryCount(); @@ -261,19 +262,19 @@ public function testOneToManyRemove() $this->assertInstanceOf(City::CLASSNAME, $city1); $this->assertEquals($entity->getCities()->get(1)->getName(), $city1->getName()); - $this->assertEquals(1, $this->secondLevelCacheLogger->getHitCount()); + $this->assertEquals(2, $this->secondLevelCacheLogger->getHitCount()); $this->assertEquals(1, $this->secondLevelCacheLogger->getRegionHitCount($this->getEntityRegion(State::CLASSNAME))); - $this->assertEquals(0, $this->secondLevelCacheLogger->getRegionHitCount($this->getCollectionRegion(State::CLASSNAME, 'cities'))); + $this->assertEquals(1, $this->secondLevelCacheLogger->getRegionHitCount($this->getCollectionRegion(State::CLASSNAME, 'cities'))); - $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); + $this->assertEquals($queryCount, $this->getCurrentQueryCount()); $state->getCities()->remove(0); $this->_em->remove($city1); $this->_em->persist($state); $this->_em->flush(); - $this->_em->clear(); + $this->secondLevelCacheLogger->clearStats(); $queryCount = $this->getCurrentQueryCount(); @@ -281,9 +282,9 @@ public function testOneToManyRemove() $this->assertCount(0, $state->getCities()); - $this->assertEquals(1, $this->secondLevelCacheLogger->getHitCount()); + $this->assertEquals(2, $this->secondLevelCacheLogger->getHitCount()); $this->assertEquals(1, $this->secondLevelCacheLogger->getRegionHitCount($this->getEntityRegion(State::CLASSNAME))); - $this->assertEquals(0, $this->secondLevelCacheLogger->getRegionHitCount($this->getCollectionRegion(State::CLASSNAME, 'cities'))); + $this->assertEquals(1, $this->secondLevelCacheLogger->getRegionHitCount($this->getCollectionRegion(State::CLASSNAME, 'cities'))); } public function testOneToManyWithEmptyRelation() @@ -346,11 +347,12 @@ public function testOneToManyCount() public function testCacheInitializeCollectionWithNewObjects() { $this->_em->clear(); + $this->evictRegions(); $traveler = new Traveler("Doctrine Bot"); - for ($i=0; $i<3; ++$i) { + for ($i = 0; $i < 3; ++$i) { $traveler->getTravels()->add(new Travel($traveler)); } @@ -373,7 +375,7 @@ public function testCacheInitializeCollectionWithNewObjects() $this->assertFalse($entity->getTravels()->isInitialized()); $this->assertCount(4, $entity->getTravels()); $this->assertTrue($entity->getTravels()->isInitialized()); - $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); + $this->assertEquals($queryCount, $this->getCurrentQueryCount()); $this->_em->flush(); $this->_em->clear();