HHH-9287 - Fix Pooled optimizer identifiers clash with INSERT rows calling sequence directly

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

@@ -86,12 +86,12 @@ public class PooledOptimizer extends AbstractOptimizer implements InitialValueAw
 			}
 			else {
 				generationState.hiValue = generationState.value;
-				generationState.value = generationState.hiValue.copy().subtract( incrementSize );
+				generationState.value = generationState.hiValue.copy().subtract( incrementSize - 1 );
 			}
 		}
-		else if ( ! generationState.hiValue.gt( generationState.value ) ) {
+		else if ( generationState.value.gt( generationState.hiValue ) ) {
 			generationState.hiValue = callback.getNextValue();
-			generationState.value = generationState.hiValue.copy().subtract( incrementSize );
+			generationState.value = generationState.hiValue.copy().subtract( incrementSize - 1 );
 		}
 
 		return generationState.value.makeValueThenIncrement();

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

@@ -85,7 +85,7 @@ public class OptimizerUnitTest extends BaseUnitTestCase {
 		// test historic sequence behavior, where the initial values start at 1...
 		SourceMock sequence = new SourceMock( 1, 10 );
 		Optimizer optimizer = buildPooledOptimizer( -1, 10 );
-		for ( int i = 1; i < 11; i++ ) {
+		for ( int i = 1; i <= 11; i++ ) {
 			next = ( Long ) optimizer.generate( sequence );
 			assertEquals( i, next.intValue() );
 		}
@@ -93,7 +93,7 @@ public class OptimizerUnitTest extends BaseUnitTestCase {
 		assertEquals( 11, sequence.getCurrentValue() );
 		// force a "clock over"
 		next = ( Long ) optimizer.generate( sequence );
-		assertEquals( 11, next.intValue() );
+		assertEquals( 12, next.intValue() );
 		assertEquals( 3, sequence.getTimesCalled() );
 		assertEquals( 21, sequence.getCurrentValue() );
 	}
@@ -110,12 +110,7 @@ public class OptimizerUnitTest extends BaseUnitTestCase {
 		assertEquals( 1001, sequence.getCurrentValue() );
 
 		Long next = (Long) optimizer.generate( sequence );
-		assertEquals( 1001, next.intValue() );
+		assertEquals( 1001 +1 , next.intValue() );
-		assertEquals( (5+1), sequence.getTimesCalled() );
-		assertEquals( (1001+3), sequence.getCurrentValue() );
-
-		next = (Long) optimizer.generate( sequence );
-		assertEquals( (1001+1), next.intValue() );
 		assertEquals( (5+1), sequence.getTimesCalled() );
 		assertEquals( (1001+3), sequence.getCurrentValue() );
 
@@ -124,9 +119,14 @@ public class OptimizerUnitTest extends BaseUnitTestCase {
 		assertEquals( (5+1), sequence.getTimesCalled() );
 		assertEquals( (1001+3), sequence.getCurrentValue() );
 
-		// force a "clock over"
 		next = (Long) optimizer.generate( sequence );
 		assertEquals( (1001+3), next.intValue() );
+		assertEquals( (5+1), sequence.getTimesCalled() );
+		assertEquals( (1001+3), sequence.getCurrentValue() );
+
+		// force a "clock over"
+		next = (Long) optimizer.generate( sequence );
+		assertEquals( (1001+4), next.intValue() );
 		assertEquals( (5+2), sequence.getTimesCalled() );
 		assertEquals( (1001+6), sequence.getCurrentValue() );
 	}
@@ -211,7 +211,7 @@ public class OptimizerUnitTest extends BaseUnitTestCase {
 		// app ends, and starts back up (we should "lose" only 2 and 3 as id values)
 		final Optimizer optimizer2 = buildPooledOptimizer( 1, 3 );
 		next = ( Long ) optimizer2.generate( sequence );
-		assertEquals( 4, next.intValue() );
+		assertEquals( 5, next.intValue() );
 		assertEquals( 3, sequence.getTimesCalled() );
 		assertEquals( 7, sequence.getCurrentValue() );
 	}

hibernate-core/src/test/java/org/hibernate/test/id/PooledHiLoSequenceIdentifierTest.java

@@ -0,0 +1,152 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) {DATE}, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.id;
+
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.List;
+import java.util.Properties;
+
+import org.jboss.logging.Logger;
+
+import org.hibernate.Session;
+import org.hibernate.Transaction;
+import org.hibernate.annotations.GenericGenerator;
+import org.hibernate.cfg.Configuration;
+import org.hibernate.jdbc.Work;
+
+import org.junit.Test;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Andrea Boriero
+ */
+@TestForIssue(jiraKey = "HHH-9287")
+public class PooledHiLoSequenceIdentifierTest extends BaseCoreFunctionalTestCase {
+	private static final Logger log = Logger.getLogger( PooledHiLoSequenceIdentifierTest.class );
+
+	@Test
+	public void testSequenceIdentifierGenerator() {
+		Session s = null;
+		Transaction tx = null;
+		try {
+			s = openSession();
+			tx = s.beginTransaction();
+
+			for ( int i = 0; i < 5; i++ ) {
+				s.persist( new SequenceIdentifier() );
+			}
+			s.flush();
+
+			assertEquals( 5, countInsertedRows( s ) );
+
+			insertNewRow( s );
+			insertNewRow( s );
+
+			assertEquals( 7, countInsertedRows( s ) );
+
+			List<Number> ids = s.createSQLQuery( "SELECT id FROM sequenceIdentifier" ).list();
+			for ( Number id : ids ) {
+				log.debug( "Found id: " + id );
+			}
+
+			for ( int i = 0; i < 3; i++ ) {
+				s.persist( new SequenceIdentifier() );
+			}
+			s.flush();
+
+			assertEquals( 10, countInsertedRows( s ) );
+		}
+		finally {
+			if ( tx != null ) {
+				tx.rollback();
+			}
+
+			s.close();
+		}
+	}
+
+	private int countInsertedRows(Session s) {
+		return ((Number) s.createSQLQuery( "SELECT COUNT(*) FROM sequenceIdentifier" )
+				.uniqueResult()).intValue();
+	}
+
+	@Override
+	protected Class[] getAnnotatedClasses() {
+		return new Class[] {SequenceIdentifier.class};
+	}
+
+	@Override
+	protected void configure(Configuration configuration) {
+		Properties properties = configuration.getProperties();
+		properties.put( "hibernate.id.new_generator_mappings", "true" );
+	}
+
+	@Entity(name = "sequenceIdentifier")
+	public static class SequenceIdentifier {
+
+		@Id
+		@GenericGenerator(name = "sampleGenerator", strategy = "enhanced-sequence",
+				parameters = {
+						@org.hibernate.annotations.Parameter(name = "optimizer", value = "pooled"),
+						@org.hibernate.annotations.Parameter(name = "initial_value", value = "1"),
+						@org.hibernate.annotations.Parameter(name = "increment_size", value = "2")
+				}
+		)
+		@GeneratedValue(strategy = GenerationType.TABLE, generator = "sampleGenerator")
+		private Long id;
+	}
+
+	private void insertNewRow(Session session) {
+		session.doWork(
+				new Work() {
+					@Override
+					public void execute(Connection connection) throws SQLException {
+						Statement statement = null;
+						try {
+							statement = connection.createStatement();
+							statement.executeUpdate(
+									"INSERT INTO sequenceIdentifier VALUES (NEXT VALUE FOR hibernate_sequence)"
+							);
+						}
+						finally {
+							if ( statement != null ) {
+								statement.close();
+							}
+						}
+					}
+				}
+		);
+	}
+}
+

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

@@ -46,10 +46,10 @@ public class PooledForcedTableSequenceTest extends BaseCoreFunctionalTestCase {
 		PooledOptimizer optimizer = (PooledOptimizer) generator.getOptimizer();
 
 		int increment = optimizer.getIncrementSize();
-		Entity[] entities = new Entity[ increment + 1 ];
+		Entity[] entities = new Entity[ increment + 2 ];
 		Session s = openSession();
 		s.beginTransaction();
-		for ( int i = 0; i < increment; i++ ) {
+		for ( int i = 0; i <= increment; i++ ) {
 			entities[i] = new Entity( "" + ( i + 1 ) );
 			s.save( entities[i] );
 			long expectedId = i + 1;
@@ -61,14 +61,14 @@ public class PooledForcedTableSequenceTest extends BaseCoreFunctionalTestCase {
 			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
 		}
 		// now force a "clock over"
-		entities[ increment ] = new Entity( "" + increment );
+		entities[increment + 1] = new Entity( "" + increment );
-		s.save( entities[ increment ] );
+		s.save( entities[increment + 1] );
-		long expectedId = optimizer.getIncrementSize() + 1;
+		long expectedId =  optimizer.getIncrementSize() + 2;
-		assertEquals( expectedId, entities[ optimizer.getIncrementSize() ].getId().longValue() );
+		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 + 1, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+		assertEquals( increment + 2, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
 		s.getTransaction().commit();
 
 		s.beginTransaction();

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

@@ -36,10 +36,10 @@ public class PooledSequenceTest extends BaseCoreFunctionalTestCase {
 		PooledOptimizer optimizer = (PooledOptimizer) generator.getOptimizer();
 
 		int increment = optimizer.getIncrementSize();
-		Entity[] entities = new Entity[ increment + 1 ];
+		Entity[] entities = new Entity[ increment + 2 ];
 		Session s = openSession();
 		s.beginTransaction();
-		for ( int i = 0; i < increment; i++ ) {
+		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
@@ -48,11 +48,11 @@ public class PooledSequenceTest extends BaseCoreFunctionalTestCase {
 			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
 		}
 		// now force a "clock over"
-		entities[ increment ] = new Entity( "" + increment );
+		entities[ increment + 1 ] = new Entity( "" + increment );
-		s.save( entities[ 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 + 1, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+		assertEquals( increment + 2, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
 		s.getTransaction().commit();
 
 		s.beginTransaction();

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

@@ -36,10 +36,10 @@ public class PooledTableTest extends BaseCoreFunctionalTestCase {
 		PooledOptimizer optimizer = (PooledOptimizer) generator.getOptimizer();
 
 		int increment = optimizer.getIncrementSize();
-		Entity[] entities = new Entity[ increment + 1 ];
+		Entity[] entities = new Entity[ increment + 2 ];
 		Session s = openSession();
 		s.beginTransaction();
-		for ( int i = 0; i < increment; i++ ) {
+		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
@@ -48,11 +48,11 @@ public class PooledTableTest extends BaseCoreFunctionalTestCase {
 			assertEquals( increment + 1, ( (BasicHolder) optimizer.getLastSourceValue() ).getActualLongValue() );
 		}
 		// now force a "clock over"
-		entities[ increment ] = new Entity( "" + increment );
+		entities[ increment + 1 ] = new Entity( "" + increment );
-		s.save( entities[ 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 + 1, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
+		assertEquals( increment + 2, ( (BasicHolder) optimizer.getLastValue() ).getActualLongValue() );
 		s.getTransaction().commit();
 
 		s.beginTransaction();