[hibernate-dev] What should happen to queued operations when a collection is detached?

[Gail Badner]

 Guillaume has reviewed my PR and I've pushed the fix to master.

I still think the inconsistency should be resolved. I have no plans to do
any more on this without any feedback.

Regards,
Gail

On Tue, Oct 2, 2018 at 6:16 PM Gail Badner <gbadner at redhat.com> wrote:

> Since I haven't heard anything back, I've gone ahead and updated my PR [1]
> to ignore queued operations when merging a detached collection. This restores
> the functionality that was in place prior to HHH-5855 (which caused
> HHH-11209).
>
> In addition I've added warnings for the following situations:
> * a detached collection is being merged that has queued operations;
> * a collection with queued operations is attached to a session;
> * a collection with queued operations is detached from a session.
>
> I've also added a test case that shows that Session#merge and
> Session#saveOrUpdate have different results when called on an entity with a
> collection with queued operations. When Session#merge is called, the queued
> operations are ignored. When Session#saveOrUpdate is called, the queued
> operations are executed on flush.
>
> I don't like this inconsistency, but I'm not sure what to do about it. I
> don't think it's a good idea to have collections floating around with
> queued operations. As far as the application knows, it is just an
> uninitialized collection. I can see that the entity could find its way into
> a new session, with surprising results.
>
> IIUC, an application probably shouldn't detach an entity with a collection
> in this state (having queued operations), so I think it makes sense that a
> warning be logged when this happens. On the other hand, the warning will
> also be logged if a transaction involving updates to a collection with
> queued operations fails.
>
> I would really appreciate some feedback on this, since HHH-11209 is a bad
> regression that really should be fixed.
>
> Thanks,
> Gail
>
> [1] https://github.com/hibernate/hibernate-orm/pull/2460
>
> On Fri, Sep 28, 2018 at 6:27 PM Gail Badner <gbadner at redhat.com> wrote:
>
>> HHH-11209 involves a bug merging a detached entity with an uninitialized
>> collection that has queued operations.
>>
>> After looking into this issue I found a bug
>> where AbstractPersistentCollection#operationQueue was not being cleared
>> after a commit, if there was no cascade mapped for the collection.
>>
>> I've fixed that in a PR. [1]
>>
>> After that fix, the detached entity in the test case attached to
>> HHH-11209 has an uninitialized entity without any queued operations after
>> commit, which can be merged successfully in a new session/transaction.
>>
>> There is still a problem when Hibernate tries to merge a detached entity
>> has an uninitialized collection with queued operations though. My PR throws
>> UnsupportedOperationException in this case, and there is a test case that
>> reproduces it.
>>
>> I've been working on a fix to add support for this, but I have my doubts
>> that it really should be supported. I see that prior to fixing HHH-5855
>> (which caused HHH-11209), Hibernate simply ignored the queued operations in
>> the detached collection. [2].
>>
>> The fix for HHH-5855 properly dealt with merging a managed collection
>> that was uninitialized with queued operations. It introduced the bug where
>> a NullPointerException would get thrown if that collection  was detached.
>>
>> If we want to support merging the queued operations, then I would
>> consider that an improvement. Would this be a worthwhile improvement? I'm
>> guessing not, but I wanted to get some opinions.
>>
>> For now, I see a couple of ways to deal with this so that HHH-11209 can
>> be wrapped up:
>> 1) ignore queued operations in a detached collection when merging, as was
>> done before HHH-5855 was fixed.
>> 2) clear the queued operations when the collection is detached.
>>
>> Comments?
>>
>> Thanks,
>> Gail
>>
>> [1] https://github.com/hibernate/hibernate-orm/pull/2460
>> [2]
>> https://github.com/hibernate/hibernate-orm/commit/c1934b72edb4f781520937618b3b750bebb84576#diff-2c7912a47063b9ea81323bf9c6628895L651
>>
>