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

Add ClassMetadataFactoryInterface to allow switching implementation #11497

Open
wants to merge 1 commit into
base: 3.2.x
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@

<rule ref="SlevomatCodingStandard.Classes.SuperfluousInterfaceNaming">
<exclude-pattern>src/EntityManagerInterface.php</exclude-pattern>
<exclude-pattern>src/Mapping/ClassMetadataFactoryInterface.php</exclude-pattern>
Copy link
Member

Choose a reason for hiding this comment

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

Why was this excluded from phpcs checks? It's going to be a new interface that you can name without the superflous naming.

Copy link
Author

Choose a reason for hiding this comment

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

Doctrine\ORM\Mapping\ClassMetadataFactory is already used (is the name of the current implementation).
Do you have a better proposal for the name of this interface?

</rule>

<rule ref="SlevomatCodingStandard.Classes.SuperfluousTraitNaming.SuperfluousSuffix">
Expand Down
8 changes: 4 additions & 4 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -96,22 +96,22 @@ parameters:
path: src/Cache/TimestampQueryCacheValidator.php

-
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactory\\) of method Doctrine\\\\ORM\\\\Decorator\\\\EntityManagerDecorator\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getMetadataFactory\\(\\)$#"
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactoryInterface\\) of method Doctrine\\\\ORM\\\\Decorator\\\\EntityManagerDecorator\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getMetadataFactory\\(\\)$#"
count: 1
path: src/Decorator/EntityManagerDecorator.php

-
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactory\\) of method Doctrine\\\\ORM\\\\Decorator\\\\EntityManagerDecorator\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManagerDecorator\\<Doctrine\\\\ORM\\\\EntityManagerInterface\\>\\:\\:getMetadataFactory\\(\\)$#"
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactoryInterface\\) of method Doctrine\\\\ORM\\\\Decorator\\\\EntityManagerDecorator\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManagerDecorator\\<Doctrine\\\\ORM\\\\EntityManagerInterface\\>\\:\\:getMetadataFactory\\(\\)$#"
count: 1
path: src/Decorator/EntityManagerDecorator.php

-
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactory\\) of method Doctrine\\\\ORM\\\\EntityManager\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getMetadataFactory\\(\\)$#"
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactoryInterface\\) of method Doctrine\\\\ORM\\\\EntityManager\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getMetadataFactory\\(\\)$#"
count: 1
path: src/EntityManager.php

-
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactory\\) of method Doctrine\\\\ORM\\\\EntityManagerInterface\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getMetadataFactory\\(\\)$#"
message: "#^Return type \\(Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataFactoryInterface\\) of method Doctrine\\\\ORM\\\\EntityManagerInterface\\:\\:getMetadataFactory\\(\\) should be compatible with return type \\(Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadataFactory\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\>\\) of method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getMetadataFactory\\(\\)$#"
count: 1
path: src/EntityManagerInterface.php

Expand Down
2 changes: 1 addition & 1 deletion psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
<code><![CDATA[$this->metadataFactory]]></code>
</InvalidReturnStatement>
<InvalidReturnType>
<code><![CDATA[ClassMetadataFactory]]></code>
<code><![CDATA[ClassMetadataFactoryInterface]]></code>
</InvalidReturnType>
<PossiblyNullArgument>
<code><![CDATA[$config->getProxyDir()]]></code>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\Persisters\Collection\CollectionPersister;
use Doctrine\ORM\Proxy\DefaultProxyClassNameResolver;
Expand All @@ -27,7 +27,7 @@
abstract class AbstractCollectionPersister implements CachedCollectionPersister
{
protected UnitOfWork $uow;
protected ClassMetadataFactory $metadataFactory;
protected ClassMetadataFactoryInterface $metadataFactory;
protected ClassMetadata $sourceEntity;
protected ClassMetadata $targetEntity;

Expand Down
4 changes: 2 additions & 2 deletions src/Cache/Persister/Entity/AbstractEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\AssociationMapping;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\PersistentCollection;
use Doctrine\ORM\Persisters\Entity\EntityPersister;
use Doctrine\ORM\Proxy\DefaultProxyClassNameResolver;
Expand All @@ -35,7 +35,7 @@
abstract class AbstractEntityPersister implements CachedEntityPersister
{
protected UnitOfWork $uow;
protected ClassMetadataFactory $metadataFactory;
protected ClassMetadataFactoryInterface $metadataFactory;

/** @var mixed[] */
protected array $queuedCache = [];
Expand Down
8 changes: 8 additions & 0 deletions src/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\ORM\Cache\CacheConfiguration;
use Doctrine\ORM\Exception\InvalidClassMetadataFactory;
use Doctrine\ORM\Exception\InvalidEntityRepository;
use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\Mapping\DefaultEntityListenerResolver;
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
use Doctrine\ORM\Mapping\DefaultQuoteStrategy;
Expand All @@ -25,6 +27,7 @@
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use LogicException;
use Psr\Cache\CacheItemPoolInterface;
use ReflectionClass;

use function class_exists;
use function is_a;
Expand Down Expand Up @@ -375,6 +378,11 @@ public function addCustomHydrationMode(string $modeName, string $hydrator): void
*/
public function setClassMetadataFactoryName(string $cmfName): void
{
$reflectionClass = new ReflectionClass($cmfName);
if (! $reflectionClass->implementsInterface(ClassMetadataFactoryInterface::class)) {
throw InvalidClassMetadataFactory::create($cmfName);
}
Comment on lines +381 to +384
Copy link
Member

Choose a reason for hiding this comment

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

This would introduce an exception when there was not one before.

Copy link
Author

Choose a reason for hiding this comment

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

Prior to this PR a call with a class different from Doctrine\ORM\Mapping\ClassMetadataFactory would cause a TypeError. So yes, it would not throw an exception here, but in a different (and completely unrelated) point of the code, which can be harder to debug.


$this->attributes['classMetadataFactoryName'] = $cmfName;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Decorator/EntityManagerDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\NativeQuery;
use Doctrine\ORM\Proxy\ProxyFactory;
use Doctrine\ORM\Query;
Expand Down Expand Up @@ -42,7 +42,7 @@ public function getRepository(string $className): EntityRepository
return $this->wrapped->getRepository($className);
}

public function getMetadataFactory(): ClassMetadataFactory
public function getMetadataFactory(): ClassMetadataFactoryInterface
{
return $this->wrapped->getMetadataFactory();
}
Expand Down
6 changes: 3 additions & 3 deletions src/EntityManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
use Doctrine\ORM\Exception\UnrecognizedIdentifierFields;
use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\Proxy\DefaultProxyClassNameResolver;
use Doctrine\ORM\Proxy\ProxyFactory;
use Doctrine\ORM\Query\Expr;
Expand Down Expand Up @@ -63,7 +63,7 @@ class EntityManager implements EntityManagerInterface
/**
* The metadata factory, used to retrieve the ORM metadata of entity classes.
*/
private ClassMetadataFactory $metadataFactory;
private ClassMetadataFactoryInterface $metadataFactory;

/**
* The UnitOfWork used to coordinate object-level transactions.
Expand Down Expand Up @@ -154,7 +154,7 @@ public function getConnection(): Connection
return $this->conn;
}

public function getMetadataFactory(): ClassMetadataFactory
public function getMetadataFactory(): ClassMetadataFactoryInterface
{
return $this->metadataFactory;
}
Expand Down
4 changes: 2 additions & 2 deletions src/EntityManagerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use Doctrine\DBAL\LockMode;
use Doctrine\ORM\Exception\ORMException;
use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\Proxy\ProxyFactory;
use Doctrine\ORM\Query\Expr;
use Doctrine\ORM\Query\FilterCollection;
Expand Down Expand Up @@ -40,7 +40,7 @@ public function getCache(): Cache|null;
*/
public function getConnection(): Connection;

public function getMetadataFactory(): ClassMetadataFactory;
public function getMetadataFactory(): ClassMetadataFactoryInterface;

/**
* Gets an ExpressionBuilder used for object-oriented construction of query expressions.
Expand Down
18 changes: 18 additions & 0 deletions src/Exception/InvalidClassMetadataFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Doctrine\ORM\Exception;

use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use LogicException;

use function sprintf;

class InvalidClassMetadataFactory extends LogicException implements ConfigurationException
{
public static function create(string $className): self
{
return new self(sprintf("Invalid class metadata factory class '%s'. It must be a %s.", $className, ClassMetadataFactoryInterface::class));
}
}
14 changes: 1 addition & 13 deletions src/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
*
* @extends AbstractClassMetadataFactory<ClassMetadata>
*/
class ClassMetadataFactory extends AbstractClassMetadataFactory
class ClassMetadataFactory extends AbstractClassMetadataFactory implements ClassMetadataFactoryInterface
{
private EntityManagerInterface|null $em = null;
private AbstractPlatform|null $targetPlatform = null;
Expand All @@ -69,18 +69,6 @@ public function setEntityManager(EntityManagerInterface $em): void
$this->em = $em;
}

/**
* @param A $maybeOwningSide
*
* @return (A is ManyToManyAssociationMapping ? ManyToManyOwningSideMapping : (
* A is OneToOneAssociationMapping ? OneToOneOwningSideMapping : (
* A is OneToManyAssociationMapping ? ManyToOneAssociationMapping : (
* A is ManyToOneAssociationMapping ? ManyToOneAssociationMapping :
* ManyToManyOwningSideMapping|OneToOneOwningSideMapping|ManyToOneAssociationMapping
* ))))
*
* @template A of AssociationMapping
*/
final public function getOwningSide(AssociationMapping $maybeOwningSide): OwningSideMapping
{
if ($maybeOwningSide instanceof OwningSideMapping) {
Expand Down
37 changes: 37 additions & 0 deletions src/Mapping/ClassMetadataFactoryInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Doctrine\ORM\Mapping;

use Doctrine\ORM\EntityManagerInterface;
use Doctrine\Persistence\Mapping\ClassMetadataFactory;
use Psr\Cache\CacheItemPoolInterface;

/** @template-extends ClassMetadataFactory<ClassMetadata> */
interface ClassMetadataFactoryInterface extends ClassMetadataFactory
{
/**
* Sets the cache for created metadata.
*/
public function setCache(CacheItemPoolInterface $cache): void;

/**
* Sets the entity manager owning the factory.
*/
public function setEntityManager(EntityManagerInterface $em): void;

/**
* @param A $maybeOwningSide
*
* @return (A is ManyToManyAssociationMapping ? ManyToManyOwningSideMapping : (
* A is OneToOneAssociationMapping ? OneToOneOwningSideMapping : (
* A is OneToManyAssociationMapping ? ManyToOneAssociationMapping : (
* A is ManyToOneAssociationMapping ? ManyToOneAssociationMapping :
* ManyToManyOwningSideMapping|OneToOneOwningSideMapping|ManyToOneAssociationMapping
* ))))
*
* @template A of AssociationMapping
*/
public function getOwningSide(AssociationMapping $maybeOwningSide): OwningSideMapping;
}
4 changes: 2 additions & 2 deletions tests/Performance/Mock/NonProxyLoadingEntityManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\NativeQuery;
use Doctrine\ORM\Proxy\ProxyFactory;
use Doctrine\ORM\Query;
Expand Down Expand Up @@ -45,7 +45,7 @@ public function getProxyFactory(): ProxyFactory
);
}

public function getMetadataFactory(): ClassMetadataFactory
public function getMetadataFactory(): ClassMetadataFactoryInterface
{
return $this->realEntityManager->getMetadataFactory();
}
Expand Down
8 changes: 7 additions & 1 deletion tests/Tests/ORM/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Doctrine\ORM\Cache\CacheConfiguration;
use Doctrine\ORM\Configuration;
use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\Exception\InvalidClassMetadataFactory;
use Doctrine\ORM\Exception\InvalidEntityRepository;
use Doctrine\ORM\Mapping as MappingNamespace;
use Doctrine\ORM\Mapping\DefaultTypedFieldMapper;
Expand All @@ -16,6 +17,7 @@
use Doctrine\ORM\Proxy\ProxyFactory;
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use Doctrine\Tests\Models\DDC753\DDC753CustomRepository;
use Doctrine\Tests\ORM\Mapping\ClassMetadataFactoryTestSubject;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\TestCase;
use Psr\Cache\CacheItemPoolInterface;
Expand Down Expand Up @@ -147,8 +149,12 @@ public function testSetCustomHydrationModes(): void
public function testSetGetClassMetadataFactoryName(): void
{
self::assertSame(MappingNamespace\ClassMetadataFactory::class, $this->configuration->getClassMetadataFactoryName());
$this->configuration->setClassMetadataFactoryName(ClassMetadataFactoryTestSubject::class);
self::assertSame(ClassMetadataFactoryTestSubject::class, $this->configuration->getClassMetadataFactoryName());

$this->expectException(InvalidClassMetadataFactory::class);
$this->expectExceptionMessage('Invalid class metadata factory class \'Doctrine\Tests\ORM\ConfigurationTest\'. It must be a Doctrine\ORM\Mapping\ClassMetadataFactoryInterface.');
$this->configuration->setClassMetadataFactoryName(self::class);
self::assertSame(self::class, $this->configuration->getClassMetadataFactoryName());
}

public function testAddGetFilters(): void
Expand Down
3 changes: 2 additions & 1 deletion tests/Tests/ORM/Mapping/MappingDriverTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Doctrine\ORM\Mapping as ORM;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataFactory;
use Doctrine\ORM\Mapping\ClassMetadataFactoryInterface;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
use Doctrine\ORM\Mapping\DefaultTypedFieldMapper;
Expand Down Expand Up @@ -92,7 +93,7 @@ public function createClassMetadata(
return $class;
}

protected function createClassMetadataFactory(EntityManagerInterface|null $em = null): ClassMetadataFactory
protected function createClassMetadataFactory(EntityManagerInterface|null $em = null): ClassMetadataFactoryInterface
{
$driver = $this->loadDriver();
$em ??= $this->getTestEntityManager();
Expand Down
Loading