From 6755bb0c7bdb1774152fddf7bb0ff20a4dd5a8a0 Mon Sep 17 00:00:00 2001 From: Bob van de Vijver Date: Wed, 8 Jan 2025 10:38:23 +0100 Subject: [PATCH 1/2] Fix Hydration when use ManyToMany[indexBy] The bug related (#11694) and fixed mapping of sql column alias to field in entity (#11783) and invalidate cache [cache/persisted/entity|cache/persisted/collection] when sql filter changes --- src/Cache/CollectionCacheKey.php | 6 +- .../AbstractCollectionPersister.php | 10 +++- ...rictReadWriteCachedCollectionPersister.php | 4 +- .../ReadWriteCachedCollectionPersister.php | 4 +- .../Entity/AbstractEntityPersister.php | 19 ++++-- .../Entity/BasicEntityPersister.php | 10 +++- src/Query/ResultSetMapping.php | 26 +++++++- .../Category.php | 43 +++++++++++++ .../CategoryTypeSQLFilter.php | 22 +++++++ .../ChangeFiltersTest.php | 60 +++++++++++++++++++ .../Company.php | 46 ++++++++++++++ 11 files changed, 235 insertions(+), 15 deletions(-) create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Category.php create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/CategoryTypeSQLFilter.php create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/ChangeFiltersTest.php create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Company.php diff --git a/src/Cache/CollectionCacheKey.php b/src/Cache/CollectionCacheKey.php index 56bf8df722c..b6c1ae803f8 100644 --- a/src/Cache/CollectionCacheKey.php +++ b/src/Cache/CollectionCacheKey.php @@ -43,7 +43,7 @@ class CollectionCacheKey extends CacheKey * @param string $association The field name that represents the association. * @param array $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); @@ -51,6 +51,8 @@ public function __construct($entityClass, $association, array $ownerIdentifier) $this->entityClass = (string) $entityClass; $this->association = (string) $association; - parent::__construct(str_replace('\\', '.', strtolower($entityClass)) . '_' . implode(' ', $ownerIdentifier) . '__' . $association); + $filterHash = $filterHash === '' ? '' : '_' . $filterHash; + + parent::__construct(str_replace('\\', '.', strtolower($entityClass)) . '_' . implode(' ', $ownerIdentifier) . '__' . $association . $filterHash); } } diff --git a/src/Cache/Persister/Collection/AbstractCollectionPersister.php b/src/Cache/Persister/Collection/AbstractCollectionPersister.php index 42d6ec8519c..18d4d7dab0e 100644 --- a/src/Cache/Persister/Collection/AbstractCollectionPersister.php +++ b/src/Cache/Persister/Collection/AbstractCollectionPersister.php @@ -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; @@ -55,6 +56,9 @@ abstract class AbstractCollectionPersister implements CachedCollectionPersister /** @var string */ protected $regionName; + /** @var FilterCollection */ + protected $filters; + /** @var CollectionHydrator */ protected $hydrator; @@ -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(); @@ -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) { @@ -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() ); $this->region->evict($key); diff --git a/src/Cache/Persister/Collection/NonStrictReadWriteCachedCollectionPersister.php b/src/Cache/Persister/Collection/NonStrictReadWriteCachedCollectionPersister.php index 2441fc9960e..cfe62a2d143 100644 --- a/src/Cache/Persister/Collection/NonStrictReadWriteCachedCollectionPersister.php +++ b/src/Cache/Persister/Collection/NonStrictReadWriteCachedCollectionPersister.php @@ -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); @@ -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'])) { diff --git a/src/Cache/Persister/Collection/ReadWriteCachedCollectionPersister.php b/src/Cache/Persister/Collection/ReadWriteCachedCollectionPersister.php index 0ec977695e2..35e7797b39b 100644 --- a/src/Cache/Persister/Collection/ReadWriteCachedCollectionPersister.php +++ b/src/Cache/Persister/Collection/ReadWriteCachedCollectionPersister.php @@ -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); @@ -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) { diff --git a/src/Cache/Persister/Entity/AbstractEntityPersister.php b/src/Cache/Persister/Entity/AbstractEntityPersister.php index 3cdd27885a4..a9d447b96bf 100644 --- a/src/Cache/Persister/Entity/AbstractEntityPersister.php +++ b/src/Cache/Persister/Entity/AbstractEntityPersister.php @@ -22,9 +22,11 @@ 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; +use function func_get_args; use function serialize; use function sha1; @@ -62,6 +64,9 @@ abstract class AbstractEntityPersister implements CachedEntityPersister /** @var Cache */ protected $cache; + /** @var FilterCollection */ + protected $filters; + /** @var CacheLogger|null */ protected $cacheLogger; @@ -91,6 +96,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(); @@ -261,7 +267,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()); } /** @@ -524,7 +530,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) { @@ -559,7 +565,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) { @@ -611,12 +617,15 @@ public function refresh(array $id, $entity, $lockMode = null) * * @return CollectionCacheKey */ - protected function buildCollectionCacheKey(array $association, $ownerId) + protected function buildCollectionCacheKey(array $association, $ownerId/*, string $filterHash */) { + $filterHash = (string) (func_get_args()[2] ?? ''); // todo: move to argument in next major release + return new CollectionCacheKey( $this->metadataFactory->getMetadataFor($association['sourceEntity'])->rootEntityName, $association['fieldName'], - $ownerId + $ownerId, + $filterHash ); } } diff --git a/src/Persisters/Entity/BasicEntityPersister.php b/src/Persisters/Entity/BasicEntityPersister.php index a9897f195fe..4073c606edc 100644 --- a/src/Persisters/Entity/BasicEntityPersister.php +++ b/src/Persisters/Entity/BasicEntityPersister.php @@ -1560,7 +1560,15 @@ protected function getSelectColumnSQL($field, ClassMetadata $class, $alias = 'r' $tableAlias = $this->getSQLTableAlias($class->name, $root); $fieldMapping = $class->fieldMappings[$field]; $sql = sprintf('%s.%s', $tableAlias, $this->quoteStrategy->getColumnName($field, $class, $this->platform)); - $columnAlias = $this->getSQLColumnAlias($fieldMapping['columnName']); + + $columnAlias = null; + if ($this->currentPersisterContext->rsm->hasColumnAliasByField($alias, $field)) { + $columnAlias = $this->currentPersisterContext->rsm->getColumnAliasByField($alias, $field); + } + + if ($columnAlias === null) { + $columnAlias = $this->getSQLColumnAlias($fieldMapping['columnName']); + } $this->currentPersisterContext->rsm->addFieldResult($alias, $columnAlias, $field); if (! empty($fieldMapping['enumType'])) { diff --git a/src/Query/ResultSetMapping.php b/src/Query/ResultSetMapping.php index b59bf8d0b99..1f3c1843f51 100644 --- a/src/Query/ResultSetMapping.php +++ b/src/Query/ResultSetMapping.php @@ -69,6 +69,13 @@ class ResultSetMapping */ public $fieldMappings = []; + /** + * Map field names for each class to alias + * + * @var array>> + */ + public $columnAliasMappings = []; + /** * Maps column names in the result set to the alias/field name to use in the mapped result. * @@ -335,7 +342,10 @@ public function addFieldResult($alias, $columnName, $fieldName, $declaringClass // column name => alias of owner $this->columnOwnerMap[$columnName] = $alias; // field name => class name of declaring class - $this->declaringClasses[$columnName] = $declaringClass ?: $this->aliasMap[$alias]; + $declaringClass = $declaringClass ?: $this->aliasMap[$alias]; + $this->declaringClasses[$columnName] = $declaringClass; + + $this->columnAliasMappings[$declaringClass][$alias][$fieldName] = $columnName; if (! $this->isMixed && $this->scalarMappings) { $this->isMixed = true; @@ -344,6 +354,20 @@ public function addFieldResult($alias, $columnName, $fieldName, $declaringClass return $this; } + public function hasColumnAliasByField(string $alias, string $fieldName): bool + { + $declaringClass = $this->aliasMap[$alias]; + + return isset($this->columnAliasMappings[$declaringClass][$alias][$fieldName]); + } + + public function getColumnAliasByField(string $alias, string $fieldName): string + { + $declaringClass = $this->aliasMap[$alias]; + + return $this->columnAliasMappings[$declaringClass][$alias][$fieldName]; + } + /** * Adds a joined entity result. * diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Category.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Category.php new file mode 100644 index 00000000000..f80d89bc98c --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Category.php @@ -0,0 +1,43 @@ +name = $name; + $this->type = $type; + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/CategoryTypeSQLFilter.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/CategoryTypeSQLFilter.php new file mode 100644 index 00000000000..459e0c2f2f7 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/CategoryTypeSQLFilter.php @@ -0,0 +1,22 @@ +getName() === Category::class) { + return sprintf('%s.%s = %s', $targetTableAlias, $targetEntity->fieldMappings['type']['fieldName'], $this->getParameter('type')); + } + + return ''; + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/ChangeFiltersTest.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/ChangeFiltersTest.php new file mode 100644 index 00000000000..c0bc57bdb10 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/ChangeFiltersTest.php @@ -0,0 +1,60 @@ +setUpEntitySchema([ + Company::class, + Category::class, + ]); + } + + private function prepareData(): void + { + $cat1 = new Category('cat1', self::CAT_FOO); + $cat2 = new Category('cat2', self::CAT_BAR); + $companyA = new Company(self::COMPANY_A, [$cat1, $cat2]); + + $this->_em->persist($cat1); + $this->_em->persist($cat2); + $this->_em->persist($companyA); + $this->_em->flush(); + $this->_em->clear(); + } + + public function testIndexAliasUpdatedWithUpdatedFilter(): void + { + $this->prepareData(); + + $company = $this->_em->getRepository(Company::class)->findOneBy([]); + + self::assertCount(2, $company->categories); + self::assertEquals([self::CAT_FOO, self::CAT_BAR], $company->categories->map(static function (Category $c): string { + return $c->type; + })->getValues()); + + $this->_em->clear(); + $this->_em->getConfiguration()->addFilter(CategoryTypeSQLFilter::class, CategoryTypeSQLFilter::class); + $this->_em->getFilters()->enable(CategoryTypeSQLFilter::class)->setParameter('type', self::CAT_FOO); + + $company = $this->_em->getRepository(Company::class)->findOneBy([]); + + self::assertCount(1, $company->categories); + self::assertEquals([self::CAT_FOO], $company->categories->map(static function (Category $c): string { + return $c->type; + })->getValues()); + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Company.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Company.php new file mode 100644 index 00000000000..c0a54cfbeeb --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilterAndIndexedRelation/Company.php @@ -0,0 +1,46 @@ + + */ + public $categories; + + /** @param Category[] $categories */ + public function __construct(string $name, array $categories) + { + $this->name = $name; + $this->categories = new ArrayCollection($categories); + } +} From 737cca5b78dafc3148c32d224ee8cc743c9a4604 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 4 Feb 2025 18:23:54 +0000 Subject: [PATCH 2/2] Bump doctrine/.github from 7.1.0 to 7.2.1 Bumps [doctrine/.github](https://github.com/doctrine/.github) from 7.1.0 to 7.2.1. - [Release notes](https://github.com/doctrine/.github/releases) - [Commits](https://github.com/doctrine/.github/compare/7.1.0...7.2.1) --- updated-dependencies: - dependency-name: doctrine/.github dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- .github/workflows/coding-standards.yml | 2 +- .github/workflows/documentation.yml | 2 +- .github/workflows/release-on-milestone-closed.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/coding-standards.yml b/.github/workflows/coding-standards.yml index efc722b1353..755956430e8 100644 --- a/.github/workflows/coding-standards.yml +++ b/.github/workflows/coding-standards.yml @@ -24,4 +24,4 @@ on: jobs: coding-standards: - uses: "doctrine/.github/.github/workflows/coding-standards.yml@7.1.0" + uses: "doctrine/.github/.github/workflows/coding-standards.yml@7.2.1" diff --git a/.github/workflows/documentation.yml b/.github/workflows/documentation.yml index 58eb5f81459..deec2b9dee8 100644 --- a/.github/workflows/documentation.yml +++ b/.github/workflows/documentation.yml @@ -17,4 +17,4 @@ on: jobs: documentation: name: "Documentation" - uses: "doctrine/.github/.github/workflows/documentation.yml@7.1.0" + uses: "doctrine/.github/.github/workflows/documentation.yml@7.2.1" diff --git a/.github/workflows/release-on-milestone-closed.yml b/.github/workflows/release-on-milestone-closed.yml index 8ed48106aca..d7ad5ab4e21 100644 --- a/.github/workflows/release-on-milestone-closed.yml +++ b/.github/workflows/release-on-milestone-closed.yml @@ -7,7 +7,7 @@ on: jobs: release: - uses: "doctrine/.github/.github/workflows/release-on-milestone-closed.yml@7.1.0" + uses: "doctrine/.github/.github/workflows/release-on-milestone-closed.yml@7.2.1" secrets: GIT_AUTHOR_EMAIL: ${{ secrets.GIT_AUTHOR_EMAIL }} GIT_AUTHOR_NAME: ${{ secrets.GIT_AUTHOR_NAME }}