Add deprecation warnings for 2.7.x

[lcobucci]

This recovers the great work done by @Majkl578 in #6869 but without using phpunit-bridge stuff to do the assertions.

[Ocramius]

Build failures affect MySQL's quoting of collations: unrelated to this patch.

[michaljusiega]

Somebody Can tell me, why you want to remove that a importants methods like clear/merge/detach in EM? It will be some replacement methods?

Second question. PHPDoc tag @deprecated mean from which version of package the specified method will be removed. So deprecated and removed in 3.0 ? I dont think so.

[Ocramius]

Somebody Can tell me, why you want to remove that a importants methods like clear/merge/detach in EM? It will be some replacement methods?

Mostly complexity and edge cases that are better solved via simpler EntityManager#clear() and smaller UnitOfWork scopes. We have closed dozens and dozens of issues related to EntityManager#merge(), which is something that was initially built after the JPA design, but is a bad fit for PHP's share-nothing architecture, and also introduces broken entity graphs in the UnitOfWork. This patch is mostly about documenting deprecations that were already agreed upon in @doctrine/doctrinecore.

PHPDoc tag @deprecated mean from which version of package the specified method will be removed. So deprecated and removed in 3.0 ? I dont think so.

The deprecated API will be removed (and we also already did remove a lot of it) in master, which will become 3.0.0

[michaljusiega]

I will never understand your approach. You don't have a good standard of changes that you can change for anything ... in my humble opinion I think that these methods are very important, they work very well and getting rid of them will prevent you from raising to version 3.0 - put yourself in the places of many programmers for whom the problem there will be a raise to 3.0. In my case, rather not possible because I use merge / clear / detach myself - and since you have stated in PHPDoc that there will be no alternative methods - praise to you!

In Symfony, if they remove something, they at least give a replacement method.

[Ocramius]

In many of the removed endpoints, the alternative is effectively "stop doing X", where "X" is identified as a problem.

Yes, this means that upgrading will require design changes in your code: that's something that was considered.

We don't take decisions in a rushed nor uninformed way: this has been discussed and agreed upon when keeping in mind that there are thousands (potentially millions) of consumers of the library.

[michaljusiega]

Okey. Last one. Can you show and explain me a equivalent code to replace a merge/clear/detach methods? Thanks.

[Ocramius]

• clear() will stay as-is: what's deprecated is clearing sub-sections of the entity graph (not supported, since it breaks the UnitOfWork internal state). If you need EntityManager#clear(), clear the entire UnitOfWork
• merge() is no longer supported: instead, if you know that you have an in-memory object that is in sync with DB state, use UnitOfWork#registerManaged(). That's very close to internal API though, so that's discouraged too.
• detach() is not to be used: if you need to store information such as which user is logged in, do not store the entire User entity (for example in a session), but rather only the user identifier.

There is NOT equivalent code: we removed these methods because people running into very bad bugs due to very very complex state permutations that shouldn't happen in first place.

[stof]

Currently, DoctrineBundle generates a proxy class (using ocramius/proxy-manager) for the entity manager, to allow resetting the entity manager (allowing to inject the entity manager in other services without having to kill the whole PHP process to rebuild all services in case an error occurs in the ORM). Will Doctrine ORM 3.0 allow reopening an entity manager instance (which of course involve it will be cleared) ? If no, this change means that using the EntityManager in a service used by a long-running process (a RabbitMQ consumer for instance) won't be possible anymore, which is quite impactful.

[lcobucci]

The architecture in v3 is quite different. The design we have been discussing is that each transaction will use a different UoW instance. This promotes better isolation and removes the need for em to be closed (in theory). There are still things to be addressed on this but that's the gist.

[stof]

well, if an em does not get closed on failure, then that's fine.

[stof]

clear is not deprecated. Partial clears (clearing only some entity classes) is, because it is unreliable (if a non-cleared entity has a relation pointing to a cleared entity, you might have managed entities referencing cleared objects, which is broken). And once you get there, you cannot recover things in any other way than clearing everything as the unit of work became unusable.

for merge, the solution would be to do a find and then build your own logic for how the state of your object and the state of the entity managed by Doctrine should be merged together

[stof]

@Ocramius my existing use case for detach is a process where I'm iterating over object results for a big result set, and detaching objects once I don't need them anymore (this process is never flushing the unit of work btw, I'm only reading in there). This detaching allows to remove the memory reference to this object from the unit of work so that the object can be destructed by PHP.
But I have some other objects (some of them being used by relations in this object btw) for which I still benefit from the entity map (and for which I'm fine with keeping them in the identity map until the end of the process, because they have a much lower cardinality).

What is the expected replacement for such usage of detach ? Would it be worth having a way to run a query which would hydrate results without registering the root entity of the results as managed by the unit of work if it is not already in it (this would of course need to be opt-in) ?
And if yes, would 2.7 provide this feature as well, alongside the deprecation of detach ?

[lcobucci]

What is the expected replacement for such usage of detach ? Would it be worth having a way to run a query which would hydrate results without registering the root entity of the results as managed by the unit of work if it is not already in it (this would of course need to be opt-in) ?
And if yes, would 2.7 provide this feature as well, alongside the deprecation of detach ?

@stof Query#iterate() + EM#clear() addresses that scenario, already in v2.6. Like:

$query = $em->createQuery(' ... ');

foreach ($query->iterate() as $result) {
    // Do whatever is needed with $result
    // ...

    // Free resources:
    unset($result[0]);
    $em->clear();
}

Regarding the relations, they would be loaded again sure. Caching should help on this (also available in v2.6).

[stof]

@lcobucci no it does not, because it also clears any other entity that might have been loaded earlier for this processing (and SLC is not an option for me, because some of these objects are also modified by direct SQL in some batch processing, so the SLC would contain the wrong info unless I also manage it myself which seems insane).

[michaljusiega]

I try myself to replace a detach method in my projects but it's hard :/

[lcobucci]

@stof have you considered enabling the SLC using the ArrayCache provider to contain the cached data to only the execution lifecycle?

That almost fully mimics the behaviour of the identity map, the only missing piece is the hydration. However, hydrating in an iteration like that should be quite cheap.

Our main goal with that move is to protect the integrity of the UoW, which is an internal component of the ORM. No application should rely on it for optimisation purposes, caching is meant to address that concern.

[stof]

@lcobucci but in my case, I want to stop tracking only my entities in the big list that I process. For other objects that I use in my processing, keeping them in the identity map is a good idea, as it ensures that I can use === being 2 variables holding them (if I clear, the second time might rehydrate a different instance).
What about my suggestion of an opt-in way to hydrate entities without adding them in the entity map for a specific query and entity (related objects would still be taken from the entity map) ? That would not corrupt the UoW, as that object would not be part of it at all (removing the need to detach it), but I would still get an instance of my class in the code (allowing to use it in my typed APIs).
In the case where I use detach, I don't actually need to have the returned instance in the UoW. but I need to have them hydrated, and having their relations being taken from the identity map.

[Ocramius]

What about my suggestion of an opt-in way to hydrate entities without adding them in the entity map

That's a very interesting idea, and could be done in future for sure. As @lcobucci stated though:

No application should rely on it for optimisation purposes, caching is meant to address that concern.

[stof]

Well, I'm not relying on the identity map for optimization purposes. that's precisely why using an ArrayCache as SLC does not fit me. I need to rely on the identity for other entities than the one I'm detaching during my processing. clearing+ in-memory SLC will optimize the loading of data, but won't preserve identity.

[beberlei]

@stof Read only sort of exists, but not as much as I remembered.

I thought the following was possible:

$query->setHint(Query::HINT_READ_ONLY, true); 

But that was removed during beta.

I added read only objects later, but they are required to be managed manually:

$entityManager->getUnitOfWork()->markReadOnly(entity);

Essentially what we just need to make the query hint automatically set entities as read only.

Read only objects are not considered during compute change sets.

[stof]

@beberlei but they are still referenced in the identity map. My issue is not about the size of computing the change set (this code does not flush at all). It is about avoiding to keep unnecessary objects in memory after processing them is done.

[thePanz]

In one of my project we are facing the same situation described by @stof in #7901 (comment), and loved the suggestion in #7901 (comment)

What about my suggestion of an opt-in way to hydrate entities without adding them in the entity map for a specific query and entity (related objects would still be taken from the entity map)? [..]

Is there any work being done in that direction that we can help with? How did you solve it @stof ?

[dbu]

the usecase of @thePanz and me is that we need to read (potentially lots of) entities that we know we don't intend to change, and we also don't need doctrine to cache them for us. at the same time, we have entities that we want to update and flush.

if i read the code in the 2.9.x branch correctly, there are mainly 2 places that might matter to us:

• the query cache that caches results based on the dbal query. this cache can be disabled per query (but as it is a cache, i assume it to discard things when it overflows, and all of it should be cheap)
• the uow identity map. the ObjectHydrator invokes UnitOfWork::createEntity, which makes the entity tracked. there are hints for this method, but there is no hint to mark that the entity should be created in detached mode. could it be an option to add a hint for that and respect it in createEntity?

we are motivated to contribute the feature, but of course we first need to figure out if the maintainer consider it a use case they want to support. if so, i suggest we create a new issue to discuss where to implement the functionality.

[stof]

Is there any work being done in that direction that we can help with? How did you solve it @stof ?

I haven't solved it yet. For now, I'm still using the deprecated detach method.

[dbu]

i chreated #8429 to further discuss the issue and think about solutions