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

[Gail Badner]

Before doing this, I'd like to get some feedback about these warnings
cropping up, just in case there is a valid use case that we're missing.

On Fri, Oct 5, 2018 at 2:46 AM andrea boriero <andrea at hibernate.org> wrote:

> I would say to log a warning when the entity is detached and then clear
> the queued operations.
>
> On Fri, 5 Oct 2018 at 07:25, Gail Badner <gbadner at redhat.com> wrote:
>
>>  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
>> >>
>> >
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>
>