Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate EntityManager::merge #8461

Closed
beberlei opened this issue Feb 6, 2021 · 9 comments
Closed

Deprecate EntityManager::merge #8461

beberlei opened this issue Feb 6, 2021 · 9 comments
Milestone

Comments

@beberlei
Copy link
Member

beberlei commented Feb 6, 2021

The merge operation as used in the Java Persistence API is when you "Merges the state of a detached entity into the persistence context", meaning you have for example an entity unserialized from somewhere else (UI, API, Session) and you want the EntityManager to know about that object properly.

It is not a common PHP practice to serialize and move around data-objects to then synchronize and store them again in the database. As such the merge operation always felt a bit weird. Its existence even lead to Symfony serializing the user object into the session, where a proper approach would only serialize the identifier.

In addition the requirement to pass an entity and get a potentially different entity back from the method is cumbersome and unintuitive.

For us maintainers additionally, merging complex object graphs through cascade=merge always had bugs in edge cases that were hard to find and fix.

Alternative:

If you use merge heavily, the alternative is instead to use EntityManager::find to fetch the actual entity with its current state from the database and then to update the state of this entity using the input/request data.

Before:

$entity = unserialize($data);
$managedEntity = $entityManager->merge($entity);
$entityManager->flush();

After:

$payload = unserialize($data);
$entity = $entityManager->find(Entity::class, $payload['id']);
$entity->setSomething($data['something']);
$entityManager->flush();

Third Party Alternatives:

  • Symfony Form
  • Laminas Hydrator
  • JMS Serializer
@beberlei beberlei added this to the 3.0.0 milestone Feb 6, 2021
@smilesrg
Copy link
Contributor

This change will make it impossible to perform operations on entities (inserts, updates) without making a select request to a database. Merge is often used in the message queues jobs.

@dbrumann
Copy link

@smilesrg Yes, but that is not necessarily bad. If you provide a detached entity in a message then the state it holds might already be outdated. It will be difficult to automatically reconcile these changes and the result might be, that your wrongly merged object is inserted instead. Whereas when you SELECT first and then manually merge in your domain you have more control over this and either with an appropriate message that tells users why the message could not be applied or manually merge in case it is safe.

When using second level cache the additional select might not even result in an actual db query, if query count/performance is what you worry about.

@smilesrg
Copy link
Contributor

@dbrumann

Yes, but that is not necessarily bad. If you provide a detached entity in a message then the state it holds might already be outdated.

I understand that, but there are cases when performance prioritized over data integrity.

When using second level cache the additional select might not even result in an actual db query, if query count/performance is what you worry about.

Yes, it is what I'm worrying about :-) second level cache is marked as experimental https://www.doctrine-project.org/projects/doctrine-orm/en/latest/reference/second-level-cache.html

Thanks for your great answer!

@smilesrg
Copy link
Contributor

smilesrg commented Apr 23, 2021

@beberlei this ticket tagged as 3.0.0 and EntityManager::merge() is already deprecated in 2.8.x. Does it mean that this ticket is about removing EntityManager::merge() ? Do we also need to deprecate/remove EntityManager::detach() ?

@james-Ballyhoo
Copy link

What is the solution for the usecase of a user's shopping basket? Fully persisting unfinished/in-progress baskets into the database and then doing more additional leg work for basket abandonment / clean up?

@derrabus
Copy link
Member

Fixed via #9488 and #9665.

@dunglas
Copy link
Contributor

dunglas commented Sep 18, 2022

This removal makes it difficult to properly implement the PUT HTTP method, which is a common use case for web applications written in PHP.

When sending a request similar to:

PUT /books/11

{
  "title": "Lorem ipsum..."
}

a new object should be created and the existing object with the same ID must be completely replaced (all existing states should be discarded).

There is currently no easy way to do this natively with Doctrine ORM because merge() has been removed and UPSERT isn't currently supported.

You cannot attach a new object to an existing DB id (it may be possible using some methods marked as internal, but this is not a good long-term option).

A workaround is to fetch the initial entity, set the new data, and reset untouched properties to their default values. This is doable using the reflection API, but this is cumbersome and hard to generalize in frameworks such as API Platform. In addition, this method is also probably slower than just creating a new object containing the new data.

@wadjeroudi
Copy link

#10209

mpdude added a commit to mpdude/doctrine2 that referenced this issue Jun 23, 2023
`EntityManager::merge()` has been deprecated in doctrine#8461 and removed in doctrine#9488.

This PR removes a few remaining references and artefacts that - to my understanding - refer to it.
mpdude added a commit to mpdude/doctrine2 that referenced this issue Jun 24, 2023
`EntityManager::merge()` has been deprecated in doctrine#8461 and removed in doctrine#9488.

This PR removes a few remaining references and artefacts that - to my understanding - refer to it.
mpdude added a commit to mpdude/doctrine2 that referenced this issue Jun 24, 2023
`EntityManager::merge()` has been deprecated in doctrine#8461 and removed in doctrine#9488.

This PR removes a few remaining references and artefacts that - to my understanding - refer to it.
mpdude added a commit to mpdude/doctrine2 that referenced this issue Jun 25, 2023
`EntityManager::merge()` has been deprecated in doctrine#8461 and removed in doctrine#9488.

This PR removes a few remaining references and artefacts that - to my understanding - refer to it.
derrabus pushed a commit that referenced this issue Jun 27, 2023
`EntityManager::merge()` has been deprecated in #8461 and removed in #9488.

This PR removes a few remaining references and artefacts that - to my understanding - refer to it.
@beberlei
Copy link
Member Author

beberlei commented Dec 15, 2023

@dunglas I get the specification for PUT in HTTP, but its not logic that makes sense in terms of entities and objects. An entity in the database exists and replacing it by resetting all fields would be domain logic to implement on the users side, not an ORM concern.

You can generically implement the logic in merge for PUT using ClassMetadata APIs such as setFieldValue. It does not require using internal APIs.

Saeven added a commit to Saeven/zf3-circlical-user that referenced this issue Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants