search

doctrine-extensions / DoctrineExtensions

Doctrine2 behavioral extensions, Translatable, Sluggable, Tree-NestedSet, Timestampable, Loggable, Sortable
MIT License
4.03k stars 1.26k forks source link

Blameable and Frankenphp creates a new entity #2843

Open tuxes3 opened 1 month ago

tuxes3 commented 1 month ago

Environment

show

``` $ composer show --latest gedmo/doctrine-extensions name : gedmo/doctrine-extensions descrip. : Doctrine behavioral extensions keywords : Blameable, behaviors, doctrine, extensions, gedmo, loggable, nestedset, odm, orm, sluggable, sortable, timestampable, translatable, tree, uploadable versions : * v3.15.0 released : 2024-02-12, 5 months ago latest : v3.16.1 released 2024-06-25, 3 weeks ago type : library license : MIT License (MIT) (OSI approved) https://spdx.org/licenses/MIT.html#licenseText homepage : http://gediminasm.org/ source : [git] https://github.com/doctrine-extensions/DoctrineExtensions.git 2a89103f4984d8970f3855284c8c04e6e6a63c0f dist : [zip] https://api.github.com/repos/doctrine-extensions/DoctrineExtensions/zipball/2a89103f4984d8970f3855284c8c04e6e6a63c0f 2a89103f4984d8970f3855284c8c04e6e6a63c0f path : /var/www/vendor/gedmo/doctrine-extensions names : gedmo/doctrine-extensions support email : gediminas.morkevicius@gmail.com issues : https://github.com/doctrine-extensions/DoctrineExtensions/issues source : https://github.com/doctrine-extensions/DoctrineExtensions/tree/v3.15.0 wiki : https://github.com/Atlantic18/DoctrineExtensions/tree/main/doc autoload psr-4 Gedmo\ => src/ requires behat/transliterator ^1.2 doctrine/collections ^1.2 || ^2.0 doctrine/common ^2.13 || ^3.0 doctrine/deprecations ^1.0 doctrine/event-manager ^1.2 || ^2.0 doctrine/persistence ^2.2 || ^3.0 php ^7.4 || ^8.0 psr/cache ^1 || ^2 || ^3 psr/clock ^1 symfony/cache ^5.4 || ^6.0 || ^7.0 requires (dev) doctrine/annotations ^1.13 || ^2.0 doctrine/cache ^1.11 || ^2.0 doctrine/dbal ^3.2 doctrine/doctrine-bundle ^2.3 doctrine/mongodb-odm ^2.3 doctrine/orm ^2.14.0 friendsofphp/php-cs-fixer ^3.14.0 nesbot/carbon ^2.71 || ^3.0 phpstan/phpstan ^1.10.2 phpstan/phpstan-doctrine ^1.0 phpstan/phpstan-phpunit ^1.0 phpunit/phpunit ^9.6 rector/rector ^0.19 symfony/console ^5.4 || ^6.0 || ^7.0 symfony/phpunit-bridge ^6.0 || ^7.0 symfony/yaml ^5.4 || ^6.0 || ^7.0 suggests doctrine/mongodb-odm to use the extensions with the MongoDB ODM doctrine/orm to use the extensions with the ORM conflicts doctrine/annotations <1.13 || >=3.0 doctrine/dbal <3.2 || >=4.0 doctrine/mongodb-odm <2.3 || >=3.0 doctrine/orm <2.14.0 || 2.16.0 || 2.16.1 || >=3.0 ```

Doctrine packages

show

``` $ composer show --latest 'doctrine/*' Direct dependencies required in composer.json: doctrine/annotations 2.0.1 2.0.1 Docblock Annotations Parser doctrine/dbal 3.8.4 4.0.4 Powerful PHP database abstraction layer (DBAL) with many features for database schema introspection and management. doctrine/doctrine-bundle 2.12.0 2.12.0 Symfony DoctrineBundle doctrine/doctrine-fixtures-bundle 3.6.1 3.6.1 Symfony DoctrineFixturesBundle doctrine/doctrine-migrations-bundle 3.3.1 3.3.1 Symfony DoctrineMigrationsBundle doctrine/migrations 3.7.4 3.8.0 PHP Doctrine Migrations project offer additional functionality on top of the database abstraction layer (DBAL) for versioning your database schema and easily de... doctrine/orm 2.19.5 3.2.1 Object-Relational-Mapper for PHP doctrine/persistence 3.3.2 3.3.3 The Doctrine Persistence project is a set of shared interfaces and functionality that the different Doctrine object mappers share. Transitive dependencies not required in composer.json: doctrine/cache 2.2.0 2.2.0 PHP Doctrine Cache library is a popular cache implementation that supports many different drivers such as redis, memcache, apc, mongodb and others. doctrine/collections 2.2.2 2.2.2 PHP Doctrine Collections library that adds additional functionality on top of PHP arrays. doctrine/common 3.4.4 3.4.4 PHP Doctrine Common project is a library that provides additional functionality that other Doctrine projects depend on such as better reflection support, proxie... doctrine/data-fixtures 1.7.0 1.7.0 Data Fixtures for all Doctrine Object Managers doctrine/deprecations 1.1.3 1.1.3 A small layer on top of trigger_error(E_USER_DEPRECATED) or PSR-3 logging with options to disable all deprecations or selectively for packages. doctrine/event-manager 2.0.1 2.0.1 The Doctrine Event Manager is a simple PHP event system that was built to be used with the various Doctrine projects. doctrine/inflector 2.0.10 2.0.10 PHP Doctrine Inflector is a small library that can perform string manipulations with regard to upper/lowercase and singular/plural forms of words. doctrine/instantiator 2.0.0 2.0.0 A small, lightweight utility to instantiate objects in PHP without invoking their constructors doctrine/lexer 3.0.1 3.0.1 PHP Doctrine Lexer parser library that can be used in Top-Down, Recursive Descent Parsers. doctrine/sql-formatter 1.4.0 1.4.0 a PHP SQL highlighting library ```

PHP version

$ php -v
PHP 8.3.7 (cli) (built: May 14 2024 03:35:46) (ZTS)
Copyright (c) The PHP Group
Zend Engine v4.3.7, Copyright (c) Zend Technologies
    with Xdebug v3.3.2, Copyright (c) 2002-2024, by Derick Rethans

Subject

when using franken-php and Gedmo\Blameable it will try to create a new user instead of just setting setting the existing relation.

I fixed this with a simple Decorator, its a bit of a ugly solution, but works meanwhile the bug exists:

// @phpstan-ignore-next-line class.extendsFinalByPhpDoc
#[AsDecorator('stof_doctrine_extensions.listener.blameable')]
final class FrankenPHPBlameableListener extends BlameableListener
{
    public function __construct(
        private readonly ManagerRegistry $managerRegistry,
    ) {
        parent::__construct();
    }

    public function getFieldValue($meta, $field, $eventAdapter)
    {
        /** @var User $user */
        $user = parent::getFieldValue($meta, $field, $eventAdapter);
        // to prevent the user from being detached from the entity
        // while uploading a file in the rich-text-editor, it would try to create a new user entity
        // this started happening after using FrankenPHP
        return $this->managerRegistry->getRepository(User::class)->find($user->getId());
    }
}

Steps to reproduce

Expected results

It should not create a new user

Actual results

It creates a new user

mbabker commented 1 month ago

TBH what you've got there is really the best solution for this without rewriting the blameable listener. Remember most of this package's code predates most long running process related code or architecture like FrankenPHP so running into issues like that isn't totally unexpected.

The only sane way I can think to improve this in the library is to support some kind of provider service for this (and other listeners) and then the application would be able to reset that service as appropriate for their setup (not using FrankenPHP myself but I imagine Symfony's ResetInterface would be used a bit here). Otherwise what you've got now where it has the extra application info (knowing you've pushed an entity into the listener, knowing what entity you're using, and a way to re-query it to deal with this specific issue) is really the nicest way to address it, even if it feels hacky.

tuxes3 commented 1 month ago

Thanks for reassuring that I am on the right path 😃

Might be worth to document this case, as long running php processes are more and more common. Furthermore to make it future proof there needs to be an interface for all the listeners. As you can see I had to extend from BlameableListener which will not work in the future (keyword final). Reason why a Decorator is not sufficient is because other Libraries are using this. For example in my case: https://github.com/stof/StofDoctrineExtensionsBundle/blob/main/src/EventListener/BlameListener.php#L23