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.

Sign up for GitHub

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

Extract mappings to their own DTOs #10405

Closed
wants to merge 33 commits into from
Closed

Extract mappings to their own DTOs #10405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
bec62b1
Extract FieldMapping in its own DTO
greg0ire Jan 17, 2023
c354e5d
Extract AssociationMapping in its own DTO
greg0ire Feb 15, 2023
d53101c
Extract discriminator column mapping into its own DTO
greg0ire Jan 23, 2023
454e0e0
Extract embedded class mapping into its own DTO
greg0ire Jan 27, 2023
f28d99a
Separate classes
greg0ire Feb 5, 2023
1286aa3
leverage To*AssociationMapping classes
greg0ire Feb 11, 2023
1229508
Introduce abstractions for owning side
greg0ire Feb 11, 2023
e208f30
Introduce AssociationMapping::isToOne()
greg0ire Feb 17, 2023
49ad40f
Remove redundant AssociationMapping::$type
greg0ire Feb 18, 2023
ef8d196
Make abstract classes that should be
greg0ire Feb 18, 2023
1290b20
Move joinColumns to where they belong
greg0ire Feb 18, 2023
b5bf1cb
Use psalm-param to fix cs issue
greg0ire Feb 18, 2023
4795e65
Use more precise/accurate docblocks
greg0ire Feb 18, 2023
895ce27
Move validation logic to dtos
greg0ire Feb 18, 2023
aeed003
Extract array access implementation into a trait
greg0ire Mar 7, 2023
1fadc99
switch to static
greg0ire Mar 23, 2023
1c869f2
Remove unused baseline entries
greg0ire Mar 23, 2023
3c6835c
Baseline remaining issues
greg0ire Mar 23, 2023
720aa4b
Make readonly properties that can be
greg0ire Mar 24, 2023
f8942cd
Finalize methods that can be finalized
greg0ire Mar 24, 2023
bdd1647
Rename JoinColumnData to JoinColumnMapping
greg0ire Mar 24, 2023
7ba408b
Remove unused constructor
greg0ire Mar 24, 2023
e66961d
Use null coalesce assignment operator
greg0ire Mar 24, 2023
b5d2ff2
Document properties as nullable
greg0ire Mar 24, 2023
f667d13
Default orphan removal to false
greg0ire Mar 24, 2023
a883fbe
Avoid array access in new files
greg0ire Mar 24, 2023
3ba09ea
Avoid array access trait when overridden
greg0ire Mar 24, 2023
1684548
Remove nullability for join columns
greg0ire Mar 25, 2023
35d7b4b
Serialize only what's necessary
greg0ire Mar 26, 2023
abc5625
Stop synchronizing properties
greg0ire Mar 26, 2023
40d46df
Narrow type down
greg0ire Mar 28, 2023
fb38580
Remove reference access
greg0ire Mar 28, 2023
2924f75
Make referencedColumnName mandatory
greg0ire Mar 28, 2023
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
19 changes: 8 additions & 11 deletions lib/Doctrine/ORM/Cache/CacheFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
use Doctrine\ORM\Cache\Persister\Collection\CachedCollectionPersister;
use Doctrine\ORM\Cache\Persister\Entity\CachedEntityPersister;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Persisters\Collection\CollectionPersister;
use Doctrine\ORM\Persisters\Entity\EntityPersister;

/**
* Contract for building second level cache regions components.
*
* @psalm-import-type AssociationMapping from ClassMetadata
*/
interface CacheFactory
{
Expand All @@ -24,12 +23,12 @@ interface CacheFactory
*/
public function buildCachedEntityPersister(EntityManagerInterface $em, EntityPersister $persister, ClassMetadata $metadata): CachedEntityPersister;

/**
* Build a collection persister for the given relation mapping.
*
* @param AssociationMapping $mapping The association mapping.
*/
public function buildCachedCollectionPersister(EntityManagerInterface $em, CollectionPersister $persister, array $mapping): CachedCollectionPersister;
/** Build a collection persister for the given relation mapping. */
public function buildCachedCollectionPersister(
EntityManagerInterface $em,
CollectionPersister $persister,
AssociationMapping $mapping,
): CachedCollectionPersister;

/**
* Build a query cache based on the given region name
Expand All @@ -43,10 +42,8 @@ public function buildEntityHydrator(EntityManagerInterface $em, ClassMetadata $m

/**
* Build a collection hydrator
*
* @param mixed[] $mapping The association mapping.
*/
public function buildCollectionHydrator(EntityManagerInterface $em, array $mapping): CollectionHydrator;
public function buildCollectionHydrator(EntityManagerInterface $em, AssociationMapping $mapping): CollectionHydrator;

/**
* Build a cache region
Expand Down
3 changes: 2 additions & 1 deletion lib/Doctrine/ORM/Cache/DefaultCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Doctrine\ORM\Cache\Persister\CachedPersister;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ToManyAssociationMapping;
use Doctrine\ORM\ORMInvalidArgumentException;
use Doctrine\ORM\UnitOfWork;

Expand Down Expand Up @@ -157,7 +158,7 @@ public function evictCollectionRegions(): void

foreach ($metadatas as $metadata) {
foreach ($metadata->associationMappings as $association) {
if (! $association['type'] & ClassMetadata::TO_MANY) {
if (! $association instanceof ToManyAssociationMapping) {
Copy link
Member

Choose a reason for hiding this comment

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

If those are all AssociationMapping, wouldn't the use of a method be an option instead of an instanceof check? You've used methods at other places too.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, I will add such a method.

continue;
}

Expand Down
17 changes: 7 additions & 10 deletions lib/Doctrine/ORM/Cache/DefaultCacheFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Doctrine\ORM\Cache\Region\FileLockRegion;
use Doctrine\ORM\Cache\Region\UpdateTimestampCache;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Persisters\Collection\CollectionPersister;
use Doctrine\ORM\Persisters\Entity\EntityPersister;
Expand Down Expand Up @@ -87,12 +88,11 @@ public function buildCachedEntityPersister(EntityManagerInterface $em, EntityPer
throw new InvalidArgumentException(sprintf('Unrecognized access strategy type [%s]', $usage));
}

/**
* {@inheritdoc}
*/
public function buildCachedCollectionPersister(EntityManagerInterface $em, CollectionPersister $persister, array $mapping): CachedCollectionPersister
{
assert(isset($mapping['cache']));
public function buildCachedCollectionPersister(
EntityManagerInterface $em,
CollectionPersister $persister,
AssociationMapping $mapping,
): CachedCollectionPersister {
$usage = $mapping['cache']['usage'];
$region = $this->getRegion($mapping['cache']);

Expand Down Expand Up @@ -128,10 +128,7 @@ public function buildQueryCache(EntityManagerInterface $em, string|null $regionN
);
}

/**
* {@inheritdoc}
*/
public function buildCollectionHydrator(EntityManagerInterface $em, array $mapping): CollectionHydrator
public function buildCollectionHydrator(EntityManagerInterface $em, AssociationMapping $mapping): CollectionHydrator
{
return new DefaultCollectionHydrator($em);
}
Expand Down
7 changes: 4 additions & 3 deletions lib/Doctrine/ORM/Cache/DefaultEntityHydrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Doctrine\Common\Util\ClassUtils;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\OneToOneAssociationMapping;
use Doctrine\ORM\Query;
use Doctrine\ORM\UnitOfWork;
use Doctrine\ORM\Utility\IdentifierFlattener;
Expand Down Expand Up @@ -57,15 +58,15 @@ public function buildCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, ob
continue;
}

if (! ($assoc['type'] & ClassMetadata::TO_ONE)) {
if (! $assoc->isToOne()) {
unset($data[$name]);

continue;
}

if (! isset($assoc['cache'])) {
$targetClassMetadata = $this->em->getClassMetadata($assoc['targetEntity']);
$owningAssociation = ! $assoc['isOwningSide']
$owningAssociation = ! $assoc->isOwningSide()
? $targetClassMetadata->associationMappings[$assoc['mappedBy']]
: $assoc;
$associationIds = $this->identifierFlattener->flattenIdentifier(
Expand Down Expand Up @@ -141,7 +142,7 @@ public function loadCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, Ent

$assocClass = $data[$name]->class;
$assocId = $data[$name]->identifier;
$isEagerLoad = ($assoc['fetch'] === ClassMetadata::FETCH_EAGER || ($assoc['type'] === ClassMetadata::ONE_TO_ONE && ! $assoc['isOwningSide']));
$isEagerLoad = ($assoc['fetch'] === ClassMetadata::FETCH_EAGER || ($assoc instanceof OneToOneAssociationMapping && ! $assoc->isOwningSide()));

if (! $isEagerLoad) {
$data[$name] = $this->em->getReference($assocClass, $assocId);
Expand Down
7 changes: 2 additions & 5 deletions lib/Doctrine/ORM/Cache/DefaultQueryCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Doctrine\ORM\Cache\Logging\CacheLogger;
use Doctrine\ORM\Cache\Persister\Entity\CachedEntityPersister;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\Query;
Expand All @@ -30,8 +31,6 @@

/**
* Default query cache implementation.
*
* @psalm-import-type AssociationMapping from ClassMetadata
*/
class DefaultQueryCache implements QueryCache
{
Expand Down Expand Up @@ -298,12 +297,10 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, mixed $result, ar
}

/**
* @param AssociationMapping $assoc
*
* @return mixed[]|null
* @psalm-return array{targetEntity: class-string, type: mixed, list?: array[], identifier?: array}|null
*/
private function storeAssociationCache(QueryCacheKey $key, array $assoc, mixed $assocValue): array|null
private function storeAssociationCache(QueryCacheKey $key, AssociationMapping $assoc, mixed $assocValue): array|null
{
$assocPersister = $this->uow->getEntityPersister($assoc['targetEntity']);
$assocMetadata = $assocPersister->getClassMetadata();
Expand Down
5 changes: 2 additions & 3 deletions lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Doctrine\ORM\Cache\Persister\Entity\CachedEntityPersister;
use Doctrine\ORM\Cache\Region;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\PersistentCollection;
Expand All @@ -23,7 +24,6 @@
use function assert;
use function count;

/** @psalm-import-type AssociationMapping from ClassMetadata */
abstract class AbstractCollectionPersister implements CachedCollectionPersister
{
protected UnitOfWork $uow;
Expand All @@ -38,12 +38,11 @@ abstract class AbstractCollectionPersister implements CachedCollectionPersister
protected CollectionHydrator $hydrator;
protected CacheLogger|null $cacheLogger;

/** @param AssociationMapping $association The association mapping. */
public function __construct(
protected CollectionPersister $persister,
protected Region $region,
EntityManagerInterface $em,
protected array $association,
protected AssociationMapping $association,
) {
$configuration = $em->getConfiguration();
$cacheConfig = $configuration->getSecondLevelCacheConfiguration();
Expand Down
12 changes: 7 additions & 5 deletions lib/Doctrine/ORM/Cache/Persister/Collection/ReadWriteCachedCollectionPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@
use Doctrine\ORM\Cache\CollectionCacheKey;
use Doctrine\ORM\Cache\ConcurrentRegion;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\Persisters\Collection\CollectionPersister;

use function spl_object_id;

/** @psalm-import-type AssociationMapping from ClassMetadata */
class ReadWriteCachedCollectionPersister extends AbstractCollectionPersister
{
/** @param AssociationMapping $association The association mapping. */
public function __construct(CollectionPersister $persister, ConcurrentRegion $region, EntityManagerInterface $em, array $association)
{
public function __construct(
CollectionPersister $persister,
ConcurrentRegion $region,
EntityManagerInterface $em,
AssociationMapping $association,
) {
parent::__construct($persister, $region, $em, $association);
}

Expand Down
31 changes: 13 additions & 18 deletions lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Doctrine\ORM\Cache\TimestampCacheKey;
use Doctrine\ORM\Cache\TimestampRegion;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\PersistentCollection;
Expand Down Expand Up @@ -86,7 +87,7 @@ public function getInserts(): array

public function getSelectSQL(
array|Criteria $criteria,
array|null $assoc = null,
AssociationMapping|null $assoc = null,
LockMode|int|null $lockMode = null,
int|null $limit = null,
int|null $offset = null,
Expand All @@ -113,7 +114,7 @@ public function getResultSetMapping(): ResultSetMapping
public function getSelectConditionStatementSQL(
string $field,
mixed $value,
array|null $assoc = null,
AssociationMapping|null $assoc = null,
string|null $comparison = null,
): string {
return $this->persister->getSelectConditionStatementSQL($field, $value, $assoc, $comparison);
Expand Down Expand Up @@ -169,8 +170,8 @@ private function storeJoinedAssociations(object $entity): void
foreach ($this->class->associationMappings as $name => $assoc) {
if (
isset($assoc['cache']) &&
($assoc['type'] & ClassMetadata::TO_ONE) &&
($assoc['fetch'] === ClassMetadata::FETCH_EAGER || ! $assoc['isOwningSide'])
($assoc->isToOne()) &&
($assoc['fetch'] === ClassMetadata::FETCH_EAGER || ! $assoc->isOwningSide())
) {
$associations[] = $name;
}
Expand Down Expand Up @@ -241,7 +242,7 @@ public function getClassMetadata(): ClassMetadata
* {@inheritdoc}
*/
public function getManyToManyCollection(
array $assoc,
AssociationMapping $assoc,
object $sourceEntity,
int|null $offset = null,
int|null $limit = null,
Expand All @@ -253,7 +254,7 @@ public function getManyToManyCollection(
* {@inheritdoc}
*/
public function getOneToManyCollection(
array $assoc,
AssociationMapping $assoc,
object $sourceEntity,
int|null $offset = null,
int|null $limit = null,
Expand Down Expand Up @@ -282,7 +283,7 @@ public function executeInserts(): array
public function load(
array $criteria,
object|null $entity = null,
array|null $assoc = null,
AssociationMapping|null $assoc = null,
array $hints = [],
LockMode|int|null $lockMode = null,
int|null $limit = null,
Expand Down Expand Up @@ -455,7 +456,7 @@ public function loadCriteria(Criteria $criteria): array
* {@inheritdoc}
*/
public function loadManyToManyCollection(
array $assoc,
AssociationMapping $assoc,
object $sourceEntity,
PersistentCollection $collection,
): array {
Expand Down Expand Up @@ -485,11 +486,8 @@ public function loadManyToManyCollection(
return $list;
}

/**
* {@inheritdoc}
*/
public function loadOneToManyCollection(
array $assoc,
AssociationMapping $assoc,
object $sourceEntity,
PersistentCollection $collection,
): mixed {
Expand Down Expand Up @@ -522,7 +520,7 @@ public function loadOneToManyCollection(
/**
* {@inheritdoc}
*/
public function loadOneToOneEntity(array $assoc, object $sourceEntity, array $identifier = []): object|null
public function loadOneToOneEntity(AssociationMapping $assoc, object $sourceEntity, array $identifier = []): object|null
{
return $this->persister->loadOneToOneEntity($assoc, $sourceEntity, $identifier);
}
Expand All @@ -543,11 +541,8 @@ public function refresh(array $id, object $entity, LockMode|int|null $lockMode =
$this->persister->refresh($id, $entity, $lockMode);
}

/**
* @param array<string, mixed> $association
* @param array<string, mixed> $ownerId
*/
protected function buildCollectionCacheKey(array $association, array $ownerId): CollectionCacheKey
/** @param array<string, mixed> $ownerId */
protected function buildCollectionCacheKey(AssociationMapping $association, array $ownerId): CollectionCacheKey
{
$metadata = $this->metadataFactory->getMetadataFor($association['sourceEntity']);
assert($metadata instanceof ClassMetadata);
Expand Down
4 changes: 1 addition & 3 deletions lib/Doctrine/ORM/Internal/Hydration/ArrayHydrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

namespace Doctrine\ORM\Internal\Hydration;

use Doctrine\ORM\Mapping\ClassMetadata;

use function array_key_last;
use function count;
use function is_array;
Expand Down Expand Up @@ -103,7 +101,7 @@ protected function hydrateRowData(array $row, array &$result): void
$relation = $parentClass->associationMappings[$relationAlias];

// Check the type of the relation (many or single-valued)
if (! ($relation['type'] & ClassMetadata::TO_ONE)) {
if (! $relation->isToOne()) {
$oneToOne = false;

if (! isset($baseElement[$relationAlias])) {
Expand Down
6 changes: 3 additions & 3 deletions lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ protected function prepare(): void
$class = $this->getClassMetadata($className);
$inverseAssoc = $class->associationMappings[$assoc['inversedBy']];

if (! ($inverseAssoc['type'] & ClassMetadata::TO_ONE)) {
if (! $inverseAssoc->isToOne()) {
continue;
}

Expand Down Expand Up @@ -364,7 +364,7 @@ protected function hydrateRowData(array $row, array &$result): void
$oid = spl_object_id($parentObject);

// Check the type of the relation (many or single-valued)
if (! ($relation['type'] & ClassMetadata::TO_ONE)) {
if (! $relation->isToOne()) {
// PATH A: Collection-valued association
$reflFieldValue = $reflField->getValue($parentObject);

Expand Down Expand Up @@ -435,7 +435,7 @@ protected function hydrateRowData(array $row, array &$result): void
// If there is an inverse mapping on the target class its bidirectional
if ($relation['inversedBy']) {
$inverseAssoc = $targetClass->associationMappings[$relation['inversedBy']];
if ($inverseAssoc['type'] & ClassMetadata::TO_ONE) {
if ($inverseAssoc->isToOne()) {
$targetClass->reflFields[$inverseAssoc['fieldName']]->setValue($element, $parentObject);
$this->uow->setOriginalEntityProperty(spl_object_id($element), $inverseAssoc['fieldName'], $parentObject);
}
Expand Down
Loading