HHH-17934 sort out version handling in upsert()

Signed-off-by: Gavin King <[email protected]>
hibernate · Aug 6, 2024 · 909a8b0 · 909a8b0
1 parent ef0cc75
commit 909a8b0
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 25 deletions.
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SqlAstTranslatorWithMerge.java b/hibernate-core/src/main/java/org/hibernate/dialect/SqlAstTranslatorWithMerge.java
@@ -50,11 +50,11 @@ public MergeOperation createMergeOperation(OptionalTableUpdate optionalTableUpda
 		);
 	}
 
-	@Override
-	public void visitOptionalTableUpdate(OptionalTableUpdate tableUpdate) {
-		renderMergeStatement(tableUpdate);
-	}
-
+//	@Override
+//	public void visitOptionalTableUpdate(OptionalTableUpdate tableUpdate) {
+//		renderMergeStatement(tableUpdate);
+//	}
+//
 	/**
 	 * Renders the OptionalTableUpdate as a MERGE query.
 	 *

diff --git a/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java
@@ -31,14 +31,14 @@
 import org.hibernate.engine.spi.PersistentAttributeInterceptor;
 import org.hibernate.engine.transaction.internal.jta.JtaStatusHelper;
 import org.hibernate.engine.transaction.jta.platform.spi.JtaPlatform;
+import org.hibernate.generator.BeforeExecutionGenerator;
+import org.hibernate.generator.Generator;
 import org.hibernate.graph.GraphSemantic;
 import org.hibernate.graph.spi.RootGraphImplementor;
 import org.hibernate.loader.ast.spi.CascadingFetchProfile;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.proxy.LazyInitializer;
-import org.hibernate.generator.Generator;
-import org.hibernate.generator.BeforeExecutionGenerator;
 import org.hibernate.tuple.entity.EntityMetamodel;
 
 import jakarta.transaction.SystemException;
@@ -173,37 +173,50 @@ public void update(String entityName, Object entity) {
 	public void upsert(String entityName, Object entity) {
 		checkOpen();
 		final EntityPersister persister = getEntityPersister( entityName, entity );
-		Object id = persister.getIdentifier( entity, this );
-		Boolean knownTransient = persister.isTransient( entity, this );
-		if ( knownTransient!=null && knownTransient ) {
-			throw new TransientObjectException(
-					"Object passed to upsert() has a null identifier: "
-							+ persister.getEntityName() );
-//			final Generator generator = persister.getGenerator();
-//			if ( !generator.generatedOnExecution() ) {
-//				id = ( (BeforeExecutionGenerator) generator).generate( this, entity, null, INSERT );
-//			}
-		}
+		final Object id = idToUpsert( entity, persister );
 		final Object[] state = persister.getValues( entity );
-		final Object oldVersion;
+		final Object oldVersion = versionToUpsert( entity, persister, state );
+		persister.merge( id, state, null, false, null, oldVersion, entity, null, this );
+	}
+
+	private Object versionToUpsert(Object entity, EntityPersister persister, Object[] state) {
 		if ( persister.isVersioned() ) {
-			oldVersion = persister.getVersion( entity );
-			if ( oldVersion == null ) {
+			final Object oldVersion = persister.getVersion( entity );
+			final Boolean knownTransient =
+					persister.getVersionMapping()
+							.getUnsavedStrategy()
+							.isUnsaved( oldVersion );
+			if ( knownTransient != null && knownTransient ) {
 				if ( seedVersion( entity, state, persister, this ) ) {
 					persister.setValues( entity, state );
 				}
+				// this is a nonsense but avoids setting version restriction
+				// parameter to null later on deep in the guts
+				return state[persister.getVersionProperty()];
 			}
 			else {
 				final Object newVersion = incrementVersion( entity, oldVersion, persister, this );
 				setVersion( state, newVersion, persister );
 				persister.setValues( entity, state );
+				return oldVersion;
 			}
 		}
 		else {
-			oldVersion = null;
+			return null;
 		}
-		persister.merge( id, state, null, false, null, oldVersion, entity, null, this );
-//		persister.setIdentifier( entity, id, this );
+	}
+
+	private Object idToUpsert(Object entity, EntityPersister persister) {
+		final Object id = persister.getIdentifier( entity, this );
+		final Boolean unsaved =
+				persister.getIdentifierMapping()
+						.getUnsavedStrategy()
+						.isUnsaved( id );
+		if ( unsaved != null && unsaved ) {
+			throw new TransientObjectException( "Object passed to upsert() has an unsaved identifier value: "
+					+ persister.getEntityName() );
+		}
+		return id;
 	}
 
 

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java
@@ -15,7 +15,7 @@
 public class UpsertVersionedTest {
     @Test void test(SessionFactoryScope scope) {
         scope.inStatelessTransaction(s-> {
-            s.upsert(new Record(123L,0L,"hello earth"));
+            s.upsert(new Record(123L,null,"hello earth"));
             s.upsert(new Record(456L,2L,"hello mars"));
         });
         scope.inStatelessTransaction(s-> {
@@ -29,6 +29,13 @@ public class UpsertVersionedTest {
             assertEquals("goodbye earth",s.get(Record.class,123L).message);
             assertEquals("hello mars",s.get(Record.class,456L).message);
         });
+        scope.inStatelessTransaction(s-> {
+            s.upsert(new Record(456L,4L,"goodbye mars"));
+        });
+        scope.inStatelessTransaction(s-> {
+            assertEquals("goodbye earth",s.get(Record.class,123L).message);
+            assertEquals("goodbye mars",s.get(Record.class,456L).message);
+        });
     }
     @Entity
     static class Record {