HHH-15118 Fix duplicate ids with PooledOptimizer when sequence value is initialValue

documentation/src/test/java/org/hibernate/userguide/envers/EntityTypeChangeAuditTest.java

@@ -9,7 +9,6 @@ package org.hibernate.userguide.envers;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import javax.persistence.Column;

hibernate-core/src/main/java/org/hibernate/id/enhanced/PooledOptimizer.java

@@ -70,22 +70,21 @@ public class PooledOptimizer extends AbstractOptimizer implements InitialValueAw
 		final GenerationState generationState = locateGenerationState( callback.getTenantIdentifier() );
 
 		if ( generationState.hiValue == null ) {
-			generationState.value = callback.getNextValue();
+			generationState.hiValue = callback.getNextValue();
 			// unfortunately not really safe to normalize this
 			// to 1 as an initial value like we do for the others
 			// because we would not be able to control this if
 			// we are using a sequence...
-			if ( generationState.value.lt( 1 ) ) {
+			if ( generationState.hiValue.lt( 1 ) ) {
-				log.pooledOptimizerReportedInitialValue( generationState.value );
+				log.pooledOptimizerReportedInitialValue( generationState.hiValue );
 			}
 			// the call to obtain next-value just gave us the initialValue
 			if ( ( initialValue == -1
-					&& generationState.value.lt( incrementSize ) )
+					&& generationState.hiValue.lt( incrementSize ) )
-					|| generationState.value.eq( initialValue ) ) {
+					|| generationState.hiValue.eq( initialValue ) ) {
-				generationState.hiValue = callback.getNextValue();
+				generationState.value = generationState.hiValue.copy();
 			}
 			else {
-				generationState.hiValue = generationState.value;
 				generationState.value = generationState.hiValue.copy().subtract( incrementSize - 1 );
 			}
 		}

hibernate-core/src/test/java/org/hibernate/id/enhanced/OptimizerUnitTest.java

@@ -226,10 +226,15 @@ public class OptimizerUnitTest extends BaseUnitTestCase {
 
 		Long next = ( Long ) optimizer.generate( sequence );
 		assertEquals( 1, next.intValue() );
+		assertEquals( 1, sequence.getTimesCalled() );
+		assertEquals( 1, sequence.getCurrentValue() );
+
+		next = ( Long ) optimizer.generate( sequence );
+		assertEquals( 2, next.intValue() );
 		assertEquals( 2, sequence.getTimesCalled() );
 		assertEquals( 4, sequence.getCurrentValue() );
 
-		// app ends, and starts back up (we should "lose" only 2 and 3 as id values)
+		// app ends, and starts back up (we should "lose" only 3 and 4 as id values)
 		final Optimizer optimizer2 = buildPooledOptimizer( 1, 3 );
 		next = ( Long ) optimizer2.generate( sequence );
 		assertEquals( 5, next.intValue() );

hibernate-core/src/test/java/org/hibernate/test/idgen/enhanced/forcedtable/PooledForcedTableSequenceTest.java

@@ -7,6 +7,7 @@
 package org.hibernate.test.idgen.enhanced.forcedtable;
 
 import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
 
 import org.hibernate.Session;
 import org.hibernate.id.IdentifierGeneratorHelper.BasicHolder;
@@ -15,6 +16,7 @@ import org.hibernate.id.enhanced.SequenceStyleGenerator;
 import org.hibernate.id.enhanced.TableStructure;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.transaction.TransactionUtil;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -23,6 +25,9 @@ import static org.junit.Assert.assertTrue;
  * @author Steve Ebersole
  */
 public class PooledForcedTableSequenceTest extends BaseCoreFunctionalTestCase {
+
+	private static final long INITIAL_VALUE = 1;
+
 	public String[] getMappings() {
 		return new String[] { "idgen/enhanced/forcedtable/Pooled.hbm.xml" };
 	}
@@ -46,37 +51,47 @@ public class PooledForcedTableSequenceTest extends BaseCoreFunctionalTestCase {
 		PooledOptimizer optimizer = (PooledOptimizer) generator.getOptimizer();
 
 		int increment = optimizer.getIncrementSize();
-		Entity[] entities = new Entity[ increment + 2 ];
+
-		Session s = openSession();
+		TransactionUtil.doInHibernate(
-		s.beginTransaction();
+				this::sessionFactory,
-		for ( int i = 0; i <= increment; i++ ) {
+				s -> {
-			entities[i] = new Entity( "" + ( i + 1 ) );
+					// The value that we get from the callback is the high value (PooledOptimizer by default)
-			s.save( entities[i] );
+					// When first increment is initialValue, we can only generate one id from it -> id 1
-			long expectedId = i + 1;
+					Entity entity = new Entity( "" + INITIAL_VALUE );
-			assertEquals( expectedId, entities[i].getId().longValue() );
+					s.save( entity );
-			// NOTE : initialization calls table twice
+
+					long expectedId = INITIAL_VALUE;
+					assertEquals( expectedId, entity.getId().longValue() );
+					assertEquals( 1, generator.getDatabaseStructure().getTimesAccessed() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+
+					// now start a full range of values, callback give us hiValue 11
+					// id : 2,3,4...,11
+					for ( int i = 1; i <= increment; i++ ) {
+						entity = new Entity( "" + ( i + INITIAL_VALUE ) );
+						s.save( entity );
+
+						expectedId = i + INITIAL_VALUE;
+						assertEquals( expectedId, entity.getId().longValue() );
 						assertEquals( 2, generator.getDatabaseStructure().getTimesAccessed() );
 						assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
-			assertEquals( i + 1, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+						assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
-			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
 					}
-		// now force a "clock over"
-		entities[increment + 1] = new Entity( "" + increment );
-		s.save( entities[increment + 1] );
-		long expectedId =  optimizer.getIncrementSize() + 2;
-		assertEquals( expectedId, entities[ increment + 1 ].getId().longValue() );
-		// initialization (2) + clock over
-		assertEquals( 3, generator.getDatabaseStructure().getTimesAccessed() );
-		assertEquals( ( increment * 2 ) + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
-		assertEquals( increment + 2, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
-		s.getTransaction().commit();
 
-		s.beginTransaction();
+					// now force a "clock over"
-		for ( int i = 0; i < entities.length; i++ ) {
+					expectedId++;
-			assertEquals( i + 1, entities[i].getId().intValue() );
+					entity = new Entity( "" + expectedId  );
-			s.delete( entities[i] );
+					s.save( entity );
+
+					assertEquals( expectedId, entity.getId().longValue() );
+					assertEquals( 3, generator.getDatabaseStructure().getTimesAccessed() );
+					assertEquals( increment * 2L + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+					assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+
+					s.createQuery( "delete Entity" ).executeUpdate();
 				}
-		s.getTransaction().commit();
+		);
-		s.close();
 	}
 }

hibernate-core/src/test/java/org/hibernate/test/idgen/enhanced/sequence/PooledSequenceTest.java

@@ -13,6 +13,7 @@ import org.hibernate.id.enhanced.PooledOptimizer;
 import org.hibernate.id.enhanced.SequenceStyleGenerator;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.transaction.TransactionUtil;
 
 import static org.hibernate.id.IdentifierGeneratorHelper.BasicHolder;
 import static org.hibernate.testing.junit4.ExtraAssertions.assertClassAssignability;
@@ -22,6 +23,9 @@ import static org.junit.Assert.assertEquals;
  * @author Steve Ebersole
  */
 public class PooledSequenceTest extends BaseCoreFunctionalTestCase {
+
+	private static final long INITIAL_VALUE = 1;
+
 	@Override
 	public String[] getMappings() {
 		return new String[] { "idgen/enhanced/sequence/Pooled.hbm.xml" };
@@ -36,31 +40,47 @@ public class PooledSequenceTest extends BaseCoreFunctionalTestCase {
 		PooledOptimizer optimizer = (PooledOptimizer) generator.getOptimizer();
 
 		int increment = optimizer.getIncrementSize();
-		Entity[] entities = new Entity[ increment + 2 ];
-		Session s = openSession();
-		s.beginTransaction();
-		for ( int i = 0; i <= increment; i++ ) {
-			entities[i] = new Entity( "" + ( i + 1 ) );
-			s.save( entities[i] );
-			assertEquals( 2, generator.getDatabaseStructure().getTimesAccessed() ); // initialization calls seq twice
-			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); // initialization calls seq twice
-			assertEquals( i + 1, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
-			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
-		}
-		// now force a "clock over"
-		entities[ increment + 1 ] = new Entity( "" + increment );
-		s.save( entities[ increment + 1 ] );
-		assertEquals( 3, generator.getDatabaseStructure().getTimesAccessed() ); // initialization (2) + clock over
-		assertEquals( ( increment * 2 ) + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); // initialization (2) + clock over
-		assertEquals( increment + 2, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
-		s.getTransaction().commit();
 
-		s.beginTransaction();
+		TransactionUtil.doInHibernate(
-		for ( int i = 0; i < entities.length; i++ ) {
+				this::sessionFactory,
-			assertEquals( i + 1, entities[i].getId().intValue() );
+				s -> {
-			s.delete( entities[i] );
+					// The value that we get from the callback is the high value (PooledOptimizer by default)
+					// When first increment is initialValue, we can only generate one id from it -> id 1
+					Entity entity = new Entity( "" + INITIAL_VALUE );
+					s.save( entity );
+
+					long expectedId = INITIAL_VALUE;
+					assertEquals( expectedId, entity.getId().longValue() );
+					assertEquals( 1, generator.getDatabaseStructure().getTimesAccessed() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+
+					// now start a full range of values, callback give us hiValue 11
+					// id : 2,3,4...,11
+					for ( int i = 1; i <= increment; i++ ) {
+						entity = new Entity( "" + ( i + INITIAL_VALUE ) );
+						s.save( entity );
+
+						expectedId = i + INITIAL_VALUE;
+						assertEquals( expectedId, entity.getId().longValue() );
+						assertEquals( 2, generator.getDatabaseStructure().getTimesAccessed() );
+						assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+						assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
 					}
-		s.getTransaction().commit();
+
-		s.close();
+					// now force a "clock over"
+					expectedId++;
+					entity = new Entity( "" + expectedId  );
+					s.save( entity );
+
+					assertEquals( expectedId, entity.getId().longValue() );
+					assertEquals( 3, generator.getDatabaseStructure().getTimesAccessed() );
+					assertEquals( increment * 2L + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+					assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+
+					s.createQuery( "delete Entity" ).executeUpdate();
+				}
+		);
 	}
 }

hibernate-core/src/test/java/org/hibernate/test/idgen/enhanced/table/PooledTableTest.java

@@ -13,6 +13,7 @@ import org.hibernate.id.enhanced.PooledOptimizer;
 import org.hibernate.id.enhanced.TableGenerator;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.transaction.TransactionUtil;
 
 import static org.hibernate.id.IdentifierGeneratorHelper.BasicHolder;
 import static org.hibernate.testing.junit4.ExtraAssertions.assertClassAssignability;
@@ -22,6 +23,9 @@ import static org.junit.Assert.assertEquals;
  * @author Steve Ebersole
  */
 public class PooledTableTest extends BaseCoreFunctionalTestCase {
+
+	private static final long INITIAL_VALUE = 1;
+
 	@Override
 	public String[] getMappings() {
 		return new String[] { "idgen/enhanced/table/Pooled.hbm.xml" };
@@ -36,31 +40,47 @@ public class PooledTableTest extends BaseCoreFunctionalTestCase {
 		PooledOptimizer optimizer = (PooledOptimizer) generator.getOptimizer();
 
 		int increment = optimizer.getIncrementSize();
-		Entity[] entities = new Entity[ increment + 2 ];
-		Session s = openSession();
-		s.beginTransaction();
-		for ( int i = 0; i <= increment; i++ ) {
-			entities[i] = new Entity( "" + ( i + 1 ) );
-			s.save( entities[i] );
-			assertEquals( 2, generator.getTableAccessCount() ); // initialization calls seq twice
-			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); // initialization calls seq twice
-			assertEquals( i + 1, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
-			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
-		}
-		// now force a "clock over"
-		entities[ increment + 1 ] = new Entity( "" + increment );
-		s.save( entities[ increment + 1 ] );
-		assertEquals( 3, generator.getTableAccessCount() ); // initialization (2) + clock over
-		assertEquals( ( increment * 2 ) + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); // initialization (2) + clock over
-		assertEquals( increment + 2, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
-		s.getTransaction().commit();
 
-		s.beginTransaction();
+		TransactionUtil.doInHibernate(
-		for ( int i = 0; i < entities.length; i++ ) {
+				this::sessionFactory,
-			assertEquals( i + 1, entities[i].getId().intValue() );
+				s -> {
-			s.delete( entities[i] );
+					// The value that we get from the callback is the high value (PooledOptimizer by default)
+					// When first increment is initialValue, we can only generate one id from it -> id 1
+					Entity entity = new Entity( "" + INITIAL_VALUE );
+					s.save( entity );
+
+					long expectedId = INITIAL_VALUE;
+					assertEquals( expectedId, entity.getId().longValue() );
+					assertEquals( 1, generator.getTableAccessCount() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+					assertEquals( INITIAL_VALUE, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+
+					// now start a full range of values, callback give us hiValue 11
+					// id : 2,3,4...,11
+					for ( int i = 1; i <= increment; i++ ) {
+						entity = new Entity( "" + ( i + INITIAL_VALUE ) );
+						s.save( entity );
+
+						expectedId = i + INITIAL_VALUE;
+						assertEquals( expectedId, entity.getId().longValue() );
+						assertEquals( 2, generator.getTableAccessCount() );
+						assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+						assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
 					}
-		s.getTransaction().commit();
+
-		s.close();
+					// now force a "clock over"
+					expectedId++;
+					entity = new Entity( "" + expectedId  );
+					s.save( entity );
+
+					assertEquals( expectedId, entity.getId().longValue() );
+					assertEquals( 3, generator.getTableAccessCount() );
+					assertEquals( increment * 2L + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
+					assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+
+					s.createQuery( "delete Entity" ).executeUpdate();
+				}
+		);
 	}
 }