HHH-8087 - Fix and test

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 ( shouldGenerateRevision( event ) ) {
-        if ( ! getAuditConfiguration().getGlobalCfg().isGenerateRevisionsForCollections() ) {
-            return;
-        }
-        if ( getAuditConfiguration().getEntCfg().isVersioned( entityName ) ) {
             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).

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 );
         }
 	}
 }

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<StrTestEntity> references = new HashSet<StrTestEntity>();
+
+	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<StrTestEntity> getReferences() {
+		return references;
+	}
+
+	public void setReferences(Set<StrTestEntity> references) {
+		this.references = references;
+	}
+
+	public String getData() {
+		return data;
+	}
+
+	public void setData(String data) {
+		this.data = data;
+	}
+}

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<StrTestEntity>() );
+		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<StrTestEntity>() );
+		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() );
+	}
+}