Bug: UnitOfWork try to set null to a read-only id column after remove

[viniciusss]

Bug Report

Q	A
BC Break	no
Version	2.11.1
PHP Version	8.1.1

Summary

I have a entity with a read-only id property, and when a try to remove the entry a receive a LogicException. Attempting to change readonly property App\Entity\Supplier::$id.

Current behavior

The UnitOfWork always try to set the id of entity to null when it' executes the executeDeletions method.

orm/lib/Doctrine/ORM/UnitOfWork.php

Lines 1254 to 1256 in ed50e3d

	//$this->entityStates[$oid] = self::STATE_NEW;
	if (! $class->isIdentifierNatural()) {
	$class->reflFields[$class->identifier[0]]->setValue($entity, null);

How to reproduce

#[ORM\Entity]
class Supplier
{
    #[ORM\Id]
    #[ORM\Column(name: 'id', type: 'integer', nullable: false)]
    #[ORM\GeneratedValue(strategy: 'AUTO')]
    public readonly int $id;

    public function __construct(
        #[ORM\Column(nullable: false)]
        public readonly string $name,
    ) {}
}

$entityManager->find(Supplier::class, 1);
$entityManager->remove($entity);
$entityManager->flush($entity);

Expected behavior

Delete the entry from database, but i dont know what's the expected behavior to the orm internals.

[Arkemlar]

For those who run on same issue there is a trick that helps to keep code clean.
If you use autoincrement Id then you anyway need a field that might be nullable and changeable.
So, create an Id class:

#[ORM\Embeddable]
class AutoincrementId implements Stringable
{
    #[ORM\Id, ORM\GeneratedValue, ORM\Column(type: 'bigint', nullable: false, options: ['unsigned' => true])]
    protected int $id;

    public function __construct(int $id = null)
    {
        if (null !== $id) {
            static::validate($id);
            $this->id = $id;
        }
    }

    //...
}

Complete file here

use Doctrine\ORM\Mapping as ORM;
use Stringable;

#[ORM\Embeddable]
class AutoincrementId implements Stringable
{
    #[ORM\Id, ORM\GeneratedValue, ORM\Column(type: 'bigint', nullable: false, options: ['unsigned' => true])]
    protected int $id;

    public function __construct(int $id = null)
    {
        if (null !== $id) {
            static::validate($id);
            $this->id = $id;
        }
    }

    public function hasId(): bool
    {
        return isset($this->id);
    }

    public function toScalar(): int
    {
        $this->ensureHasId();

        return $this->id;
    }

   public function isEqualsTo(object $other): bool
    {
        $this->ensureHasId();

        return parent::isEqualsTo($other);
    }

    public function __toString(): string
    {
        $this->ensureHasId();

        return parent::__toString();
    }

    public static function isValid($value): bool
    {
        return \is_int($value) && $value > 0;
    }

    protected static function validate($id): void
    {
        if (!static::isValid($id)) {
            throw new InvalidIdException($id);
        }
    }

    protected function ensureHasId(): void
    {
        if (!$this->hasId()) {
            throw new AutoincrementIdException();
        }
    }
}

And then use it in your class:

#[ORM\Entity]
class ProductUnit
{
    #[ORM\Embedded(columnPrefix: false)]
    public readonly AutoincrementId $id;  // it is public because readonly and is set in constructor
}

So in your entity you have readonly field and that's fine. And this also correponds to DDD practices, especially if you create custom id class and use it instead of AutoincrementId in your entity:

#[ORM\Embeddable]
final class ProductUnitId extends AutoincrementId
{
}

#[ORM\Entity]
class ProductUnit
{
    #[ORM\Embedded(columnPrefix: false)]
    public readonly ProductUnitId $id;
}

The only drawback is that AutoincrementId (or custom id) is not a 100% ValueObject since it is changeable, but it is changeable by doctrine only so it is sort of compromise.

This techinque works perfectly in my project.