HHH-18487 align behavior of UnsavedValueFactory with semantics of persist()

Signed-off-by: Gavin King <[email protected]>

Author: gavinking

hibernate-core/src/main/java/org/hibernate/engine/internal/UnsavedValueFactory.java

@@ -21,6 +21,8 @@
 import org.hibernate.type.descriptor.java.VersionJavaType;
 import org.hibernate.type.descriptor.java.spi.PrimitiveJavaType;
 
+import static org.hibernate.engine.internal.Versioning.isNullInitialVersion;
+
 /**
  * Helper for dealing with unsaved value handling
  *
@@ -81,25 +83,20 @@ else if ( idJtd instanceof PrimitiveJavaType ) {
 	public static <T> VersionValue getUnsavedVersionValue(
 			KeyValue bootVersionMapping,
 			VersionJavaType<T> jtd,
-			Long length,
-			Integer precision,
-			Integer scale,
 			Getter getter,
-			Supplier<?> templateInstanceAccess,
-			SessionFactoryImplementor sessionFactory) {
+			Supplier<?> templateInstanceAccess) {
 		final String unsavedValue = bootVersionMapping.getNullValue();
 		if ( unsavedValue == null ) {
 			if ( getter != null && templateInstanceAccess != null ) {
-				Object templateInstance = templateInstanceAccess.get();
+				final Object templateInstance = templateInstanceAccess.get();
 				@SuppressWarnings("unchecked")
 				final T defaultValue = (T) getter.get( templateInstance );
-
-				// if the version of a newly instantiated object is not the same
-				// as the version seed value, use that as the unsaved-value
-				final T seedValue = jtd.seed( length, precision, scale, mockSession( sessionFactory ) );
-				return jtd.areEqual( seedValue, defaultValue )
-						? VersionValue.UNDEFINED
-						: new VersionValue( defaultValue );
+				// if the version of a newly instantiated object is null
+				// or a negative number, use that value as the unsaved-value,
+				// otherwise assume it's the initial version set by program
+				return isNullInitialVersion( defaultValue )
+						? new VersionValue( defaultValue )
+						: VersionValue.UNDEFINED;
 			}
 			else {
 				return VersionValue.UNDEFINED;

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EntityVersionMappingImpl.java

@@ -54,7 +54,7 @@ public class EntityVersionMappingImpl implements EntityVersionMapping, FetchOpti
 	private final Integer scale;
 	private final Integer temporalPrecision;
 
-	private final BasicType versionBasicType;
+	private final BasicType<?> versionBasicType;
 
 	private final VersionValue unsavedValueStrategy;
 
@@ -90,15 +90,10 @@ public EntityVersionMappingImpl(
 		unsavedValueStrategy = UnsavedValueFactory.getUnsavedVersionValue(
 				(KeyValue) bootEntityDescriptor.getVersion().getValue(),
 				(VersionJavaType<?>) versionBasicType.getJavaTypeDescriptor(),
-				length,
-				precision,
-				scale,
-				declaringType
-						.getRepresentationStrategy()
+				declaringType.getRepresentationStrategy()
 						.resolvePropertyAccess( bootEntityDescriptor.getVersion() )
 						.getGetter(),
-				templateInstanceAccess,
-				creationProcess.getCreationContext().getSessionFactory()
+				templateInstanceAccess
 		);
 	}
 

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

@@ -16,6 +16,10 @@
 public interface VersionJavaType<T> extends JavaType<T> {
 	/**
 	 * Generate an initial version.
+	 * <p>
+	 * Note that this operation is only used when the program sets a null or negative
+	 * number as the value of the entity version field. It is not called when the
+	 * program sets the version field to a sensible-looking version.
 	 *
 	 * @param length The length of the type
 	 * @param precision The precision of the type

hibernate-core/src/test/java/org/hibernate/orm/test/ops/MergeExplicitInitialVersionTest.java

@@ -0,0 +1,71 @@
+package org.hibernate.orm.test.ops;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.Id;
+import jakarta.persistence.Version;
+import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
+import org.hibernate.testing.orm.junit.Jpa;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@Jpa(annotatedClasses = {MergeExplicitInitialVersionTest.E.class,
+        MergeExplicitInitialVersionTest.F.class,
+        MergeExplicitInitialVersionTest.G.class})
+public class MergeExplicitInitialVersionTest {
+    @Test public void testGeneratedId(EntityManagerFactoryScope scope) {
+        E e = new E();
+        scope.inTransaction(s->s.persist(e));
+        assertEquals(e.version, 1);
+        e.text = "hello";
+        E e2 = scope.fromTransaction(s->s.merge(e));
+        assertEquals(e2.version, 2);
+    }
+    @Test public void testAssignedId(EntityManagerFactoryScope scope) {
+        F f = new F();
+        scope.inTransaction(s->s.persist(f));
+        assertEquals(f.version, 1);
+        f.text = "hello";
+        F f2 = scope.fromTransaction(s->s.merge(f));
+        assertEquals(f2.version, 2);
+    }
+    @Test public void testNegativeVersion(EntityManagerFactoryScope scope) {
+        G g = new G();
+        scope.inTransaction(s->s.persist(g));
+        assertEquals(g.version, 0);
+        g.text = "hello";
+        G g2 = scope.fromTransaction(s->s.merge(g));
+        assertEquals(g2.version, 1);
+    }
+
+    @Entity
+    static class E {
+        @Id
+        @GeneratedValue
+        long id;
+        @Version
+        int version = 1;
+        String text;
+    }
+
+    @Entity
+    static class F {
+        @Id
+        long id = 5;
+        @Version
+        int version = 1;
+        String text;
+    }
+
+    @Entity
+    static class G {
+        @Id
+        @GeneratedValue
+        long id;
+        @Version
+        int version = -1;
+        String text;
+    }
+
+}