[DX] providing a id via constructor gets overwritten from @ORM\CustomIdGenerator #6257

New Issue

admin · 2026-01-22T15:29:42+01:00

admin commented

2026-01-22 15:29:42 +01:00

Originally created by @c33s on GitHub (Jun 24, 2019).

Originally assigned to: @lcobucci on GitHub.

Bug Report

Q	A
BC Break	no?
Version	v2.6.3

doctrine/doctrine-bundle: 1.9.1

Summary

using CustomIdGenerator

    /**
     * @ORM\Id
     * @ORM\Column(type="uuid", unique=true)
     * @ORM\GeneratedValue(strategy="CUSTOM")
     * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator")
     */

and additionally provide the id via constructor

Type.php:

    public function __construct(UuidInterface $id = null)
    {
        $this->id = ($id ? $id : Uuid::uuid4());
    }

leads to CustomIdGenerator always overwrite the id already set via constructor. the event calling the generator should do something llke checking if id is null and only then call the generator.

this would increase the UX/DX a lot.

Current behavior

on new entities the CustomIdGenerator always "wins" which will create a new id even if already set via set or constructor.

How to reproduce

create an entity using CustomIdGenerator and also allow the id to be set via construtor

 $type = new Type('my type', Uuid::fromString('59ba2202-449a-462a-a8bd-013b38dd57be'));
 dump($type);
 $em->persist($type);
 dump($type);

the first dump will show the right id the 2nd will have a new generated id.

Expected behavior

if id is already set, do not create a new id

related issues: https://github.com/ramsey/uuid-doctrine/issues/81

for googles current suggested workarounds (quoted from symfony slack):

greg_wtm [7:35 PM]

because it's an event - you cannot mix these two
simply use ramsey uuid factory in constructor with generator strategy = none and call it a day

greg_wtm [7:46 PM]

in such case you can cheat and change the ID with reflections 🙂
since the field is mapped anyway so it cannot change
that way you do __construct which sets the ID and you can tinker with it before flush
in our case we have a simple trait which has 3 methods: regenrateId() which sets $this->id and __construct() + __clone() - it covers most of the cases
you can even make the id protected and simply create your objects using new class(yourId) extends RealEntity {...} with custom constructor to not expose the id in real > constructor so that nobody can mess with that in a real code

tarlepp [7:53 PM]

what about these "solutions":

use data fixtures so that you know what id's there are - eg. reflection here to change that id

first create that entity and then use that in next case

and for first one just remove that @ORM\CustomIdGenerator from your entities and create that UUID in entity constructor - so that you can change that later in your test > fixtures - not so pretty but it works

greg_wtm [7:58 PM]

I think the cleanest is the extend with anonymous class 😉
but even refelction in this case is fine since that's exactly how doctrine does access to properties

Originally created by @c33s on GitHub (Jun 24, 2019). Originally assigned to: @lcobucci on GitHub. ### Bug Report | Q | A |------------ | ------ | BC Break | no? | Version | v2.6.3 doctrine/doctrine-bundle: 1.9.1 #### Summary using `CustomIdGenerator` ```php /** * @ORM\Id * @ORM\Column(type="uuid", unique=true) * @ORM\GeneratedValue(strategy="CUSTOM") * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator") */ ``` and additionally provide the id via constructor `Type.php`: ```php public function __construct(UuidInterface $id = null) { $this->id = ($id ? $id : Uuid::uuid4()); } ``` leads to `CustomIdGenerator` always overwrite the id already set via constructor. the event calling the generator should do something llke checking if `id` is null and only then call the generator. this would increase the UX/DX a lot. #### Current behavior on new entities the `CustomIdGenerator` always "wins" which will create a new id even if already set via set or constructor. #### How to reproduce create an entity using `CustomIdGenerator` and also allow the id to be set via construtor ```php $type = new Type('my type', Uuid::fromString('59ba2202-449a-462a-a8bd-013b38dd57be')); dump($type); $em->persist($type); dump($type); ``` the first dump will show the right id the 2nd will have a new generated id. #### Expected behavior if id is already set, do not create a new id related issues: https://github.com/ramsey/uuid-doctrine/issues/81 for googles current suggested workarounds (quoted from symfony slack): greg_wtm [7:35 PM] > because it's an event - you cannot mix these two > simply use ramsey uuid factory in constructor with generator strategy = none and call it a day greg_wtm [7:46 PM] > in such case you can cheat and change the ID with reflections :slightly_smiling_face: > since the field is mapped anyway so it cannot change > that way you do `__construct` which sets the ID and you can tinker with it before flush > in our case we have a simple trait which has 3 methods: `regenrateId()` which sets `$this->id` and `__construct()` + `__clone()` - it covers most of the cases > you can even make the `id` protected and simply create your objects using `new class(yourId) extends RealEntity {...}` with custom constructor to not expose the `id` in real > constructor so that nobody can mess with that in a real code tarlepp [7:53 PM] > what about these "solutions": > 1. use data fixtures so that you know what id's there are - eg. reflection here to change that id > 2. first create that entity and then use that in next case > > and for first one just remove that `@ORM\CustomIdGenerator` from your entities and create that UUID in entity constructor - so that you can change that later in your test > fixtures - not so pretty but it works greg_wtm [7:58 PM] > I think the cleanest is the extend with anonymous class :wink: > but even refelction in this case is fine since that's exactly how doctrine does access to properties

admin added the Won't Fix label 2026-01-22 15:29:42 +01:00

admin closed this issue

2026-01-22 15:29:42 +01:00

admin commented

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

@Ocramius commented on GitHub (Jun 24, 2019):

if id is already set, do not create a new id

I'd rather say that either the identifier is generated or it isn't, not both scenarios.

@Ocramius commented on GitHub (Jun 24, 2019): > if id is already set, do not create a new id I'd rather say that either the identifier is generated or it isn't, not both scenarios.

admin commented

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

@ramsey commented on GitHub (Jun 24, 2019):

Hypothetically, I might have a use-case where I want API clients to generate a UUID and send that as part of the body to create a new entity, but if they don't, I want to generate it. In this case, do I just turn off the custom generator and initialize it always in the constructor, as you see in @c33s's example?

@ramsey commented on GitHub (Jun 24, 2019): Hypothetically, I might have a use-case where I want API clients to generate a UUID and send that as part of the body to create a new entity, but if they don't, I want to generate it. In this case, do I just turn off the custom generator and initialize it always in the constructor, as you see in @c33s's example?

admin commented

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

@lcobucci commented on GitHub (Jun 25, 2019):

@ramsey I'd say so.

The idea of an identifier generator is to remove the need for the application to create ids. If your software has special requirements regarding that process, you should rather pull this responsibility from the ORM and provide the required logic.

@lcobucci commented on GitHub (Jun 25, 2019): @ramsey I'd say so. The idea of an identifier generator is to remove the need for the application to create ids. If your software has special requirements regarding that process, you should rather pull this responsibility from the ORM and provide the required logic.

admin commented

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

@TomasVotruba commented on GitHub (Jul 12, 2019):

This is how we solved it: https://github.com/ramsey/uuid-doctrine/issues/81#issuecomment-510790171

@TomasVotruba commented on GitHub (Jul 12, 2019): This is how we solved it: https://github.com/ramsey/uuid-doctrine/issues/81#issuecomment-510790171

admin commented

2026-01-22 15:29:45 +01:00

@lcobucci commented on GitHub (Oct 2, 2019):

Closing here since this the designed and expected behaviour of the library and we don't plan to change (at least for now).

@lcobucci commented on GitHub (Oct 2, 2019): Closing here since this the designed and expected behaviour of the library and we don't plan to change (at least for now).

admin referenced this issue

2026-01-22 16:05:51 +01:00

[PR #6257] Adding a note about the `Doctrine\ORM\QueryBuilder#addCriteria()` method #9921

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#6257