Skip to content

Commit

Permalink
invalidate cache when sql filter changes (#11783)
Browse files Browse the repository at this point in the history
  • Loading branch information
dbannik committed Jan 17, 2025
1 parent dae1522 commit b0766d6
Show file tree
Hide file tree
Showing 13 changed files with 50 additions and 38 deletions.
4 changes: 2 additions & 2 deletions src/Cache/CollectionCacheKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ class CollectionCacheKey extends CacheKey
* @param string $association The field name that represents the association.
* @param array<string, mixed> $ownerIdentifier The identifier of the owning entity.
*/
public function __construct($entityClass, $association, array $ownerIdentifier)
public function __construct($entityClass, $association, array $ownerIdentifier, string $filterHash)
{
ksort($ownerIdentifier);

$this->ownerIdentifier = $ownerIdentifier;
$this->entityClass = (string) $entityClass;
$this->association = (string) $association;

parent::__construct(str_replace('\\', '.', strtolower($entityClass)) . '_' . implode(' ', $ownerIdentifier) . '__' . $association);
parent::__construct(str_replace('\\', '.', strtolower($entityClass)) . '_' . implode(' ', $ownerIdentifier) . '__' . $association . '_' . $filterHash);
}
}
2 changes: 1 addition & 1 deletion src/Cache/DefaultCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ private function buildCollectionCacheKey(
$ownerIdentifier = $this->toIdentifierArray($metadata, $ownerIdentifier);
}

return new CollectionCacheKey($metadata->rootEntityName, $association, $ownerIdentifier);
return new CollectionCacheKey($metadata->rootEntityName, $association, $ownerIdentifier, '');
}

/**
Expand Down
10 changes: 8 additions & 2 deletions src/Cache/Persister/Collection/AbstractCollectionPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\Persisters\Collection\CollectionPersister;
use Doctrine\ORM\Proxy\DefaultProxyClassNameResolver;
use Doctrine\ORM\Query\FilterCollection;
use Doctrine\ORM\UnitOfWork;

use function array_values;
Expand Down Expand Up @@ -55,6 +56,9 @@ abstract class AbstractCollectionPersister implements CachedCollectionPersister
/** @var string */
protected $regionName;

/** @var FilterCollection */
protected $filters;

/** @var CollectionHydrator */
protected $hydrator;

Expand All @@ -76,6 +80,7 @@ public function __construct(CollectionPersister $persister, Region $region, Enti
$this->region = $region;
$this->persister = $persister;
$this->association = $association;
$this->filters = $em->getFilters();
$this->regionName = $region->getName();
$this->uow = $em->getUnitOfWork();
$this->metadataFactory = $em->getMetadataFactory();
Expand Down Expand Up @@ -189,7 +194,7 @@ public function containsKey(PersistentCollection $collection, $key)
public function count(PersistentCollection $collection)
{
$ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId);
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId, $this->filters->getHash());
$entry = $this->region->get($key);

if ($entry !== null) {
Expand Down Expand Up @@ -241,7 +246,8 @@ protected function evictCollectionCache(PersistentCollection $collection)
$key = new CollectionCacheKey(
$this->sourceEntity->rootEntityName,
$this->association['fieldName'],
$this->uow->getEntityIdentifier($collection->getOwner())
$this->uow->getEntityIdentifier($collection->getOwner()),
$this->filters->getHash()

Check warning on line 250 in src/Cache/Persister/Collection/AbstractCollectionPersister.php

View check run for this annotation

Codecov / codecov/patch

src/Cache/Persister/Collection/AbstractCollectionPersister.php#L249-L250

Added lines #L249 - L250 were not covered by tests
);

$this->region->evict($key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function afterTransactionRolledBack()
public function delete(PersistentCollection $collection)
{
$ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId);
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId, $this->filters->getHash());

$this->persister->delete($collection);

Expand All @@ -65,7 +65,7 @@ public function update(PersistentCollection $collection)
}

$ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId);
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId, $this->filters->getHash());

// Invalidate non initialized collections OR ordered collection
if ($isDirty && ! $isInitialized || isset($this->association['orderBy'])) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function afterTransactionRolledBack()
public function delete(PersistentCollection $collection)
{
$ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId);
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId, $this->filters->getHash());
$lock = $this->region->lock($key);

$this->persister->delete($collection);
Expand Down Expand Up @@ -98,7 +98,7 @@ public function update(PersistentCollection $collection)
$this->persister->update($collection);

$ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId);
$key = new CollectionCacheKey($this->sourceEntity->rootEntityName, $this->association['fieldName'], $ownerId, $this->filters->getHash());
$lock = $this->region->lock($key);

if ($lock === null) {
Expand Down
16 changes: 11 additions & 5 deletions src/Cache/Persister/Entity/AbstractEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\Persisters\Entity\EntityPersister;
use Doctrine\ORM\Proxy\DefaultProxyClassNameResolver;
use Doctrine\ORM\Query\FilterCollection;
use Doctrine\ORM\UnitOfWork;

use function array_merge;
Expand Down Expand Up @@ -62,6 +63,9 @@ abstract class AbstractEntityPersister implements CachedEntityPersister
/** @var Cache */
protected $cache;

/** @var FilterCollection */
protected $filters;

/** @var CacheLogger|null */
protected $cacheLogger;

Expand Down Expand Up @@ -91,6 +95,7 @@ public function __construct(EntityPersister $persister, Region $region, EntityMa
$this->region = $region;
$this->persister = $persister;
$this->cache = $em->getCache();
$this->filters = $em->getFilters();
$this->regionName = $region->getName();
$this->uow = $em->getUnitOfWork();
$this->metadataFactory = $em->getMetadataFactory();
Expand Down Expand Up @@ -261,7 +266,7 @@ protected function getHash($query, $criteria, ?array $orderBy = null, $limit = n
? $this->persister->expandCriteriaParameters($criteria)
: $this->persister->expandParameters($criteria);

return sha1($query . serialize($params) . serialize($orderBy) . $limit . $offset);
return sha1($query . serialize($params) . serialize($orderBy) . $limit . $offset . $this->filters->getHash());
}

/**
Expand Down Expand Up @@ -524,7 +529,7 @@ public function loadManyToManyCollection(array $assoc, $sourceEntity, Persistent
}

$ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
$key = $this->buildCollectionCacheKey($assoc, $ownerId);
$key = $this->buildCollectionCacheKey($assoc, $ownerId, $this->filters->getHash());
$list = $persister->loadCollectionCache($collection, $key);

if ($list !== null) {
Expand Down Expand Up @@ -559,7 +564,7 @@ public function loadOneToManyCollection(array $assoc, $sourceEntity, PersistentC
}

$ownerId = $this->uow->getEntityIdentifier($collection->getOwner());
$key = $this->buildCollectionCacheKey($assoc, $ownerId);
$key = $this->buildCollectionCacheKey($assoc, $ownerId, $this->filters->getHash());
$list = $persister->loadCollectionCache($collection, $key);

if ($list !== null) {
Expand Down Expand Up @@ -611,12 +616,13 @@ public function refresh(array $id, $entity, $lockMode = null)
*
* @return CollectionCacheKey
*/
protected function buildCollectionCacheKey(array $association, $ownerId)
protected function buildCollectionCacheKey(array $association, $ownerId, string $filterHash)
{
return new CollectionCacheKey(
$this->metadataFactory->getMetadataFor($association['sourceEntity'])->rootEntityName,
$association['fieldName'],
$ownerId
$ownerId,
$filterHash
);
}
}
16 changes: 8 additions & 8 deletions tests/Tests/ORM/Cache/CacheKeyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,32 +41,32 @@ public function testEntityCacheKeyIdentifierOrder(): void

public function testCollectionCacheKeyIdentifierType(): void
{
$key1 = new CollectionCacheKey('Foo', 'assoc', ['id' => 1]);
$key2 = new CollectionCacheKey('Foo', 'assoc', ['id' => '1']);
$key1 = new CollectionCacheKey('Foo', 'assoc', ['id' => 1], '');
$key2 = new CollectionCacheKey('Foo', 'assoc', ['id' => '1'], '');

self::assertEquals($key1->hash, $key2->hash);
}

public function testCollectionCacheKeyIdentifierOrder(): void
{
$key1 = new CollectionCacheKey('Foo', 'assoc', ['foo_bar' => 1, 'bar_foo' => 2]);
$key2 = new CollectionCacheKey('Foo', 'assoc', ['bar_foo' => 2, 'foo_bar' => 1]);
$key1 = new CollectionCacheKey('Foo', 'assoc', ['foo_bar' => 1, 'bar_foo' => 2], '');
$key2 = new CollectionCacheKey('Foo', 'assoc', ['bar_foo' => 2, 'foo_bar' => 1], '');

self::assertEquals($key1->hash, $key2->hash);
}

public function testCollectionCacheKeyIdentifierCollision(): void
{
$key1 = new CollectionCacheKey('Foo', 'assoc', ['id' => 1]);
$key2 = new CollectionCacheKey('Bar', 'assoc', ['id' => 1]);
$key1 = new CollectionCacheKey('Foo', 'assoc', ['id' => 1], '');
$key2 = new CollectionCacheKey('Bar', 'assoc', ['id' => 1], '');

self::assertNotEquals($key1->hash, $key2->hash);
}

public function testCollectionCacheKeyAssociationCollision(): void
{
$key1 = new CollectionCacheKey('Foo', 'assoc1', ['id' => 1]);
$key2 = new CollectionCacheKey('Foo', 'assoc2', ['id' => 1]);
$key1 = new CollectionCacheKey('Foo', 'assoc1', ['id' => 1], '');
$key2 = new CollectionCacheKey('Foo', 'assoc2', ['id' => 1], '');

self::assertNotEquals($key1->hash, $key2->hash);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Tests/ORM/Cache/CacheLoggerChainTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function testEntityCacheChain(): void
public function testCollectionCacheChain(): void
{
$name = 'my_collection_region';
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');

$this->logger->setLogger('mock', $this->mock);

Expand Down
2 changes: 1 addition & 1 deletion tests/Tests/ORM/Cache/DefaultCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ private function putEntityCacheEntry(string $className, array $identifier, array
private function putCollectionCacheEntry(string $className, string $association, array $ownerIdentifier, array $data): void
{
$metadata = $this->em->getClassMetadata($className);
$cacheKey = new CollectionCacheKey($metadata->name, $association, $ownerIdentifier);
$cacheKey = new CollectionCacheKey($metadata->name, $association, $ownerIdentifier, '');
$cacheEntry = new CollectionCacheEntry($data);
$persister = $this->em->getUnitOfWork()->getCollectionPersister($metadata->getAssociationMapping($association));

Expand Down
2 changes: 1 addition & 1 deletion tests/Tests/ORM/Cache/DefaultCollectionHydratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function testLoadCacheCollection(): void

$sourceClass = $this->_em->getClassMetadata(State::class);
$targetClass = $this->_em->getClassMetadata(City::class);
$key = new CollectionCacheKey($sourceClass->name, 'cities', ['id' => 21]);
$key = new CollectionCacheKey($sourceClass->name, 'cities', ['id' => 21], '');
$collection = new PersistentCollection($this->_em, $targetClass, new ArrayCollection());
$list = $this->structure->loadCacheEntry($sourceClass, $key, $entry, $collection);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function testDeleteShouldLockItem(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');

$this->region->expects(self::once())
->method('lock')
Expand All @@ -52,7 +52,7 @@ public function testUpdateShouldLockItem(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');

$this->region->expects(self::once())
->method('lock')
Expand All @@ -70,7 +70,7 @@ public function testUpdateTransactionRollBackShouldEvictItem(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');

$this->region->expects(self::once())
->method('lock')
Expand All @@ -94,7 +94,7 @@ public function testDeleteTransactionRollBackShouldEvictItem(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');

$this->region->expects(self::once())
->method('lock')
Expand All @@ -118,7 +118,7 @@ public function testTransactionRollBackDeleteShouldClearQueue(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');
$property = new ReflectionProperty(ReadWriteCachedCollectionPersister::class, 'queuedCache');

$property->setAccessible(true);
Expand Down Expand Up @@ -150,7 +150,7 @@ public function testTransactionRollBackUpdateShouldClearQueue(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');
$property = new ReflectionProperty(ReadWriteCachedCollectionPersister::class, 'queuedCache');

$property->setAccessible(true);
Expand Down Expand Up @@ -182,7 +182,7 @@ public function testTransactionRollCommitDeleteShouldClearQueue(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');
$property = new ReflectionProperty(ReadWriteCachedCollectionPersister::class, 'queuedCache');

$property->setAccessible(true);
Expand Down Expand Up @@ -214,7 +214,7 @@ public function testTransactionRollCommitUpdateShouldClearQueue(): void
$lock = Lock::createLockRead();
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');
$property = new ReflectionProperty(ReadWriteCachedCollectionPersister::class, 'queuedCache');

$property->setAccessible(true);
Expand Down Expand Up @@ -245,7 +245,7 @@ public function testDeleteLockFailureShouldIgnoreQueue(): void
$entity = new State('Foo');
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');
$property = new ReflectionProperty(ReadWriteCachedCollectionPersister::class, 'queuedCache');

$property->setAccessible(true);
Expand All @@ -270,7 +270,7 @@ public function testUpdateLockFailureShouldIgnoreQueue(): void
$entity = new State('Foo');
$persister = $this->createPersisterDefault();
$collection = $this->createCollection($entity);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');
$property = new ReflectionProperty(ReadWriteCachedCollectionPersister::class, 'queuedCache');

$property->setAccessible(true);
Expand Down
4 changes: 2 additions & 2 deletions tests/Tests/ORM/Cache/StatisticsCacheLoggerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function testEntityCache(): void
public function testCollectionCache(): void
{
$name = 'my_collection_region';
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$key = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');

$this->logger->collectionCacheHit($name, $key);
$this->logger->collectionCachePut($name, $key);
Expand Down Expand Up @@ -81,7 +81,7 @@ public function testMultipleCaches(): void
$entityRegion = 'my_entity_region';
$queryRegion = 'my_query_region';

$coolKey = new CollectionCacheKey(State::class, 'cities', ['id' => 1]);
$coolKey = new CollectionCacheKey(State::class, 'cities', ['id' => 1], '');
$entityKey = new EntityCacheKey(State::class, ['id' => 1]);
$queryKey = new QueryCacheKey('my_query_hash');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public function testBasicConcurrentCollectionReadLock(): void
$this->secondLevelCacheLogger->clearStats();

$stateId = $this->states[0]->getId();
$cacheId = new CollectionCacheKey(State::class, 'cities', ['id' => $stateId]);
$cacheId = new CollectionCacheKey(State::class, 'cities', ['id' => $stateId], '');
$region = $this->_em->getCache()->getCollectionCacheRegion(State::class, 'cities');
assert($region instanceof ConcurrentRegionMock);

Expand Down

0 comments on commit b0766d6

Please sign in to comment.