diff --git a/composer.json b/composer.json index 69dcaee9846f44d8ac448e484ac975a741a68560..e2e96ef6907450b8a26675653b3552e6f430bc51 100644 --- a/composer.json +++ b/composer.json @@ -53,6 +53,7 @@ "doctrine/annotations": "^1.13.3 || ^2.0", "doctrine/dbal": "^3.6.0", "doctrine/event-manager": "^2.0", + "doctrine/instantiator": "^2.0", "doctrine/lexer": "^2.0 || ^3.0", "egulias/email-validator": "^4.0", "enshrined/svg-sanitize": "^0.15.4", diff --git a/composer.lock b/composer.lock index 953e3ff788103a8c99b5972889acd2fee7c28065..3e2a570f686d064f55cc5f02ad726c3ef1df70b5 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "66545c37f72bcd4c288a999d36081c4e", + "content-hash": "cb5b95c7553a5605827f042e046e8393", "packages": [ { "name": "bacon/bacon-qr-code", @@ -581,6 +581,76 @@ ], "time": "2022-10-12T20:59:15+00:00" }, + { + "name": "doctrine/instantiator", + "version": "2.0.0", + "source": { + "type": "git", + "url": "https://github.com/doctrine/instantiator.git", + "reference": "c6222283fa3f4ac679f8b9ced9a4e23f163e80d0" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/doctrine/instantiator/zipball/c6222283fa3f4ac679f8b9ced9a4e23f163e80d0", + "reference": "c6222283fa3f4ac679f8b9ced9a4e23f163e80d0", + "shasum": "" + }, + "require": { + "php": "^8.1" + }, + "require-dev": { + "doctrine/coding-standard": "^11", + "ext-pdo": "*", + "ext-phar": "*", + "phpbench/phpbench": "^1.2", + "phpstan/phpstan": "^1.9.4", + "phpstan/phpstan-phpunit": "^1.3", + "phpunit/phpunit": "^9.5.27", + "vimeo/psalm": "^5.4" + }, + "type": "library", + "autoload": { + "psr-4": { + "Doctrine\\Instantiator\\": "src/Doctrine/Instantiator/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Marco Pivetta", + "email": "ocramius@gmail.com", + "homepage": "https://ocramius.github.io/" + } + ], + "description": "A small, lightweight utility to instantiate objects in PHP without invoking their constructors", + "homepage": "https://www.doctrine-project.org/projects/instantiator.html", + "keywords": [ + "constructor", + "instantiate" + ], + "support": { + "issues": "https://github.com/doctrine/instantiator/issues", + "source": "https://github.com/doctrine/instantiator/tree/2.0.0" + }, + "funding": [ + { + "url": "https://www.doctrine-project.org/sponsorship.html", + "type": "custom" + }, + { + "url": "https://www.patreon.com/phpdoctrine", + "type": "patreon" + }, + { + "url": "https://tidelift.com/funding/github/packagist/doctrine%2Finstantiator", + "type": "tidelift" + } + ], + "time": "2022-12-30T00:23:10+00:00" + }, { "name": "doctrine/lexer", "version": "3.0.0", @@ -6038,76 +6108,6 @@ ], "time": "2022-02-25T21:32:43+00:00" }, - { - "name": "doctrine/instantiator", - "version": "2.0.0", - "source": { - "type": "git", - "url": "https://github.com/doctrine/instantiator.git", - "reference": "c6222283fa3f4ac679f8b9ced9a4e23f163e80d0" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/doctrine/instantiator/zipball/c6222283fa3f4ac679f8b9ced9a4e23f163e80d0", - "reference": "c6222283fa3f4ac679f8b9ced9a4e23f163e80d0", - "shasum": "" - }, - "require": { - "php": "^8.1" - }, - "require-dev": { - "doctrine/coding-standard": "^11", - "ext-pdo": "*", - "ext-phar": "*", - "phpbench/phpbench": "^1.2", - "phpstan/phpstan": "^1.9.4", - "phpstan/phpstan-phpunit": "^1.3", - "phpunit/phpunit": "^9.5.27", - "vimeo/psalm": "^5.4" - }, - "type": "library", - "autoload": { - "psr-4": { - "Doctrine\\Instantiator\\": "src/Doctrine/Instantiator/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Marco Pivetta", - "email": "ocramius@gmail.com", - "homepage": "https://ocramius.github.io/" - } - ], - "description": "A small, lightweight utility to instantiate objects in PHP without invoking their constructors", - "homepage": "https://www.doctrine-project.org/projects/instantiator.html", - "keywords": [ - "constructor", - "instantiate" - ], - "support": { - "issues": "https://github.com/doctrine/instantiator/issues", - "source": "https://github.com/doctrine/instantiator/tree/2.0.0" - }, - "funding": [ - { - "url": "https://www.doctrine-project.org/sponsorship.html", - "type": "custom" - }, - { - "url": "https://www.patreon.com/phpdoctrine", - "type": "patreon" - }, - { - "url": "https://tidelift.com/funding/github/packagist/doctrine%2Finstantiator", - "type": "tidelift" - } - ], - "time": "2022-12-30T00:23:10+00:00" - }, { "name": "friendsofphp/php-cs-fixer", "version": "v3.15.1", diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php index bab6bf923b6623decdda94ed3be3640744cfb809..320a0112f57c76b9414bf7941970ede2a15f51db 100644 --- a/typo3/sysext/core/Classes/Utility/GeneralUtility.php +++ b/typo3/sysext/core/Classes/Utility/GeneralUtility.php @@ -2995,8 +2995,10 @@ class GeneralUtility * * @param class-string $className Base class name to evaluate * @return class-string Final class name to instantiate with `new [classname]` + * @internal This is not a public API method, do not use in own extensions. + * Public to be accessible by extbase hydration. */ - protected static function getClassName($className) + public static function getClassName($className) { if (class_exists($className)) { while (static::classHasImplementation($className)) { diff --git a/typo3/sysext/core/Documentation/Changelog/12.4/Important-100207-LetDataMappercreateEmptyObjectUseDoctrineinstantiator.rst b/typo3/sysext/core/Documentation/Changelog/12.4/Important-100207-LetDataMappercreateEmptyObjectUseDoctrineinstantiator.rst new file mode 100644 index 0000000000000000000000000000000000000000..31f6173a20a73e0eb7acd80ae9e0506e7357db8f --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/12.4/Important-100207-LetDataMappercreateEmptyObjectUseDoctrineinstantiator.rst @@ -0,0 +1,173 @@ +.. include:: /Includes.rst.txt + +.. _important-100207-1679414752: + +================================================================================== +Important: #100207 - Let DataMapper::createEmptyObject() use doctrine/instantiator +================================================================================== + +See :issue:`100207` + +Description +=========== + +Introduction +------------ + +This document explains the intended way in which the Extbase ORM thaws/hydrates objects. + +Hydrating Objects +----------------- + +Hydrating (the term originates from `doctrine/orm`), or in Extbase terms thawing, is +the act of creating an object from a given database row. The responsible class involved +is the :php:`DataMapper`. During the process of hydrating, the :php:`DataMapper` creates +objects to map the raw database data onto. + +Before diving into the framework internals, let's take a look at models from the +users perspective. + +Creating Objects with Constructor Arguments +------------------------------------------- + +Imagine you have a table `tx_extension_domain_model_blog` and a corresponding model +or entity (entity is used as a synonym here) :php:`Vendor\Extension\Domain\Model\Blog`. + +Now, also imagine there is a domain rule which states, that all Blogs must have a +title. This rule can easily be followed by letting the blog class have a constructor +with a required argument :php:`string $title`. + +.. code-block:: php + + class Blog extends AbstractEntity + { + protected ObjectStorage $posts; + + public function __construct(protected string $title) + { + $this->posts = new ObjectStorage(); + } + } + +This example also shows how the `posts` property is initialized. It is done in +the constructor because PHP does not allow setting a default value that is of +type object. + +Hydrating Objects with Constructor Arguments +-------------------------------------------- + +Whenever the user creates new blog objects in extension code, the aforementioned +domain rule is followed. It's also possible to work on the `posts` `ObjectStorage` +without further initialization. :php:`new Blog('title')` is all I need to create +a blog object with a valid state. + +What happens in the :php:`DataMapper` however, is a totally different thing. When +hydrating an object, the :php:`DataMapper` cannot follow any domain rules. Its only +job is to map the raw database values onto a `Blog` instance. The :php:`DataMapper` +could of course detect constructor arguments and try to guess which argument +corresponds to what property but only if there is an easy mapping, i.e. if the +constructor takes argument :php:`string $title` and updates property `title` with it. + +To avoid possible errors due to guessing, the :php:`DataMapper` simply +ignores the constructor at all. It does so with the help of the library `doctrine/instantiator`. + +This pretty much explains the title of this document in detail. But there is more +to all this. + +Initializing Objects +-------------------- + +Have a look at the :php:`$posts` property in the example above. If the :php:`DataMapper` +ignores the constructor, that property is in an invalid state, i.e. uninitialized. + +To address this problem and possible others, the :php:`DataMapper` will call the method +`initializeObject(): void` on models, if it exists. + +Here is an updated version of the model: + +.. code-block:: php + + class Blog extends AbstractEntity + { + protected ObjectStorage $posts; + + public function __construct(protected string $title) + { + $this->initializeObject(); + } + + public function initializeObject(): void + { + $this->posts = new ObjectStorage(); + } + } + +This example demonstrates how Extbase expects the user to set up their model(s). If +method :php:`initializeObject()` is used for initialization logic that needs to be +triggered on initial creation AND on hydration. Please mind that :php:`__construct()` +**SHOULD** call :php:`initializeObject()`. + +If there are no domain rules to follow, the recommended way to set up a model +would then still be to define a :php:`__construct()` and :php:`initializeObject()` +method like this: + +.. code-block:: php + + class Blog extends AbstractEntity + { + protected ObjectStorage $posts; + + public function __construct() + { + $this->initializeObject(); + } + + public function initializeObject(): void + { + $this->posts = new ObjectStorage(); + } + } + +Mutating Objects +---------------- + +I'd like to add a few more words on mutators (setter, adder, etc.). One might think that +:php:`DataMapper` uses mutators during object hydration but it DOES NOT. `mutators` +are the only way for the user (developer) to implement business rules besides +using the constructor. + +The :php:`DataMapper` uses `@internal` method :php:`AbstractDomainObject::_setProperty()` +to update object properties. This looks a bit dirty and is a way around all business +rules but that's what the :php:`DataMapper` needs in order to leave the `mutators` to +the users. + +.. warning:: + + While :php:`DataMapper` does not use any mutators, other parts of Extbase do. + Both, validation and property mapping, either use existing mutators or gather + type information from them. This will change in the future but as of TYPO3 12 LTS + this information is correct. + +Property Visibility +------------------- + +One important thing to know is that Extbase needs entity properties to be protected +or public. As written in the former paragraph, :php:`AbstractDomainObject::_setProperty()` +is used to bypass setters. :php:`AbstractDomainObject` however, is not able to access +private properties of child classes, hence the need to have protected or public +properties. + + +Dependency Injection +-------------------- + +Without digging too deep into this topic the following statements have to be made. +Extbase expects entities to be so called prototypes, i.e. classes that do have a +different state per instance. DataMapper DOES NOT use Dependency Injection for the +creation of entities, i.e. it does not query the object container. This also means, +that Dependency Injection is not possible in entities. + +If you think that your entities need to use/access services, you need to find other +ways to implement it. + +.. index:: PHP-API, ext:extbase diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php index 17ad59ec81562d12a1a42e92e5b35e94013fbdbe..8760ba9166c014b18d8286b76b99c2b653c01c23 100644 --- a/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php +++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Mapper/DataMapper.php @@ -15,6 +15,7 @@ namespace TYPO3\CMS\Extbase\Persistence\Generic\Mapper; +use Doctrine\Instantiator\InstantiatorInterface; use Psr\EventDispatcher\EventDispatcherInterface; use Symfony\Component\PropertyInfo\Type; use TYPO3\CMS\Core\Context\Context; @@ -53,32 +54,20 @@ use TYPO3\CMS\Extbase\Utility\TypeHandlingUtility; */ class DataMapper { - protected ReflectionService $reflectionService; - protected QueryObjectModelFactory $qomFactory; - protected Session $persistenceSession; - protected DataMapFactory $dataMapFactory; - protected QueryFactoryInterface $queryFactory; - protected EventDispatcherInterface $eventDispatcher; - /** * @var QueryInterface|null */ protected $query; public function __construct( - ReflectionService $reflectionService, - QueryObjectModelFactory $qomFactory, - Session $persistenceSession, - DataMapFactory $dataMapFactory, - QueryFactoryInterface $queryFactory, - EventDispatcherInterface $eventDispatcher + private readonly ReflectionService $reflectionService, + private readonly QueryObjectModelFactory $qomFactory, + private readonly Session $persistenceSession, + private readonly DataMapFactory $dataMapFactory, + private readonly QueryFactoryInterface $queryFactory, + private readonly EventDispatcherInterface $eventDispatcher, + private readonly InstantiatorInterface $instantiator, ) { - $this->reflectionService = $reflectionService; - $this->qomFactory = $qomFactory; - $this->persistenceSession = $persistenceSession; - $this->dataMapFactory = $dataMapFactory; - $this->queryFactory = $queryFactory; - $this->eventDispatcher = $eventDispatcher; } public function setQuery(QueryInterface $query): void @@ -160,16 +149,18 @@ class DataMapper } /** - * Creates a skeleton of the specified object + * Creates a skeleton of the specified object. This is + * designed to *not* call class constructor when hydrating, + * but *do call* initializeObject() if exists and obey + * eventually registered implementation overrides ("xclass"). * - * @param string $className Name of the class to create a skeleton for + * @param class-string $className Name of the class to create a skeleton for * @throws InvalidClassException - * @return DomainObjectInterface The object skeleton * @template T of DomainObjectInterface * @phpstan-param class-string<T> $className * @phpstan-return T */ - protected function createEmptyObject($className) + protected function createEmptyObject(string $className): DomainObjectInterface { // Note: The class_implements() function also invokes autoload to assure that the interfaces // and the class are loaded. Would end up with __PHP_Incomplete_Class without it. @@ -177,7 +168,12 @@ class DataMapper throw new InvalidClassException('Cannot create empty instance of the class "' . $className . '" because it does not implement the TYPO3\\CMS\\Extbase\\DomainObject\\DomainObjectInterface.', 1234386924); } - return GeneralUtility::makeInstance($className); + // Use GU::getClassName() to obey class implementation overrides. + $object = $this->instantiator->instantiate(GeneralUtility::getClassName($className)); + if (is_callable($callable = [$object, 'initializeObject'])) { + $callable(); + } + return $object; } /** diff --git a/typo3/sysext/extbase/Configuration/Services.yaml b/typo3/sysext/extbase/Configuration/Services.yaml index 8a0b05c118b90e5f6fc509d18fb4985540d464dd..1f05cf92a0411c31f8b777efb210b97a3643bd33 100644 --- a/typo3/sysext/extbase/Configuration/Services.yaml +++ b/typo3/sysext/extbase/Configuration/Services.yaml @@ -196,3 +196,6 @@ services: priority: 10 target: TYPO3\CMS\Extbase\Domain\Model\Folder sources: string + + Doctrine\Instantiator\InstantiatorInterface: + class: \Doctrine\Instantiator\Instantiator diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php index 4a4f809875da676b9f263c12bf8df8d59112acdf..50c70d608dea745f830960751f475a5425ea0705 100644 --- a/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php +++ b/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/DataMapperTest.php @@ -26,6 +26,7 @@ use TYPO3\CMS\Core\Authentication\BackendUserAuthentication; use TYPO3\CMS\Core\Cache\CacheManager; use TYPO3\CMS\Core\Core\SystemEnvironmentBuilder; use TYPO3\CMS\Core\Http\ServerRequest; +use TYPO3\CMS\Extbase\Persistence\Generic\Exception\InvalidClassException; use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\ColumnMap; use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper; use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\Exception\UnknownPropertyTypeException; @@ -510,4 +511,30 @@ class DataMapperTest extends FunctionalTestCase array_map(static fn (Comment $comment): int => $comment->getUid(), $comments) ); } + + /** + * @test + */ + public function createEmptyObjectThrowsInvalidClassExceptionIfClassNameDoesNotImplementDomainObjectInterface(): void + { + self::expectException(InvalidClassException::class); + self::expectExceptionCode(1234386924); + $subject = $this->get(DataMapper::class); + // Reflection since the method is protected + $subjectReflection = new \ReflectionObject($subject); + $subjectReflection->getMethod('createEmptyObject')->invoke($subject, \stdClass::class); + } + + /** + * @test + */ + public function createEmptyObjectInstantiatesWithoutCallingTheConstructorButCallsInitializeObject(): void + { + self::expectException(\RuntimeException::class); + // 1680071491 is thrown, but not 1680071490! + self::expectExceptionCode(1680071491); + $subject = $this->get(DataMapper::class); + $subjectReflection = new \ReflectionObject($subject); + $subjectReflection->getMethod('createEmptyObject')->invoke($subject, Fixtures\HydrationFixtureEntity::class); + } } diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/Fixtures/HydrationFixtureEntity.php b/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/Fixtures/HydrationFixtureEntity.php new file mode 100644 index 0000000000000000000000000000000000000000..898c2cf5e13865e053118b8422335326ac8e6b22 --- /dev/null +++ b/typo3/sysext/extbase/Tests/Functional/Persistence/Generic/Mapper/Fixtures/HydrationFixtureEntity.php @@ -0,0 +1,35 @@ +<?php + +declare(strict_types=1); + +/* + * This file is part of the TYPO3 CMS project. + * + * It is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License, either version 2 + * of the License, or any later version. + * + * For the full copyright and license information, please read the + * LICENSE.txt file that was distributed with this source code. + * + * The TYPO3 project - inspiring people to share! + */ + +namespace TYPO3\CMS\Extbase\Tests\Functional\Persistence\Generic\Mapper\Fixtures; + +use TYPO3\CMS\Extbase\DomainObject\AbstractEntity; + +class HydrationFixtureEntity extends AbstractEntity +{ + public function __construct() + { + // Used to verify __construct() *is not* called by DataMapper when hydrating. + throw new \RuntimeException('constructor called', 1680071490); + } + + public function initializeObject(): void + { + // Used to verify initializeObject() *is* called by DataMapper when hydrating. + throw new \RuntimeException('initializeObject called', 1680071491); + } +} diff --git a/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php b/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php index 01a81259327dbfcd9e5799af399087e4b9090191..695effdc5f181d2c3cc8db8b36b4659dc2c3395e 100644 --- a/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php +++ b/typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Mapper/DataMapperTest.php @@ -17,6 +17,7 @@ declare(strict_types=1); namespace TYPO3\CMS\Extbase\Tests\Unit\Persistence\Generic\Mapper; +use Doctrine\Instantiator\InstantiatorInterface; use Psr\EventDispatcher\EventDispatcherInterface; use TYPO3\CMS\Core\Cache\CacheManager; use TYPO3\CMS\Extbase\Configuration\ConfigurationManager; @@ -66,6 +67,7 @@ class DataMapperTest extends UnitTestCase $dataMapFactory, $this->createMock(QueryFactory::class), $this->createMock(EventDispatcherInterface::class), + $this->createMock(InstantiatorInterface::class), ); } diff --git a/typo3/sysext/extbase/composer.json b/typo3/sysext/extbase/composer.json index 94a5db1f29633fdd933c37cd173c3b1536b60301..f357fe6f7cebe30092b09c8d597dc6d8fb186d74 100644 --- a/typo3/sysext/extbase/composer.json +++ b/typo3/sysext/extbase/composer.json @@ -19,6 +19,7 @@ "sort-packages": true }, "require": { + "doctrine/instantiator": "^2.0", "phpdocumentor/reflection-docblock": "^5.2", "phpdocumentor/type-resolver": "^1.4", "symfony/dependency-injection": "^6.2",