From 6a428c6064fa0104edee8696dd456f5990d748fb Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 13:36:23 +0200 Subject: [PATCH 01/11] Allow to retreive association cache defaults --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index dd47c9788..1dfb93349 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -1078,18 +1078,29 @@ class ClassMetadataInfo implements ClassMetadata * @return void */ public function enableAssociationCache($fieldName, array $cache) + { + $this->associationMappings[$fieldName]['cache'] = $this->getAssociationCacheDefaults ($fieldName, $cache); + } + + /** + * @param string $fieldName + * @param array $cache + * + * @return array + */ + public function getAssociationCacheDefaults ($fieldName, array $cache) { if ( ! isset($cache['usage'])) { $cache['usage'] = isset($this->cache['usage']) - ? $this->cache['usage'] - : self::CACHE_USAGE_READ_ONLY; + ? $this->cache['usage'] + : self::CACHE_USAGE_READ_ONLY; } if ( ! isset($cache['region'])) { $cache['region'] = strtolower(str_replace('\\', '_', $this->rootEntityName)) . '__' . $fieldName; } - $this->associationMappings[$fieldName]['cache'] = $cache; + return $cache; } /** From c685255fe3f2510805626e6b66834083edf1671c Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 13:37:09 +0200 Subject: [PATCH 02/11] Check for non-cacheable associations directly on the class metada info --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 1dfb93349..2947b19ee 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -28,6 +28,7 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use ReflectionClass; use Doctrine\Common\Persistence\Mapping\ClassMetadata; use Doctrine\Common\ClassLoader; +use Doctrine\ORM\Cache\CacheException; /** * A ClassMetadata instance holds all the object-relational mapping metadata @@ -1486,6 +1487,10 @@ class ClassMetadataInfo implements ClassMetadata if ( ! $this->isIdentifierComposite && count($this->identifier) > 1) { $this->isIdentifierComposite = true; } + + if ($this->cache && !isset($mapping['cache'])) { + throw CacheException::nonCacheableEntityAssociation($this->name, $mapping['fieldName']); + } } // Mandatory attributes for both sides From 11be4fae86c9155d24bb7d6774473629ebafae34 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 13:38:01 +0200 Subject: [PATCH 03/11] Do not check at runtime for non-cacheable associations --- lib/Doctrine/ORM/Cache/DefaultQueryCache.php | 4 ---- .../Cache/Persister/Entity/AbstractEntityPersister.php | 8 -------- 2 files changed, 12 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php index 87bb13663..ce3463c06 100644 --- a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php +++ b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php @@ -266,10 +266,6 @@ class DefaultQueryCache implements QueryCache continue; } - if ( ! isset($assoc['cache'])) { - throw CacheException::nonCacheableEntityAssociation($entityName, $name); - } - $assocPersister = $this->uow->getEntityPersister($assoc['targetEntity']); $assocRegion = $assocPersister->getCacheRegion(); $assocMetadata = $assocPersister->getClassMetadata(); diff --git a/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php b/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php index eaead7cf7..68e5c034d 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php @@ -234,14 +234,6 @@ abstract class AbstractEntityPersister implements CachedEntityPersister $class = $this->metadataFactory->getMetadataFor($className); } - if ($class->containsForeignIdentifier) { - foreach ($class->associationMappings as $name => $assoc) { - if (!empty($assoc['id']) && !isset($assoc['cache'])) { - throw CacheException::nonCacheableEntityAssociation($class->name, $name); - } - } - } - $entry = $this->hydrator->buildCacheEntry($class, $key, $entity); $cached = $this->region->put($key, $entry); From f4f32a5213d3e687563549caeb5c9ac89d5e8943 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 13:41:03 +0200 Subject: [PATCH 04/11] Annotation driver uses getAssociationCacheDefaults for SLC mapping --- .../ORM/Mapping/Driver/AnnotationDriver.php | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index df8b7b80c..e8af86e84 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php @@ -273,7 +273,13 @@ class AnnotationDriver extends AbstractAnnotationDriver $mapping = array(); $mapping['fieldName'] = $property->getName(); - + // Evaluate @Cache annotation + if (($cacheAnnot = $this->reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\Cache')) !== null) { + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], array( + 'usage' => constant('Doctrine\ORM\Mapping\ClassMetadata::CACHE_USAGE_' . $cacheAnnot->usage), + 'region' => $cacheAnnot->region, + )); + } // Check for JoinColumn/JoinColumns annotations $joinColumns = array(); @@ -396,14 +402,6 @@ class AnnotationDriver extends AbstractAnnotationDriver $mapping['columnPrefix'] = $embeddedAnnot->columnPrefix; $metadata->mapEmbedded($mapping); } - - // Evaluate @Cache annotation - if (($cacheAnnot = $this->reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\Cache')) !== null) { - $metadata->enableAssociationCache($mapping['fieldName'], array( - 'usage' => constant('Doctrine\ORM\Mapping\ClassMetadata::CACHE_USAGE_' . $cacheAnnot->usage), - 'region' => $cacheAnnot->region, - )); - } } // Evaluate AssociationOverrides annotation From 7d64be915cf12bde12474b7956af5e274877a544 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 13:41:13 +0200 Subject: [PATCH 05/11] XML driver uses getAssociationCacheDefaults for SLC mapping --- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index c917eb245..7f9a7ecc9 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php @@ -384,12 +384,12 @@ class XmlDriver extends FileDriver $mapping['orphanRemoval'] = $this->evaluateBoolean($oneToOneElement['orphan-removal']); } - $metadata->mapOneToOne($mapping); - // Evaluate second level cache if (isset($oneToOneElement->cache)) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($oneToOneElement->cache)); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($oneToOneElement->cache)); } + + $metadata->mapOneToOne($mapping); } } @@ -428,12 +428,12 @@ class XmlDriver extends FileDriver throw new \InvalidArgumentException(" is not a valid tag"); } - $metadata->mapOneToMany($mapping); - // Evaluate second level cache if (isset($oneToManyElement->cache)) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($oneToManyElement->cache)); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($oneToManyElement->cache)); } + + $metadata->mapOneToMany($mapping); } } @@ -473,12 +473,13 @@ class XmlDriver extends FileDriver $mapping['cascade'] = $this->_getCascadeMappings($manyToOneElement->cascade); } - $metadata->mapManyToOne($mapping); - // Evaluate second level cache if (isset($manyToOneElement->cache)) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($manyToOneElement->cache)); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($manyToOneElement->cache)); } + + $metadata->mapManyToOne($mapping); + } } @@ -543,12 +544,12 @@ class XmlDriver extends FileDriver throw new \InvalidArgumentException(" is not a valid tag"); } - $metadata->mapManyToMany($mapping); - // Evaluate second level cache if (isset($manyToManyElement->cache)) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($manyToManyElement->cache)); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($manyToManyElement->cache)); } + + $metadata->mapManyToMany($mapping); } } From acbda4bc0ecfe6335e50291ae18545deda761846 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 13:41:24 +0200 Subject: [PATCH 06/11] YAML driver uses getAssociationCacheDefaults for SLC mapping --- .../ORM/Mapping/Driver/YamlDriver.php | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php index 95d148318..aa288d0cd 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php @@ -388,12 +388,12 @@ class YamlDriver extends FileDriver $mapping['orphanRemoval'] = (bool)$oneToOneElement['orphanRemoval']; } - $metadata->mapOneToOne($mapping); - // Evaluate second level cache if (isset($oneToOneElement['cache'])) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($oneToOneElement['cache'])); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($oneToOneElement['cache'])); } + + $metadata->mapOneToOne($mapping); } } @@ -426,12 +426,13 @@ class YamlDriver extends FileDriver $mapping['indexBy'] = $oneToManyElement['indexBy']; } - $metadata->mapOneToMany($mapping); // Evaluate second level cache if (isset($oneToManyElement['cache'])) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($oneToManyElement['cache'])); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($oneToManyElement['cache'])); } + + $metadata->mapOneToMany($mapping); } } @@ -475,12 +476,12 @@ class YamlDriver extends FileDriver $mapping['cascade'] = $manyToOneElement['cascade']; } - $metadata->mapManyToOne($mapping); - // Evaluate second level cache if (isset($manyToOneElement['cache'])) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($manyToOneElement['cache'])); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($manyToOneElement['cache'])); } + + $metadata->mapManyToOne($mapping); } } @@ -552,12 +553,12 @@ class YamlDriver extends FileDriver $mapping['orphanRemoval'] = (bool)$manyToManyElement['orphanRemoval']; } - $metadata->mapManyToMany($mapping); - // Evaluate second level cache if (isset($manyToManyElement['cache'])) { - $metadata->enableAssociationCache($mapping['fieldName'], $this->cacheToArray($manyToManyElement['cache'])); + $mapping['cache'] = $metadata->getAssociationCacheDefaults($mapping['fieldName'], $this->cacheToArray($manyToManyElement['cache'])); } + + $metadata->mapManyToMany($mapping); } } From 012367a371e2c0ed2b41589ce72b933c460147fd Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 13:42:33 +0200 Subject: [PATCH 07/11] Removed runtime check test for non-cacheable entities --- .../Tests/ORM/Cache/DefaultQueryCacheTest.php | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php index 3af1a3bc4..0c537b2c5 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php @@ -437,30 +437,6 @@ class DefaultQueryCacheTest extends OrmTestCase $this->assertNull($this->queryCache->get($key, $rsm)); } - /** - * @expectedException Doctrine\ORM\Cache\CacheException - * @expectedExceptionMessage Entity association field "Doctrine\Tests\Models\Cache\City#travels" not configured as part of the second-level cache. - */ - public function testQueryNotCacheableAssociationException() - { - $uow = $this->em->getUnitOfWork(); - $key = new QueryCacheKey('query.key1', 0); - $rsm = new ResultSetMappingBuilder($this->em); - $cityClass = $this->em->getClassMetadata(City::CLASSNAME); - $city = new City("City 1", null); - $result = array( - $city - ); - - $cityClass->setFieldValue($city, 'id', 1); - - $rsm->addRootEntityFromClassMetadata(City::CLASSNAME, 'c'); - $rsm->addJoinedEntityFromClassMetadata(Travel::CLASSNAME, 't', 'c', 'travels', array('id' => 't_id')); - $uow->registerManaged($city, array('id' => $city->getId()), array('name' => $city->getName(), 'state' => null)); - - $this->queryCache->put($key, $rsm, $result); - } - /** * @expectedException Doctrine\ORM\Cache\CacheException * @expectedExceptionMessage Second level cache does not support scalar results. From 5f2922b3a7b6f0d8e5df363952a2218ba9f465ff Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 14:24:01 +0200 Subject: [PATCH 08/11] Test annotation driver with failing SLC mapping --- .../ORM/Mapping/AnnotationDriverTest.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php index 927755f99..409dec663 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php @@ -23,6 +23,19 @@ class AnnotationDriverTest extends AbstractMappingDriverTest $annotationDriver->loadMetadataForClass('stdClass', $cm); } + /** + * @expectedException Doctrine\ORM\Cache\CacheException + * @expectedExceptionMessage Entity association field "Doctrine\Tests\ORM\Mapping\AnnotationSLC#foo" not configured as part of the second-level cache. + */ + public function testFailingSecondLevelCacheAssociation() + { + $className = 'Doctrine\Tests\ORM\Mapping\AnnotationSLC'; + $mappingDriver = $this->_loadDriver(); + + $class = new ClassMetadata($className); + $mappingDriver->loadMetadataForClass($className, $class); + } + /** * @group DDC-268 */ @@ -351,3 +364,26 @@ class InvalidFetchOption */ private $collection; } + +/** + * @Entity + * @Cache + */ +class AnnotationSLC +{ + /** + * @Id + * @ManyToOne(targetEntity="AnnotationSLCFoo") + */ + public $foo; +} +/** + * @Entity + */ +class AnnotationSLCFoo +{ + /** + * @Column(type="string") + */ + public $id; +} From 4da0ee9db8c3636f056a3b780f78ae419f6069a0 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 14:24:19 +0200 Subject: [PATCH 09/11] Test PHP driver with failing SLC mapping --- .../Tests/ORM/Mapping/PHPMappingDriverTest.php | 13 +++++++++++++ .../php/Doctrine.Tests.ORM.Mapping.PHPSLC.php | 12 ++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.PHPSLC.php diff --git a/tests/Doctrine/Tests/ORM/Mapping/PHPMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/PHPMappingDriverTest.php index 5d06983ca..142455542 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/PHPMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/PHPMappingDriverTest.php @@ -33,4 +33,17 @@ class PHPMappingDriverTest extends AbstractMappingDriverTest { $this->createClassMetadata('Doctrine\Tests\Models\DDC889\DDC889Class'); } + + /** + * @expectedException Doctrine\ORM\Cache\CacheException + * @expectedExceptionMessage Entity association field "Doctrine\Tests\ORM\Mapping\PHPSLC#foo" not configured as part of the second-level cache. + */ + public function testFailingSecondLevelCacheAssociation() + { + $className = 'Doctrine\Tests\ORM\Mapping\PHPSLC'; + $mappingDriver = $this->_loadDriver(); + + $class = new ClassMetadata($className); + $mappingDriver->loadMetadataForClass($className, $class); + } } diff --git a/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.PHPSLC.php b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.PHPSLC.php new file mode 100644 index 000000000..c9341958a --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.PHPSLC.php @@ -0,0 +1,12 @@ +enableCache(array( + 'usage' => ClassMetadataInfo::CACHE_USAGE_READ_ONLY +)); +$metadata->mapManyToOne(array( + 'fieldName' => 'foo', + 'id' => true, + 'targetEntity' => 'PHPSLCFoo' +)); \ No newline at end of file From 32f0fefec7fb1a1ca57baca63c2ee4b3fbf6d035 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 20 Jun 2015 14:24:45 +0200 Subject: [PATCH 10/11] Test XML driver with failing SLC mapping --- .../ORM/Mapping/XmlMappingDriverTest.php | 22 +++++++++++++++++++ .../Doctrine.Tests.ORM.Mapping.XMLSLC.dcm.xml | 13 +++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.XMLSLC.dcm.xml diff --git a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php index 3ca82d08d..c2822f178 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php @@ -32,6 +32,19 @@ class XmlMappingDriverTest extends AbstractMappingDriverTest $this->assertEquals($expectedMap, $class->discriminatorMap); } + /** + * @expectedException Doctrine\ORM\Cache\CacheException + * @expectedExceptionMessage Entity association field "Doctrine\Tests\ORM\Mapping\XMLSLC#foo" not configured as part of the second-level cache. + */ + public function testFailingSecondLevelCacheAssociation() + { + $className = 'Doctrine\Tests\ORM\Mapping\XMLSLC'; + $mappingDriver = $this->_loadDriver(); + + $class = new ClassMetadata($className); + $mappingDriver->loadMetadataForClass($className, $class); + } + public function testIdentifierWithAssociationKey() { $driver = $this->_loadDriver(); @@ -176,3 +189,12 @@ class CTI class CTIFoo extends CTI {} class CTIBar extends CTI {} class CTIBaz extends CTI {} + +class XMLSLC +{ + public $foo; +} +class XMLSLCFoo +{ + public $id; +} diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.XMLSLC.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.XMLSLC.dcm.xml new file mode 100644 index 000000000..ca8092b50 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.XMLSLC.dcm.xml @@ -0,0 +1,13 @@ + + + + + + + + + + From 3a7b2991e888f3e48c5e1779f42960adf142441a Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Thu, 16 Jul 2015 16:20:36 +0200 Subject: [PATCH 11/11] PSR-2 CS improvements --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 2947b19ee..4e437b2ef 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -1089,12 +1089,12 @@ class ClassMetadataInfo implements ClassMetadata * * @return array */ - public function getAssociationCacheDefaults ($fieldName, array $cache) + public function getAssociationCacheDefaults($fieldName, array $cache) { if ( ! isset($cache['usage'])) { $cache['usage'] = isset($this->cache['usage']) - ? $this->cache['usage'] - : self::CACHE_USAGE_READ_ONLY; + ? $this->cache['usage'] + : self::CACHE_USAGE_READ_ONLY; } if ( ! isset($cache['region'])) {