StatefulPersistenceContext.entityEntryContext does not work properly for ManagedEntity associated with a different StatefulPersistenceContext

Description

EntityEntryContext does not "know" which StatefulPersistenceContext owns it. This can cause problems when a ManagedEntity that is associated with a different StatefulPersistenceContext is passed to EntityEntryContext methods.

When a ManagedEntity is passed as an argument to an EntityEntryContext method, the method always assumes that the ManagedEntity is associated with the EntityEntryContext if {{ManagedEntity.$$_hibernate_getEntityEntry() != null }}.

If the ManagedEntity is associated with a different EntityEntryContext, then methods like #addEntityEntry and #removeEntityEntry will overwrite data in the ManagedEntity, causing inconsistencies in the other EntityEntryContext.

In addition, Session.contains( managedEntity) will always return true:

{{s = getFactory().openSession()
loadedParent = s.get( Parent.class, loadedParent.getId() );
managedParent = (ManagedEntity) loadedParent;
// now open a new session (s2) and assert that the new session does not contain it
Session s2 = getFactory().openSession();
assertFalse( s2.contains( managedParent ) );}} <== FAILS

EntityEntryContext should have a reference to its owning StatefulPersistenceContext. That way it can determine if ManagedEntity.$$_hibernate_getEntityEntry() is associated with the same StatefulPersistenceContext, so EntityEntryContext methods can deal with the ManagedEntity appropriately.

Environment

None

Activity

Show:
Steve Ebersole
July 22, 2016, 5:28 PM

to your larger comment/question...

That is why I said I think that this needs a clean-room look at the design of our PC. We essentially have multiple new use-cases that the old design was never meant to handle. We need a better design that covers all of the use-case we wish to support.

And clearly a shared entity reference (both in terms of L2 cache and PC) is one of those.

Steve Ebersole
July 23, 2016, 4:40 PM

you asked me to comment on your "short-term proposal". Assuming this is that proposal:

One idea is to wrap the immutable ManagedEntity objects with a ManagedEntityHolder that implements ManagedEntity, adding ManagedEntityHolder#getManagedEntity to retrieve the actual immutable ManagedEntity (with null previous/next references). Instead, ManagedEntityHolder would hold the previous/next references specific to that PersistenceContext.

Sure that would work. But realize its essentially the same as what we used to do. In effect it is reverting much of the performance gain.

Previously we would create a new EntityEntry for each immutable-entity + Session combo. It is this object creation that the performance work targeted. Unless I miss something, your proposal is to now create a new ManagedEntityHolder instance for each immutable-entity + Session combo. Will it "work"? Sure, it will address the bugs. But from a performance perspective you will wipe out all that work.

Emmanuel Bernard
July 25, 2016, 11:28 AM

WRT ordering, batching is what comes to mind. Also we I think (used to) guarantee that session.save(a); session.save(b); would apply INSERT a / INSERT b in the same order in case our FK operation ordering algorithm was getting faulty.

Gail Badner
July 25, 2016, 11:02 PM

All of the pull requests retain the order in which entities were added to the EntityEntryContext.

Gail Badner
July 30, 2016, 1:21 AM

Fixed in master, 5.1, and 5.0 branches.

Assignee

Gail Badner

Reporter

Gail Badner

Fix versions

Labels

backPortable

None

Suitable for new contributors

None

Requires Release Note

None

Pull Request

None

backportDecision

None

Components

Affects versions

Priority

Critical
Configure