diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/event/BaseEnversCollectionEventListener.java b/hibernate-envers/src/main/java/org/hibernate/envers/event/BaseEnversCollectionEventListener.java index b6ee70c6de..58ec003b78 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/event/BaseEnversCollectionEventListener.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/event/BaseEnversCollectionEventListener.java @@ -50,6 +50,7 @@ import org.hibernate.persister.collection.AbstractCollectionPersister; * @author HernпїЅn Chanfreau * @author Steve Ebersole * @author Michal Skowronek (mskowr at o2 dot pl) + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) */ public abstract class BaseEnversCollectionEventListener extends BaseEnversEventListener { protected BaseEnversCollectionEventListener(AuditConfiguration enversConfiguration) { @@ -65,15 +66,12 @@ public abstract class BaseEnversCollectionEventListener extends BaseEnversEventL PersistentCollection newColl, Serializable oldColl, CollectionEntry collectionEntry) { - String entityName = event.getAffectedOwnerEntityName(); - if ( ! getAuditConfiguration().getGlobalCfg().isGenerateRevisionsForCollections() ) { - return; - } - if ( getAuditConfiguration().getEntCfg().isVersioned( entityName ) ) { + if ( shouldGenerateRevision( event ) ) { checkIfTransactionInProgress(event.getSession()); AuditProcess auditProcess = getAuditConfiguration().getSyncManager().get(event.getSession()); + String entityName = event.getAffectedOwnerEntityName(); String ownerEntityName = ((AbstractCollectionPersister) collectionEntry.getLoadedPersister()).getOwnerEntityName(); String referencingPropertyName = collectionEntry.getRole().substring(ownerEntityName.length() + 1); @@ -123,6 +121,26 @@ public abstract class BaseEnversCollectionEventListener extends BaseEnversEventL } } + /** + * @param event Collection event. + * @return Initialized persistent collection. + */ + protected Serializable getInitializedCollection(AbstractCollectionEvent event) { + event.getCollection().forceInitialization(); + return event.getCollection().getStoredSnapshot(); + } + + /** + * Checks whether modification of not-owned relation field triggers new revision and owner entity is versioned. + * @param event Collection event. + * @return {@code true} if revision based on given event should be generated, {@code false} otherwise. + */ + protected boolean shouldGenerateRevision(AbstractCollectionEvent event) { + final String entityName = event.getAffectedOwnerEntityName(); + return getAuditConfiguration().getGlobalCfg().isGenerateRevisionsForCollections() + && getAuditConfiguration().getEntCfg().isVersioned( entityName ); + } + /** * Looks up a relation description corresponding to the given property in the given entity. If no description is * found in the given entity, the parent entity is checked (so that inherited relations work). diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/event/EnversPreCollectionRemoveEventListenerImpl.java b/hibernate-envers/src/main/java/org/hibernate/envers/event/EnversPreCollectionRemoveEventListenerImpl.java index ab90d7ca26..f46d8980b3 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/event/EnversPreCollectionRemoveEventListenerImpl.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/event/EnversPreCollectionRemoveEventListenerImpl.java @@ -23,6 +23,8 @@ */ package org.hibernate.envers.event; +import java.io.Serializable; + import org.hibernate.engine.spi.CollectionEntry; import org.hibernate.envers.configuration.AuditConfiguration; import org.hibernate.event.spi.PreCollectionRemoveEvent; @@ -32,6 +34,7 @@ import org.hibernate.event.spi.PreCollectionRemoveEventListener; * @author Adam Warski (adam at warski dot org) * @author HernпїЅn Chanfreau * @author Steve Ebersole + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) */ public class EnversPreCollectionRemoveEventListenerImpl extends BaseEnversCollectionEventListener @@ -45,7 +48,12 @@ public class EnversPreCollectionRemoveEventListenerImpl public void onPreRemoveCollection(PreCollectionRemoveEvent event) { CollectionEntry collectionEntry = getCollectionEntry( event ); if ( collectionEntry != null && !collectionEntry.getLoadedPersister().isInverse() ) { - onCollectionAction( event, null, collectionEntry.getSnapshot(), collectionEntry ); + Serializable oldColl = collectionEntry.getSnapshot(); + if ( !event.getCollection().wasInitialized() && shouldGenerateRevision( event ) ) { + // In case of uninitialized collection we need a fresh snapshot to properly calculate audit data. + oldColl = getInitializedCollection( event ); + } + onCollectionAction( event, null, oldColl, collectionEntry ); } } } diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/entities/manytomany/unidirectional/JoinTableEntity.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/entities/manytomany/unidirectional/JoinTableEntity.java new file mode 100644 index 0000000000..1e5c162b08 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/entities/manytomany/unidirectional/JoinTableEntity.java @@ -0,0 +1,95 @@ +package org.hibernate.envers.test.entities.manytomany.unidirectional; + +import java.io.Serializable; +import java.util.HashSet; +import java.util.Set; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.JoinTable; +import javax.persistence.ManyToMany; + +import org.hibernate.envers.Audited; +import org.hibernate.envers.test.entities.StrTestEntity; + +/** + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) + */ +@Entity +@Audited +public class JoinTableEntity implements Serializable { + @Id + @GeneratedValue + private Long id; + + private String data; + + @ManyToMany + @JoinTable(name = "test_join_table", + joinColumns = @JoinColumn(name = "assoc_id1"), + inverseJoinColumns = @JoinColumn(name = "assoc_id2") + ) + private Set references = new HashSet(); + + public JoinTableEntity() { + } + + public JoinTableEntity(String data) { + this.data = data; + } + + public JoinTableEntity(Long id, String data) { + this.id = id; + this.data = data; + } + + @Override + public boolean equals(Object o) { + if ( this == o ) return true; + if ( !( o instanceof JoinTableEntity ) ) return false; + + JoinTableEntity that = (JoinTableEntity) o; + + if ( data != null ? !data.equals( that.data ) : that.data != null ) return false; + if ( id != null ? !id.equals( that.id ) : that.id != null ) return false; + + return true; + } + + @Override + public int hashCode() { + int result = id != null ? id.hashCode() : 0; + result = 31 * result + ( data != null ? data.hashCode() : 0 ); + return result; + } + + @Override + public String toString() { + return "JoinTableEntity(id = " + id + ", data = " + data + ")"; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public Set getReferences() { + return references; + } + + public void setReferences(Set references) { + this.references = references; + } + + public String getData() { + return data; + } + + public void setData(String data) { + this.data = data; + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytomany/unidirectional/JoinTableDetachedTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytomany/unidirectional/JoinTableDetachedTest.java new file mode 100644 index 0000000000..78b6a86d72 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/manytomany/unidirectional/JoinTableDetachedTest.java @@ -0,0 +1,137 @@ +package org.hibernate.envers.test.integration.manytomany.unidirectional; + +import java.util.Arrays; +import java.util.HashSet; +import javax.persistence.EntityManager; + +import org.junit.Assert; +import org.junit.Test; + +import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase; +import org.hibernate.envers.test.Priority; +import org.hibernate.envers.test.entities.StrTestEntity; +import org.hibernate.envers.test.entities.manytomany.unidirectional.JoinTableEntity; +import org.hibernate.testing.TestForIssue; + +/** + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) + */ +@TestForIssue( jiraKey = "HHH-8087" ) +public class JoinTableDetachedTest extends BaseEnversJPAFunctionalTestCase { + private Long collectionEntityId = null; + private Integer element1Id = null; + private Integer element2Id = null; + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { JoinTableEntity.class, StrTestEntity.class }; + } + + @Test + @Priority(10) + public void initData() { + EntityManager em = getEntityManager(); + + // Revision 1 - addition + em.getTransaction().begin(); + JoinTableEntity collectionEntity = new JoinTableEntity( "some data" ); + StrTestEntity element1 = new StrTestEntity( "str1" ); + StrTestEntity element2 = new StrTestEntity( "str2" ); + collectionEntity.getReferences().add( element1 ); + collectionEntity.getReferences().add( element2 ); + em.persist( element1 ); + em.persist( element2 ); + em.persist( collectionEntity ); + em.getTransaction().commit(); + + collectionEntityId = collectionEntity.getId(); + element1Id = element1.getId(); + element2Id = element2.getId(); + + em.close(); + em = getEntityManager(); + + // Revision 2 - simple modification + em.getTransaction().begin(); + collectionEntity = em.find( JoinTableEntity.class, collectionEntity.getId() ); + collectionEntity.setData( "some other data" ); + collectionEntity = em.merge( collectionEntity ); + em.getTransaction().commit(); + + em.close(); + em = getEntityManager(); + + // Revision 3 - remove detached object from collection + em.getTransaction().begin(); + collectionEntity = em.find( JoinTableEntity.class, collectionEntity.getId() ); + collectionEntity.getReferences().remove( element1 ); + collectionEntity = em.merge( collectionEntity ); + em.getTransaction().commit(); + + em.close(); + em = getEntityManager(); + + // Revision 4 - replace the collection + em.getTransaction().begin(); + collectionEntity = em.find( JoinTableEntity.class, collectionEntity.getId() ); + collectionEntity.setReferences( new HashSet() ); + collectionEntity = em.merge( collectionEntity ); + em.getTransaction().commit(); + + em.close(); + em = getEntityManager(); + + // Revision 5 - add to collection + em.getTransaction().begin(); + collectionEntity = em.find( JoinTableEntity.class, collectionEntity.getId() ); + collectionEntity.getReferences().add( element1 ); + collectionEntity = em.merge( collectionEntity ); + em.getTransaction().commit(); + + em.close(); + } + + @Test + public void testRevisionsCounts() { + Assert.assertEquals( Arrays.asList( 1, 2, 3, 4, 5 ), getAuditReader().getRevisions(JoinTableEntity.class, collectionEntityId ) ); + Assert.assertEquals( Arrays.asList( 1 ), getAuditReader().getRevisions(StrTestEntity.class, element1Id ) ); + Assert.assertEquals( Arrays.asList( 1 ), getAuditReader().getRevisions(StrTestEntity.class, element2Id ) ); + } + + @Test + public void testHistoryOfCollectionEntity() { + // Revision 1 + JoinTableEntity collectionEntity = new JoinTableEntity( collectionEntityId, "some data" ); + StrTestEntity element1 = new StrTestEntity( "str1", element1Id ); + StrTestEntity element2 = new StrTestEntity( "str2", element2Id ); + collectionEntity.getReferences().add( element1 ); + collectionEntity.getReferences().add( element2 ); + JoinTableEntity ver1 = getAuditReader().find( JoinTableEntity.class, collectionEntityId, 1 ); + Assert.assertEquals( collectionEntity, ver1 ); + Assert.assertEquals( collectionEntity.getReferences(), ver1.getReferences() ); + + // Revision 2 + collectionEntity.setData( "some other data" ); + JoinTableEntity ver2 = getAuditReader().find( JoinTableEntity.class, collectionEntityId, 2 ); + Assert.assertEquals( collectionEntity, ver2 ); + Assert.assertEquals( collectionEntity.getReferences(), ver2.getReferences() ); + + // Revision 3 + collectionEntity.getReferences().remove( element1 ); + JoinTableEntity ver3 = getAuditReader().find( JoinTableEntity.class, collectionEntityId, 3 ); + Assert.assertEquals( collectionEntity, ver3 ); + Assert.assertEquals( collectionEntity.getReferences(), ver3.getReferences() ); + + // Revision 4 + collectionEntity.setReferences( new HashSet() ); + JoinTableEntity ver4 = getAuditReader().find( JoinTableEntity.class, collectionEntityId, 4 ); + Assert.assertEquals( collectionEntity, ver4 ); + Assert.assertEquals( collectionEntity.getReferences(), ver4.getReferences() ); + + // Revision 5 + collectionEntity.getReferences().add( element1 ); + JoinTableEntity ver5 = getAuditReader().find( JoinTableEntity.class, collectionEntityId, 5 ); + Assert.assertEquals( collectionEntity, ver5 ); + Assert.assertEquals( collectionEntity.getReferences(), ver5.getReferences() ); + } +}