doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.94k stars 2.51k forks source link

Exception when deleting entity with readonly ID property in model (PHP 8.1 feature) #10032

Open mrcage opened 2 years ago

mrcage commented 2 years ago

Bug Report

Q A
BC Break no
Version 2.13.1
PHP 8.1.6
Database MariaDB 10.4.24

Summary

When I try to remove an entity with a model which as a readonly ID property through the entity manager, an exception is thrown. I am using PHP 8.1 which added support for readonly properties, and this is supported since ORM 2.11 according to the release notes.

I am using Slim 4 as a framework.

Current behavior

The following exception is thrown when trying to call the delete method on my entity repository class:

Type: LogicException Code: 0 Message: Attempting to change readonly property App\Models\Text::$id. File: C:...\vendor\doctrine\orm\lib\Doctrine\ORM\Mapping\ReflectionReadonlyProperty.php

Trace:

#0 C:\...\vendor\doctrine\orm\lib\Doctrine\ORM\UnitOfWork.php(1260): Doctrine\ORM\Mapping\ReflectionReadonlyProperty->setValue(Object(App\Models\Text), NULL)
#1 C:\...\vendor\doctrine\orm\lib\Doctrine\ORM\UnitOfWork.php(453): Doctrine\ORM\UnitOfWork->executeDeletions(Object(Doctrine\ORM\Mapping\ClassMetadata))
#2 C:\...\vendor\doctrine\orm\lib\Doctrine\ORM\EntityManager.php(403): Doctrine\ORM\UnitOfWork->commit(NULL)
#3 C:\...\src\Repository\TextRepository.php(67): Doctrine\ORM\EntityManager->flush()
#4 C:\...\src\Controllers\Backend\TextController.php(33): App\Repository\TextRepository->delete(15)
#5 [internal function]: App\Controllers\Backend\TextController->index(Object(Slim\Psr7\Request), Object(Slim\Psr7\Response), Object(App\Repository\TextRepository))
#6 C:\...\vendor\php-di\invoker\src\Invoker.php(74): call_user_func_array(Array, Array)
#7 C:\...\vendor\php-di\slim-bridge\src\ControllerInvoker.php(47): Invoker\Invoker->call(Array, Array)
#8 C:\...\vendor\slim\slim\Slim\Routing\Route.php(384): DI\Bridge\Slim\ControllerInvoker->__invoke(Array, Object(Slim\Psr7\Request), Object(Slim\Psr7\Response), Array)
#9 C:\...\vendor\slim\slim\Slim\MiddlewareDispatcher.php(81): Slim\Routing\Route->handle(Object(Slim\Psr7\Request))
#10 C:\...\vendor\tuupola\slim-basic-auth\src\HttpBasicAuthentication.php(187): Slim\MiddlewareDispatcher->handle(Object(Slim\Psr7\Request))
#11 C:\...\vendor\slim\slim\Slim\MiddlewareDispatcher.php(147): Tuupola\Middleware\HttpBasicAuthentication->process(Object(Slim\Psr7\Request), Object(Slim\MiddlewareDispatcher))
#12 C:\...\vendor\slim\slim\Slim\MiddlewareDispatcher.php(81): Psr\Http\Server\RequestHandlerInterface@anonymous->handle(Object(Slim\Psr7\Request))
#13 C:\...\vendor\slim\slim\Slim\Routing\Route.php(341): Slim\MiddlewareDispatcher->handle(Object(Slim\Psr7\Request))
#14 C:\...\vendor\slim\slim\Slim\Routing\RouteRunner.php(84): Slim\Routing\Route->run(Object(Slim\Psr7\Request))
#15 C:\...\vendor\slim\slim\Slim\Middleware\ErrorMiddleware.php(107): Slim\Routing\RouteRunner->handle(Object(Slim\Psr7\Request))
#16 C:\...\vendor\slim\slim\Slim\MiddlewareDispatcher.php(147): Slim\Middleware\ErrorMiddleware->process(Object(Slim\Psr7\Request), Object(Slim\Routing\RouteRunner))
#17 C:\...\vendor\slim\twig-view\src\TwigMiddleware.php(115): Psr\Http\Server\RequestHandlerInterface@anonymous->handle(Object(Slim\Psr7\Request))
#18 C:\...\vendor\slim\slim\Slim\MiddlewareDispatcher.php(147): Slim\Views\TwigMiddleware->process(Object(Slim\Psr7\Request), Object(Psr\Http\Server\RequestHandlerInterface@anonymous))
#19 C:\...\vendor\bryanjhv\slim-session\src\Slim\Middleware\Session.php(82): Psr\Http\Server\RequestHandlerInterface@anonymous->handle(Object(Slim\Psr7\Request))
#20 C:\...\vendor\slim\slim\Slim\MiddlewareDispatcher.php(209): Slim\Middleware\Session->__invoke(Object(Slim\Psr7\Request), Object(Psr\Http\Server\RequestHandlerInterface@anonymous))
#21 C:\...\vendor\slim\slim\Slim\MiddlewareDispatcher.php(81): Psr\Http\Server\RequestHandlerInterface@anonymous->handle(Object(Slim\Psr7\Request))
#22 C:\...\vendor\slim\slim\Slim\App.php(215): Slim\MiddlewareDispatcher->handle(Object(Slim\Psr7\Request))
#23 C:\...\vendor\slim\slim\Slim\App.php(199): Slim\App->handle(Object(Slim\Psr7\Request))
#24 C:\...\src\bootstrap.php(92): Slim\App->run()
#25 C:\...\public\index.php(3): require('C:\\devel\\php\\et...')
#26 {main}

How to reproduce

My model:

<?php

namespace App\Models;

use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\Table;

#[Entity, Table(name: 'texts')]
class Text
{
    #[Column(name: 'text_id'), Id, GeneratedValue]
    public readonly int $id;

    #[Column(name: 'text_keyword', unique: true)]
    public string $keyword;

    #[Column(name: 'text_text')]
    public string $content;
}

My Repository:

<?php

namespace App\Repository;

use App\Models\Text;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\EntityRepository;

final class TextRepository
{
    private EntityRepository $repo;

    public function __construct(private EntityManager $em)
    {
        $this->repo = $em->getRepository(Text::class);
    }

    // ... some more methods

    public function delete(int $id): void
    {
        $text = $this->em->getReference(Text::class, $id);
        $this->em->remove($text);
        $this->em->flush();
    }
}

SQL used to create the relevant database table:

CREATE TABLE `texts` (
  `text_id` int(11) NOT NULL,
  `text_keyword` varchar(255) NOT NULL,
  `text_text` text NOT NULL
) ENGINE=MyISAM DEFAULT CHARSET=utf8;

ALTER TABLE `texts`
  ADD PRIMARY KEY (`text_id`),
  ADD UNIQUE KEY `text_keyword` (`text_keyword`);

Expected behavior

No exception should be thrown.

Nayte91 commented 2 years ago

I confirm this issue, I experience the same problem on my home project and on a synthetic test project, typically the base demo for API Platform; With an enhanced readonly $id on a given Entity, you can get, post, put, but no delete. Note that is the same problem with: public readonly ?int $id; and: public readonly int $id;

Problem is that this exact example is given in the Doctrine's blog post, so it's kind of a flagship feature.

Hope it can help,

Nayte91 commented 2 years ago

The LogicException is thrown by ORM/Mapping/ReflectionReadonlyProperty::setValue()L46;

It seems that the condition tests if parent::getValue($objectOrValue), that returns our readonly property value (in our examples, our Entity's readonly $id), is different from $value, that is a parameter of the public function setValue(mixed $objectOrValue, mixed $value = null): void.

The setValue() method is called by ORM\UnitOfWork::executeDeletions()L1256, with a $value = null;

I wonder it should try to make such a move when a property is readonly, by wanting to set a value. Or if ReflectionReadonlyProperty::setValue() act like it should. Either UnitOfWork::executeDeletions() should test if we face a ReflectionReadonlyProperty before asking to set a value, or ReflectionReadonlyProperty::setValue() shouldn't change the value tell it's ok (but this is lying and lying is bad so I'm not fond of this one).

I don't really understand the strategy around UnitOfWork::executeDeletions() that transforms entities into "new ones" by setting ids to null:

// Entity with this $oid after deletion treated as NEW, even if the $oid // is obtained by a new entity because the old one went out of scope. //$this->entityStates[$oid] = self::STATE_NEW;

So I can't take a decision about what to do here to fix.