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

11783 failure with indexed relation #11792

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/Cache/CollectionCacheKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,16 @@ 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);
$filterHash = $filterHash === '' ? '' : '_' . $filterHash;

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 @@
/** @var string */
protected $regionName;

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

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

Expand All @@ -76,6 +80,7 @@
$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 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 @@
$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
19 changes: 14 additions & 5 deletions src/Cache/Persister/Entity/AbstractEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -62,6 +64,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 +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();
Expand Down Expand Up @@ -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());
}

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
);
}
}
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