-
15. Re: adding more information to revision entity
I have just pushed a change that reverts EntityTrackingRevisionListener to its original state and have created a new EntityIncludedTrackingRevisionListener that has the supplied entity.
Anyone already using the EntityTrackingRevisionListener will still work fine (no breaking change) and only internal classes have been updated.
Projects wanting to take advantage of this new feaure only need to change their implementation to implement the EntityIncludedTrackingRevisionListener to get the actual entity,
https://github.com/stevemac007/hibernate-core/tree/Branch_4.0.0.Final
Just for completeness I cloned this from the 4.0.0.Final (as named in the Branch) so I could use this as a drop in replacement for 4.0.0 in our production environment.
I have no tests for this but would be happy to add some if anyone has pointers how to do so.
I'm much more happy that this could be contributed back to the core now, so am I best to create a github pull request, or create a patch for application against a JIRA issue?
-
16. Re: adding more information to revision entity
Hi all!
I should have visited this forum more often :).
@Vyacheslav Sakhno:
IMO if you save several entities of the same type during one transaction, there would be just one entry in REVCHANGES table. Please see DefaultTrackingModifiedEntitiesRevisionEntity.modifiedEntityNames mapping and Java type.
@Steve Mactaggart:
If you query for an entity that has just been modified in your implementation of EntityTrackingRevisionListener.entityChanged() method (using provided entity class, name and id), Hibernate does not hit database actually. This entity should exist in the session's first level cache. Do you agree?
I think we can add another parameter to EntityTrackingRevisionListener.entityChanged() as Steve suggests, but in the future release (compatibility reasons; 5.0 maybe?). Adam, do you think that adding third revision listener in 4.x would be a good idea?
Regards,
Lukasz Antoniak
-
17. Re: adding more information to revision entity
Lukasz,
If you query for an entity that has just been modified in your implementation of EntityTrackingRevisionListener.entityChanged() method (using provided entity class, name and id), Hibernate does not hit database actually. This entity should exist in the session's first level cache. Do you agree?
I was not sure if this was true at the time I started working on this fix, and being part of "Audit" code and therefore running on every database interaction I wanted to ensure that the performance overhead was small. Also initially was looking at trying to get the actual changes, ie name changed from 'old' --> 'new', but then (for my personal requirements) I found that wasn't exactly what I needed so reverted to just attaching the entire adjusted entity.
I think we can add another parameter to EntityTrackingRevisionListener.entityChanged() as Steve suggests, but in the future release (compatibility reasons; 5.0 maybe?). Adam, do you think that adding third revision listener in 4.x would be a good idea?
I'm happy either way with this, I am using my own custom build of 4.0.0 at the moment, but we are probably not in a positon to wait till the 5.0 timeline. If I need to I can maintain my own fork until something like this gets approved.
-
18. Re: adding more information to revision entity
Can you create a pull request and a JIRA issue? If the issue gets some votes, I'll merge it.
Adam
-
19. Re: adding more information to revision entity
Here is why i'm asking, this is a proposal for feature request request maybe:
different types of audited entities can be in a group which is identified by group Identifier and also can require for entities to have some group info, in my case group info is the same for all entity types and is an id of group root entity(class+id);
I need to query for entities changed in revision with common to all types of them costraint/criteria:
Criteria criteria = persistenceManagerHibernate.getCriteria(EntityChange.class);
criteria.createAlias("revision", "r");
criteria.add(Restrictions.eq("groupEntityId", groupEntityId));
criteria.add(Restrictions.eq("groupName", groupName));
criteria.addOrder(Order.asc("r.timestamp"));
criteria.setFirstResult(start);
criteria.setMaxResults(limit);
Current implementation of
has no method
AuditQueryCreator createQuery();
do.
And also implementation of this is partially a perfomance issue:
1) either query for each entity type by a separate query
2) or track entities(not entity types) changed in revision with group info copied to entity change table. And query that table for entity change in revision by criteria:
So i've decided to implement second case:
@Entity
@Table(name = "ENTITY_CHANGED_IN_REVISION")
public class EntityChange {
@Id
@NotNull
@GeneratedValue
@Column(name = "ID")
private Long id;
@Column(name="GROUP_NAME")
@Size(max = 50)
private String groupName;
@Column(name="GROUP_ENTITY_ID")
@Size(max = 50)
private String groupEntityId;
@Column(name="ENTITY_ID")
//TODO implement
@Size(max = 200)
private String entityId;
@Column(name="ENTITY_NAME")
private String entityName;
@Column(name="REVISION_TYPE")
private Integer revisionType;
@ManyToOne(fetch = FetchType.LAZY, optional = false)
@JoinColumn(name = "REV_ID", updatable = false)
private Revision revision;
.....
@Entity
@Table(name = "REVISIONS")
@org.hibernate.envers.RevisionEntity(CustomRevisionListener.class)
public class Revision implements Serializable {
private static final long serialVersionUID = -1255842407304508513L;
@Id
@GeneratedValue
@RevisionNumber
private long id;
@RevisionTimestamp
@NotNull
@Column(name = "TIMESTAMP")
private Date timestamp;
@Column(name="USERNAME")
private String username;
@OneToMany(fetch = FetchType.LAZY, mappedBy = "revision", cascade = { CascadeType.ALL }, orphanRemoval = true)
private Set<EntityChange> entitiesChanges = new HashSet<EntityChange>();
...
@Service
public class CustomRevisionListener implements EntityTrackingRevisionListener, ApplicationContextAware {
...
@Override
public void entityChanged(@SuppressWarnings("rawtypes") Class entityClass, String entityName, Serializable entityId, RevisionType revisionType, Object revisionEntity) {
try {
Revision revision = (Revision) revisionEntity;
//TODO it is a workaround of entityChanged double invokation during relationship audit
boolean isPresentInDb = getEntityChangeService().isPresentInDb(revision, revisionType, entityName, entityId);
if (!isPresentInDb) {
Auditable auditable = getEntityRevisionService().loadLastAvailableObjectState(entityClass, entityId);
auditable.getAuditGroupName();
auditable.getAuditGroupId();
EntityChange entityChange = getEntityChangeService().createDetached(revision, revisionType, entityName, entityId, auditable);
getEntityChangeService().save(entityChange);
revision.getEntitiesChanges().add(entityChange);
getRevisionService().save(revision);
}
} catch (Exception e) {
logger.error(e.getMessage());
e.printStackTrace();
}
}
}
-
20. Re: adding more information to revision entity
This is in fact a separate auditing strategy - to keep all changes in one entity and be able to query them.
Your way would be nice to have in Envers as well, of course
Adam
-
21. Re: adding more information to revision entity
Hi guys,
Do you have any plans on providing new type of revision listener in the Envers 4 branch?
My use case is probably quite similar the ones already mentioned: I want to extract some information from entity which was modified and store it as part of the revision. More precisely, all audited entities in our system belong to a container (which we call a "workspace"). When an entity is being modified, and a new revision is being generated, I would like to extract the container information (workspace ID) from the modified entity and store it as part of the revision entity. Was a bit surprised when EntityTrackingRevisionListener did not allow this as it did not expose the actual entity being changed, just the class and ID.
Vjacheslav's approach of searching the entity by classname and ID in the cache would probably work, but looks slightly tricky for a number of reasons:
- If the entity was deleted, we'll probably need to look in the audit history instead.
- Searching for an entity which was just modified looks dangerous... if, by any chance, this entity will not be cache, then we will try to query from DB the entity which is currently being updated, which would result in a deadlock.
- As Envers code creates the revision listener via reflection, Spring dependency injection is not performed, and we'll have to proceed with static field and ApplicationContextAware workaround, which makes me worry about thread safety and bean reusability stuff.
In other words, accepting Steve's pull request would definitely be a nicer way. Is there any JIRA issue I can vote for to make this accepted?
Sergey
-
22. Re: adding more information to revision entity
Hmm your clarification actually makes sense. The only issue is the backward compatibility in 4.x branch .
-
23. Re: adding more information to revision entity
After some thought, I think I understand your concern now. Proposed patch is adding new EntityIncludedTrackingRevisionListener interface, extending RevisionListener with new entityChanged(..., Object entity) method. If you add this in 4.x, this would be backwards compatible for all existing code which is using RevisionListener or EntityTrackingRevisionListener interfaces. However, to be clean, you would prefer to modify EntityTrackingRevisionListener.entityChanged() method, adding Object entity parameter. This change would break backwards compatibility, so acceptable only in 5.x branch.
I would be quite happy to get this functionality in 4.x branch though... I don't have any information of release plans of Hibernate 5 but doesn't look like it's going to be any time soon.
Why can't we implement both - add EntityIncludedTrackingRevisionListener to 4.x code, and modify EntityTrackingRevisionListener in 5.x code? This would mean that any code written for Envers 4 using EntityIncludedTrackingRevisionListener will have to be updated when migrating to Envers 5 to use EntityTrackingRevisionListener instead. This, however, should be normal if you don't expect 5.x to be fully backwards compatible with 4.x?
Also, why would you prefer (according to your comment in the pull request discussion thread) to have EntityIncludedTrackingRevisionListener extending EntityTrackingRevisionListener? This would mean that if I implement EntityIncludedTrackingRevisionListener I also need to implement old entityChanged() method - which would probably be a useless thing to do?
-
24. Re: adding more information to revision entity
Apologies if I made myself not clear. In the pull request comment I was talking about adding a new parameter to existing EntityTrackingRevisionListener. I hope to validate pull request during coming weekend, and:
1) Create new and deprecated EntityIncludedTrackingRevisionListener in 4.x.
2) Extend existing EntityTrackingRevisionListener in 5.x.
I have to check what gets passed as entity object to entityChanged() method in various mapping options.
-
25. Re: adding more information to revision entity
I'll be sure to try using it once it gets to a snapshot build. Thanks a lot!
-
26. Re: adding more information to revision entity
I have commented on the pull request and it seems that few more changes need to be done. I will write some test cases verifying entityChang() behavior and provide a link. Two scenarios that look wrong to me are:
1) "Fake" bidirectional relation. Such relation uses @OneToMany + @JoinColumn on the one side, and @ManyToOne + @Column(insertable=false, updatable=false) on the many side. Example test case: org.hibernate.envers.test.integration.onetomany.detached.JoinColumnBidirectionalList.
2) Map relations. Example test case: org.hibernate.envers.test.integration.manytomany.BasicMap.
-
27. Re: adding more information to revision entity
Please see my changes and read test cases (all pass at the moment) here: https://github.com/lukasz-antoniak/hibernate-core/compare/notification
Every notification that user is going to receive is shown ordered with expectNotification() method. I suppose that notifications don't work as expected in every scenario. Thoughts?
@Adam, could you take a look as well?
Edit:
When you click on the commit link (https://github.com/lukasz-antoniak/hibernate-core/commit/19bde340c61aa130eeb6a73afe35c737ef834f8a) you can see my inline comments.
-
-
29. Re: adding more information to revision entity
Thanks Adam. Persistent collection callback has been added plus arguments encapsulated into dedicated classes.
Not sure if notifications work 100% correct in case of fake bi-directional relations, for example:
ing2.getReferences().remove(ed2);ing1.getReferences().add(ed2);Assuming we get
expectNotification( ListJoinColumnBidirectionalRefEdEntity.class, ed2.getId(), ed2, RevisionType.ADD );, shouldn't we expect the removal notification as well? The root cause might be FakeBidirectionalRelationWorkUnit.FakeRelationChange#merge(FakeRelationChange, FakeRelationChange) ("DEL, ADD - return ADD (points to new owner)"). This would be really hard to patch and not sure if it is worthy doing so.I have just noticed that I have forgotten about CollectionChangeWorkUnit in collection changed collback.