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

New Issue

admin · 2026-01-22T15:41:43+01:00

admin commented

2026-01-22 15:41:43 +01:00

Originally created by @viniciusss on GitHub (Feb 22, 2022).

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. ed50e3d967/lib/Doctrine/ORM/UnitOfWork.php (L1254-L1256)

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.

Originally created by @viniciusss on GitHub (Feb 22, 2022). ### 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. https://github.com/doctrine/orm/blob/ed50e3d9671e26857da6897c8e9323e78f6e34c1/lib/Doctrine/ORM/UnitOfWork.php#L1254-L1256 #### How to reproduce ```php #[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, ) {} } ``` ```php $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.

admin commented

2026-01-22 15:41:44 +01:00

@Arkemlar commented on GitHub (Dec 20, 2023):

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.

@Arkemlar commented on GitHub (Dec 20, 2023): 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: ```php #[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; } } //... } ``` <details> <summary>Complete file here</summary> ```php 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(); } } } ``` </details> And then use it in your class: ```php #[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: ```php #[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.

admin referenced this issue

2026-01-22 16:06:52 +01:00

[PR #6936] Remove support for same-namespace class resolution #10230

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6936