From c9253ef64b60cdee969cef2476439fed4db31fbc Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Thu, 2 Nov 2023 13:46:15 -0400 Subject: [PATCH 1/4] feat: make `PersistentCollection::first()` "extra" lazy --- docs/en/tutorials/extra-lazy-associations.rst | 1 + src/PersistentCollection.php | 14 +++++ .../Functional/ExtraLazyCollectionTest.php | 57 +++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/docs/en/tutorials/extra-lazy-associations.rst b/docs/en/tutorials/extra-lazy-associations.rst index 4c31357eecc..1dae001a36c 100644 --- a/docs/en/tutorials/extra-lazy-associations.rst +++ b/docs/en/tutorials/extra-lazy-associations.rst @@ -17,6 +17,7 @@ can be called without triggering a full load of the collection: - ``Collection#contains($entity)`` - ``Collection#containsKey($key)`` - ``Collection#count()`` +- ``Collection#first()`` - ``Collection#get($key)`` - ``Collection#slice($offset, $length = null)`` diff --git a/src/PersistentCollection.php b/src/PersistentCollection.php index d54d3d1b997..ef5f28c3cb3 100644 --- a/src/PersistentCollection.php +++ b/src/PersistentCollection.php @@ -504,6 +504,20 @@ public function __wakeup(): void $this->em = null; } + /** + * {@inheritDoc} + */ + public function first() + { + if (! $this->initialized && ! $this->isDirty && $this->getMapping()['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) { + $persister = $this->getUnitOfWork()->getCollectionPersister($this->getMapping()); + + return array_values($persister->slice($this, 0, 1))[0] ?? false; + } + + return parent::first(); + } + /** * Extracts a slice of $length elements starting at position $offset from the Collection. * diff --git a/tests/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Tests/ORM/Functional/ExtraLazyCollectionTest.php index a092f6555cd..569ea135009 100644 --- a/tests/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -179,6 +179,63 @@ public function testCountOneToManyJoinedInheritance(): void self::assertCount(2, $otherClass->childClasses); } + #[Group('non-cacheable')] + public function testFirstWhenInitialized(): void + { + $user = $this->_em->find(CmsUser::class, $this->userId); + $this->getQueryLog()->reset()->enable(); + $user->groups->toArray(); + + self::assertTrue($user->groups->isInitialized()); + self::assertInstanceOf(CmsGroup::class, $user->groups->first()); + $this->assertQueryCount(1, 'Should only execute one query to initialize collection, no extra query for first().'); + } + + public function testFirstOnEmptyCollectionWhenInitialized(): void + { + foreach ($this->_em->getRepository(CmsGroup::class)->findAll() as $group) { + $this->_em->remove($group); + } + + $this->_em->flush(); + + $user = $this->_em->find(CmsUser::class, $this->userId); + $this->getQueryLog()->reset()->enable(); + $user->groups->toArray(); + + self::assertTrue($user->groups->isInitialized()); + self::assertFalse($user->groups->first()); + $this->assertQueryCount(1, 'Should only execute one query to initialize collection, no extra query for first().'); + } + + public function testFirstWhenNotInitialized(): void + { + $user = $this->_em->find(CmsUser::class, $this->userId); + $this->getQueryLog()->reset()->enable(); + + self::assertFalse($user->groups->isInitialized()); + self::assertInstanceOf(CmsGroup::class, $user->groups->first()); + self::assertFalse($user->groups->isInitialized()); + $this->assertQueryCount(1, 'Should only execute one query for first().'); + } + + public function testFirstOnEmptyCollectionWhenNotInitialized(): void + { + foreach ($this->_em->getRepository(CmsGroup::class)->findAll() as $group) { + $this->_em->remove($group); + } + + $this->_em->flush(); + + $user = $this->_em->find(CmsUser::class, $this->userId); + $this->getQueryLog()->reset()->enable(); + + self::assertFalse($user->groups->isInitialized()); + self::assertFalse($user->groups->first()); + self::assertFalse($user->groups->isInitialized()); + $this->assertQueryCount(1, 'Should only execute one query for first().'); + } + #[Group('DDC-546')] public function testFullSlice(): void { From 81c0d599c956071e974f9ba4134a8cab9d07565c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Thu, 27 Jun 2024 23:22:59 +0200 Subject: [PATCH 2/4] Implement compatibility with Persistence 4 --- composer.json | 2 +- phpcs.xml.dist | 2 ++ phpstan-baseline.neon | 5 +++ phpstan-dbal3.neon | 5 +++ phpstan.neon | 5 +++ psalm-baseline.xml | 23 ++++++++---- src/EntityManager.php | 4 +-- src/Mapping/ClassMetadata.php | 12 ++----- .../Driver/LoadMappingFileImplementation.php | 35 +++++++++++++++++++ src/Mapping/Driver/XmlDriver.php | 8 ++--- .../GetReflectionClassImplementation.php | 33 +++++++++++++++++ .../Mock/NonProxyLoadingEntityManager.php | 8 +++++ tests/Tests/Mocks/MetadataDriverMock.php | 6 ++-- .../ORM/Functional/Ticket/DDC3103Test.php | 14 ++++++-- tests/Tests/ORM/Mapping/ClassMetadataTest.php | 5 +++ 15 files changed, 139 insertions(+), 28 deletions(-) create mode 100644 src/Mapping/Driver/LoadMappingFileImplementation.php create mode 100644 src/Mapping/GetReflectionClassImplementation.php diff --git a/composer.json b/composer.json index ec2ff1fd5a7..7269a3c08a1 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "doctrine/inflector": "^1.4 || ^2.0", "doctrine/instantiator": "^1.3 || ^2", "doctrine/lexer": "^3", - "doctrine/persistence": "^3.3.1", + "doctrine/persistence": "^3.3.1 || ^4", "psr/cache": "^1 || ^2 || ^3", "symfony/console": "^5.4 || ^6.0 || ^7.0", "symfony/var-exporter": "^6.3.9 || ^7.0" diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 4fde4cbf5e5..b14bbc6c1d8 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -48,6 +48,8 @@ + src/Mapping/Driver/LoadMappingFileImplementation.php + src/Mapping/GetReflectionClassImplementation.php tests/* diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7c8ec81ae84..1b6362a6d8e 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -120,6 +120,11 @@ parameters: count: 1 path: src/EntityRepository.php + - + message: "#^If condition is always true\\.$#" + count: 1 + path: src/Mapping/ClassMetadata.php + - message: "#^If condition is always true\\.$#" count: 1 diff --git a/phpstan-dbal3.neon b/phpstan-dbal3.neon index 724fe2003f7..889d94c1df3 100644 --- a/phpstan-dbal3.neon +++ b/phpstan-dbal3.neon @@ -34,3 +34,8 @@ parameters: - message: '~deprecated class Doctrine\\DBAL\\Tools\\Console\\Command\\ReservedWordsCommand\:~' path: src/Tools/Console/ConsoleRunner.php + + # Compatibility with Persistence 3 + - + message: '#Expression on left side of \?\? is not nullable.#' + path: src/Mapping/Driver/AttributeDriver.php diff --git a/phpstan.neon b/phpstan.neon index d90ec9fe41f..f98eb8d00b8 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -45,3 +45,8 @@ parameters: message: '#Negated boolean expression is always false\.#' paths: - src/Mapping/Driver/AttributeDriver.php + + # Compatibility with Persistence 3 + - + message: '#Expression on left side of \?\? is not nullable.#' + path: src/Mapping/Driver/AttributeDriver.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c7897f4d614..cef61546b74 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -298,15 +298,9 @@ - - - - - reflClass]]> - @@ -340,6 +334,7 @@ + reflClass]]> @@ -474,6 +469,14 @@ + + + + + + + + @@ -526,6 +529,14 @@ + + + + + + + + diff --git a/src/EntityManager.php b/src/EntityManager.php index eb5a123d0b6..5324d9cac7b 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -565,9 +565,9 @@ public function initializeObject(object $obj): void /** * {@inheritDoc} */ - public function isUninitializedObject($obj): bool + public function isUninitializedObject($value): bool { - return $this->unitOfWork->isUninitializedObject($obj); + return $this->unitOfWork->isUninitializedObject($value); } public function getFilters(): FilterCollection diff --git a/src/Mapping/ClassMetadata.php b/src/Mapping/ClassMetadata.php index 7c9020805a5..b91bd9b5ac1 100644 --- a/src/Mapping/ClassMetadata.php +++ b/src/Mapping/ClassMetadata.php @@ -73,6 +73,8 @@ */ class ClassMetadata implements PersistenceClassMetadata, Stringable { + use GetReflectionClassImplementation; + /* The inheritance mapping types */ /** * NONE means the class does not participate in an inheritance hierarchy @@ -932,16 +934,6 @@ public function validateLifecycleCallbacks(ReflectionService $reflService): void } } - /** - * {@inheritDoc} - * - * Can return null when using static reflection, in violation of the LSP - */ - public function getReflectionClass(): ReflectionClass|null - { - return $this->reflClass; - } - /** @psalm-param array{usage?: mixed, region?: mixed} $cache */ public function enableCache(array $cache): void { diff --git a/src/Mapping/Driver/LoadMappingFileImplementation.php b/src/Mapping/Driver/LoadMappingFileImplementation.php new file mode 100644 index 00000000000..df351889ba2 --- /dev/null +++ b/src/Mapping/Driver/LoadMappingFileImplementation.php @@ -0,0 +1,35 @@ +doLoadMappingFile($file); + } + } +} else { + /** @internal */ + trait LoadMappingFileImplementation + { + /** + * {@inheritDoc} + */ + protected function loadMappingFile($file) + { + return $this->doLoadMappingFile($file); + } + } +} diff --git a/src/Mapping/Driver/XmlDriver.php b/src/Mapping/Driver/XmlDriver.php index 30e85900e2e..e11b6b61d6f 100644 --- a/src/Mapping/Driver/XmlDriver.php +++ b/src/Mapping/Driver/XmlDriver.php @@ -43,6 +43,8 @@ */ class XmlDriver extends FileDriver { + use LoadMappingFileImplementation; + public const DEFAULT_FILE_EXTENSION = '.dcm.xml'; /** @@ -878,10 +880,8 @@ private function getCascadeMappings(SimpleXMLElement $cascadeElement): array return $cascades; } - /** - * {@inheritDoc} - */ - protected function loadMappingFile($file) + /** @return array */ + private function doLoadMappingFile(string $file): array { $this->validateMapping($file); $result = []; diff --git a/src/Mapping/GetReflectionClassImplementation.php b/src/Mapping/GetReflectionClassImplementation.php new file mode 100644 index 00000000000..780015c3680 --- /dev/null +++ b/src/Mapping/GetReflectionClassImplementation.php @@ -0,0 +1,33 @@ +reflClass; + } + } +} else { + trait GetReflectionClassImplementation + { + /** + * {@inheritDoc} + * + * Can return null when using static reflection, in violation of the LSP + */ + public function getReflectionClass(): ReflectionClass|null + { + return $this->reflClass; + } + } +} diff --git a/tests/Performance/Mock/NonProxyLoadingEntityManager.php b/tests/Performance/Mock/NonProxyLoadingEntityManager.php index 20f233e0089..bff330ab9be 100644 --- a/tests/Performance/Mock/NonProxyLoadingEntityManager.php +++ b/tests/Performance/Mock/NonProxyLoadingEntityManager.php @@ -212,4 +212,12 @@ public function contains(object $object): bool { return $this->realEntityManager->contains($object); } + + /** + * {@inheritDoc} + */ + public function isUninitializedObject($value): bool + { + return $this->realEntityManager->isUninitializedObject($value); + } } diff --git a/tests/Tests/Mocks/MetadataDriverMock.php b/tests/Tests/Mocks/MetadataDriverMock.php index b3be873c596..a2472cf8c46 100644 --- a/tests/Tests/Mocks/MetadataDriverMock.php +++ b/tests/Tests/Mocks/MetadataDriverMock.php @@ -15,14 +15,14 @@ class MetadataDriverMock implements MappingDriver /** * {@inheritDoc} */ - public function loadMetadataForClass($className, ClassMetadata $metadata) + public function loadMetadataForClass($className, ClassMetadata $metadata): void { } /** * {@inheritDoc} */ - public function isTransient($className) + public function isTransient($className): bool { return false; } @@ -30,7 +30,7 @@ public function isTransient($className) /** * {@inheritDoc} */ - public function getAllClassNames() + public function getAllClassNames(): array { return []; } diff --git a/tests/Tests/ORM/Functional/Ticket/DDC3103Test.php b/tests/Tests/ORM/Functional/Ticket/DDC3103Test.php index ec505c07741..5ead8d58698 100644 --- a/tests/Tests/ORM/Functional/Ticket/DDC3103Test.php +++ b/tests/Tests/ORM/Functional/Ticket/DDC3103Test.php @@ -7,10 +7,12 @@ use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\Column; use Doctrine\ORM\Mapping\Embeddable; +use Doctrine\Persistence\Mapping\StaticReflectionService; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Group; +use function class_exists; use function serialize; use function unserialize; @@ -18,6 +20,15 @@ #[Group('DDC-3103')] class DDC3103Test extends OrmFunctionalTestCase { + protected function setUp(): void + { + if (! class_exists(StaticReflectionService::class)) { + self::markTestSkipped('This test is not supported by the current installed doctrine/persistence version'); + } + + parent::setUp(); + } + public function testIssue(): void { $classMetadata = new ClassMetadata(DDC3103ArticleId::class); @@ -39,7 +50,6 @@ public function testIssue(): void #[Embeddable] class DDC3103ArticleId { - /** @var string */ #[Column(name: 'name', type: 'string', length: 255)] - protected $nameValue; + protected string $nameValue; } diff --git a/tests/Tests/ORM/Mapping/ClassMetadataTest.php b/tests/Tests/ORM/Mapping/ClassMetadataTest.php index 38b2bde7ca9..60b1bc1822f 100644 --- a/tests/Tests/ORM/Mapping/ClassMetadataTest.php +++ b/tests/Tests/ORM/Mapping/ClassMetadataTest.php @@ -56,6 +56,7 @@ use stdClass; use function assert; +use function class_exists; use function count; use function serialize; use function str_contains; @@ -979,6 +980,10 @@ public function testCanInstantiateInternalPhpClassSubclassFromUnserializedMetada public function testWakeupReflectionWithEmbeddableAndStaticReflectionService(): void { + if (! class_exists(StaticReflectionService::class)) { + self::markTestSkipped('This test is not supported by the current installed doctrine/persistence version'); + } + $classMetadata = new ClassMetadata(TestEntity1::class); $classMetadata->mapEmbedded( From da51234d5aaba3901e91461982c07e54d97cce9c Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 12 May 2022 22:26:34 +0200 Subject: [PATCH 3/4] [GH-3519] Deprecate passing the same class with different discriminator values. --- UPGRADE.md | 9 +++++++-- src/Mapping/ClassMetadata.php | 20 +++++++++++++++++++ tests/Tests/ORM/Mapping/ClassMetadataTest.php | 8 ++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 6da5e0be57c..12e67dee2cd 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,8 @@ +# Upgrade to 3.4 + +Using the same class several times in a discriminator map is deprecated. +In 4.0, this will be an error. + # Upgrade to 3.3 ## Deprecate `DatabaseDriver` @@ -737,7 +742,7 @@ Use `toIterable()` instead. Output walkers should implement the new `\Doctrine\ORM\Query\OutputWalker` interface and create `Doctrine\ORM\Query\Exec\SqlFinalizer` instances instead of `Doctrine\ORM\Query\Exec\AbstractSqlExecutor`s. -The output walker must not base its workings on the query `firstResult`/`maxResult` values, so that the +The output walker must not base its workings on the query `firstResult`/`maxResult` values, so that the `SqlFinalizer` can be kept in the query cache and used regardless of the actual `firstResult`/`maxResult` values. Any operation dependent on `firstResult`/`maxResult` should take place within the `SqlFinalizer::createExecutor()` method. Details can be found at https://github.com/doctrine/orm/pull/11188. @@ -750,7 +755,7 @@ change in behavior. Progress on this is tracked at https://github.com/doctrine/orm/issues/11624 . -## PARTIAL DQL syntax is undeprecated +## PARTIAL DQL syntax is undeprecated Use of the PARTIAL keyword is not deprecated anymore in DQL, because we will be able to support PARTIAL objects with PHP 8.4 Lazy Objects and diff --git a/src/Mapping/ClassMetadata.php b/src/Mapping/ClassMetadata.php index b91bd9b5ac1..a72099bd4d8 100644 --- a/src/Mapping/ClassMetadata.php +++ b/src/Mapping/ClassMetadata.php @@ -25,7 +25,10 @@ use Stringable; use function array_column; +use function array_count_values; use function array_diff; +use function array_filter; +use function array_flip; use function array_intersect; use function array_key_exists; use function array_keys; @@ -39,6 +42,7 @@ use function defined; use function enum_exists; use function explode; +use function implode; use function in_array; use function interface_exists; use function is_string; @@ -2178,6 +2182,22 @@ final public function getDiscriminatorColumn(): DiscriminatorColumnMapping */ public function setDiscriminatorMap(array $map): void { + if (count(array_flip($map)) !== count($map)) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/issues/3519', + <<<'DEPRECATION' + Mapping a class to multiple discriminator values is deprecated, + and the discriminator mapping of %s contains duplicate values + for the following discriminator values: %s. + DEPRECATION, + $this->name, + implode(', ', array_keys(array_filter(array_count_values($map), static function (int $value): bool { + return $value > 1; + }))), + ); + } + foreach ($map as $value => $className) { $this->addDiscriminatorMapClass($value, $className); } diff --git a/tests/Tests/ORM/Mapping/ClassMetadataTest.php b/tests/Tests/ORM/Mapping/ClassMetadataTest.php index 60b1bc1822f..57736cc68a8 100644 --- a/tests/Tests/ORM/Mapping/ClassMetadataTest.php +++ b/tests/Tests/ORM/Mapping/ClassMetadataTest.php @@ -1101,6 +1101,14 @@ public function testClassNameMappingDiscriminatorValue(): void $xmlElement->children()->{'discriminator-map'}->{'discriminator-mapping'}[0]->attributes()['value'], ); } + + public function testDiscriminatorMapWithSameClassMultipleTimesDeprecated(): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/3519'); + + $cm = new ClassMetadata(CMS\CmsUser::class); + $cm->setDiscriminatorMap(['foo' => CMS\CmsUser::class, 'bar' => CMS\CmsUser::class]); + } } #[MappedSuperclass] From dd3604f523cc35283f0c4f4c36329b322845e57e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sat, 23 Nov 2024 21:06:56 +0100 Subject: [PATCH 4/4] Use properties over array keys Using array access is deprecated. --- src/PersistentCollection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PersistentCollection.php b/src/PersistentCollection.php index ef5f28c3cb3..876a92a261a 100644 --- a/src/PersistentCollection.php +++ b/src/PersistentCollection.php @@ -509,7 +509,7 @@ public function __wakeup(): void */ public function first() { - if (! $this->initialized && ! $this->isDirty && $this->getMapping()['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) { + if (! $this->initialized && ! $this->isDirty && $this->getMapping()->fetch === ClassMetadata::FETCH_EXTRA_LAZY) { $persister = $this->getUnitOfWork()->getCollectionPersister($this->getMapping()); return array_values($persister->slice($this, 0, 1))[0] ?? false;