From 9d1a4973ae2d8ddedd6b66e958e32ebb58cdf3d6 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 19 Mar 2024 09:19:25 +0100 Subject: [PATCH 1/5] Improve lazy ghost performance by avoiding self-referencing closure. (#11376) * Improve lazy ghost performance by avoiding self-referencing closure. Co-authored-by: Nicolas Grekas * update baselien --------- Co-authored-by: Nicolas Grekas --- psalm-baseline.xml | 8 ++------ src/Proxy/ProxyFactory.php | 14 ++++++-------- tests/Tests/ORM/Proxy/ProxyFactoryTest.php | 6 +++--- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c175642e62b..7a3b729e849 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1492,6 +1492,7 @@ + @@ -1501,13 +1502,8 @@ - + - - - diff --git a/src/Proxy/ProxyFactory.php b/src/Proxy/ProxyFactory.php index 0660c423f0d..5b2d2eca0c9 100644 --- a/src/Proxy/ProxyFactory.php +++ b/src/Proxy/ProxyFactory.php @@ -360,11 +360,11 @@ private function createInitializer(ClassMetadata $classMetadata, EntityPersister */ private function createLazyInitializer(ClassMetadata $classMetadata, EntityPersister $entityPersister, IdentifierFlattener $identifierFlattener): Closure { - return static function (InternalProxy $proxy, InternalProxy $original) use ($entityPersister, $classMetadata, $identifierFlattener): void { - $identifier = $classMetadata->getIdentifierValues($original); - $entity = $entityPersister->loadById($identifier, $original); + return static function (InternalProxy $proxy) use ($entityPersister, $classMetadata, $identifierFlattener): void { + $identifier = $classMetadata->getIdentifierValues($proxy); + $original = $entityPersister->loadById($identifier); - if ($entity === null) { + if ($original === null) { throw EntityNotFoundException::fromClassNameAndIdentifier( $classMetadata->getName(), $identifierFlattener->flattenIdentifier($classMetadata, $identifier) @@ -382,7 +382,7 @@ private function createLazyInitializer(ClassMetadata $classMetadata, EntityPersi continue; } - $property->setValue($proxy, $property->getValue($entity)); + $property->setValue($proxy, $property->getValue($original)); } }; } @@ -468,9 +468,7 @@ private function getProxyFactory(string $className): Closure $identifierFields = array_intersect_key($class->getReflectionProperties(), $identifiers); $proxyFactory = Closure::bind(static function (array $identifier) use ($initializer, $skippedProperties, $identifierFields, $className): InternalProxy { - $proxy = self::createLazyGhost(static function (InternalProxy $object) use ($initializer, &$proxy): void { - $initializer($object, $proxy); - }, $skippedProperties); + $proxy = self::createLazyGhost($initializer, $skippedProperties); foreach ($identifierFields as $idField => $reflector) { if (! isset($identifier[$idField])) { diff --git a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php index 3739aaf4dd5..d0989953def 100644 --- a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -65,7 +65,7 @@ public function testReferenceProxyDelegatesLoadingToThePersister(): void { $identifier = ['id' => 42]; $proxyClass = 'Proxies\__CG__\Doctrine\Tests\Models\ECommerce\ECommerceFeature'; - $persister = $this->getMockBuilderWithOnlyMethods(BasicEntityPersister::class, ['load']) + $persister = $this->getMockBuilderWithOnlyMethods(BasicEntityPersister::class, ['loadById']) ->disableOriginalConstructor() ->getMock(); @@ -75,8 +75,8 @@ public function testReferenceProxyDelegatesLoadingToThePersister(): void $persister ->expects(self::atLeastOnce()) - ->method('load') - ->with(self::equalTo($identifier), self::isInstanceOf($proxyClass)) + ->method('loadById') + ->with(self::equalTo($identifier)) ->will(self::returnValue($proxy)); $proxy->getDescription(); From d15624f72f920cfc79ea86b7d3c0a7586ad71574 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 20 Mar 2024 15:45:47 +0100 Subject: [PATCH 2/5] [GH-11185] Bugfix: do not use collection batch loading for indexBy assocations. (#11380) --- src/UnitOfWork.php | 2 +- .../Ticket/GH11149/EagerProduct.php | 41 ++++++++++++++++ .../GH11149/EagerProductTranslation.php | 45 ++++++++++++++++++ .../Functional/Ticket/GH11149/GH11149Test.php | 47 +++++++++++++++++++ .../ORM/Functional/Ticket/GH11149/Locale.php | 27 +++++++++++ 5 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php create mode 100644 tests/Tests/ORM/Functional/Ticket/GH11149/EagerProductTranslation.php create mode 100644 tests/Tests/ORM/Functional/Ticket/GH11149/GH11149Test.php create mode 100644 tests/Tests/ORM/Functional/Ticket/GH11149/Locale.php diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 3bb9b8efeb6..25b88221583 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -3166,7 +3166,7 @@ public function createEntity($className, array $data, &$hints = []) if ($hints['fetchMode'][$class->name][$field] === ClassMetadata::FETCH_EAGER) { $isIteration = isset($hints[Query::HINT_INTERNAL_ITERATION]) && $hints[Query::HINT_INTERNAL_ITERATION]; - if ($assoc['type'] === ClassMetadata::ONE_TO_MANY && ! $isIteration && ! $targetClass->isIdentifierComposite) { + if ($assoc['type'] === ClassMetadata::ONE_TO_MANY && ! $isIteration && ! $targetClass->isIdentifierComposite && ! isset($assoc['indexBy'])) { $this->scheduleCollectionForBatchLoading($pColl, $class); } else { $this->loadCollection($pColl); diff --git a/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php b/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php new file mode 100644 index 00000000000..448f7d82eb6 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php @@ -0,0 +1,41 @@ + + */ + public $translations; + + public function __construct(int $id) + { + $this->id = $id; + $this->translations = new ArrayCollection(); + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProductTranslation.php b/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProductTranslation.php new file mode 100644 index 00000000000..32c501868ec --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProductTranslation.php @@ -0,0 +1,45 @@ +id = $id; + $this->product = $product; + $this->locale = $locale; + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/GH11149/GH11149Test.php b/tests/Tests/ORM/Functional/Ticket/GH11149/GH11149Test.php new file mode 100644 index 00000000000..28bab541b90 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH11149/GH11149Test.php @@ -0,0 +1,47 @@ +setUpEntitySchema([ + Locale::class, + EagerProduct::class, + EagerProductTranslation::class, + ]); + } + + public function testFetchEagerModeWithIndexBy(): void + { + // Load entities into database + $this->_em->persist($product = new EagerProduct(11149)); + $this->_em->persist($locale = new Locale('fr_FR')); + $this->_em->persist(new EagerProductTranslation(11149, $product, $locale)); + $this->_em->flush(); + $this->_em->clear(); + + // Fetch entity from database + $product = $this->_em->find(EagerProduct::class, 11149); + + // Assert associated entity is loaded eagerly + static::assertInstanceOf(EagerProduct::class, $product); + static::assertInstanceOf(PersistentCollection::class, $product->translations); + static::assertTrue($product->translations->isInitialized()); + static::assertCount(1, $product->translations); + + // Assert associated entity is indexed by given property + $translation = $product->translations->get('fr_FR'); + static::assertInstanceOf(EagerProductTranslation::class, $translation); + static::assertNotInstanceOf(Proxy::class, $translation); + } +} diff --git a/tests/Tests/ORM/Functional/Ticket/GH11149/Locale.php b/tests/Tests/ORM/Functional/Ticket/GH11149/Locale.php new file mode 100644 index 00000000000..d54eda636ff --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH11149/Locale.php @@ -0,0 +1,27 @@ +code = $code; + } +} From 5ccbc201bff51cdc2006db8585b0d22d6695859d Mon Sep 17 00:00:00 2001 From: Thomas Landauer Date: Wed, 20 Mar 2024 23:34:10 +0100 Subject: [PATCH 3/5] [Documentation] Removing "Doctrine Mapping Types" ... (#11384) ... in favor of https://www.doctrine-project.org/projects/doctrine-dbal/en/3.8/reference/types.html#reference Page: https://www.doctrine-project.org/projects/doctrine-orm/en/2.19/reference/basic-mapping.html#doctrine-mapping-types As announced in https://github.com/doctrine/dbal/pull/6336#issuecomment-2003720361 , the goal is to remove this duplicated type information from ORM and replace it with a link to DBAL. In https://github.com/doctrine/dbal/pull/6341 , I'm adding any detail which I'm deleting here to the DBAL. --- docs/en/reference/basic-mapping.rst | 48 +++-------------------------- 1 file changed, 5 insertions(+), 43 deletions(-) diff --git a/docs/en/reference/basic-mapping.rst b/docs/en/reference/basic-mapping.rst index a61e5d8d0e4..32f8626806b 100644 --- a/docs/en/reference/basic-mapping.rst +++ b/docs/en/reference/basic-mapping.rst @@ -300,50 +300,12 @@ and a custom ``Doctrine\ORM\Mapping\TypedFieldMapper`` implementation. Doctrine Mapping Types ---------------------- -The ``type`` option used in the ``@Column`` accepts any of the existing -Doctrine types or even your own custom types. A Doctrine type defines +The ``type`` option used in the ``@Column`` accepts any of the +`existing Doctrine DBAL types `_ +or :doc:`your own custom mapping types +<../cookbook/custom-mapping-types>`. A Doctrine type defines the conversion between PHP and SQL types, independent from the database vendor -you are using. All Mapping Types that ship with Doctrine are fully portable -between the supported database systems. - -As an example, the Doctrine Mapping Type ``string`` defines the -mapping from a PHP string to a SQL VARCHAR (or VARCHAR2 etc. -depending on the RDBMS brand). Here is a quick overview of the -built-in mapping types: - -- ``string``: Type that maps a SQL VARCHAR to a PHP string. -- ``integer``: Type that maps a SQL INT to a PHP integer. -- ``smallint``: Type that maps a database SMALLINT to a PHP - integer. -- ``bigint``: Type that maps a database BIGINT to a PHP string. -- ``boolean``: Type that maps a SQL boolean or equivalent (TINYINT) to a PHP boolean. -- ``decimal``: Type that maps a SQL DECIMAL to a PHP string. -- ``date``: Type that maps a SQL DATETIME to a PHP DateTime - object. -- ``time``: Type that maps a SQL TIME to a PHP DateTime object. -- ``datetime``: Type that maps a SQL DATETIME/TIMESTAMP to a PHP - DateTime object. -- ``datetimetz``: Type that maps a SQL DATETIME/TIMESTAMP to a PHP - DateTime object with timezone. -- ``text``: Type that maps a SQL CLOB to a PHP string. -- ``object``: Type that maps a SQL CLOB to a PHP object using - ``serialize()`` and ``unserialize()`` -- ``array``: Type that maps a SQL CLOB to a PHP array using - ``serialize()`` and ``unserialize()`` -- ``simple_array``: Type that maps a SQL CLOB to a PHP array using - ``implode()`` and ``explode()``, with a comma as delimiter. *IMPORTANT* - Only use this type if you are sure that your values cannot contain a ",". -- ``json_array``: Type that maps a SQL CLOB to a PHP array using - ``json_encode()`` and ``json_decode()`` -- ``float``: Type that maps a SQL Float (Double Precision) to a - PHP double. *IMPORTANT*: Works only with locale settings that use - decimal points as separator. -- ``guid``: Type that maps a database GUID/UUID to a PHP string. Defaults to - varchar but uses a specific type if the platform supports it. -- ``blob``: Type that maps a SQL BLOB to a PHP resource stream - -A cookbook article shows how to define :doc:`your own custom mapping types -<../cookbook/custom-mapping-types>`. +you are using. .. note:: From db6e702088a251f7f0adc2fcdaa2003f46a5a4f6 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Thu, 21 Mar 2024 10:32:55 +0100 Subject: [PATCH 4/5] Remove unused variable (#11391) --- tests/Tests/ORM/Proxy/ProxyFactoryTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php index d0989953def..e6335c46c8f 100644 --- a/tests/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -64,7 +64,6 @@ protected function setUp(): void public function testReferenceProxyDelegatesLoadingToThePersister(): void { $identifier = ['id' => 42]; - $proxyClass = 'Proxies\__CG__\Doctrine\Tests\Models\ECommerce\ECommerceFeature'; $persister = $this->getMockBuilderWithOnlyMethods(BasicEntityPersister::class, ['loadById']) ->disableOriginalConstructor() ->getMock(); From 95795c87a8924fa28b1465dec67ad8a5b85d8213 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Thu, 21 Mar 2024 10:38:59 +0100 Subject: [PATCH 5/5] Add missing import --- tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php b/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php index 448f7d82eb6..e2528089abf 100644 --- a/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php +++ b/tests/Tests/ORM/Functional/Ticket/GH11149/EagerProduct.php @@ -5,6 +5,7 @@ namespace Doctrine\Tests\ORM\Functional\Ticket\GH11149; use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; /**