HHH-8280 - validity audit strategy breaks when entity IDs are reused

hibernate-envers/src/main/java/org/hibernate/envers/strategy/ValidityAuditStrategy.java

@@ -98,8 +98,12 @@ public class ValidityAuditStrategy implements AuditStrategy {
 		session.save( auditedEntityName, data );
 		sessionCacheCleaner.scheduleAuditDataRemoval( session, data );
 
-		// Update the end date of the previous row if this operation is expected to have a previous row
-		if ( getRevisionType( auditCfg, data ) != RevisionType.ADD ) {
+		// Update the end date of the previous row.
+		//
+		// The statement will no-op if an entity with a given identifier has been
+		// inserted for the first time. But in case a deleted primary key value was
+		// reused, this guarantees correct strategy behavior: exactly one row with
+		// a null end date exists for each identifier.
 			final Queryable productionEntityQueryable = getQueryable( entityName, sessionImplementor );
 			final Queryable rootProductionEntityQueryable = getQueryable(
 					productionEntityQueryable.getRootEntityName(),
@@ -238,13 +242,12 @@ public class ValidityAuditStrategy implements AuditStrategy {
 					}
 			);
 
-			if ( rowCount != 1 ) {
+			if ( rowCount != 1 && getRevisionType(auditCfg, data) != RevisionType.ADD ) {
 				throw new RuntimeException(
 						"Cannot update previous revision for entity " + auditedEntityName + " and id " + id
 				);
 			}
 	}
-	}
 
 	private Queryable getQueryable(String entityName, SessionImplementor sessionImplementor) {
 		return (Queryable) sessionImplementor.getFactory().getEntityPersister( entityName );

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/strategy/ValidityAuditStrategyIdentifierReuseTest.java

@@ -0,0 +1,67 @@
+package org.hibernate.envers.test.integration.strategy;
+
+import static org.junit.Assert.*;
+
+import java.util.Map;
+
+import javax.persistence.EntityManager;
+import javax.persistence.EntityTransaction;
+
+import org.hibernate.envers.configuration.EnversSettings;
+import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase;
+import org.hibernate.envers.test.entities.IntNoAutoIdTestEntity;
+import org.hibernate.testing.TestForIssue;
+import org.junit.Test;
+
+/**
+ * Tests that reusing identifiers doesn't cause {@link ValidityAuditstrategy}
+ * to misbehave.
+ *
+ * @author adar
+ *
+ */
+@TestForIssue(jiraKey = "HHH-8280")
+public class ValidityAuditStrategyIdentifierReuseTest extends
+BaseEnversJPAFunctionalTestCase {
+
+  @SuppressWarnings({ "rawtypes", "unchecked" })
+  @Override
+  protected void addConfigOptions(Map options) {
+    options.put(EnversSettings.AUDIT_STRATEGY,
+        "org.hibernate.envers.strategy.ValidityAuditStrategy");
+  }
+
+  @Override
+  protected Class<?>[] getAnnotatedClasses() {
+    return new Class[] { IntNoAutoIdTestEntity.class };
+  }
+
+  private void saveRemoveEntity(EntityManager em, Integer id) {
+    EntityTransaction et = em.getTransaction();
+
+    et.begin();
+    IntNoAutoIdTestEntity e = new IntNoAutoIdTestEntity(0, id);
+    em.persist(e);
+    assertEquals(id, e.getId());
+    et.commit();
+
+    et.begin();
+    e = em.find(IntNoAutoIdTestEntity.class, id);
+    assertNotNull(e);
+    em.remove(e);
+    et.commit();
+  }
+
+  @Test
+  public void testValidityAuditStrategyDoesntThrowWhenIdentifierIsReused() {
+    final Integer reusedID = 1;
+
+    EntityManager em = getEntityManager();
+    try {
+      saveRemoveEntity(em, reusedID);
+      saveRemoveEntity(em, reusedID);
+    } finally {
+      em.close();
+    }
+  }
+}