Merge pull request #1433 from goetas/slc-check-to-classmetadatainfo

Check for non-cacheable entities on metadata level, not at runtime

lib/Doctrine/ORM/Cache/DefaultQueryCache.php

@@ -267,10 +267,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();

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);
 

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 <tt>ClassMetadata</tt> instance holds all the object-relational mapping metadata
@@ -1078,6 +1079,17 @@ 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'])
@@ -1089,7 +1101,7 @@ class ClassMetadataInfo implements ClassMetadata
             $cache['region'] = strtolower(str_replace('\\', '_', $this->rootEntityName)) . '__' . $fieldName;
         }
 
-        $this->associationMappings[$fieldName]['cache'] = $cache;
+        return $cache;
     }
 
     /**
@@ -1475,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

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

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("<index-by /> 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("<index-by /> 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);
             }
         }
 

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);
             }
         }
 

tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php

@@ -482,30 +482,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.

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;
+}

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);
+    }
 }

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;
+}

tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.ORM.Mapping.PHPSLC.php

@@ -0,0 +1,12 @@
+<?php
+
+use Doctrine\ORM\Mapping\ClassMetadataInfo;
+
+$metadata->enableCache(array(
+    'usage' => ClassMetadataInfo::CACHE_USAGE_READ_ONLY
+));
+$metadata->mapManyToOne(array(
+    'fieldName'      => 'foo',
+    'id'         => true,
+    'targetEntity'   => 'PHPSLCFoo'
+));

tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.XMLSLC.dcm.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
+      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+      xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
+                          http://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
+    <entity name="Doctrine\Tests\ORM\Mapping\XMLSLC">
+        <cache usage="NONSTRICT_READ_WRITE" />
+        <id name="foo" association-key="true"/>
+        <many-to-one field="foo" target-entity="Doctrine\Tests\ORM\Mapping\XMLSLCFoo">
+            <join-column name="foo_id" referenced-column-name="id" />
+        </many-to-one>
+    </entity>
+</doctrine-mapping>