DDC-2406: Merging of new detached entities with PrePersist lifecycle callback breaks #3020

Open
opened 2026-01-22 14:10:03 +01:00 by admin · 10 comments
Owner

Originally created by @doctrinebot on GitHub (Apr 19, 2013).

Originally assigned to: @beberlei on GitHub.

Jira issue originally created by user deatheriam:

Merging of new detached entities with PrePersist lifecycle callback breaks:

Code snippet:

    class A
    {
       /****
        *  @ORM\ManyToOne(targetEntity= ...
        *  @ORM\JoinColumn(name=" ...
        */
        protected $b;

        public function getB()
        {
            return $this->b;
        }

        public function setB($b)
        {
            $this->b = $b;
        }

        /****
         *
         * @ORM\PrePersist
         *
         * @return void
         */
        public function onPrePersist()
        {
           if ($this->getB() === null) {
                throw new \Exception('B instance must be defined);
           }
           ....
        }
    }

    class B 
    {
    }

    $a = new A();
    $b = $em->find('B', 1);
    $a->setB($b);
    $em->persist($a); // works fine as B instance is set
    $em->detach($a);

    $a = $em->merge($a) // breaks in onPrePersist

The reason it happens is that the merge operation is trying to persist a new entity created by uow::newInstance($class) without populating its properties first:

 // If there is no ID, it is actually NEW.
    ....
    if ( ! $id) {
        $managedCopy = $this->newInstance($class);

        $this->persistNew($class, $managedCopy);
    } else {
    ....

This should happen first for the $managedCopy:

    // Merge state of $entity into existing (managed) entity
    foreach ($class->reflClass->getProperties() as $prop) {
        ....
Originally created by @doctrinebot on GitHub (Apr 19, 2013). Originally assigned to: @beberlei on GitHub. Jira issue originally created by user deatheriam: Merging of new detached entities with PrePersist lifecycle callback breaks: Code snippet: ``` class A { /**** * @ORM\ManyToOne(targetEntity= ... * @ORM\JoinColumn(name=" ... */ protected $b; public function getB() { return $this->b; } public function setB($b) { $this->b = $b; } /**** * * @ORM\PrePersist * * @return void */ public function onPrePersist() { if ($this->getB() === null) { throw new \Exception('B instance must be defined); } .... } } class B { } $a = new A(); $b = $em->find('B', 1); $a->setB($b); $em->persist($a); // works fine as B instance is set $em->detach($a); $a = $em->merge($a) // breaks in onPrePersist ``` The reason it happens is that the merge operation is trying to persist a new entity created by uow::newInstance($class) without populating its properties first: ``` // If there is no ID, it is actually NEW. .... if ( ! $id) { $managedCopy = $this->newInstance($class); $this->persistNew($class, $managedCopy); } else { .... ``` This should happen first for the $managedCopy: ``` // Merge state of $entity into existing (managed) entity foreach ($class->reflClass->getProperties() as $prop) { .... ```
admin added the Bug label 2026-01-22 14:10:03 +01:00
Author
Owner

@doctrinebot commented on GitHub (Apr 28, 2013):

Comment created by @FabioBatSilva:

[~beberlei], Is this an expected behavior ?

I mean.. This issue is about dispatch the event before copy the original values into the managed instance.
But overall, should *$em->detach()* trigger @PrePersist events ?

@doctrinebot commented on GitHub (Apr 28, 2013): Comment created by @FabioBatSilva: [~beberlei], Is this an expected behavior ? I mean.. This issue is about dispatch the event before copy the original values into the managed instance. But overall, should **$em->detach()\* trigger *@PrePersist** events ?
Author
Owner

@doctrinebot commented on GitHub (May 1, 2013):

Comment created by @beberlei:

[~fabio.bat.silva] he talks about $em->merge() on a detached entity calling pre persist. This should only happen on a NEW entity, not on a DETACHED one.

@doctrinebot commented on GitHub (May 1, 2013): Comment created by @beberlei: [~fabio.bat.silva] he talks about $em->merge() on a detached entity calling pre persist. This should only happen on a NEW entity, not on a DETACHED one.
Author
Owner

@doctrinebot commented on GitHub (May 1, 2013):

Comment created by deatheriam:

I tend to disagree with the statement above about pre persist that should not happen on a detached entity being merged back in. If this event handler contains a business logic that this entity needs to be checked against and the detached entity was modified before the merge operation in a way that invalidates it in the prePersist than I will end up with the invalid entity in the identity map. If the merge operation calls persist it must run the prePersist event handler as well for consistency.

If there is a logic that prevents persisting invalid entities why should it bypassed in the merge operation?

@doctrinebot commented on GitHub (May 1, 2013): Comment created by deatheriam: I tend to disagree with the statement above about pre persist that should not happen on a detached entity being merged back in. If this event handler contains a business logic that this entity needs to be checked against and the detached entity was modified before the merge operation in a way that invalidates it in the prePersist than I will end up with the invalid entity in the identity map. If the merge operation calls persist it must run the prePersist event handler as well for consistency. If there is a logic that prevents persisting invalid entities why should it bypassed in the merge operation?
Author
Owner

@doctrinebot commented on GitHub (Nov 28, 2013):

Comment created by fodagus:

I can confirm that this bug has not been fixed while using doctrine 2.4

Exactly as Oleg Namaka has described, my organization is trying to use @PrePersist callbacks to enforce validation on new entities.

However, we use an extensive client side framework that sends json back to our server. Our workflow is then:

deserializeJson into detached entity
merge detached entity to get it managed (this will apply our edits to an existing entity, or create a new one if this one is new)
persist

However, some entities run into the above problem while using this workflow, so our validation is not run. I can provide more code samples if required.

@doctrinebot commented on GitHub (Nov 28, 2013): Comment created by fodagus: I can confirm that this bug has not been fixed while using doctrine 2.4 Exactly as Oleg Namaka has described, my organization is trying to use @PrePersist callbacks to enforce validation on new entities. However, we use an extensive client side framework that sends json back to our server. Our workflow is then: deserializeJson into detached entity merge detached entity to get it managed (this will apply our edits to an existing entity, or create a new one if this one is new) persist However, some entities run into the above problem while using this workflow, so our validation is not run. I can provide more code samples if required.
Author
Owner

@doctrinebot commented on GitHub (Apr 29, 2014):

Comment created by deatheriam:

Whose feedback is it awaiting for?

@doctrinebot commented on GitHub (Apr 29, 2014): Comment created by deatheriam: Whose feedback is it awaiting for?
Author
Owner

@doctrinebot commented on GitHub (May 30, 2014):

Comment created by romaricdrigon:

I can confirm this issue.

My use case (which I guess is pretty standard):

  • I deserialize a new entity (with relationships to already existing ones)
  • I merge it to attach existing entities
  • I persist it

PrePersist is called on an entity with all properties set to null. Any modifications made inside this entity won't be saved to the DB.

@doctrinebot commented on GitHub (May 30, 2014): Comment created by romaricdrigon: I can confirm this issue. My use case (which I guess is pretty standard): - I deserialize a new entity (with relationships to already existing ones) - I merge it to attach existing entities - I persist it PrePersist is called on an entity with all properties set to null. Any modifications made inside this entity won't be saved to the DB.
Author
Owner

@doctrinebot commented on GitHub (May 30, 2014):

Comment created by fodagus:

Is there any reason the proposed solution can't be implemented? I've patched a local version of Doctrine with the proposed changes and ran it through my application's tests without any issues, so I believe this would fix the problem.

@doctrinebot commented on GitHub (May 30, 2014): Comment created by fodagus: Is there any reason the proposed solution can't be implemented? I've patched a local version of Doctrine with the proposed changes and ran it through my application's tests without any issues, so I believe this would fix the problem.
Author
Owner

@doctrinebot commented on GitHub (Sep 14, 2014):

Comment created by chrissjohnson00:

  • on this one. I also have a similar situation where I get a detached entity from an API and use merge to update any changed elements. Obviously there is a workaround (trigger the lifecycle's function manually), but would be great not to have to do this.
@doctrinebot commented on GitHub (Sep 14, 2014): Comment created by chrissjohnson00: - on this one. I also have a similar situation where I get a detached entity from an API and use merge to update any changed elements. Obviously there is a workaround (trigger the lifecycle's function manually), but would be great not to have to do this.
Author
Owner

@mhallnoinc commented on GitHub (Sep 23, 2016):

did anyone figure this out?

@mhallnoinc commented on GitHub (Sep 23, 2016): did anyone figure this out?
Author
Owner

@cclose commented on GitHub (Mar 29, 2018):

I think this might be fixed by https://github.com/doctrine/doctrine2/pull/6177

@cclose commented on GitHub (Mar 29, 2018): I think this might be fixed by https://github.com/doctrine/doctrine2/pull/6177
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#3020