HHH-13500 Fix audit strategy sub-query return incorrect results

In order to maintain backward compatibility with long-standing behavior,
this introduces a new configuration option which can be toggled to have
AuditReader#find implementations adhere to returning an exact match on
revision-number rather than one which is less-than or equal-to the
provided argument.

So a new configuration option org.hibernate.envers.find_by_revision_exact_match
provides users with the ability to be able to force this new behavior
while allowing legacy behavior to remain the default.

documentation/src/main/asciidoc/userguide/chapters/envers/Envers.adoc

@@ -298,6 +298,15 @@ Guarantees proper validity audit strategy behavior when application reuses ident
 `*org.hibernate.envers.original_id_prop_name*` (default: `originalId` )::
 Specifies the composite-id key property name used by the audit table mappings.
 
+`*org.hibernate.envers.find_by_revision_exact_match*` (default: `false` )::
+Specifies whether or not `AuditReader#find` methods which accept a revision-number argument are to find results based on fuzzy-match or exact-match behavior.
++
+The old (legacy) behavior has always been to perform a fuzzy-match where these methods would return a match if any revision existed for the primary-key with a revision-number less-than or equal-to the revision method argument.
+This behavior is great when you want to find the snapshot of a non-related entity based on another entity's revision number.
++
+The new (optional) behavior when this option is enabled forces the query to perform an exact-match instead.
+In order for these methods to return a non-`null` value, a revision entry must exist for the entity with the specified primary key and revision number; otherwise the result will be `null`.
+
 [IMPORTANT]
 ====
 The following configuration options have been added recently and should be regarded as experimental:
@@ -306,6 +315,7 @@ The following configuration options have been added recently and should be regar
 .  `org.hibernate.envers.using_modified_flag`
 .  `org.hibernate.envers.modified_flag_suffix`
 .  `org.hibernate.envers.original_id_prop_name`
+.  `org.hibernate.envers.find_by_revision_exact_match`
 ====
 
 [[envers-additional-mappings]]

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

@@ -119,4 +119,18 @@ public interface EnversSettings {
 	 * Exactly one row with {@code null} end date exists for each identifier.
 	 */
 	String ALLOW_IDENTIFIER_REUSE = "org.hibernate.envers.allow_identifier_reuse";
+
+	/**
+	 * Forces {@code AuditReader#find} implementations that accept a revision-number argument to perform an exact
+	 * match against the supplied revision number rather than potentially returning hits that are less-than or
+	 * equal-to the supplied revision number.
+	 *
+	 * This option is meant to maintain backward compatibility while attempting to correct a bug in behavior without
+	 * impacting existing users who may use the current behavior.
+	 * 
+	 * Defaults to {@literal false}.
+	 *
+	 * @since 5.4.4
+	 */
+	String FIND_BY_REVISION_EXACT_MATCH = "org.hibernate.envers.find_by_revision_exact_match";
 }

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

@@ -66,6 +66,9 @@ public class GlobalConfiguration {
 	// Support reused identifiers of previously deleted entities
 	private final boolean allowIdentifierReuse;
 
+	// Forces audit reader find by revision methods to perform exact match
+	private final boolean findByRevisionExactMatch;
+
 	/*
 		 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 ...).
@@ -156,6 +159,10 @@ public class GlobalConfiguration {
 		allowIdentifierReuse = ConfigurationHelper.getBoolean(
 				EnversSettings.ALLOW_IDENTIFIER_REUSE, properties, false
 		);
+
+		findByRevisionExactMatch = ConfigurationHelper.getBoolean(
+				EnversSettings.FIND_BY_REVISION_EXACT_MATCH, properties, false
+		);
 	}
 
 	public EnversService getEnversService() {
@@ -221,4 +228,8 @@ public class GlobalConfiguration {
 	public boolean isAllowIdentifierReuse() {
 		return allowIdentifierReuse;
 	}
+
+	public boolean isAuditReaderFindAtRevisionExactMatch() {
+		return findByRevisionExactMatch;
+	}
 }

hibernate-envers/src/main/java/org/hibernate/envers/query/internal/impl/EntitiesAtRevisionQuery.java

@@ -97,6 +97,11 @@ public class EntitiesAtRevisionQuery extends AbstractAuditQuery {
 				true
 		);
 
+		if ( enversService.getGlobalConfiguration().isAuditReaderFindAtRevisionExactMatch() ) {
+			// When EnversSettings#FIND_BY_REVISION_EXACT_MATCH is true, this forces this condition
+			qb.getRootParameters().addWhereWithNamedParam( revisionPropertyPath, "=", REVISION_PARAMETER );
+		}
+
 		if ( !includeDeletions ) {
 			// e.revision_type != DEL
 			qb.getRootParameters().addWhereWithParam( verEntCfg.getRevisionTypePropName(), "<>", RevisionType.DEL );

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/auditReader/FindByRevisionExactMatchOptionTest.java

@@ -0,0 +1,98 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.envers.test.integration.auditReader;
+
+import java.util.Arrays;
+import java.util.Map;
+
+import org.hibernate.envers.AuditReader;
+import org.hibernate.envers.configuration.EnversSettings;
+import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase;
+import org.hibernate.envers.test.Priority;
+import org.hibernate.envers.test.entities.IntNoAutoIdTestEntity;
+import org.junit.Test;
+
+import org.hibernate.testing.TestForIssue;
+
+import static junit.framework.TestCase.assertNull;
+import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * A test which verifies the behavior of the various {@link AuditReader} find implementations when the
+ * configuration option {@link EnversSettings#FIND_BY_REVISION_EXACT_MATCH} is enabled.
+ *
+ * @author Chris Cranford
+ */
+@TestForIssue(jiraKey = "HHH-13500")
+public class FindByRevisionExactMatchOptionTest extends BaseEnversJPAFunctionalTestCase {
+	@Override
+	protected void addConfigOptions(Map options) {
+		super.addConfigOptions( options );
+
+		options.put( EnversSettings.FIND_BY_REVISION_EXACT_MATCH, "true" );
+	}
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] { IntNoAutoIdTestEntity.class };
+	}
+
+	@Priority(10)
+	@Test
+	public void initData() {
+		// Insert entity with id=1, numVal=1, revision 1
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final IntNoAutoIdTestEntity entity = new IntNoAutoIdTestEntity( 1, 1 );
+			entityManager.persist( entity );
+		} );
+
+		// Update entity with id=1, setting numVal=11, revision 2
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final IntNoAutoIdTestEntity entity = entityManager.find( IntNoAutoIdTestEntity.class, 1 );
+			entity.setNumVal( 11 );
+			entityManager.merge( entity );
+		} );
+
+		// Insert entity with id=2, numVal=2, revision 3
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final IntNoAutoIdTestEntity entity = new IntNoAutoIdTestEntity( 2, 2 );
+			entityManager.persist( entity );
+		} );
+
+		// Update entity with id=2, setting numVal=22, revision 4
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final IntNoAutoIdTestEntity entity = entityManager.find( IntNoAutoIdTestEntity.class, 2 );
+			entity.setNumVal( 22 );
+			entityManager.merge( entity );
+		} );
+	}
+
+	@Test
+	public void testRevisionCounts() {
+		assertEquals( Arrays.asList( 1, 2 ), getAuditReader().getRevisions( IntNoAutoIdTestEntity.class, 1 ) );
+		assertEquals( Arrays.asList( 3, 4 ), getAuditReader().getRevisions( IntNoAutoIdTestEntity.class, 2 ) );
+	}
+
+	@Test
+	public void testFindEntityId1() {
+		final AuditReader auditReader = getAuditReader();
+		assertEquals( new IntNoAutoIdTestEntity( 1, 1 ), auditReader.find( IntNoAutoIdTestEntity.class, 1, 1 ) );
+		assertEquals( new IntNoAutoIdTestEntity( 11, 1 ), auditReader.find( IntNoAutoIdTestEntity.class, 1, 2 ) );
+		assertNull( auditReader.find( IntNoAutoIdTestEntity.class, 1, 3 ) );
+		assertNull( auditReader.find( IntNoAutoIdTestEntity.class, 1, 4 ) );
+	}
+
+	@Test
+	public void testFindEntityId2() {
+		final AuditReader auditReader = getAuditReader();
+		assertNull( auditReader.find( IntNoAutoIdTestEntity.class, 2, 1 ) );
+		assertNull( auditReader.find( IntNoAutoIdTestEntity.class, 2, 2 ) );
+		assertEquals( new IntNoAutoIdTestEntity( 2, 2 ), auditReader.find( IntNoAutoIdTestEntity.class, 2, 3 ) );
+		assertEquals( new IntNoAutoIdTestEntity( 22, 2 ), auditReader.find( IntNoAutoIdTestEntity.class, 2, 4 ) );
+	}
+}