From c57d39444552388a72b06c337fd15e0c4414b6ac Mon Sep 17 00:00:00 2001 From: CHAPEL Guillaume Date: Sat, 12 Mar 2022 11:42:10 +0100 Subject: [PATCH] HHH-15118 Fix duplicate ids with PooledOptimizer when sequence value is initialValue --- .../envers/EntityTypeChangeAuditTest.java | 3 +- .../id/enhanced/PooledOptimizer.java | 13 +++---- .../test/id/enhanced/OptimizerUnitTest.java | 5 +++ .../PooledForcedTableSequenceTest.java | 38 ++++++++++++------ .../enhanced/sequence/PooledSequenceTest.java | 39 +++++++++++++++---- .../idgen/enhanced/table/PooledTableTest.java | 38 +++++++++++++----- 6 files changed, 99 insertions(+), 37 deletions(-) diff --git a/documentation/src/test/java/org/hibernate/userguide/envers/EntityTypeChangeAuditTest.java b/documentation/src/test/java/org/hibernate/userguide/envers/EntityTypeChangeAuditTest.java index 88b1660435..47d89db279 100644 --- a/documentation/src/test/java/org/hibernate/userguide/envers/EntityTypeChangeAuditTest.java +++ b/documentation/src/test/java/org/hibernate/userguide/envers/EntityTypeChangeAuditTest.java @@ -110,8 +110,7 @@ public class EntityTypeChangeAuditTest extends BaseEntityManagerFunctionalTestCa AuditReaderFactory .get(entityManager) .getCrossTypeRevisionChangesReader() - // 52 is the next id on the DB due to allocationSize defaulting to 50 - .findEntityTypes(52) + .findEntityTypes(2) .iterator().next() .getFirst() ); diff --git a/hibernate-core/src/main/java/org/hibernate/id/enhanced/PooledOptimizer.java b/hibernate-core/src/main/java/org/hibernate/id/enhanced/PooledOptimizer.java index 74501d8cc9..55de6b3ca0 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/enhanced/PooledOptimizer.java +++ b/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 ) ) { - log.pooledOptimizerReportedInitialValue( generationState.value ); + if ( generationState.hiValue.lt( 1 ) ) { + log.pooledOptimizerReportedInitialValue( generationState.hiValue ); } // the call to obtain next-value just gave us the initialValue if ( ( initialValue == -1 - && generationState.value.lt( incrementSize ) ) - || generationState.value.eq( initialValue ) ) { - generationState.hiValue = callback.getNextValue(); + && generationState.hiValue.lt( incrementSize ) ) + || generationState.hiValue.eq( initialValue ) ) { + generationState.value = generationState.hiValue.copy(); } else { - generationState.hiValue = generationState.value; generationState.value = generationState.hiValue.copy().subtract( incrementSize - 1 ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/OptimizerUnitTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/OptimizerUnitTest.java index de86d12da1..f779c02990 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/OptimizerUnitTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/id/enhanced/OptimizerUnitTest.java @@ -235,6 +235,11 @@ public class OptimizerUnitTest { 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() ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/forcedtable/PooledForcedTableSequenceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/forcedtable/PooledForcedTableSequenceTest.java index a9b00684fa..f4433b7e33 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/forcedtable/PooledForcedTableSequenceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/forcedtable/PooledForcedTableSequenceTest.java @@ -19,8 +19,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.hamcrest.Matchers.instanceOf; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; @DomainModel( xmlMappings = "org/hibernate/orm/test/idgen/enhanced/forcedtable/Pooled.hbm.xml" @@ -28,6 +28,8 @@ import static org.junit.Assert.assertThat; @SessionFactory public class PooledForcedTableSequenceTest { + private static final long INITIAL_VALUE = 1; + @Test public void testNormalBoundary(SessionFactoryScope scope) { EntityPersister persister = scope.getSessionFactory() @@ -44,27 +46,41 @@ public class PooledForcedTableSequenceTest { scope.inTransaction( (s) -> { - for ( int i = 0; i <= increment; i++ ) { - final Entity entity = new Entity( "" + ( i + 1 ) ); + // 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 ); - long expectedId = i + 1; + expectedId = i + INITIAL_VALUE; assertEquals( expectedId, entity.getId().longValue() ); - // NOTE : initialization calls table twice 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" - final Entity entity = new Entity( "" + increment ); + expectedId++; + entity = new Entity( "" + expectedId ); s.save( entity ); - long expectedId = optimizer.getIncrementSize() + 2; + assertEquals( expectedId, entity.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() ); + assertEquals( increment * 2L + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); + assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() ); } ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/sequence/PooledSequenceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/sequence/PooledSequenceTest.java index e0b3ff46fb..5a2c0bac05 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/sequence/PooledSequenceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/sequence/PooledSequenceTest.java @@ -18,8 +18,8 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.Matchers.instanceOf; import static org.hibernate.id.IdentifierGeneratorHelper.BasicHolder; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; /** * @author Steve Ebersole @@ -28,6 +28,8 @@ import static org.junit.Assert.assertThat; @SessionFactory public class PooledSequenceTest { + private static final long INITIAL_VALUE = 1; + @Test public void testNormalBoundary(SessionFactoryScope scope) { final EntityPersister persister = scope.getSessionFactory() @@ -43,20 +45,41 @@ public class PooledSequenceTest { scope.inTransaction( (session) -> { - for ( int i = 0; i <= increment; i++ ) { - final Entity entity = new Entity( "" + ( i + 1 ) ); + // 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 ); + session.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 ) ); session.save( entity ); + + expectedId = i + INITIAL_VALUE; + assertEquals( expectedId, entity.getId().longValue() ); 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( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() ); assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); } + // now force a "clock over" - final Entity entity = new Entity( "" + increment ); + expectedId++; + entity = new Entity( "" + expectedId ); session.save( entity ); - 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() ); + + assertEquals( expectedId, entity.getId().longValue() ); + assertEquals( 3, generator.getDatabaseStructure().getTimesAccessed() ); + assertEquals( increment * 2L + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); + assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() ); } ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/table/PooledTableTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/table/PooledTableTest.java index 7a8a2b26ba..d5f7464ceb 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/table/PooledTableTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/idgen/enhanced/table/PooledTableTest.java @@ -18,12 +18,15 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.Matchers.instanceOf; import static org.hibernate.id.IdentifierGeneratorHelper.BasicHolder; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; @DomainModel( xmlMappings = "org/hibernate/orm/test/idgen/enhanced/table/Pooled.hbm.xml" ) @SessionFactory public class PooledTableTest { + + private static final long INITIAL_VALUE = 1; + @Test public void testNormalBoundary(SessionFactoryScope scope) { final EntityPersister persister = scope.getSessionFactory() @@ -39,24 +42,41 @@ public class PooledTableTest { scope.inTransaction( (s) -> { - for ( int i = 0; i <= increment; i++ ) { - final Entity entity = new Entity( "" + ( i + 1 ) ); + // 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 ); - // initialization calls seq twice + + expectedId = i + INITIAL_VALUE; + assertEquals( expectedId, entity.getId().longValue() ); assertEquals( 2, generator.getTableAccessCount() ); 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" - final Entity entity = new Entity( "" + increment ); + expectedId++; + entity = new Entity( "" + expectedId ); s.save( entity ); - // initialization (2) + clock over + assertEquals( expectedId, entity.getId().longValue() ); assertEquals( 3, generator.getTableAccessCount() ); - assertEquals( ( increment * 2 ) + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); - assertEquals( increment + 2, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() ); + assertEquals( increment * 2L + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() ); + assertEquals( expectedId, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() ); } ); }