Improve detection of entity state

Hibernate use 0 or user provided positive initial value as seed of integer version.
EclipseLink always use 1 as seed of integer version.

Author: quaff

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java

@@ -55,6 +55,7 @@
  * @author Mark Paluch
  * @author Jens Schauder
  * @author Greg Turnquist
+ * @author Yanming Zhou
  */
 public class JpaMetamodelEntityInformation<T, ID> extends JpaEntityInformationSupport<T, ID> {
 
@@ -219,13 +220,38 @@ public Object getCompositeIdAttributeValue(Object id, String idAttribute) {
 	@Override
 	public boolean isNew(T entity) {
 
-		if (versionAttribute.isEmpty()
-				|| versionAttribute.map(Attribute::getJavaType).map(Class::isPrimitive).orElse(false)) {
+		if (versionAttribute.isEmpty()) {
 			return super.isNew(entity);
 		}
 
 		BeanWrapper wrapper = new DirectFieldAccessFallbackBeanWrapper(entity);
 
+		if (versionAttribute.map(Attribute::getJavaType).map(Class::isPrimitive).orElse(false)) {
+			return versionAttribute.map(it -> {
+				Object version = wrapper.getPropertyValue(it.getName());
+				if (version instanceof Number value) {
+					PersistenceProvider provider = PersistenceProvider.fromMetamodel(metamodel);
+					if (provider == PersistenceProvider.HIBERNATE) {
+						// Hibernate use 0 or user provided positive initial value as seed of integer version
+						if (value.longValue() < 0) {
+							// see org.hibernate.engine.internal.Versioning#isNullInitialVersion()
+							return true;
+						}
+						// TODO Compare version to initial value (field value of transient entity)
+						// It's unknown if equals because entity maybe transient or just persisted
+						// But it's absolute not new if not equals
+					} else if (provider == PersistenceProvider.ECLIPSELINK) {
+						// EclipseLink always use 1 as seed of integer version
+						if (value.longValue() < 1) {
+							return true;
+						}
+						// It's unknown because the value may be initial value or persisted entity version
+					}
+				}
+				return super.isNew(entity);
+			}).orElseThrow(); // should never throw
+		}
+
 		return versionAttribute.map(it -> wrapper.getPropertyValue(it.getName()) == null).orElse(true);
 	}
 

spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/SampleWithPrimitiveVersion.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2013-2024 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.jpa.domain.sample;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Version;
+
+/**
+ * @author Yanming Zhou
+ */
+@Entity
+public class SampleWithPrimitiveVersion {
+
+	@Id private Long id;
+
+	@Version private long version = -1;
+
+	public void setId(Long id) {
+		this.id = id;
+	}
+
+	public long getVersion() {
+		return version;
+	}
+
+	public void setVersion(long version) {
+		this.version = version;
+	}
+}

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java

@@ -24,6 +24,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.springframework.data.jpa.domain.AbstractPersistable;
+import org.springframework.data.jpa.domain.sample.SampleWithPrimitiveVersion;
+import org.springframework.data.repository.core.EntityInformation;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 
@@ -33,6 +35,7 @@
  * @author Oliver Gierke
  * @author Jens Schauder
  * @author Greg Turnquist
+ * @author Yanming Zhou
  */
 @ExtendWith(SpringExtension.class)
 @ContextConfiguration({ "classpath:infrastructure.xml", "classpath:eclipselink.xml" })
@@ -79,4 +82,21 @@ void correctlyDeterminesIdValueForNestedIdClassesWithNonPrimitiveNonManagedType(
 	@Override
 	@Disabled
 	void prefersPrivateGetterOverFieldAccess() {}
+
+	@Override
+	@Disabled
+	// superseded by #nonPositiveVersionedEntityIsNew()
+	void considersEntityAsNotNewWhenHavingIdSetAndUsingPrimitiveTypeForVersionProperty() {}
+
+	@Test
+	void nonPositiveVersionedEntityIsNew() {
+		EntityInformation<SampleWithPrimitiveVersion, Long> information = new JpaMetamodelEntityInformation<>(SampleWithPrimitiveVersion.class,
+				em.getMetamodel(), em.getEntityManagerFactory().getPersistenceUnitUtil());
+
+		SampleWithPrimitiveVersion entity = new SampleWithPrimitiveVersion();
+		entity.setId(23L); // assigned
+		assertThat(information.isNew(entity)).isTrue();
+		entity.setVersion(0);
+		assertThat(information.isNew(entity)).isTrue();
+	}
 }

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java

@@ -17,14 +17,19 @@
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.springframework.data.jpa.domain.sample.SampleWithPrimitiveVersion;
 import org.springframework.data.jpa.util.DisabledOnHibernate61;
+import org.springframework.data.repository.core.EntityInformation;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 /**
  * Hibernate execution for {@link JpaMetamodelEntityInformationIntegrationTests}.
  *
  * @author Greg Turnquist
+ * @author Yanming Zhou
  */
 @ExtendWith(SpringExtension.class)
 @ContextConfiguration("classpath:infrastructure.xml")
@@ -55,4 +60,14 @@ void prefersPrivateGetterOverFieldAccess() {
 	void findsIdClassOnMappedSuperclass() {
 		super.findsIdClassOnMappedSuperclass();
 	}
+
+	@Test
+	void negativeVersionedEntityIsNew() {
+		EntityInformation<SampleWithPrimitiveVersion, Long> information = new JpaMetamodelEntityInformation<>(SampleWithPrimitiveVersion.class,
+				em.getMetamodel(), em.getEntityManagerFactory().getPersistenceUnitUtil());
+
+		SampleWithPrimitiveVersion entity = new SampleWithPrimitiveVersion();
+		entity.setId(23L); // assigned
+		assertThat(information.isNew(entity)).isTrue();
+	}
 }

spring-data-jpa/src/test/resources/META-INF/persistence.xml

@@ -46,6 +46,7 @@
 		<class>org.springframework.data.jpa.domain.sample.SampleEntityPK</class>
 		<class>org.springframework.data.jpa.domain.sample.SampleWithIdClass</class>
 		<class>org.springframework.data.jpa.domain.sample.SampleWithPrimitiveId</class>
+		<class>org.springframework.data.jpa.domain.sample.SampleWithPrimitiveVersion</class>
 		<class>org.springframework.data.jpa.domain.sample.SampleWithTimestampVersion</class>
 		<class>org.springframework.data.jpa.domain.sample.Site</class>
 		<class>org.springframework.data.jpa.domain.sample.SpecialUser</class>