HHH-10413 : byte[] as the version attribute broken

hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java

@@ -994,6 +994,8 @@ public class ModelBinder {
 				rootEntityDescriptor.getTable()
 		);
 
+		versionValue.makeVersion();
+
 		bindSimpleValueType(
 				sourceDocument,
 				versionAttributeSource.getTypeInformation(),

hibernate-core/src/main/java/org/hibernate/cfg/annotations/SimpleValueBinder.java

@@ -97,6 +97,9 @@ public class SimpleValueBinder {
 
 	public void setVersion(boolean isVersion) {
 		this.isVersion = isVersion;
+		if ( isVersion && simpleValue != null ) {
+			simpleValue.makeVersion();
+		}
 	}
 
 	public void setTimestampVersionType(String versionType) {
@@ -397,6 +400,9 @@ public class SimpleValueBinder {
 			table = columns[0].getTable();
 		}
 		simpleValue = new SimpleValue( buildingContext.getMetadataCollector(), table );
+		if ( isVersion ) {
+			simpleValue.makeVersion();
+		}
 		if ( isNationalized ) {
 			simpleValue.makeNationalized();
 		}

hibernate-core/src/main/java/org/hibernate/internal/util/compare/RowVersionComparator.java

@@ -0,0 +1,37 @@
+/*
+ * 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.internal.util.compare;
+
+import java.util.Comparator;
+
+/**
+ * @author Gail Badner
+ */
+public final class RowVersionComparator implements Comparator<byte[]> {
+	public static RowVersionComparator INSTANCE = new RowVersionComparator();
+
+	private RowVersionComparator() {
+	}
+
+	@Override
+	@SuppressWarnings({ "unchecked" })
+	public int compare(byte[] o1, byte[] o2) {
+		final int lengthToCheck = Math.min( o1.length, o2.length );
+
+		for ( int i = 0 ; i < lengthToCheck ; i++ ) {
+			// must do an unsigned int comparison
+			final int comparison = ComparableComparator.INSTANCE.compare(
+						Byte.toUnsignedInt( o1[i] ),
+						Byte.toUnsignedInt( o2[i] )
+			);
+			if ( comparison != 0 ) {
+				return comparison;
+			}
+		}
+		return o1.length - o2.length;
+	}
+}

hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java

@@ -37,6 +37,8 @@ import org.hibernate.internal.CoreLogging;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.util.ReflectHelper;
 import org.hibernate.service.ServiceRegistry;
+import org.hibernate.type.BinaryType;
+import org.hibernate.type.RowVersionType;
 import org.hibernate.type.Type;
 import org.hibernate.type.descriptor.JdbcTypeNameMapper;
 import org.hibernate.type.descriptor.converter.AttributeConverterSqlTypeDescriptorAdapter;
@@ -65,6 +67,7 @@ public class SimpleValue implements KeyValue {
 
 	private String typeName;
 	private Properties typeParameters;
+	private boolean isVersion;
 	private boolean isNationalized;
 	private boolean isLob;
 
@@ -168,6 +171,13 @@ public class SimpleValue implements KeyValue {
 		this.typeName = typeName;
 	}
 
+	public void makeVersion() {
+		this.isVersion = true;
+	}
+
+	public boolean isVersion() {
+		return isVersion;
+	}
 	public void makeNationalized() {
 		this.isNationalized = true;
 	}
@@ -408,6 +418,12 @@ public class SimpleValue implements KeyValue {
 		}
 
 		Type result = metadata.getTypeResolver().heuristicType( typeName, typeParameters );
+		// if this is a byte[] version/timestamp, then we need to use RowVersionType
+		// instead of BinaryType (HHH-10413)
+		if ( isVersion && BinaryType.class.isInstance( result ) ) {
+			log.debug( "version is BinaryType; changing to RowVersionType" );
+			result = RowVersionType.INSTANCE;
+		}
 		if ( result == null ) {
 			String msg = "Could not determine type for: " + typeName;
 			if ( table != null ) {

hibernate-core/src/main/java/org/hibernate/type/BasicTypeRegistry.java

@@ -74,6 +74,7 @@ public class BasicTypeRegistry implements Serializable {
 
 		register( BinaryType.INSTANCE );
 		register( WrapperBinaryType.INSTANCE );
+		register( RowVersionType.INSTANCE );
 		register( ImageType.INSTANCE );
 		register( CharArrayType.INSTANCE );
 		register( CharacterArrayType.INSTANCE );

hibernate-core/src/main/java/org/hibernate/type/BinaryType.java

@@ -15,6 +15,9 @@ import org.hibernate.type.descriptor.sql.VarbinaryTypeDescriptor;
 /**
  * A type that maps between a {@link java.sql.Types#VARBINARY VARBINARY} and {@code byte[]}
  *
+ * Implementation of the {@link VersionType} interface should be considered deprecated.
+ * For binary entity versions/timestamps, {@link RowVersionType} should be used instead.
+ *
  * @author Gavin King
  * @author Steve Ebersole
  */
@@ -37,7 +40,15 @@ public class BinaryType
 		return new String[] { getName(), "byte[]", byte[].class.getName() };
 	}
 
+	/**
+	 * Generate an initial version.
+	 *
+	 * @param session The session from which this request originates.
+	 * @return an instance of the type
+	 * @deprecated use {@link RowVersionType} for binary entity versions/timestamps
+	 */
 	@Override
+	@Deprecated
 	public byte[] seed(SharedSessionContractImplementor session) {
 		// Note : simply returns null for seed() and next() as the only known
 		// 		application of binary types for versioning is for use with the
@@ -46,12 +57,28 @@ public class BinaryType
 		return null;
 	}
 
+	/**
+	 * Increment the version.
+	 *
+	 * @param session The session from which this request originates.
+	 * @param current the current version
+	 * @return an instance of the type
+	 * @deprecated use {@link RowVersionType} for binary entity versions/timestamps
+	 */
 	@Override
+	@Deprecated
 	public byte[] next(byte[] current, SharedSessionContractImplementor session) {
 		return current;
 	}
 
+	/**
+	 * Get a comparator for version values.
+	 *
+	 * @return The comparator to use to compare different version values.
+	 * @deprecated use {@link RowVersionType} for binary entity versions/timestamps
+	 */
 	@Override
+	@Deprecated
 	public Comparator<byte[]> getComparator() {
 		return PrimitiveByteArrayTypeDescriptor.INSTANCE.getComparator();
 	}

hibernate-core/src/main/java/org/hibernate/type/RowVersionType.java

@@ -0,0 +1,62 @@
+/*
+ * 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.type;
+
+import java.util.Comparator;
+
+import org.hibernate.engine.spi.SharedSessionContractImplementor;
+import org.hibernate.internal.util.compare.RowVersionComparator;
+import org.hibernate.type.descriptor.java.PrimitiveByteArrayTypeDescriptor;
+import org.hibernate.type.descriptor.java.RowVersionTypeDescriptor;
+import org.hibernate.type.descriptor.sql.VarbinaryTypeDescriptor;
+
+/**
+ * A type that maps between a {@link java.sql.Types#VARBINARY VARBINARY} and {@code byte[]}
+ * specifically for entity versions/timestamps.
+ *
+ * @author Gavin King
+ * @author Steve Ebersole
+ * @author Gail Badner
+ */
+public class RowVersionType
+		extends AbstractSingleColumnStandardBasicType<byte[]>
+		implements VersionType<byte[]> {
+
+	public static final RowVersionType INSTANCE = new RowVersionType();
+
+	public String getName() {
+		return "row_version";
+	}
+
+	public RowVersionType() {
+		super( VarbinaryTypeDescriptor.INSTANCE, RowVersionTypeDescriptor.INSTANCE );
+	}
+
+	@Override
+	public String[] getRegistrationKeys() {
+		return new String[] { getName() };
+	}
+
+	@Override
+	public byte[] seed(SharedSessionContractImplementor session) {
+		// Note : simply returns null for seed() and next() as the only known
+		// 		application of binary types for versioning is for use with the
+		// 		TIMESTAMP datatype supported by Sybase and SQL Server, which
+		// 		are completely db-generated values...
+		return null;
+	}
+
+	@Override
+	public byte[] next(byte[] current, SharedSessionContractImplementor session) {
+		return current;
+	}
+
+	@Override
+	public Comparator<byte[]> getComparator() {
+		return getJavaTypeDescriptor().getComparator();
+	}
+}

hibernate-core/src/main/java/org/hibernate/type/StandardBasicTypes.java

@@ -230,6 +230,14 @@ public final class StandardBasicTypes {
 	 */
 	public static final WrapperBinaryType WRAPPER_BINARY = WrapperBinaryType.INSTANCE;
 
+	/**
+	 * The standard Hibernate type for mapping {@code byte[]} to JDBC {@link java.sql.Types#VARBINARY VARBINARY},
+	 * specifically for entity versions/timestamps.
+	 *
+	 * @see RowVersionType
+	 */
+	public static final RowVersionType ROW_VERSION = RowVersionType.INSTANCE;
+
 	/**
 	 * The standard Hibernate type for mapping {@code byte[]} to JDBC {@link java.sql.Types#LONGVARBINARY LONGVARBINARY}.
 	 *

hibernate-core/src/main/java/org/hibernate/type/descriptor/java/RowVersionTypeDescriptor.java

@@ -0,0 +1,131 @@
+/*
+ * 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.type.descriptor.java;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.sql.Blob;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Comparator;
+
+import org.hibernate.HibernateException;
+import org.hibernate.engine.jdbc.BinaryStream;
+import org.hibernate.engine.jdbc.internal.BinaryStreamImpl;
+import org.hibernate.internal.util.compare.RowVersionComparator;
+import org.hibernate.type.descriptor.WrapperOptions;
+
+/**
+ * Descriptor for {@code byte[]} handling specifically used for specifically for entity versions/timestamps.
+ *
+ * @author Steve Ebersole
+ * @author Gail Badner
+ */
+public class RowVersionTypeDescriptor extends AbstractTypeDescriptor<byte[]> {
+	public static final RowVersionTypeDescriptor INSTANCE = new RowVersionTypeDescriptor();
+
+	@SuppressWarnings({ "unchecked" })
+	public RowVersionTypeDescriptor() {
+		super( byte[].class, ArrayMutabilityPlan.INSTANCE );
+	}
+
+	@Override
+	public boolean areEqual(byte[] one, byte[] another) {
+		return one == another
+				|| ( one != null && another != null && Arrays.equals( one, another ) );
+	}
+
+	@Override
+	public int extractHashCode(byte[] bytes) {
+		int hashCode = 1;
+		for ( byte aByte : bytes ) {
+			hashCode = 31 * hashCode + aByte;
+		}
+		return hashCode;
+	}
+
+	public String toString(byte[] bytes) {
+		final StringBuilder buf = new StringBuilder( bytes.length * 2 );
+		for ( byte aByte : bytes ) {
+			final String hexStr = Integer.toHexString( aByte - Byte.MIN_VALUE );
+			if ( hexStr.length() == 1 ) {
+				buf.append( '0' );
+			}
+			buf.append( hexStr );
+		}
+		return buf.toString();
+	}
+
+	@Override
+	public String extractLoggableRepresentation(byte[] value) {
+		return (value == null) ? super.extractLoggableRepresentation( null ) : Arrays.toString( value );
+	}
+
+	public byte[] fromString(String string) {
+		if ( string == null ) {
+			return null;
+		}
+		if ( string.length() % 2 != 0 ) {
+			throw new IllegalArgumentException( "The string is not a valid string representation of a binary content." );
+		}
+		byte[] bytes = new byte[string.length() / 2];
+		for ( int i = 0; i < bytes.length; i++ ) {
+			final String hexStr = string.substring( i * 2, (i + 1) * 2 );
+			bytes[i] = (byte) (Integer.parseInt(hexStr, 16) + Byte.MIN_VALUE);
+		}
+		return bytes;
+	}
+
+	@Override
+	@SuppressWarnings({ "unchecked" })
+	public Comparator<byte[]> getComparator() {
+		return RowVersionComparator.INSTANCE;
+	}
+
+	@SuppressWarnings({ "unchecked" })
+	public <X> X unwrap(byte[] value, Class<X> type, WrapperOptions options) {
+		if ( value == null ) {
+			return null;
+		}
+		if ( byte[].class.isAssignableFrom( type ) ) {
+			return (X) value;
+		}
+		if ( InputStream.class.isAssignableFrom( type ) ) {
+			return (X) new ByteArrayInputStream( value );
+		}
+		if ( BinaryStream.class.isAssignableFrom( type ) ) {
+			return (X) new BinaryStreamImpl( value );
+		}
+		if ( Blob.class.isAssignableFrom( type ) ) {
+			return (X) options.getLobCreator().createBlob( value );
+		}
+
+		throw unknownUnwrap( type );
+	}
+
+	public <X> byte[] wrap(X value, WrapperOptions options) {
+		if ( value == null ) {
+			return null;
+		}
+		if ( byte[].class.isInstance( value ) ) {
+			return (byte[]) value;
+		}
+		if ( InputStream.class.isInstance( value ) ) {
+			return DataHelper.extractBytes( (InputStream) value );
+		}
+		if ( Blob.class.isInstance( value ) || DataHelper.isNClob( value.getClass() ) ) {
+			try {
+				return DataHelper.extractBytes( ( (Blob) value ).getBinaryStream() );
+			}
+			catch ( SQLException e ) {
+				throw new HibernateException( "Unable to access lob stream", e );
+			}
+		}
+
+		throw unknownWrap( value.getClass() );
+	}
+}

hibernate-core/src/test/java/org/hibernate/test/util/RowVersionComparatorTest.java

@@ -0,0 +1,139 @@
+/*
+ * 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.test.util;
+
+import org.junit.Test;
+
+import org.hibernate.internal.util.compare.RowVersionComparator;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * @author Gail Badner
+ */
+public class RowVersionComparatorTest extends BaseUnitTestCase {
+
+	@Test
+	public void testNull() {
+		try {
+			RowVersionComparator.INSTANCE.compare( null, null );
+			fail( "should have thrown NullPointerException" );
+		}
+		catch ( NullPointerException ex ) {
+			// expected
+		}
+
+		try {
+			RowVersionComparator.INSTANCE.compare( null, new byte[] { 1 } );
+			fail( "should have thrown NullPointerException" );
+		}
+		catch ( NullPointerException ex ) {
+			// expected
+		}
+
+		try {
+			RowVersionComparator.INSTANCE.compare( new byte[] { 1 }, null );
+			fail( "should have thrown NullPointerException" );
+		}
+		catch ( NullPointerException ex ) {
+			// expected
+		}
+	}
+
+	@Test
+	public void testArraysSameLength() {
+		assertEquals(
+				0,
+				RowVersionComparator.INSTANCE.compare(
+						new byte[] {},
+						new byte[] {}
+				)
+		);
+		assertEquals(
+				0,
+				RowVersionComparator.INSTANCE.compare(
+						new byte[] { 1 },
+						new byte[] { 1 }
+				)
+		);
+		assertEquals(
+				0,
+				RowVersionComparator.INSTANCE.compare(
+						new byte[] { 1, 2 },
+						new byte[] { 1, 2 }
+				)
+		);
+		assertTrue(
+				RowVersionComparator.INSTANCE.compare(
+						new byte[] { 0, 2 },
+						new byte[] { 1, 2 }
+				) < 0
+		);
+
+		assertTrue(
+				RowVersionComparator.INSTANCE.compare(
+						new byte[] { 1, 1 },
+						new byte[] { 1, 2 }
+				) < 0
+		);
+
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] { 2, 2 },
+						new byte[] { 1, 2 }
+				) > 0
+		);
+
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] { 2, 2 },
+						new byte[] { 2, 1 }
+				) > 0
+		);
+	}
+
+	@Test
+	public void testArraysDifferentLength() {
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] {},
+						new byte[] { 1 }
+				) < 0
+		);
+
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] { 1 },
+						new byte[] {}
+				) > 0
+		);
+
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] { 1 },
+						new byte[] { 1, 2 }
+				) < 0
+		);
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] { 1, 2 },
+						new byte[] { 1 }
+				) > 0
+		);
+
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] { 2 },
+						new byte[] { 1, 2 }
+				) > 0
+		);
+		assertTrue( RowVersionComparator.INSTANCE.compare(
+						new byte[] { 1, 2 },
+						new byte[] { 2 }
+				) < 0
+		);
+
+	}
+
+
+}

hibernate-core/src/test/java/org/hibernate/test/version/sybase/SybaseTimestampComparisonAnnotationsTest.java

@@ -0,0 +1,100 @@
+/*
+ * 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.test.version.sybase;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.Table;
+import javax.persistence.Version;
+
+import org.junit.Test;
+
+import org.hibernate.Session;
+import org.hibernate.annotations.Generated;
+import org.hibernate.annotations.GenerationTime;
+import org.hibernate.annotations.Source;
+import org.hibernate.annotations.SourceType;
+import org.hibernate.annotations.Type;
+import org.hibernate.dialect.SybaseASE15Dialect;
+import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.type.RowVersionType;
+import org.hibernate.type.VersionType;
+
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author Gail Badner
+ */
+@RequiresDialect( SybaseASE15Dialect.class )
+public class SybaseTimestampComparisonAnnotationsTest extends BaseCoreFunctionalTestCase {
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-10413" )
+	public void testComparableTimestamps() {
+		final VersionType versionType =
+				sessionFactory().getEntityPersister( Thing.class.getName() ).getVersionType();
+		assertSame( RowVersionType.INSTANCE, versionType );
+
+		Session s = openSession();
+		s.getTransaction().begin();
+		Thing thing = new Thing();
+		thing.name = "n";
+		s.persist( thing );
+		s.getTransaction().commit();
+		s.close();
+
+		byte[] previousVersion = thing.version;
+		for ( int i = 0 ; i < 20 ; i++ ) {
+			try {
+				Thread.sleep(1000);                 //1000 milliseconds is one second.
+			} catch(InterruptedException ex) {
+				Thread.currentThread().interrupt();
+			}
+
+			s = openSession();
+			s.getTransaction().begin();
+			thing.name = "n" + i;
+			thing = (Thing) s.merge( thing );
+			s.getTransaction().commit();
+			s.close();
+
+			assertTrue( versionType.compare( previousVersion, thing.version ) < 0 );
+			previousVersion = thing.version;
+		}
+
+		s = openSession();
+		s.getTransaction().begin();
+		s.delete( thing );
+		s.getTransaction().commit();
+		s.close();
+	}
+
+	@Override
+	protected Class[] getAnnotatedClasses() {
+		return new Class[] { Thing.class };
+	}
+
+	@Entity
+	@Table(name="thing")
+	public static class Thing {
+		@Id
+		private long id;
+
+		@Version
+		@Generated(GenerationTime.ALWAYS)
+		@Column(name = "ver", columnDefinition = "timestamp")
+		private byte[] version;
+
+		private String name;
+
+	}
+
+}

hibernate-core/src/test/java/org/hibernate/test/version/sybase/SybaseTimestampVersioningTest.java

@@ -6,17 +6,22 @@
  */
 package org.hibernate.test.version.sybase;
 
+import javax.persistence.OptimisticLockException;
+
 import org.junit.Test;
 
-import org.hibernate.HibernateException;
 import org.hibernate.Session;
 import org.hibernate.Transaction;
 import org.hibernate.dialect.SybaseASE15Dialect;
 import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
 import org.hibernate.type.BinaryType;
+import org.hibernate.type.RowVersionType;
+import org.hibernate.type.VersionType;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -64,7 +69,7 @@ public class SybaseTimestampVersioningTest extends BaseCoreFunctionalTestCase {
 				t2.commit();
 				fail( "optimistic lock check did not fail" );
 			}
-			catch( HibernateException e ) {
+			catch( OptimisticLockException e ) {
 				// expected...
 				try {
 					t2.rollback();
@@ -216,4 +221,45 @@ public class SybaseTimestampVersioningTest extends BaseCoreFunctionalTestCase {
 		t.commit();
 		s.close();
 	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-10413" )
+	public void testComparableTimestamps() {
+		final VersionType versionType =
+				sessionFactory().getEntityPersister( User.class.getName() ).getVersionType();
+		assertSame( RowVersionType.INSTANCE, versionType );
+
+		Session s = openSession();
+		s.getTransaction().begin();
+		User user = new User();
+		user.setUsername( "n" );
+		s.persist( user );
+		s.getTransaction().commit();
+		s.close();
+
+		byte[] previousTimestamp = user.getTimestamp();
+		for ( int i = 0 ; i < 20 ; i++ ) {
+			try {
+				Thread.sleep(1000);                 //1000 milliseconds is one second.
+			} catch(InterruptedException ex) {
+				Thread.currentThread().interrupt();
+			}
+
+			s = openSession();
+			s.getTransaction().begin();
+			user.setUsername( "n" + i );
+			user = (User) s.merge( user );
+			s.getTransaction().commit();
+			s.close();
+
+			assertTrue( versionType.compare( previousTimestamp, user.getTimestamp() ) < 0 );
+			previousTimestamp = user.getTimestamp();
+		}
+
+		s = openSession();
+		s.getTransaction().begin();
+		s.delete( user );
+		s.getTransaction().commit();
+		s.close();
+	}
 }