HHH-8280 - Identifiers reuse option

documentation/src/main/docbook/devguide/en-US/Envers.xml

@@ -326,6 +326,19 @@
                             Name of column used for storing ordinal of the change in sets of embeddable elements.
                         </entry>
                     </row>
+                    <row>
+                        <entry>
+                            <property>org.hibernate.envers.allow_identifier_reuse</property>
+                        </entry>
+                        <entry>
+                            false
+                        </entry>
+                        <entry>
+                            Guarantees proper validity audit strategy behavior when application reuses identifiers
+                            of deleted entities. Exactly one row with <literal>null</literal> end date exists
+                            for each identifier.
+                        </entry>
+                    </row>
                 </tbody>
             </tgroup>
         </table>

hibernate-envers/src/main/java/org/hibernate/envers/configuration/EnversSettings.java

@@ -125,4 +125,10 @@ public interface EnversSettings {
 	 * Name of column used for storing ordinal of the change in sets of embeddable elements. Defaults to {@literal SETORDINAL}.
 	 */
 	public static final String EMBEDDABLE_SET_ORDINAL_FIELD_NAME = "org.hibernate.envers.embeddable_set_ordinal_field_name";
+
+	/**
+	 * Guarantees proper validity audit strategy behavior when application reuses identifiers of deleted entities.
+	 * Exactly one row with {@code null} end date exists for each identifier.
+	 */
+	public static final String ALLOW_IDENTIFIER_REUSE = "org.hibernate.envers.allow_identifier_reuse";
 }

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/GlobalConfiguration.java

@@ -75,12 +75,15 @@ public class GlobalConfiguration {
 	// Use revision entity with native id generator
 	private final boolean useRevisionEntityWithNativeId;
 
+	// Support reused identifiers of previously deleted entities
+	private final boolean allowIdentifierReuse;
+
 	/*
 		 Which operator to use in correlated subqueries (when we want a property to be equal to the result of
 		 a correlated subquery, for example: e.p <operator> (select max(e2.p) where e2.p2 = e.p2 ...).
 		 Normally, this should be "=". However, HSQLDB has an issue related to that, so as a workaround,
 		 "in" is used. See {@link org.hibernate.envers.test.various.HsqlTest}.
-		 */
+	*/
 	private final String correlatedSubqueryOperator;
 
 	public GlobalConfiguration(Properties properties, ClassLoaderService classLoaderService) {
@@ -131,6 +134,10 @@ public class GlobalConfiguration {
 		else {
 			revisionListenerClass = null;
 		}
+
+		allowIdentifierReuse = ConfigurationHelper.getBoolean(
+				EnversSettings.ALLOW_IDENTIFIER_REUSE, properties, false
+		);
 	}
 
 	public boolean isGenerateRevisionsForCollections() {
@@ -184,4 +191,8 @@ public class GlobalConfiguration {
 	public boolean isUseRevisionEntityWithNativeId() {
 		return useRevisionEntityWithNativeId;
 	}
+
+	public boolean isAllowIdentifierReuse() {
+		return allowIdentifierReuse;
+	}
 }

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

@@ -100,10 +100,13 @@ public class ValidityAuditStrategy implements AuditStrategy {
 
 		// Update the end date of the previous row.
 		//
-		// The statement will no-op if an entity with a given identifier has been
+		// When application reuses identifiers of previously removed entities:
+		// The UPDATE 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.
+		// null end date exists for each identifier.
+		final boolean reuseEntityIdentifier = auditCfg.getGlobalCfg().isAllowIdentifierReuse();
+		if ( reuseEntityIdentifier || getRevisionType( auditCfg, data ) != RevisionType.ADD ) {
 			final Queryable productionEntityQueryable = getQueryable( entityName, sessionImplementor );
 			final Queryable rootProductionEntityQueryable = getQueryable(
 					productionEntityQueryable.getRootEntityName(),
@@ -242,11 +245,12 @@ public class ValidityAuditStrategy implements AuditStrategy {
 					}
 			);
 
-			if ( rowCount != 1 && getRevisionType(auditCfg, data) != RevisionType.ADD ) {
+			if ( rowCount != 1 && ( !reuseEntityIdentifier || ( 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) {

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

@@ -0,0 +1,73 @@
+package org.hibernate.envers.test.integration.strategy;
+
+import javax.persistence.EntityManager;
+import javax.persistence.EntityTransaction;
+import java.util.Arrays;
+import java.util.Map;
+
+import org.hibernate.envers.configuration.EnversSettings;
+import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase;
+import org.hibernate.envers.test.entities.IntNoAutoIdTestEntity;
+
+import org.junit.Test;
+
+import org.hibernate.testing.TestForIssue;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * Tests that reusing identifiers doesn't cause auditing misbehavior.
+ *
+ * @author adar
+ */
+@TestForIssue(jiraKey = "HHH-8280")
+public class IdentifierReuseTest extends BaseEnversJPAFunctionalTestCase {
+	@Override
+	protected void addConfigOptions(Map options) {
+		options.put( EnversSettings.ALLOW_IDENTIFIER_REUSE, "true" );
+	}
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] { IntNoAutoIdTestEntity.class };
+	}
+
+	@Test
+	public void testIdentifierReuse() {
+		final Integer reusedId = 1;
+
+		EntityManager entityManager = getEntityManager();
+		saveUpdateAndRemoveEntity( entityManager, reusedId );
+		saveUpdateAndRemoveEntity( entityManager, reusedId );
+		entityManager.close();
+
+		assertEquals(
+				Arrays.asList( 1, 2, 3, 4, 5, 6 ),
+				getAuditReader().getRevisions( IntNoAutoIdTestEntity.class, reusedId )
+		);
+	}
+
+	private void saveUpdateAndRemoveEntity(EntityManager entityManager, Integer id) {
+		EntityTransaction transaction = entityManager.getTransaction();
+
+		transaction.begin();
+		IntNoAutoIdTestEntity entity = new IntNoAutoIdTestEntity( 0, id );
+		entityManager.persist( entity );
+		assertEquals( id, entity.getId() );
+		transaction.commit();
+
+		transaction.begin();
+		entity = entityManager.find( IntNoAutoIdTestEntity.class, id );
+		entity.setNumVal( 1 );
+		entity = entityManager.merge( entity );
+		assertEquals( id, entity.getId() );
+		transaction.commit();
+
+		transaction.begin();
+		entity = entityManager.find( IntNoAutoIdTestEntity.class, id );
+		assertNotNull( entity );
+		entityManager.remove( entity );
+		transaction.commit();
+	}
+}

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

@@ -1,67 +0,0 @@
-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();
-    }
-  }
-}