From 74e93e4cadad24a46631e76a3512134a9b9bfc02 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 00:11:59 +0100 Subject: [PATCH 01/18] Added multi-get support for second level cached collections --- .../ORM/Cache/DefaultCacheFactory.php | 22 +++- .../ORM/Cache/DefaultCollectionHydrator.php | 25 ++++- .../ORM/Cache/MultiGetCollectionHydrator.php | 106 ++++++++++++++++++ lib/Doctrine/ORM/Cache/MultiGetRegion.php | 43 +++++++ .../Cache/Region/DefaultMultiGetRegion.php | 61 ++++++++++ 5 files changed, 253 insertions(+), 4 deletions(-) create mode 100644 lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php create mode 100644 lib/Doctrine/ORM/Cache/MultiGetRegion.php create mode 100644 lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php index 79171856905..1c322ce8d45 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php +++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php @@ -21,6 +21,7 @@ namespace Doctrine\ORM\Cache; use Doctrine\Common\Cache\CacheProvider; +use Doctrine\Common\Cache\MultiGetCache; use Doctrine\ORM\Cache; use Doctrine\ORM\Cache\Region; @@ -29,6 +30,8 @@ use Doctrine\ORM\Cache\Region\DefaultRegion; use Doctrine\ORM\Cache\Region\FileLockRegion; use Doctrine\ORM\Cache\Region\UpdateTimestampCache; +use Doctrine\ORM\Cache\Region\DefaultMultiGetRegion; +use Doctrine\ORM\Cache\Persister\CachedPersister; use Doctrine\ORM\Persisters\Entity\EntityPersister; use Doctrine\ORM\Persisters\Collection\CollectionPersister; use Doctrine\ORM\Cache\Persister\Entity\ReadOnlyCachedEntityPersister; @@ -178,6 +181,19 @@ public function buildQueryCache(EntityManagerInterface $em, $regionName = null) */ public function buildCollectionHydrator(EntityManagerInterface $em, array $mapping) { + if ($mapping['cache']) { + $targetPersister = $em->getUnitOfWork()->getEntityPersister($mapping['targetEntity']); + + if ($targetPersister instanceof CachedPersister) { + + $targetRegion = $targetPersister->getCacheRegion(); + + if ($targetRegion instanceof MultiGetRegion) { + return new MultiGetCollectionHydrator($em); + } + } + } + return new DefaultCollectionHydrator($em); } @@ -202,7 +218,11 @@ public function getRegion(array $cache) $cacheAdapter->setNamespace($cache['region']); - $region = new DefaultRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])); + if ($cacheAdapter instanceof MultiGetCache) { + $region = new DefaultMultiGetRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])); + } else { + $region = new DefaultRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])); + } if ($cache['usage'] === ClassMetadata::CACHE_USAGE_READ_WRITE) { diff --git a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php index cedd891da25..9440f5c95be 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php +++ b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php @@ -81,15 +81,34 @@ public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, $targetRegion = $targetPersister->getCacheRegion(); $list = array(); + $keys = array(); foreach ($entry->identifiers as $index => $identifier) { + $keys[$index] = new EntityCacheKey($assoc['targetEntity'], $identifier); + } + - $entityEntry = $targetRegion->get(new EntityCacheKey($assoc['targetEntity'], $identifier)); + if ($targetRegion instanceof MultiGetRegion) { + $entityEntries = $targetRegion->getMulti($keys); - if ($entityEntry === null) { + if ($entityEntries === null) { return null; } - $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->resolveAssociationEntries($this->em), self::$hints); + foreach ($entityEntries as $index => $entityEntry) { + $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->data, self::$hints); + } + + } else { + foreach ($entry->identifiers as $index => $identifier) { + + $entityEntry = $targetRegion->get(new EntityCacheKey($assoc['targetEntity'], $identifier)); + + if ($entityEntry === null) { + return null; + } + + $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->resolveAssociationEntries($this->em), self::$hints); + } } array_walk($list, function($entity, $index) use ($collection) { diff --git a/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php b/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php new file mode 100644 index 00000000000..dbf5c20dcd6 --- /dev/null +++ b/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php @@ -0,0 +1,106 @@ +. + */ + +namespace Doctrine\ORM\Cache; + +use Doctrine\ORM\Query; +use Doctrine\ORM\PersistentCollection; +use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\EntityManagerInterface; + +/** + * Collection hydrator that expects a target region to be instance of {Doctrine\ORM\Cache\MultiGetRegion}, + * to enable loading the entire collection with one cache request. + * + * @since 2.5 + * @author Asmir Mustafic + */ +class MultiGetCollectionHydrator implements CollectionHydrator +{ + /** + * @var \Doctrine\ORM\EntityManagerInterface + */ + private $em; + + /** + * @var \Doctrine\ORM\UnitOfWork + */ + private $uow; + + /** + * @var array + */ + private static $hints = array(Query::HINT_CACHE_ENABLED => true); + + /** + * @param \Doctrine\ORM\EntityManagerInterface $em The entity manager. + */ + public function __construct(EntityManagerInterface $em) + { + $this->em = $em; + $this->uow = $em->getUnitOfWork(); + } + + /** + * {@inheritdoc} + */ + public function buildCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, $collection) + { + $data = array(); + + foreach ($collection as $index => $entity) { + $data[$index] = $this->uow->getEntityIdentifier($entity); + } + + return new CollectionCacheEntry($data); + } + + /** + * {@inheritdoc} + */ + public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, CollectionCacheEntry $entry, PersistentCollection $collection) + { + $assoc = $metadata->associationMappings[$key->association]; + $targetPersister = $this->uow->getEntityPersister($assoc['targetEntity']); + $targetRegion = $targetPersister->getCacheRegion(); + $list = array(); + + $keys = array(); + foreach ($entry->identifiers as $index => $identifier) { + $keys[$index] = new EntityCacheKey($assoc['targetEntity'], $identifier); + } + + $entityEntries = $targetRegion->getMulti($keys); + + if ($entityEntries === null) { + return null; + } + + foreach ($entityEntries as $index => $entityEntry) { + $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->data, self::$hints); + } + + array_walk($list, function ($entity, $index) use ($collection) { + $collection->hydrateSet($index, $entity); + }); + + return $list; + } +} diff --git a/lib/Doctrine/ORM/Cache/MultiGetRegion.php b/lib/Doctrine/ORM/Cache/MultiGetRegion.php new file mode 100644 index 00000000000..ca63837c68a --- /dev/null +++ b/lib/Doctrine/ORM/Cache/MultiGetRegion.php @@ -0,0 +1,43 @@ +. + */ + +namespace Doctrine\ORM\Cache; + +use Doctrine\ORM\Cache\CacheKey; + +/** + * Defines a region that supports multi-get reading. + * + * With one method call we can get multipe items. + * + * @since 2.5 + * @author Asmir Mustafic + */ +interface MultiGetRegion extends Region +{ + /** + * Get all items from the cache indentifed by $keys. + * It returns NULL if some elements can not be found. + * + * @param CacheKey[] $key The keys of the items to be retrieved. + * @return array The cached entries or NULL if one or more entries can not be found + */ + public function getMulti(array $keys); +} diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php new file mode 100644 index 00000000000..876c6dce30b --- /dev/null +++ b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php @@ -0,0 +1,61 @@ +. + */ + +namespace Doctrine\ORM\Cache\Region; + +use Doctrine\ORM\Cache\Lock; +use Doctrine\ORM\Cache\Region; +use Doctrine\ORM\Cache\CacheKey; +use Doctrine\ORM\Cache\CacheEntry; +use Doctrine\Common\Cache\CacheProvider; +use Doctrine\ORM\Cache\MultiGetRegion; +use Doctrine\Common\Cache\CacheMultiGet; + +/** + * A cache region that enables the retrieval of multiple elements with one call + * + * @since 2.5 + * @author Asmir Mustafic + */ +class DefaultMultiGetRegion extends DefaultRegion implements MultiGetRegion +{ + /** + * {@inheritdoc} + */ + public function getMulti(array $keys) + { + $keysToRetrieve = array(); + foreach ($keys as $index => $key) { + $keysToRetrieve[$index] = $this->name . '_' . $key->hash; + } + + $items = $this->cache->fetchMultiple($keysToRetrieve); + if (count($items) !== count($keysToRetrieve)) { + return null; + } + + $returnableItems = array(); + foreach ($keysToRetrieve as $index => $key) { + $returnableItems[$index] = $items[$key]; + } + + return $returnableItems; + } +} From b0792330e466b38a232333d501d6e49fe7331af1 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 00:13:47 +0100 Subject: [PATCH 02/18] Added test case for MultiGetRegion --- .../Tests/ORM/Cache/MultiGetRegionTest.php | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php diff --git a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php new file mode 100644 index 00000000000..fd0c0bda235 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php @@ -0,0 +1,39 @@ + + */ +class MultiGetRegionTest extends AbstractRegionTest +{ + protected function createRegion() + { + return new DefaultMultiGetRegion('default.region.test', $this->cache); + } + + public function testGetMulti() + { + $key1 = new CacheKeyMock('key.1'); + $value1 = new CacheEntryMock(array('id'=>1, 'name' => 'bar')); + + $key2 = new CacheKeyMock('key.2'); + $value2 = new CacheEntryMock(array('id'=>2, 'name' => 'bar')); + + $this->assertFalse($this->region->contains($key1)); + $this->assertFalse($this->region->contains($key2)); + + $this->region->put($key1, $value1); + $this->region->put($key2, $value2); + + $actual = $this->region->getMulti(array($key1, $key2)); + + $this->assertEquals($value1, $actual[0]); + $this->assertEquals($value2, $actual[1]); + } +} From 77c2e24215695e1b9de8aba1d9f0d07dc6b7af0d Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 00:15:27 +0100 Subject: [PATCH 03/18] Added test case for buildCachedCollectioHydrator on DefaultCacheFactory --- .../Tests/ORM/Cache/DefaultCacheFactoryTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php index c6dc3b5013f..ea5b9cf9e15 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php @@ -281,4 +281,18 @@ public function testBuildsNewNamespacedCacheInstancePerRegionInstance() $this->assertSame('foo', $fooRegion->getCache()->getNamespace()); $this->assertSame('bar', $barRegion->getCache()->getNamespace()); } + + public function testBuildCachedCollectioHydrator() + { + $em = $this->em; + $entityName = 'Doctrine\Tests\Models\Cache\State'; + $metadata = $em->getClassMetadata($entityName); + $mapping = $metadata->associationMappings['cities']; + + $mapping['cache']['usage'] = ClassMetadata::CACHE_USAGE_READ_ONLY; + + $hydrator = $this->factory->buildCollectionHydrator($em, $mapping); + + $this->assertInstanceOf('Doctrine\ORM\Cache\MultiGetCollectionHydrator', $hydrator); + } } From 779af8ce8e11273a36de5efaae6b2b21d9e7a6c1 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 10:09:13 +0100 Subject: [PATCH 04/18] Added dependency with doctrine/cache 1.4 --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 5e99449293f..83dc4bc2b90 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ "doctrine/dbal": ">=2.5-dev,<2.6-dev", "doctrine/instantiator": "~1.0.1", "doctrine/common": ">=2.5-dev,<2.6-dev", + "doctrine/cache": "~1.4", "symfony/console": "~2.5" }, "require-dev": { From 1bfa68d94f9d335c35065e64080a8d8593076d56 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 10:15:07 +0100 Subject: [PATCH 05/18] Removed dependency with Region interface --- lib/Doctrine/ORM/Cache/MultiGetRegion.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Cache/MultiGetRegion.php b/lib/Doctrine/ORM/Cache/MultiGetRegion.php index ca63837c68a..bbec206aa13 100644 --- a/lib/Doctrine/ORM/Cache/MultiGetRegion.php +++ b/lib/Doctrine/ORM/Cache/MultiGetRegion.php @@ -30,7 +30,7 @@ * @since 2.5 * @author Asmir Mustafic */ -interface MultiGetRegion extends Region +interface MultiGetRegion { /** * Get all items from the cache indentifed by $keys. From 5ec201405143ce7adfe790a16d5fd3b574c20024 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 10:27:04 +0100 Subject: [PATCH 06/18] MultiGetCollectionHydrator depends knows the multi-get region --- lib/Doctrine/ORM/Cache/DefaultCacheFactory.php | 2 +- .../ORM/Cache/MultiGetCollectionHydrator.php | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php index 1c322ce8d45..04baf0de034 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php +++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php @@ -189,7 +189,7 @@ public function buildCollectionHydrator(EntityManagerInterface $em, array $mappi $targetRegion = $targetPersister->getCacheRegion(); if ($targetRegion instanceof MultiGetRegion) { - return new MultiGetCollectionHydrator($em); + return new MultiGetCollectionHydrator($em, $targetRegion); } } } diff --git a/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php b/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php index dbf5c20dcd6..8770a48e2f9 100644 --- a/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php +++ b/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php @@ -44,6 +44,11 @@ class MultiGetCollectionHydrator implements CollectionHydrator */ private $uow; + /** + * @var MultiGetRegion + */ + private $targetRegion; + /** * @var array */ @@ -52,10 +57,11 @@ class MultiGetCollectionHydrator implements CollectionHydrator /** * @param \Doctrine\ORM\EntityManagerInterface $em The entity manager. */ - public function __construct(EntityManagerInterface $em) + public function __construct(EntityManagerInterface $em, MultiGetRegion $targetRegion) { $this->em = $em; $this->uow = $em->getUnitOfWork(); + $this->targetRegion = $targetRegion; } /** @@ -78,8 +84,6 @@ public function buildCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, CollectionCacheEntry $entry, PersistentCollection $collection) { $assoc = $metadata->associationMappings[$key->association]; - $targetPersister = $this->uow->getEntityPersister($assoc['targetEntity']); - $targetRegion = $targetPersister->getCacheRegion(); $list = array(); $keys = array(); @@ -87,7 +91,7 @@ public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, $keys[$index] = new EntityCacheKey($assoc['targetEntity'], $identifier); } - $entityEntries = $targetRegion->getMulti($keys); + $entityEntries = $this->targetRegion->getMulti($keys); if ($entityEntries === null) { return null; From 1b4eee6d0dd22d332a1776a7af8975e67938d639 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 11:52:39 +0100 Subject: [PATCH 07/18] Fixed strange test case with CmsUser and second-level cache --- lib/Doctrine/ORM/Cache/DefaultCacheFactory.php | 12 ++++++------ .../Tests/ORM/Functional/ExtraLazyCollectionTest.php | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php index 04baf0de034..88a80a1a924 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php +++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php @@ -184,13 +184,13 @@ public function buildCollectionHydrator(EntityManagerInterface $em, array $mappi if ($mapping['cache']) { $targetPersister = $em->getUnitOfWork()->getEntityPersister($mapping['targetEntity']); - if ($targetPersister instanceof CachedPersister) { - - $targetRegion = $targetPersister->getCacheRegion(); + if (! ($targetPersister instanceof CachedPersister)) { + throw CacheException::nonCacheableEntity($mapping['targetEntity']); + } + $targetRegion = $targetPersister->getCacheRegion(); - if ($targetRegion instanceof MultiGetRegion) { - return new MultiGetCollectionHydrator($em, $targetRegion); - } + if ($targetRegion instanceof MultiGetRegion) { + return new MultiGetCollectionHydrator($em, $targetRegion); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 3bede3f15a3..2807ccfd7c1 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -39,6 +39,10 @@ public function setUp() $class->associationMappings['phonenumbers']['fetch'] = ClassMetadataInfo::FETCH_EXTRA_LAZY; $class->associationMappings['phonenumbers']['indexBy'] = 'phonenumber'; + unset($class->associationMappings['phonenumbers']['cache']); + unset($class->associationMappings['articles']['cache']); + unset($class->associationMappings['users']['cache']); + $class = $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsGroup'); $class->associationMappings['users']['fetch'] = ClassMetadataInfo::FETCH_EXTRA_LAZY; $class->associationMappings['users']['indexBy'] = 'username'; From e73bd9e9bbd3e93e19fc0932036882f6d5d0e4f9 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 18:35:28 +0100 Subject: [PATCH 08/18] New buildCacheEntry way bo build a entry for a cached collection --- .../ORM/Cache/DefaultCollectionHydrator.php | 28 ++--- .../ORM/Cache/MultiGetCollectionHydrator.php | 110 ------------------ lib/Doctrine/ORM/Cache/MultiGetRegion.php | 6 +- .../AbstractCollectionPersister.php | 3 +- .../Cache/Region/DefaultMultiGetRegion.php | 7 +- .../Tests/ORM/Cache/MultiGetRegionTest.php | 6 +- 6 files changed, 18 insertions(+), 142 deletions(-) delete mode 100644 lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php diff --git a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php index 9440f5c95be..716bd846ff3 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php +++ b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php @@ -65,9 +65,8 @@ public function buildCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key $data = array(); foreach ($collection as $index => $entity) { - $data[$index] = $this->uow->getEntityIdentifier($entity); + $data[$index] = new EntityCacheKey($metadata->name, $this->uow->getEntityIdentifier($entity)); } - return new CollectionCacheEntry($data); } @@ -81,36 +80,25 @@ public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, $targetRegion = $targetPersister->getCacheRegion(); $list = array(); - $keys = array(); - foreach ($entry->identifiers as $index => $identifier) { - $keys[$index] = new EntityCacheKey($assoc['targetEntity'], $identifier); - } - - if ($targetRegion instanceof MultiGetRegion) { - $entityEntries = $targetRegion->getMulti($keys); + $entityEntries = $targetRegion->getMulti($entry); if ($entityEntries === null) { return null; } - - foreach ($entityEntries as $index => $entityEntry) { - $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->data, self::$hints); - } - } else { + $entityEntries = array(); foreach ($entry->identifiers as $index => $identifier) { - - $entityEntry = $targetRegion->get(new EntityCacheKey($assoc['targetEntity'], $identifier)); - - if ($entityEntry === null) { + if (null === ($entityEntries[$index] = $targetRegion->get($identifier))) { return null; } - - $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->resolveAssociationEntries($this->em), self::$hints); } } + foreach ($entityEntries as $index => $entityEntry) { + $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->data, self::$hints); + } + array_walk($list, function($entity, $index) use ($collection) { $collection->hydrateSet($index, $entity); }); diff --git a/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php b/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php deleted file mode 100644 index 8770a48e2f9..00000000000 --- a/lib/Doctrine/ORM/Cache/MultiGetCollectionHydrator.php +++ /dev/null @@ -1,110 +0,0 @@ -. - */ - -namespace Doctrine\ORM\Cache; - -use Doctrine\ORM\Query; -use Doctrine\ORM\PersistentCollection; -use Doctrine\ORM\Mapping\ClassMetadata; -use Doctrine\ORM\EntityManagerInterface; - -/** - * Collection hydrator that expects a target region to be instance of {Doctrine\ORM\Cache\MultiGetRegion}, - * to enable loading the entire collection with one cache request. - * - * @since 2.5 - * @author Asmir Mustafic - */ -class MultiGetCollectionHydrator implements CollectionHydrator -{ - /** - * @var \Doctrine\ORM\EntityManagerInterface - */ - private $em; - - /** - * @var \Doctrine\ORM\UnitOfWork - */ - private $uow; - - /** - * @var MultiGetRegion - */ - private $targetRegion; - - /** - * @var array - */ - private static $hints = array(Query::HINT_CACHE_ENABLED => true); - - /** - * @param \Doctrine\ORM\EntityManagerInterface $em The entity manager. - */ - public function __construct(EntityManagerInterface $em, MultiGetRegion $targetRegion) - { - $this->em = $em; - $this->uow = $em->getUnitOfWork(); - $this->targetRegion = $targetRegion; - } - - /** - * {@inheritdoc} - */ - public function buildCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, $collection) - { - $data = array(); - - foreach ($collection as $index => $entity) { - $data[$index] = $this->uow->getEntityIdentifier($entity); - } - - return new CollectionCacheEntry($data); - } - - /** - * {@inheritdoc} - */ - public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, CollectionCacheEntry $entry, PersistentCollection $collection) - { - $assoc = $metadata->associationMappings[$key->association]; - $list = array(); - - $keys = array(); - foreach ($entry->identifiers as $index => $identifier) { - $keys[$index] = new EntityCacheKey($assoc['targetEntity'], $identifier); - } - - $entityEntries = $this->targetRegion->getMulti($keys); - - if ($entityEntries === null) { - return null; - } - - foreach ($entityEntries as $index => $entityEntry) { - $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->data, self::$hints); - } - - array_walk($list, function ($entity, $index) use ($collection) { - $collection->hydrateSet($index, $entity); - }); - - return $list; - } -} diff --git a/lib/Doctrine/ORM/Cache/MultiGetRegion.php b/lib/Doctrine/ORM/Cache/MultiGetRegion.php index bbec206aa13..96ca5867b6a 100644 --- a/lib/Doctrine/ORM/Cache/MultiGetRegion.php +++ b/lib/Doctrine/ORM/Cache/MultiGetRegion.php @@ -20,8 +20,6 @@ namespace Doctrine\ORM\Cache; -use Doctrine\ORM\Cache\CacheKey; - /** * Defines a region that supports multi-get reading. * @@ -36,8 +34,8 @@ interface MultiGetRegion * Get all items from the cache indentifed by $keys. * It returns NULL if some elements can not be found. * - * @param CacheKey[] $key The keys of the items to be retrieved. + * @param CollectionCacheEntry[] $collection The collection of the items to be retrieved. * @return array The cached entries or NULL if one or more entries can not be found */ - public function getMulti(array $keys); + public function getMulti(CollectionCacheEntry $collection); } diff --git a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php index 33d437253a1..f6215676307 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php @@ -167,8 +167,7 @@ public function storeCollectionCache(CollectionCacheKey $key, $elements) $targetHydrator = $targetPersister->getEntityHydrator(); $entry = $this->hydrator->buildCacheEntry($this->targetEntity, $key, $elements); - foreach ($entry->identifiers as $index => $identifier) { - $entityKey = new EntityCacheKey($this->targetEntity->rootEntityName, $identifier); + foreach ($entry->identifiers as $index => $entityKey) { if ($targetRegion->contains($entityKey)) { continue; diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php index 876c6dce30b..ef7b2b912fb 100644 --- a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php @@ -26,7 +26,7 @@ use Doctrine\ORM\Cache\CacheEntry; use Doctrine\Common\Cache\CacheProvider; use Doctrine\ORM\Cache\MultiGetRegion; -use Doctrine\Common\Cache\CacheMultiGet; +use Doctrine\ORM\Cache\CollectionCacheEntry; /** * A cache region that enables the retrieval of multiple elements with one call @@ -39,10 +39,10 @@ class DefaultMultiGetRegion extends DefaultRegion implements MultiGetRegion /** * {@inheritdoc} */ - public function getMulti(array $keys) + public function getMulti(CollectionCacheEntry $collection) { $keysToRetrieve = array(); - foreach ($keys as $index => $key) { + foreach ($collection->identifiers as $index => $key) { $keysToRetrieve[$index] = $this->name . '_' . $key->hash; } @@ -55,7 +55,6 @@ public function getMulti(array $keys) foreach ($keysToRetrieve as $index => $key) { $returnableItems[$index] = $items[$key]; } - return $returnableItems; } } diff --git a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php index fd0c0bda235..00ad09e026f 100644 --- a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php @@ -6,6 +6,8 @@ use Doctrine\Tests\Mocks\CacheEntryMock; use Doctrine\Tests\Mocks\CacheKeyMock; use Doctrine\ORM\Cache\Region\DefaultMultiGetRegion; +use Doctrine\ORM\Cache\CollectionCacheEntry; +use Doctrine\ORM\Cache\EntityCacheKey; /** * @author Asmir Mustafic @@ -21,7 +23,7 @@ public function testGetMulti() { $key1 = new CacheKeyMock('key.1'); $value1 = new CacheEntryMock(array('id'=>1, 'name' => 'bar')); - + $key2 = new CacheKeyMock('key.2'); $value2 = new CacheEntryMock(array('id'=>2, 'name' => 'bar')); @@ -31,7 +33,7 @@ public function testGetMulti() $this->region->put($key1, $value1); $this->region->put($key2, $value2); - $actual = $this->region->getMulti(array($key1, $key2)); + $actual = $this->region->getMulti(new CollectionCacheEntry(array($key1, $key2))); $this->assertEquals($value1, $actual[0]); $this->assertEquals($value2, $actual[1]); From 3f64f3252bb51d9f18ffc680ce64ccb7f90d13c2 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 17 Jan 2015 18:37:06 +0100 Subject: [PATCH 09/18] Changed some tests to be compatible with the new implementation of multiget region --- .../ORM/Cache/DefaultCacheFactory.php | 24 ++++--------------- .../ORM/Cache/DefaultCacheFactoryTest.php | 14 ----------- .../Cache/DefaultCollectionHydratorTest.php | 7 +++--- 3 files changed, 9 insertions(+), 36 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php index 88a80a1a924..b3d6e0f46cf 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php +++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php @@ -181,20 +181,8 @@ public function buildQueryCache(EntityManagerInterface $em, $regionName = null) */ public function buildCollectionHydrator(EntityManagerInterface $em, array $mapping) { - if ($mapping['cache']) { - $targetPersister = $em->getUnitOfWork()->getEntityPersister($mapping['targetEntity']); - - if (! ($targetPersister instanceof CachedPersister)) { - throw CacheException::nonCacheableEntity($mapping['targetEntity']); - } - $targetRegion = $targetPersister->getCacheRegion(); - - if ($targetRegion instanceof MultiGetRegion) { - return new MultiGetCollectionHydrator($em, $targetRegion); - } - } - - return new DefaultCollectionHydrator($em); + $targetPersister = $em->getUnitOfWork()->getEntityPersister($mapping['targetEntity']); + return new DefaultCollectionHydrator($em, $targetPersister->getCacheRegion()); } /** @@ -218,11 +206,9 @@ public function getRegion(array $cache) $cacheAdapter->setNamespace($cache['region']); - if ($cacheAdapter instanceof MultiGetCache) { - $region = new DefaultMultiGetRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])); - } else { - $region = new DefaultRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])); - } + $region = ($cacheAdapter instanceof MultiGetCache) + ? new DefaultMultiGetRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])) + : new DefaultRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])); if ($cache['usage'] === ClassMetadata::CACHE_USAGE_READ_WRITE) { diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php index ea5b9cf9e15..cb872e52077 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php @@ -64,7 +64,6 @@ public function testBuildCachedEntityPersisterReadOnly() ->with($this->equalTo($metadata->cache)) ->will($this->returnValue($region)); - $cachedPersister = $this->factory->buildCachedEntityPersister($em, $persister, $metadata); $this->assertInstanceOf('Doctrine\ORM\Cache\Persister\Entity\CachedEntityPersister', $cachedPersister); @@ -282,17 +281,4 @@ public function testBuildsNewNamespacedCacheInstancePerRegionInstance() $this->assertSame('bar', $barRegion->getCache()->getNamespace()); } - public function testBuildCachedCollectioHydrator() - { - $em = $this->em; - $entityName = 'Doctrine\Tests\Models\Cache\State'; - $metadata = $em->getClassMetadata($entityName); - $mapping = $metadata->associationMappings['cities']; - - $mapping['cache']['usage'] = ClassMetadata::CACHE_USAGE_READ_ONLY; - - $hydrator = $this->factory->buildCollectionHydrator($em, $mapping); - - $this->assertInstanceOf('Doctrine\ORM\Cache\MultiGetCollectionHydrator', $hydrator); - } } diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultCollectionHydratorTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultCollectionHydratorTest.php index 74082d8a7bb..ed0456e3908 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultCollectionHydratorTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultCollectionHydratorTest.php @@ -29,7 +29,8 @@ protected function setUp() $this->enableSecondLevelCache(); parent::setUp(); - $this->structure = new DefaultCollectionHydrator($this->_em); + $targetPersister = $this->_em->getUnitOfWork()->getEntityPersister(City::CLASSNAME); + $this->structure = new DefaultCollectionHydrator($this->_em, $targetPersister); } public function testImplementsCollectionEntryStructure() @@ -41,8 +42,8 @@ public function testLoadCacheCollection() { $targetRegion = $this->_em->getCache()->getEntityCacheRegion(City::CLASSNAME); $entry = new CollectionCacheEntry(array( - array('id'=>31), - array('id'=>32), + new EntityCacheKey(City::CLASSNAME, array('id'=>31)), + new EntityCacheKey(City::CLASSNAME, array('id'=>32)), )); $targetRegion->put(new EntityCacheKey(City::CLASSNAME, array('id'=>31)), new EntityCacheEntry(City::CLASSNAME, array('id'=>31, 'name'=>'Foo'))); From 3c5a794691e8df8b7c57da52f81a79b0dea87c39 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:10:49 +0100 Subject: [PATCH 10/18] #954 DDC-2982 - Making cache `Region` always a `MultiGetRegion` (no need to segregate the interface here) --- .../ORM/Cache/CollectionCacheEntry.php | 8 +++-- lib/Doctrine/ORM/Cache/MultiGetRegion.php | 7 ++-- lib/Doctrine/ORM/Cache/Region.php | 2 +- .../Cache/Region/DefaultMultiGetRegion.php | 4 +-- .../ORM/Cache/Region/DefaultRegion.php | 32 +++++++++++++++++++ .../ORM/Cache/Region/FileLockRegion.php | 13 ++++++++ .../Doctrine/Tests/Mocks/CacheRegionMock.php | 11 +++++++ .../Tests/Mocks/ConcurrentRegionMock.php | 13 ++++++++ .../Tests/ORM/Cache/MultiGetRegionTest.php | 2 +- .../AbstractCollectionPersisterTest.php | 9 ++++-- ...ReadWriteCachedCollectionPersisterTest.php | 1 + .../Entity/AbstractEntityPersisterTest.php | 10 ++++-- .../ReadWriteCachedEntityPersisterTest.php | 1 + 13 files changed, 98 insertions(+), 15 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/CollectionCacheEntry.php b/lib/Doctrine/ORM/Cache/CollectionCacheEntry.php index 58c8757e0cc..6c634ce311d 100644 --- a/lib/Doctrine/ORM/Cache/CollectionCacheEntry.php +++ b/lib/Doctrine/ORM/Cache/CollectionCacheEntry.php @@ -31,12 +31,12 @@ class CollectionCacheEntry implements CacheEntry /** * READ-ONLY: Public only for performance reasons, it should be considered immutable. * - * @var array The list of entity identifiers hold by the collection + * @var CacheKey[] The list of entity identifiers hold by the collection */ public $identifiers; /** - * @param array $identifiers List of entity identifiers hold by the collection + * @param CacheKey[] $identifiers List of entity identifiers hold by the collection */ public function __construct(array $identifiers) { @@ -46,9 +46,11 @@ public function __construct(array $identifiers) /** * Creates a new CollectionCacheEntry * - * This method allow Doctrine\Common\Cache\PhpFileCache compatibility + * This method allows for Doctrine\Common\Cache\PhpFileCache compatibility * * @param array $values array containing property values + * + * @return self */ public static function __set_state(array $values) { diff --git a/lib/Doctrine/ORM/Cache/MultiGetRegion.php b/lib/Doctrine/ORM/Cache/MultiGetRegion.php index 96ca5867b6a..6de9c888d62 100644 --- a/lib/Doctrine/ORM/Cache/MultiGetRegion.php +++ b/lib/Doctrine/ORM/Cache/MultiGetRegion.php @@ -34,8 +34,9 @@ interface MultiGetRegion * Get all items from the cache indentifed by $keys. * It returns NULL if some elements can not be found. * - * @param CollectionCacheEntry[] $collection The collection of the items to be retrieved. - * @return array The cached entries or NULL if one or more entries can not be found + * @param CollectionCacheEntry $collection The collection of the items to be retrieved. + * + * @return CacheEntry[]|null The cached entries or NULL if one or more entries can not be found */ - public function getMulti(CollectionCacheEntry $collection); + public function getMultiple(CollectionCacheEntry $collection); } diff --git a/lib/Doctrine/ORM/Cache/Region.php b/lib/Doctrine/ORM/Cache/Region.php index 894fd2f9b92..3bffbbce747 100644 --- a/lib/Doctrine/ORM/Cache/Region.php +++ b/lib/Doctrine/ORM/Cache/Region.php @@ -26,7 +26,7 @@ * @since 2.5 * @author Fabio B. Silva */ -interface Region +interface Region extends MultiGetRegion { /** * Retrieve the name of this region. diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php index ef7b2b912fb..b037732a8b5 100644 --- a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php @@ -34,12 +34,12 @@ * @since 2.5 * @author Asmir Mustafic */ -class DefaultMultiGetRegion extends DefaultRegion implements MultiGetRegion +class DefaultMultiGetRegion extends DefaultRegion { /** * {@inheritdoc} */ - public function getMulti(CollectionCacheEntry $collection) + public function getMultiple(CollectionCacheEntry $collection) { $keysToRetrieve = array(); foreach ($collection->identifiers as $index => $key) { diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php index a1362989f33..c5758f0ac42 100644 --- a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php @@ -20,6 +20,7 @@ namespace Doctrine\ORM\Cache\Region; +use Doctrine\ORM\Cache\CollectionCacheEntry; use Doctrine\ORM\Cache\Lock; use Doctrine\ORM\Cache\Region; use Doctrine\ORM\Cache\CacheKey; @@ -93,6 +94,37 @@ public function get(CacheKey $key) return $this->cache->fetch($this->name . '_' . $key->hash) ?: null; } + /** + * {@inheritdoc} + */ + public function getMultiple(CollectionCacheEntry $collection) + { + $keysToRetrieve = array(); + + foreach ($collection->identifiers as $index => $key) { + $keysToRetrieve[$index] = $this->name . '_' . $key->hash; + } + + $items = array_filter( + array_map([$this->cache, 'fetch'], $keysToRetrieve), + function ($retrieved) { + return false !== $retrieved; + } + ); + + if (count($items) !== count($keysToRetrieve)) { + return null; + } + + $returnableItems = array(); + + foreach ($keysToRetrieve as $index => $key) { + $returnableItems[$index] = $items[$key]; + } + + return $returnableItems; + } + /** * {@inheritdoc} */ diff --git a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php index 083329c5540..69167bc901a 100644 --- a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php @@ -20,6 +20,7 @@ namespace Doctrine\ORM\Cache\Region; +use Doctrine\ORM\Cache\CollectionCacheEntry; use Doctrine\ORM\Cache\Lock; use Doctrine\ORM\Cache\Region; use Doctrine\ORM\Cache\CacheKey; @@ -172,6 +173,18 @@ public function get(CacheKey $key) return $this->region->get($key); } + /** + * {@inheritdoc} + */ + public function getMultiple(CollectionCacheEntry $collection) + { + if (array_filter(array_map([$this, 'isLocked'], $collection->identifiers))) { + return null; + } + + return $this->region->getMultiple($collection); + } + /** * {inheritdoc} */ diff --git a/tests/Doctrine/Tests/Mocks/CacheRegionMock.php b/tests/Doctrine/Tests/Mocks/CacheRegionMock.php index 05366379e99..519e724941c 100644 --- a/tests/Doctrine/Tests/Mocks/CacheRegionMock.php +++ b/tests/Doctrine/Tests/Mocks/CacheRegionMock.php @@ -4,6 +4,7 @@ use Doctrine\ORM\Cache\CacheEntry; use Doctrine\ORM\Cache\CacheKey; +use Doctrine\ORM\Cache\CollectionCacheEntry; use Doctrine\ORM\Cache\Lock; use Doctrine\ORM\Cache\Region; @@ -94,6 +95,16 @@ public function get(CacheKey $key) return $this->getReturn(__FUNCTION__, null); } + /** + * {@inheritdoc} + */ + public function getMultiple(CollectionCacheEntry $collection) + { + $this->calls[__FUNCTION__][] = array('collection' => $collection); + + return $this->getReturn(__FUNCTION__, null); + } + /** * {@inheritdoc} */ diff --git a/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php b/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php index fc488b4b2a2..82685b7d798 100644 --- a/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php +++ b/tests/Doctrine/Tests/Mocks/ConcurrentRegionMock.php @@ -3,6 +3,7 @@ namespace Doctrine\Tests\Mocks; +use Doctrine\ORM\Cache\CollectionCacheEntry; use Doctrine\ORM\Cache\ConcurrentRegion; use Doctrine\ORM\Cache\LockException; use Doctrine\ORM\Cache\CacheEntry; @@ -131,6 +132,18 @@ public function get(CacheKey $key) return $this->region->get($key); } + /** + * {@inheritdoc} + */ + public function getMultiple(CollectionCacheEntry $collection) + { + $this->calls[__FUNCTION__][] = array('collection' => $collection); + + $this->throwException(__FUNCTION__); + + return $this->region->getMultiple($collection); + } + /** * {@inheritdoc} */ diff --git a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php index 00ad09e026f..4c3258a1292 100644 --- a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php @@ -33,7 +33,7 @@ public function testGetMulti() $this->region->put($key1, $value1); $this->region->put($key2, $value2); - $actual = $this->region->getMulti(new CollectionCacheEntry(array($key1, $key2))); + $actual = $this->region->getMultiple(new CollectionCacheEntry(array($key1, $key2))); $this->assertEquals($value1, $actual[0]); $this->assertEquals($value2, $actual[1]); diff --git a/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php b/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php index c3db83a44e7..17e7c4d7e3f 100644 --- a/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php @@ -38,6 +38,7 @@ abstract class AbstractCollectionPersisterTest extends OrmTestCase 'getName', 'contains', 'get', + 'getMultiple', 'put', 'evict', 'evictAll' @@ -56,6 +57,7 @@ abstract class AbstractCollectionPersisterTest extends OrmTestCase 'removeElement', 'removeKey', 'get', + 'getMultiple', 'loadCriteria' ); @@ -65,7 +67,7 @@ abstract class AbstractCollectionPersisterTest extends OrmTestCase * @param \Doctrine\ORM\Cache\Region $region * @param array $mapping * - * @return Doctrine\ORM\Cache\Persister\Collection\AbstractCollectionPersister + * @return \Doctrine\ORM\Cache\Persister\Collection\AbstractCollectionPersister */ abstract protected function createPersister(EntityManager $em, CollectionPersister $persister, Region $region, array $mapping); @@ -77,7 +79,10 @@ protected function setUp() $this->em = $this->_getTestEntityManager(); $this->region = $this->createRegion(); - $this->collectionPersister = $this->getMock('Doctrine\ORM\Persisters\Collection\CollectionPersister', $this->collectionPersisterMockMethods); + $this->collectionPersister = $this->getMock( + 'Doctrine\ORM\Persisters\Collection\CollectionPersister', + $this->collectionPersisterMockMethods + ); } /** diff --git a/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/ReadWriteCachedCollectionPersisterTest.php b/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/ReadWriteCachedCollectionPersisterTest.php index 0bb6f58ed12..529da203fa0 100644 --- a/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/ReadWriteCachedCollectionPersisterTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/ReadWriteCachedCollectionPersisterTest.php @@ -19,6 +19,7 @@ class ReadWriteCachedCollectionPersisterTest extends AbstractCollectionPersister 'getName', 'contains', 'get', + 'getMultiple', 'put', 'evict', 'evictAll', diff --git a/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php b/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php index 116339c87a1..662a9dfe6fa 100644 --- a/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php @@ -41,6 +41,7 @@ abstract class AbstractEntityPersisterTest extends OrmTestCase 'getName', 'contains', 'get', + 'getMultiple', 'put', 'evict', 'evictAll' @@ -85,7 +86,7 @@ abstract class AbstractEntityPersisterTest extends OrmTestCase * @param \Doctrine\ORM\Cache\Region $region * @param \Doctrine\ORM\Mapping\ClassMetadata $metadata * - * @return Doctrine\ORM\Cache\Persister\Entity\AbstractEntityPersister + * @return \Doctrine\ORM\Cache\Persister\Entity\AbstractEntityPersister */ abstract protected function createPersister(EntityManager $em, EntityPersister $persister, Region $region, ClassMetadata $metadata); @@ -97,7 +98,10 @@ protected function setUp() $this->em = $this->_getTestEntityManager(); $this->region = $this->createRegion(); - $this->entityPersister = $this->getMock('Doctrine\ORM\Persisters\Entity\EntityPersister', $this->entityPersisterMockMethods); + $this->entityPersister = $this->getMock( + 'Doctrine\ORM\Persisters\Entity\EntityPersister', + $this->entityPersisterMockMethods + ); } /** @@ -109,7 +113,7 @@ protected function createRegion() } /** - * @return Doctrine\ORM\Cache\Persister\AbstractEntityPersister + * @return \Doctrine\ORM\Cache\Persister\AbstractEntityPersister */ protected function createPersisterDefault() { diff --git a/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersisterTest.php b/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersisterTest.php index 1f598a903a5..054bf951dc7 100644 --- a/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersisterTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/ReadWriteCachedEntityPersisterTest.php @@ -20,6 +20,7 @@ class ReadWriteCachedEntityPersisterTest extends AbstractEntityPersisterTest 'getName', 'contains', 'get', + 'getMultiple', 'put', 'evict', 'evictAll', From 8ddcc4b270105f8ce48dc93c9c1a32f62a323b52 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:11:14 +0100 Subject: [PATCH 11/18] #954 DDC-2982 - No need to check if a `Region` is a `MultiGetRegion` --- .../ORM/Cache/DefaultCollectionHydrator.php | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php index 716bd846ff3..5ca1e59df65 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php +++ b/lib/Doctrine/ORM/Cache/DefaultCollectionHydrator.php @@ -76,25 +76,18 @@ public function buildCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key public function loadCacheEntry(ClassMetadata $metadata, CollectionCacheKey $key, CollectionCacheEntry $entry, PersistentCollection $collection) { $assoc = $metadata->associationMappings[$key->association]; + /* @var $targetPersister \Doctrine\ORM\Cache\Persister\CachedPersister */ $targetPersister = $this->uow->getEntityPersister($assoc['targetEntity']); $targetRegion = $targetPersister->getCacheRegion(); $list = array(); - if ($targetRegion instanceof MultiGetRegion) { - $entityEntries = $targetRegion->getMulti($entry); - - if ($entityEntries === null) { - return null; - } - } else { - $entityEntries = array(); - foreach ($entry->identifiers as $index => $identifier) { - if (null === ($entityEntries[$index] = $targetRegion->get($identifier))) { - return null; - } - } + $entityEntries = $targetRegion->getMultiple($entry); + + if ($entityEntries === null) { + return null; } + /* @var $entityEntries \Doctrine\ORM\Cache\EntityCacheEntry[] */ foreach ($entityEntries as $index => $entityEntry) { $list[$index] = $this->uow->createEntity($entityEntry->class, $entityEntry->data, self::$hints); } From 95c6cca336b98cf6ac7efe78a29163851069a4d2 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:17:07 +0100 Subject: [PATCH 12/18] #954 DDC-2982 - Minor CS fixes/IDE hints --- lib/Doctrine/ORM/Cache/DefaultCacheFactory.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php index b3d6e0f46cf..c1547e52eb5 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php +++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php @@ -181,8 +181,13 @@ public function buildQueryCache(EntityManagerInterface $em, $regionName = null) */ public function buildCollectionHydrator(EntityManagerInterface $em, array $mapping) { + /* @var $targetPersister \Doctrine\ORM\Cache\Persister\CachedPersister */ $targetPersister = $em->getUnitOfWork()->getEntityPersister($mapping['targetEntity']); - return new DefaultCollectionHydrator($em, $targetPersister->getCacheRegion()); + + return new DefaultCollectionHydrator( + $em, + $targetPersister->getCacheRegion() + ); } /** From 0e4a7caf0b79f49ba4595509f09cea7b9796cb54 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:30:20 +0100 Subject: [PATCH 13/18] #954 DDC-2982 - Evicting all cache entries is not supported with a generic cache adapter --- tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php index f449cfe6de9..94cc99e35b8 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php @@ -60,4 +60,16 @@ public function testDoesNotModifyCacheNamespace() $this->assertSame('foo', $cache->getNamespace()); } + + public function testEvictAllWithGenericCacheThrowsUnsupportedException() + { + /* @var $cache \Doctrine\Common\Cache\Cache */ + $cache = $this->getMock('Doctrine\Common\Cache\Cache'); + + $region = new DefaultRegion('foo', $cache); + + $this->setExpectedException('BadMethodCallException'); + + $region->evictAll(); + } } \ No newline at end of file From 564624814b449c93d0340def5840bb029b5766ec Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:30:37 +0100 Subject: [PATCH 14/18] #954 DDC-2982 - Evicting all cache entries is not supported with a generic cache adapter --- .../ORM/Cache/Region/DefaultRegion.php | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php index c5758f0ac42..f4525ee1131 100644 --- a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php @@ -20,12 +20,13 @@ namespace Doctrine\ORM\Cache\Region; +use Doctrine\Common\Cache\Cache as CacheAdapter; +use Doctrine\Common\Cache\ClearableCache; +use Doctrine\ORM\Cache\CacheEntry; +use Doctrine\ORM\Cache\CacheKey; use Doctrine\ORM\Cache\CollectionCacheEntry; use Doctrine\ORM\Cache\Lock; use Doctrine\ORM\Cache\Region; -use Doctrine\ORM\Cache\CacheKey; -use Doctrine\ORM\Cache\CacheEntry; -use Doctrine\Common\Cache\CacheProvider; /** * The simplest cache region compatible with all doctrine-cache drivers. @@ -36,7 +37,7 @@ class DefaultRegion implements Region { /** - * @var \Doctrine\Common\Cache\CacheProvider + * @var CacheAdapter */ protected $cache; @@ -51,11 +52,11 @@ class DefaultRegion implements Region protected $lifetime = 0; /** - * @param string $name - * @param \Doctrine\Common\Cache\CacheProvider $cache - * @param integer $lifetime + * @param string $name + * @param CacheAdapter $cache + * @param integer $lifetime */ - public function __construct($name, CacheProvider $cache, $lifetime = 0) + public function __construct($name, CacheAdapter $cache, $lifetime = 0) { $this->cache = $cache; $this->name = (string) $name; @@ -146,6 +147,13 @@ public function evict(CacheKey $key) */ public function evictAll() { + if (! $this->cache instanceof ClearableCache) { + throw new \BadMethodCallException(sprintf( + 'Clearing all cache entries is not supported by the supplied cache adapter of type %s', + get_class($this->cache) + )); + } + return $this->cache->deleteAll(); } } From 95fe03b18265cf74248504f3cb9a26e13130cc4b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:42:15 +0100 Subject: [PATCH 15/18] #954 DDC-2982 - Coverage for different instantiation of single-/multi-get cache regions --- .../ORM/Cache/DefaultCacheFactoryTest.php | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php index cb872e52077..cd41726d5d9 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultCacheFactoryTest.php @@ -281,4 +281,36 @@ public function testBuildsNewNamespacedCacheInstancePerRegionInstance() $this->assertSame('bar', $barRegion->getCache()->getNamespace()); } + public function testBuildsDefaultCacheRegionFromGenericCacheRegion() + { + /* @var $cache \Doctrine\Common\Cache\Cache */ + $cache = $this->getMock('Doctrine\Common\Cache\Cache'); + + $factory = new DefaultCacheFactory($this->regionsConfig, $cache); + + $this->assertInstanceOf( + 'Doctrine\ORM\Cache\Region\DefaultRegion', + $factory->getRegion(array( + 'region' => 'bar', + 'usage' => ClassMetadata::CACHE_USAGE_READ_ONLY, + )) + ); + } + + public function testBuildsMultiGetCacheRegionFromGenericCacheRegion() + { + /* @var $cache \Doctrine\Common\Cache\CacheProvider */ + $cache = $this->getMockForAbstractClass('Doctrine\Common\Cache\CacheProvider'); + + $factory = new DefaultCacheFactory($this->regionsConfig, $cache); + + $this->assertInstanceOf( + 'Doctrine\ORM\Cache\Region\DefaultMultiGetRegion', + $factory->getRegion(array( + 'region' => 'bar', + 'usage' => ClassMetadata::CACHE_USAGE_READ_ONLY, + )) + ); + } + } From 624b98544a82b81b5d64b82110afdb9362a82891 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:42:49 +0100 Subject: [PATCH 16/18] #954 DDC-2982 - `DefaultCacheFactory` now supports generic `Doctrine\Common\Cache\Cache` instances --- .../ORM/Cache/DefaultCacheFactory.php | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php index c1547e52eb5..69d2fd1c16b 100644 --- a/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php +++ b/lib/Doctrine/ORM/Cache/DefaultCacheFactory.php @@ -20,26 +20,25 @@ namespace Doctrine\ORM\Cache; +use Doctrine\Common\Cache\Cache as CacheAdapter; use Doctrine\Common\Cache\CacheProvider; use Doctrine\Common\Cache\MultiGetCache; - use Doctrine\ORM\Cache; +use Doctrine\ORM\Cache\Persister\Collection\NonStrictReadWriteCachedCollectionPersister; +use Doctrine\ORM\Cache\Persister\Collection\ReadOnlyCachedCollectionPersister; +use Doctrine\ORM\Cache\Persister\Collection\ReadWriteCachedCollectionPersister; +use Doctrine\ORM\Cache\Persister\Entity\NonStrictReadWriteCachedEntityPersister; +use Doctrine\ORM\Cache\Persister\Entity\ReadOnlyCachedEntityPersister; +use Doctrine\ORM\Cache\Persister\Entity\ReadWriteCachedEntityPersister; use Doctrine\ORM\Cache\Region; -use Doctrine\ORM\Mapping\ClassMetadata; -use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Cache\Region\DefaultMultiGetRegion; use Doctrine\ORM\Cache\Region\DefaultRegion; use Doctrine\ORM\Cache\Region\FileLockRegion; use Doctrine\ORM\Cache\Region\UpdateTimestampCache; -use Doctrine\ORM\Cache\Region\DefaultMultiGetRegion; -use Doctrine\ORM\Cache\Persister\CachedPersister; -use Doctrine\ORM\Persisters\Entity\EntityPersister; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Persisters\Collection\CollectionPersister; -use Doctrine\ORM\Cache\Persister\Entity\ReadOnlyCachedEntityPersister; -use Doctrine\ORM\Cache\Persister\Entity\ReadWriteCachedEntityPersister; -use Doctrine\ORM\Cache\Persister\Collection\ReadOnlyCachedCollectionPersister; -use Doctrine\ORM\Cache\Persister\Collection\ReadWriteCachedCollectionPersister; -use Doctrine\ORM\Cache\Persister\Entity\NonStrictReadWriteCachedEntityPersister; -use Doctrine\ORM\Cache\Persister\Collection\NonStrictReadWriteCachedCollectionPersister; +use Doctrine\ORM\Persisters\Entity\EntityPersister; /** * @since 2.5 @@ -48,7 +47,7 @@ class DefaultCacheFactory implements CacheFactory { /** - * @var \Doctrine\Common\Cache\CacheProvider + * @var CacheAdapter */ private $cache; @@ -73,10 +72,10 @@ class DefaultCacheFactory implements CacheFactory private $fileLockRegionDirectory; /** - * @param \Doctrine\ORM\Cache\RegionsConfiguration $cacheConfig - * @param \Doctrine\Common\Cache\CacheProvider $cache + * @param RegionsConfiguration $cacheConfig + * @param CacheAdapter $cache */ - public function __construct(RegionsConfiguration $cacheConfig, CacheProvider $cache) + public function __construct(RegionsConfiguration $cacheConfig, CacheAdapter $cache) { $this->cache = $cache; $this->regionsConfig = $cacheConfig; @@ -209,11 +208,16 @@ public function getRegion(array $cache) $cacheAdapter = clone $this->cache; - $cacheAdapter->setNamespace($cache['region']); + if ($cacheAdapter instanceof CacheProvider) { + $cacheAdapter->setNamespace($cache['region']); + } + + $name = $cache['region']; + $lifetime = $this->regionsConfig->getLifetime($cache['region']); $region = ($cacheAdapter instanceof MultiGetCache) - ? new DefaultMultiGetRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])) - : new DefaultRegion($cache['region'], $cacheAdapter, $this->regionsConfig->getLifetime($cache['region'])); + ? new DefaultMultiGetRegion($name, $cacheAdapter, $lifetime) + : new DefaultRegion($name, $cacheAdapter, $lifetime); if ($cache['usage'] === ClassMetadata::CACHE_USAGE_READ_WRITE) { From b1474768fea3ec1abb8543865e2a382373fdeeda Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:43:34 +0100 Subject: [PATCH 17/18] #954 DDC-2982 - Better type-safety in `Doctrine\ORM\Cache\Region\DefaultMultiGetRegion` instantiation logic --- .../Cache/Region/DefaultMultiGetRegion.php | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php index b037732a8b5..39bf9c9e866 100644 --- a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php @@ -20,12 +20,8 @@ namespace Doctrine\ORM\Cache\Region; -use Doctrine\ORM\Cache\Lock; +use Doctrine\Common\Cache\MultiGetCache; use Doctrine\ORM\Cache\Region; -use Doctrine\ORM\Cache\CacheKey; -use Doctrine\ORM\Cache\CacheEntry; -use Doctrine\Common\Cache\CacheProvider; -use Doctrine\ORM\Cache\MultiGetRegion; use Doctrine\ORM\Cache\CollectionCacheEntry; /** @@ -36,6 +32,25 @@ */ class DefaultMultiGetRegion extends DefaultRegion { + /** + * Note that the multiple type is due to doctrine/cache not integrating the MultiGetCache interface + * in its signature due to BC in 1.x + * + * @var MultiGetCache|\Doctrine\Common\Cache\Cache + */ + protected $cache; + + /** + * {@inheritDoc} + * + * @param MultiGetCache $cache + */ + public function __construct($name, MultiGetCache $cache, $lifetime = 0) + { + /* @var $cache \Doctrine\Common\Cache\Cache */ + parent::__construct($name, $cache, $lifetime); + } + /** * {@inheritdoc} */ From d5f6b4440a385a94df284ca023186fc1fac31f22 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 17 Jan 2015 23:44:40 +0100 Subject: [PATCH 18/18] #954 DDC-2982 - s/`CacheProvider`/`Cache` in documentation --- docs/en/reference/second-level-cache.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/reference/second-level-cache.rst b/docs/en/reference/second-level-cache.rst index d88e83fc634..5408ca97c8a 100644 --- a/docs/en/reference/second-level-cache.rst +++ b/docs/en/reference/second-level-cache.rst @@ -185,7 +185,7 @@ To enable the second-level-cache, you should provide a cache factory