Skip to content

Commit

Permalink
Fix Hydration when use ManyToMany[indexBy]
Browse files Browse the repository at this point in the history
The bug related (doctrine#11694) and fixed mapping of sql column alias to field in entity (doctrine#11783) and
invalidate cache [cache/persisted/entity|cache/persisted/collection] when sql filter changes
  • Loading branch information
bobvandevijver authored and dbannik committed Jan 17, 2025
1 parent e3cabad commit 43d33aa
Show file tree
Hide file tree
Showing 11 changed files with 230 additions and 15 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);
}
}
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()
);

$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
);
}
}
10 changes: 9 additions & 1 deletion src/Persisters/Entity/BasicEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand Down
26 changes: 25 additions & 1 deletion src/Query/ResultSetMapping.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ class ResultSetMapping
*/
public $fieldMappings = [];

/**
* Map field names for each class to alias
*
* @var array<class-string, array<string, array<string, string>>>
*/
public $columnAliasMappings = [];

/**
* Maps column names in the result set to the alias/field name to use in the mapped result.
*
Expand Down Expand Up @@ -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;
Expand All @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilterAndIndexedRelation;

use Doctrine\ORM\Mapping as ORM;

/**
* @ORM\Entity
* @ORM\Table(name="Category_Master")
*/
class Category
{
/**
* @ORM\Id
* @ORM\Column(type="integer")
* @ORM\GeneratedValue(strategy="AUTO")
*
* @var int
*/
public $id;

/**
* @ORM\Column(type="string")
*
* @var string
*/
public $name;

/**
* @ORM\Column(type="string")
*
* @var string
*/
public $type;

public function __construct(string $name, string $type)
{
$this->name = $name;
$this->type = $type;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilterAndIndexedRelation;

use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query\Filter\SQLFilter;

use function sprintf;

class CategoryTypeSQLFilter extends SQLFilter
{
public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias): string
{
if ($targetEntity->getName() === Category::class) {
return sprintf('%s.%s = %s', $targetTableAlias, $targetEntity->fieldMappings['type']['fieldName'], $this->getParameter('type'));
}

return '';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilterAndIndexedRelation;

use Doctrine\Tests\OrmFunctionalTestCase;

final class ChangeFiltersTest extends OrmFunctionalTestCase
{
private const COMPANY_A = 'A';
private const CAT_BAR = 'bar';
private const CAT_FOO = 'foo';

public function setUp(): void
{
parent::setUp();

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

0 comments on commit 43d33aa

Please sign in to comment.