Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Jul 16, 2015
4 changes: 0 additions & 4 deletions lib/Doctrine/ORM/Cache/DefaultQueryCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,6 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, $result, array $h
continue;
}

if ( ! isset($assoc['cache'])) {
throw CacheException::nonCacheableEntityAssociation($entityName, $name);
}

$assocPersister = $this->uow->getEntityPersister($assoc['targetEntity']);
$assocRegion = $assocPersister->getCacheRegion();
$assocMetadata = $assocPersister->getClassMetadata();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,14 +234,6 @@ public function storeEntityCache($entity, EntityCacheKey $key)
$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);

Expand Down
22 changes: 19 additions & 3 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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
Expand Down Expand Up @@ -1078,18 +1079,29 @@ public function enableCache(array $cache)
* @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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space between the function name and open parenthesis

{
if ( ! isset($cache['usage'])) {
$cache['usage'] = isset($this->cache['usage'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing indentation level, it should be :

$cache['usage'] = isset($this->cache['usage'])
    ? $this->cache['usage']
    : self::CACHE_USAGE_READ_ONLY;

? $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;
}

/**
Expand Down Expand Up @@ -1475,6 +1487,10 @@ protected function _validateAndCompleteAssociationMapping(array $mapping)
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
Expand Down
16 changes: 7 additions & 9 deletions lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,13 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)

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

Expand Down Expand Up @@ -396,14 +402,6 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$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
Expand Down
25 changes: 13 additions & 12 deletions lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$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);
}
}

Expand Down Expand Up @@ -428,12 +428,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
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);
}
}

Expand Down Expand Up @@ -473,12 +473,13 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$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);

}
}

Expand Down Expand Up @@ -543,12 +544,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
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);
}
}

Expand Down
23 changes: 12 additions & 11 deletions lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -388,12 +388,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$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);
}
}

Expand Down Expand Up @@ -426,12 +426,13 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$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);
}
}

Expand Down Expand Up @@ -475,12 +476,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$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);
}
}

Expand Down Expand Up @@ -552,12 +553,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
$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);
}
}

Expand Down
24 changes: 0 additions & 24 deletions tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -437,30 +437,6 @@ public function testGetShouldIgnoreMissingEntityQueryCacheEntry()
$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.
Expand Down
36 changes: 36 additions & 0 deletions tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ public function testLoadMetadataForNonEntityThrowsException()
$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
*/
Expand Down Expand Up @@ -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;
}
13 changes: 13 additions & 0 deletions tests/Doctrine/Tests/ORM/Mapping/PHPMappingDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,17 @@ public function testinvalidEntityOrMappedSuperClassShouldMentionParentClasses()
{
$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);
}
}
22 changes: 22 additions & 0 deletions tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ public function testClassTableInheritanceDiscriminatorMap()
$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();
Expand Down Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
@@ -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'
));
Original file line number Diff line number Diff line change
@@ -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>