Rephrase ObjectManager::merge() BC break doc

[greg0ire]

It refers to another deprecation that has been reverted.

[malarzm]

Thanks @greg0ire!

[gnutix]

Sorry to say that here, but every time I read this line of the doc, I feel like I've been given a 🖕. :/

Our project uses merge extensively in legacy code, and I have no idea how in hell I will be able to migrate our code when the time comes. I find it quite absurd for a library to just say "we've removed one of our feature because it was inconvenient for us, just deal with it" without providing an upgrade path, code examples, an external package or anything really...

[derrabus]

I find it quite absurd for a library to just say "we've removed one of our feature because it was inconvenient for us, just deal with it"

For the record: Nobody says that.

[gnutix]

Thanks for answering without answering... For the record : the quote were not meant to imply someone said exactly that, but rather that that's how I interpreted every written sentence (in doc, issues, etc) related to the removal of this method I've ever read (and there's been some, I've been looking out for that problem for years now). Not once have I seen a comment that gave any support to the users of the library.

So again, my question is : is it possible to provide such a supportive answer ? One that provide an upgrade path, in the means of code examples, some separate repository with the code, or else... ?

Or is this method just impossible to migrate in any way, and every one using it have two choices : either throw their code away and close their business, or be stuck on a soon-to-be-outdated library version ? If so, I think this deprecation notice should have a bit more context and explain how everyone's using it is damned.

[malarzm]

It's not possible to provide a clear migration path as there's too many ways one could use merge:

• fetching data from some cache and merging -> SLC probably
• fetching unserialized object from session and merging - it's asking for trouble anyway, just refetch the object and store id in session
• random merge calls across the codebase - well, how can we say what to do? I've even seen random calls to merge that were discarding returned object. How can we advise?

That's just 3 usages from the top of my head I've seen over the years, but not the only ones. Upgrade path depends on why somebody used merge in the first place. It's just my personal opinion, but I've been using Doctrine for many years, wrote apps big and small, and never HAD to use merge. There were moments I could, it even could have been handy, but I decided not to and never looked back.

[gnutix]

I don't know what SLC refers to? Here's two of the most common usages in our codebase :

    public function save(Alert $alert)
    {
        if (!$alert->getId()) {
            $this->em->persist($alert);
        } else {
            $this->em->merge($alert);
        }
        $this->em->flush();

        return $alert;
    }

And sometimes, it's even :

    public function save(AbsenceRequest $absenceRequest)
    {
        $entity = $this->em->merge($absenceRequest);
        $this->em->flush($entity);

        return $entity;
    }

There are some more exotic usages too unfortunately that I can't really share as-is.

[malarzm]

I don't know what SLC refers to?

Second Level Cache

Here's two of the most common usages in our codebase :

Your first example is exactly what I had in mind with "I've even seen random calls to merge that were discarding returned object. How can we advise?". Why are you merging the object, discarding what merge told you, and returning the original from method? This is not how merge is supposed to be used. Next, it's impossible to advise you without knowing the story why merge is even used. Why is it needed? In normal flow you'd be operating on an entity fetched from database and that's it. Why do you need to discard whatever is in UoW with what you're passing to the method? Do you rely on behavious that it's safe-ish to use merge on a managed entity but you want cascade to kick in? Do you even consider passing unmanaged entities to merge?

[derrabus]

Sorry to interrupt again and not being helpful, but this is a merge request that fixed a tiny piece of documentation, not a support forum. Can we please use https://github.com/doctrine/orm/discussions for support questions (assuming that the ORM is the persitence implementation that is being used here)?

[malarzm]

Feel free to mention me in a discussion thread if you create one as I'm not following all ORM activity. From my perspective your demonstrated usage of merge is a prominent example of why merge should be gone as it doesn't seem to be used properly (basing on provided examples).