Author: steve.ebersole(a)jboss.com Date: 2010-05-11 15:27:20 -0400 (Tue, 11 May 2010) New Revision: 19470 Added: core/trunk/core/src/test/java/org/hibernate/id/SequenceHiLoGeneratorTest.java Modified: core/trunk/core/src/main/java/org/hibernate/id/MultipleHiLoPerTableGenerator.java core/trunk/core/src/main/java/org/hibernate/id/SequenceGenerator.java core/trunk/core/src/main/java/org/hibernate/id/SequenceHiLoGenerator.java core/trunk/core/src/main/java/org/hibernate/id/enhanced/OptimizerFactory.java core/trunk/parent/pom.xml Log: HHH-5042 - TableGenerator does not increment hibernate_sequences.next_hi_value anymore after having exhausted the current lo-range Modified: core/trunk/core/src/main/java/org/hibernate/id/MultipleHiLoPerTableGenerator.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/id/MultipleHiLoPerTableGenerator.java 2010-05-11 19:24:54 UTC (rev 19469) +++ core/trunk/core/src/main/java/org/hibernate/id/MultipleHiLoPerTableGenerator.java 2010-05-11 19:27:20 UTC (rev 19470) @@ -37,6 +37,8 @@ import org.hibernate.LockMode; import org.hibernate.MappingException; import org.hibernate.cfg.ObjectNameNormalizer; +import org.hibernate.id.enhanced.AccessCallback; +import org.hibernate.id.enhanced.OptimizerFactory; import org.hibernate.jdbc.util.FormatStyle; import org.hibernate.dialect.Dialect; import org.hibernate.engine.SessionImplementor; @@ -104,8 +106,7 @@ public static final String MAX_LO = "max_lo"; private int maxLo; - private int lo; - private IntegralDataTypeHolder value; + private OptimizerFactory.LegacyHiLoAlgorithmOptimizer hiloOptimizer; private Class returnClass; private int keySize; @@ -149,19 +150,15 @@ IntegralDataTypeHolder value = IdentifierGeneratorHelper.getIntegralDataTypeHolder( returnClass ); int rows; do { - // The loop ensures atomicity of the - // select + update even for no transaction - // or read committed isolation level - - //sql = query; - SQL_STATEMENT_LOGGER.logStatement( sql, FormatStyle.BASIC ); - PreparedStatement qps = conn.prepareStatement(query); + SQL_STATEMENT_LOGGER.logStatement( query, FormatStyle.BASIC ); + PreparedStatement qps = conn.prepareStatement( query ); PreparedStatement ips = null; try { ResultSet rs = qps.executeQuery(); boolean isInitialized = rs.next(); if ( !isInitialized ) { value.initialize( 0 ); + SQL_STATEMENT_LOGGER.logStatement( insert, FormatStyle.BASIC ); ips = conn.prepareStatement( insert ); value.bind( ips, 1 ); ips.execute(); @@ -182,7 +179,8 @@ qps.close(); } - PreparedStatement ups = conn.prepareStatement(update); + SQL_STATEMENT_LOGGER.logStatement( update, FormatStyle.BASIC ); + PreparedStatement ups = conn.prepareStatement( update ); try { value.copy().increment().bind( ups, 1 ); value.bind( ups, 2 ); @@ -195,12 +193,12 @@ finally { ups.close(); } - } - while (rows==0); + } while ( rows==0 ); + return value; } - public synchronized Serializable generate(SessionImplementor session, Object obj) + public synchronized Serializable generate(final SessionImplementor session, Object obj) throws HibernateException { // maxLo < 1 indicates a hilo generator with no hilo :? if ( maxLo < 1 ) { @@ -212,15 +210,13 @@ return value.makeValue(); } - if ( lo > maxLo ) { - IntegralDataTypeHolder hiVal = (IntegralDataTypeHolder) doWorkInNewTransaction( session ); - lo = ( hiVal.eq( 0 ) ) ? 1 : 0; - value = hiVal.copy().multiplyBy( maxLo+1 ).add( lo ); - if ( log.isDebugEnabled() ) { - log.debug("new hi value: " + hiVal); - } - } - return value.makeValueThenIncrement(); + return hiloOptimizer.generate( + new AccessCallback() { + public IntegralDataTypeHolder getNextValue() { + return (IntegralDataTypeHolder) doWorkInNewTransaction( session ); + } + } + ); } public void configure(Type type, Properties params, Dialect dialect) throws MappingException { @@ -281,7 +277,8 @@ //hilo config maxLo = PropertiesHelper.getInt(MAX_LO, params, Short.MAX_VALUE); - lo = maxLo + 1; // so we "clock over" on the first invocation returnClass = type.getReturnedClass(); + + hiloOptimizer = new OptimizerFactory.LegacyHiLoAlgorithmOptimizer( returnClass, maxLo ); } } Modified: core/trunk/core/src/main/java/org/hibernate/id/SequenceGenerator.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/id/SequenceGenerator.java 2010-05-11 19:24:54 UTC (rev 19469) +++ core/trunk/core/src/main/java/org/hibernate/id/SequenceGenerator.java 2010-05-11 19:27:20 UTC (rev 19470) @@ -72,6 +72,10 @@ private Type identifierType; private String sql; + protected Type getIdentifierType() { + return identifierType; + } + public void configure(Type type, Properties params, Dialect dialect) throws MappingException { ObjectNameNormalizer normalizer = ( ObjectNameNormalizer ) params.get( IDENTIFIER_NORMALIZER ); sequenceName = normalizer.normalizeIdentifierQuoting( @@ -103,7 +107,7 @@ protected IntegralDataTypeHolder generateHolder(SessionImplementor session) { try { - PreparedStatement st = session.getBatcher().prepareSelectStatement(sql); + PreparedStatement st = session.getBatcher().prepareSelectStatement( sql ); try { ResultSet rs = st.executeQuery(); try { Modified: core/trunk/core/src/main/java/org/hibernate/id/SequenceHiLoGenerator.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/id/SequenceHiLoGenerator.java 2010-05-11 19:24:54 UTC (rev 19469) +++ core/trunk/core/src/main/java/org/hibernate/id/SequenceHiLoGenerator.java 2010-05-11 19:27:20 UTC (rev 19470) @@ -26,11 +26,11 @@ import java.io.Serializable; import java.util.Properties; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.hibernate.MappingException; import org.hibernate.dialect.Dialect; import org.hibernate.engine.SessionImplementor; +import org.hibernate.id.enhanced.AccessCallback; +import org.hibernate.id.enhanced.OptimizerFactory; import org.hibernate.type.Type; import org.hibernate.util.PropertiesHelper; @@ -50,23 +50,24 @@ * @author Gavin King */ public class SequenceHiLoGenerator extends SequenceGenerator { - public static final String MAX_LO = "max_lo"; - private static final Logger log = LoggerFactory.getLogger(SequenceHiLoGenerator.class); - private int maxLo; - private int lo; - private IntegralDataTypeHolder value; + private OptimizerFactory.LegacyHiLoAlgorithmOptimizer hiloOptimizer; public void configure(Type type, Properties params, Dialect d) throws MappingException { super.configure(type, params, d); - maxLo = PropertiesHelper.getInt(MAX_LO, params, 9); - lo = maxLo + 1; // so we "clock over" on the first invocation + + maxLo = PropertiesHelper.getInt( MAX_LO, params, 9 ); + + hiloOptimizer = new OptimizerFactory.LegacyHiLoAlgorithmOptimizer( + getIdentifierType().getReturnedClass(), + maxLo + ); } - public synchronized Serializable generate(SessionImplementor session, Object obj) { + public synchronized Serializable generate(final SessionImplementor session, Object obj) { // maxLo < 1 indicates a hilo generator with no hilo :? if ( maxLo < 1 ) { //keep the behavior consistent even for boundary usages @@ -77,16 +78,21 @@ return value.makeValue(); } - if ( lo > maxLo ) { - IntegralDataTypeHolder hiVal = generateHolder( session ); - lo = ( hiVal.eq( 0 ) ) ? 1 : 0; - value = hiVal.copy().multiplyBy( maxLo+1 ).add( lo ); - if ( log.isDebugEnabled() ) { - log.debug("new hi value: " + hiVal); - } - } + return hiloOptimizer.generate( + new AccessCallback() { + public IntegralDataTypeHolder getNextValue() { + return generateHolder( session ); + } + } + ); + } - return value.makeValueThenIncrement(); + /** + * For testing/assertion purposes + * + * @return The optimizer + */ + OptimizerFactory.LegacyHiLoAlgorithmOptimizer getHiloOptimizer() { + return hiloOptimizer; } - } Modified: core/trunk/core/src/main/java/org/hibernate/id/enhanced/OptimizerFactory.java =================================================================== --- core/trunk/core/src/main/java/org/hibernate/id/enhanced/OptimizerFactory.java 2010-05-11 19:24:54 UTC (rev 19469) +++ core/trunk/core/src/main/java/org/hibernate/id/enhanced/OptimizerFactory.java 2010-05-11 19:27:20 UTC (rev 19470) @@ -48,6 +48,7 @@ private static Class[] CTOR_SIG = new Class[] { Class.class, int.class }; + @SuppressWarnings({ "UnnecessaryBoxing" }) public static Optimizer buildOptimizer(String type, Class returnClass, int incrementSize) { String optimizerClassName; if ( NONE.equals( type ) ) { @@ -66,7 +67,7 @@ try { Class optimizerClass = ReflectHelper.classForName( optimizerClassName ); Constructor ctor = optimizerClass.getConstructor( CTOR_SIG ); - return ( Optimizer ) ctor.newInstance( new Object[] { returnClass, new Integer( incrementSize ) } ); + return ( Optimizer ) ctor.newInstance( returnClass, Integer.valueOf( incrementSize ) ); } catch( Throwable ignore ) { // intentionally empty @@ -265,6 +266,67 @@ } } + public static class LegacyHiLoAlgorithmOptimizer extends OptimizerSupport { + private long maxLo; + private long lo; + private IntegralDataTypeHolder hi; + + private IntegralDataTypeHolder lastSourceValue; + private IntegralDataTypeHolder value; + + + public LegacyHiLoAlgorithmOptimizer(Class returnClass, int incrementSize) { + super( returnClass, incrementSize ); + if ( incrementSize < 1 ) { + throw new HibernateException( "increment size cannot be less than 1" ); + } + if ( log.isTraceEnabled() ) { + log.trace( "creating hilo optimizer (legacy) with [incrementSize=" + incrementSize + "; returnClass=" + returnClass.getName() + "]" ); + } + + maxLo = incrementSize; + lo = maxLo+1; + } + + /** + * {@inheritDoc} + */ + public synchronized Serializable generate(AccessCallback callback) { + if ( lo > maxLo ) { + lastSourceValue = callback.getNextValue(); + lo = lastSourceValue.eq( 0 ) ? 1 : 0; + hi = lastSourceValue.copy().multiplyBy( maxLo+1 ); + } + value = hi.copy().add( lo++ ); + return value.makeValue(); + } + + /** + * {@inheritDoc} + */ + public IntegralDataTypeHolder getLastSourceValue() { + return lastSourceValue.copy(); + } + + /** + * {@inheritDoc} + */ + public boolean applyIncrementSizeToSourceValues() { + return false; + } + + /** + * Getter for property 'lastValue'. + * <p/> + * Exposure intended for testing purposes. + * + * @return Value for property 'lastValue'. + */ + public IntegralDataTypeHolder getLastValue() { + return value; + } + } + /** * Optimizer which uses a pool of values, storing the next low value of the * range in the database. Added: core/trunk/core/src/test/java/org/hibernate/id/SequenceHiLoGeneratorTest.java =================================================================== --- core/trunk/core/src/test/java/org/hibernate/id/SequenceHiLoGeneratorTest.java (rev 0) +++ core/trunk/core/src/test/java/org/hibernate/id/SequenceHiLoGeneratorTest.java 2010-05-11 19:27:20 UTC (rev 19470) @@ -0,0 +1,169 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2010, 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.id; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.Properties; + +import junit.framework.TestCase; + +import org.hibernate.Hibernate; +import org.hibernate.Session; +import org.hibernate.cfg.Configuration; +import org.hibernate.cfg.Environment; +import org.hibernate.cfg.NamingStrategy; +import org.hibernate.cfg.ObjectNameNormalizer; +import org.hibernate.dialect.Dialect; +import org.hibernate.dialect.H2Dialect; +import org.hibernate.engine.SessionFactoryImplementor; +import org.hibernate.impl.SessionImpl; +import org.hibernate.jdbc.Work; +import org.hibernate.mapping.SimpleAuxiliaryDatabaseObject; + +/** + * I went back to 3.3 source and grabbed the code/logic as it existed back then and crafted this + * unit test so that we can make sure the value keep being generated in the expected manner + * + * @author Steve Ebersole + */ +@SuppressWarnings({ "deprecation" }) +public class SequenceHiLoGeneratorTest extends TestCase { + private static final String TEST_SEQUENCE = "test_sequence"; + + private Configuration cfg; + private SessionFactoryImplementor sessionFactory; + private SequenceHiLoGenerator generator; + + @Override + protected void setUp() throws Exception { + super.setUp(); + + Properties properties = new Properties(); + properties.setProperty( SequenceGenerator.SEQUENCE, TEST_SEQUENCE ); + properties.setProperty( SequenceHiLoGenerator.MAX_LO, "3" ); + properties.put( + PersistentIdentifierGenerator.IDENTIFIER_NORMALIZER, + new ObjectNameNormalizer() { + @Override + protected boolean isUseQuotedIdentifiersGlobally() { + return false; + } + + @Override + protected NamingStrategy getNamingStrategy() { + return cfg.getNamingStrategy(); + } + } + ); + + Dialect dialect = new H2Dialect(); + + generator = new SequenceHiLoGenerator(); + generator.configure( Hibernate.LONG, properties, dialect ); + + cfg = new Configuration() + .setProperty( Environment.DRIVER, "org.h2.Driver" ) + .setProperty( Environment.URL, "jdbc:h2:mem:db1;DB_CLOSE_DELAY=-1" ) + .setProperty( Environment.USER, "sa" ) + .setProperty( Environment.HBM2DDL_AUTO, "create-drop" ); + cfg.addAuxiliaryDatabaseObject( + new SimpleAuxiliaryDatabaseObject( + generator.sqlCreateStrings( dialect )[0], + generator.sqlDropStrings( dialect )[0] + ) + ); + + sessionFactory = (SessionFactoryImplementor) cfg.buildSessionFactory(); + } + + @Override + protected void tearDown() throws Exception { + if ( sessionFactory != null ) { + sessionFactory.close(); + } + + super.tearDown(); + } + + public void testHiLoAlgorithm() { + SessionImpl session = (SessionImpl) sessionFactory.openSession(); + session.beginTransaction(); + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // initially sequence should be uninitialized + assertEquals( 0L, extractSequenceValue( session ) ); + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // historically the hilo generators skipped the initial block of values; + // so the first generated id value is maxlo + 1, here be 4 + Long generatedValue = (Long) generator.generate( session, null ); + assertEquals( 4L, generatedValue.longValue() ); + // which should also perform the first read on the sequence which should set it to its "start with" value (1) + assertEquals( 1L, extractSequenceValue( session ) ); + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + generatedValue = (Long) generator.generate( session, null ); + assertEquals( 5L, generatedValue.longValue() ); + assertEquals( 1L, extractSequenceValue( session ) ); + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + generatedValue = (Long) generator.generate( session, null ); + assertEquals( 6L, generatedValue.longValue() ); + assertEquals( 1L, extractSequenceValue( session ) ); + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + generatedValue = (Long) generator.generate( session, null ); + assertEquals( 7L, generatedValue.longValue() ); + // unlike the newer strategies, the db value will not get update here. It gets updated on the next invocation + // after a clock over + assertEquals( 1L, extractSequenceValue( session ) ); + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + generatedValue = (Long) generator.generate( session, null ); + assertEquals( 8L, generatedValue.longValue() ); + // this should force an increment in the sequence value + assertEquals( 2L, extractSequenceValue( session ) ); + + session.getTransaction().commit(); + session.close(); + } + + private long extractSequenceValue(Session session) { + class WorkImpl implements Work { + private long value; + public void execute(Connection connection) throws SQLException { + PreparedStatement query = connection.prepareStatement( "select currval('" + TEST_SEQUENCE + "');" ); + ResultSet resultSet = query.executeQuery(); + resultSet.next(); + value = resultSet.getLong( 1 ); + } + } + WorkImpl work = new WorkImpl(); + session.doWork( work ); + return work.value; + } +} Modified: core/trunk/parent/pom.xml =================================================================== --- core/trunk/parent/pom.xml 2010-05-11 19:24:54 UTC (rev 19469) +++ core/trunk/parent/pom.xml 2010-05-11 19:27:20 UTC (rev 19470) @@ -593,6 +593,7 @@ <groupId>com.h2database</groupId> <artifactId>h2</artifactId> <version>1.2.134</version> + <scope>test</scope> </dependency> </dependencies> <properties> @@ -738,6 +739,7 @@ <groupId>com.ibm</groupId> <artifactId>db2jcc_license_cu</artifactId> <version>3.1.57</version> + <scope>test</scope> </dependency> </dependencies> <properties> @@ -764,6 +766,7 @@ <groupId>com.ibm</groupId> <artifactId>db2jcc_license_cu</artifactId> <version>3.8.47</version> + <scope>test</scope> </dependency> </dependencies> <properties> @@ -790,6 +793,7 @@ <groupId>com.ibm</groupId> <artifactId>db2jcc_license_cu</artifactId> <version>3.57.86</version> + <scope>test</scope> </dependency> </dependencies> <properties>