Actual entity being change available when using envers revision listener #241

Closed
wants to merge 3 commits into
from

Projects

None yet

4 participants

@stevemac007

As per the conversation here http://community.jboss.org/thread/176142

This change adds a new interface for Entity Revision tracking that allows the listener to recieve the actual entity that has been changed.

stevemac007 added some commits Dec 22, 2011
@stevemac007 stevemac007 Initial discussion idea on how to pass the Audited entity to the
Listener.

I know that this commit breaks public API's, and will need to be
refactored to either create a new API or some sort of sub-interface.

But I wanted to commit this to get discussion and feedback on this
approach and why/whynot this would work.

I also need to build a working copy of this so that I can test that this
sort of fix will actually help us with our issue in our product.
fed9eda
@stevemac007 stevemac007 Reverted the EntityTrackingRevisionListener and created a new Listener
that accepts the changed entities.
4ffbf2c
@stevemac007 stevemac007 Another attempt to commit the fix to get the recent fix to compile. 55cb670

Hi Steve,

Do you have any news from Envers guys on the status of this pull request? Is there any JIRA issue I can vote for to get this accepted?

Member

I can validate the pull request but would prefer to extend EntityTrackingRevisionListener instead of adding new interface with one extra parameter. Would you require to release proposed changes in 4.x stream, or 5.x is acceptable (compatibility issues)? BTW, entity that has just been saved to the database should exist in the first level cache, so looking it up will not hit database. Am I right?

I believe there was a jira raised.. I will try and find the link and update it here.

I believe there was a jira raised.. I will try and find the link and update it here.

@lukasz-antoniak lukasz-antoniak commented on the diff Aug 25, 2012
...onization/work/FakeBidirectionalRelationWorkUnit.java
@@ -112,7 +112,7 @@ public AuditWorkUnit merge(FakeBidirectionalRelationWorkUnit second) {
secondFakeRelationChanges.get(propertyName)));
}
- return new FakeBidirectionalRelationWorkUnit(this, mergedFakeRelationChanges, mergedNested);
+ return new FakeBidirectionalRelationWorkUnit(this, mergedFakeRelationChanges, mergedNested, null);
lukasz-antoniak
lukasz-antoniak Aug 25, 2012 Member

Here I would suggest passing mergedNested.getEntity() instead of null.

@lukasz-antoniak lukasz-antoniak commented on the diff Aug 25, 2012
...onization/work/FakeBidirectionalRelationWorkUnit.java
@@ -124,7 +124,7 @@ public static AuditWorkUnit merge(FakeBidirectionalRelationWorkUnit frwu, AuditW
AuditWorkUnit nestedMerged = nestedSecond.dispatch(nestedFirst);
// Creating a new fake relation work unit with the nested merged data
- return new FakeBidirectionalRelationWorkUnit(frwu, nestedMerged);
+ return new FakeBidirectionalRelationWorkUnit(frwu, nestedMerged, null);
lukasz-antoniak
lukasz-antoniak Aug 25, 2012 Member

Here I would suggest passing nestedMerged.getEntity() instead of null.

@lukasz-antoniak lukasz-antoniak commented on the diff Aug 25, 2012
...nization/work/PersistentCollectionChangeWorkUnit.java
@@ -156,7 +156,7 @@ public AuditWorkUnit dispatch(WorkUnitMergeVisitor first) {
mergedChanges.addAll(newChangesIdMap.values());
return new PersistentCollectionChangeWorkUnit(sessionImplementor, entityName, verCfg, id, mergedChanges,
- referencingPropertyName);
+ referencingPropertyName, null);
lukasz-antoniak
lukasz-antoniak Aug 25, 2012 Member

Here also I wouldn't like to pass null. The actual modification here has to be consistent with EntityChangeNotifier#entityChanged(Session, Object, AuditWorkUnit) code which notifies about a change in collection owner.

Member
brmeyer commented Jul 10, 2013

Hey @lukasz-antoniak , any follow ups to this?

Member

I will close this pull request for now. Not all cases work as expected. Please follow discussion starting here: https://community.jboss.org/message/755951#755951. I would be really happy if someone could work on my forked branch and fix the scenario that fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment