Bug 1286697 - Removing LazyCollectionOption.EXTRA annotation from Pool.entitlements breaks spectests
Removing LazyCollectionOption.EXTRA annotation from Pool.entitlements breaks ...
Status: ASSIGNED
Product: Candlepin
Classification: Community
Component: candlepin (Show other bugs)
1.2
Unspecified Unspecified
low Severity low
: ---
: ---
Assigned To: Alex Wood
Katello QA List
: Triaged
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-30 09:25 EST by Filip Nguyen
Modified: 2017-12-11 08:35 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Filip Nguyen 2015-11-30 09:25:11 EST
Description of problem:
Disabling EXTRA lazy loading on Pool.entitlements breaks several spec tests. I didn't spend any time to understand this yet but disabling any kind of LAZY loading shouldn't affect functionality of the application.

Version-Release number of selected component (if applicable):
2

How reproducible:
always

Steps to Reproduce:
1. Comment out @LazyCollection(LazyCollectionOption.EXTRA) in Pool.entitlements
2. server/bin/deploy -g && cd server && buildr rspec ;

Note you must use '-g' switch to see the errors

Actual results:
Spec test failures

e.g. Entitlement Resource
  should allow consumer to change entitlement quantity (FAILED - 5)
  should not allow consumer to change entitlement quantity out of bounds (FAILED - 6)


Expected results:
No failures
Comment 1 Alex Wood 2016-03-02 17:04:36 EST
I spent some time investigating this today and made a little progress.  The code is as follows:

  pool.getEntitlements().remove(entitlement);
  poolCurator.merge(pool);
  entitlementCurator.delete(entitlement);

The error message itself is "Runtime Error deleted instance passed to merge: [org.candlepin.model.Entitlement#<null>] at org.hibernate.event.internal.DefaultMergeEventListener.onMerge:160", and the cause (near as I can tell) is that we have CascadeType.ALL set on a Pool's entitlement collection.  I'm guessing that the merge and delete are being deferred to Hibernate lifecycle management and that by the time the actual update (via the merge) occurs, the entitlement object has been marked as being in the removed state.  When we attempt to commit the update that results in an attempt to cascade the update to the entitlement which is in the removed state.

Fixing it is another matter.  My inclination is to set orphanRemoval to true on the OneToMany association between Pool and Entitlement.  Then remove the entitlement from the collection when the time comes and flush the pool afterwards to immediately delete the orphan entitlement.  This approach seems to work (the spec tests pass at least) but I wouldn't mind getting a second opinion on whether this is the correct procedure from a JPA perspective.
Comment 2 Filip Nguyen 2016-03-14 12:42:01 EDT
Yeah now I think the problem is that during this revoke, in this particular case, the Pool.entitlements collection is initialized. Because its initialized you must remove any entitlement e from pool.entitlements in case you want to also delete it by EntityManager.remove(e). The necessity to do that is, I think, because Cascade CREATE/MERGE is in place.

Apart from your solutions, I think there are may be other straightforward options as well: 
  - do remove an Entitlement e from the collection as soon as its removed by EntityManager. While doing this, one has to be careful not to initialize lazy load of Pool.entitlements if unnecessary (this might be a performance problem)
  - remove cascade MERGE, CREATE from pool.entitlements :-) This would be good, but I am unsure we want to go down this road, since some part of Candlepin may depend on cascading

Note You need to log in before you can comment on or make changes to this bug.