From 1d1f395951d0b601a653ebd778ef9315f62c0104 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Thu, 5 Sep 2024 17:10:14 +0800 Subject: [PATCH 1/2] 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. --- .../JpaMetamodelEntityInformation.java | 30 ++++++++++++- .../sample/SampleWithPrimitiveVersion.java | 43 +++++++++++++++++++ ...odelEntityInformationIntegrationTests.java | 20 +++++++++ ...odelEntityInformationIntegrationTests.java | 15 +++++++ .../test/resources/META-INF/persistence.xml | 1 + 5 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/SampleWithPrimitiveVersion.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java index 9979fc773b..8c9dca2fe2 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java +++ b/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 extends JpaEntityInformationSupport { @@ -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); } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/SampleWithPrimitiveVersion.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/domain/sample/SampleWithPrimitiveVersion.java new file mode 100644 index 0000000000..1f76e60c2b --- /dev/null +++ b/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; + } +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java index 4922461995..889e126e21 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java +++ b/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 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(); + } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java index c305e9b4bc..6ba5f1cc4a 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/HibernateJpaMetamodelEntityInformationIntegrationTests.java +++ b/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 information = new JpaMetamodelEntityInformation<>(SampleWithPrimitiveVersion.class, + em.getMetamodel(), em.getEntityManagerFactory().getPersistenceUnitUtil()); + + SampleWithPrimitiveVersion entity = new SampleWithPrimitiveVersion(); + entity.setId(23L); // assigned + assertThat(information.isNew(entity)).isTrue(); + } } diff --git a/spring-data-jpa/src/test/resources/META-INF/persistence.xml b/spring-data-jpa/src/test/resources/META-INF/persistence.xml index 1c3be472e0..410f9d69d3 100644 --- a/spring-data-jpa/src/test/resources/META-INF/persistence.xml +++ b/spring-data-jpa/src/test/resources/META-INF/persistence.xml @@ -46,6 +46,7 @@ org.springframework.data.jpa.domain.sample.SampleEntityPK org.springframework.data.jpa.domain.sample.SampleWithIdClass org.springframework.data.jpa.domain.sample.SampleWithPrimitiveId + org.springframework.data.jpa.domain.sample.SampleWithPrimitiveVersion org.springframework.data.jpa.domain.sample.SampleWithTimestampVersion org.springframework.data.jpa.domain.sample.Site org.springframework.data.jpa.domain.sample.SpecialUser From 3f283405a97fa390f8d883e011b1977c9e81ec63 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Mon, 9 Sep 2024 10:12:37 +0800 Subject: [PATCH 2/2] Polishing --- .../support/JpaMetamodelEntityInformation.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java index 8c9dca2fe2..cb2913dd88 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java @@ -61,7 +61,7 @@ public class JpaMetamodelEntityInformation extends JpaEntityInformationSu private final IdMetadata idMetadata; private final Optional> versionAttribute; - private final Metamodel metamodel; + private final PersistenceProvider persistenceProvider; private final @Nullable String entityName; private final PersistenceUnitUtil persistenceUnitUtil; @@ -78,7 +78,7 @@ public JpaMetamodelEntityInformation(Class domainClass, Metamodel metamodel, super(domainClass); Assert.notNull(metamodel, "Metamodel must not be null"); - this.metamodel = metamodel; + this.persistenceProvider = PersistenceProvider.fromMetamodel(metamodel); ManagedType type = metamodel.managedType(domainClass); @@ -92,7 +92,7 @@ public JpaMetamodelEntityInformation(Class domainClass, Metamodel metamodel, throw new IllegalArgumentException("The given domain class does not contain an id attribute"); } - this.idMetadata = new IdMetadata<>(identifiableType, PersistenceProvider.fromMetamodel(metamodel)); + this.idMetadata = new IdMetadata<>(identifiableType, persistenceProvider); this.versionAttribute = findVersionAttribute(identifiableType, metamodel); Assert.notNull(persistenceUnitUtil, "PersistenceUnitUtil must not be null"); @@ -149,8 +149,6 @@ public String getEntityName() { public ID getId(T entity) { // check if this is a proxy. If so use Proxy mechanics to access the id. - PersistenceProvider persistenceProvider = PersistenceProvider.fromMetamodel(metamodel); - if (persistenceProvider.shouldUseAccessorFor(entity)) { return (ID) persistenceProvider.getIdentifierFrom(entity); } @@ -230,8 +228,7 @@ public boolean isNew(T entity) { return versionAttribute.map(it -> { Object version = wrapper.getPropertyValue(it.getName()); if (version instanceof Number value) { - PersistenceProvider provider = PersistenceProvider.fromMetamodel(metamodel); - if (provider == PersistenceProvider.HIBERNATE) { + if (persistenceProvider == 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() @@ -240,7 +237,7 @@ public boolean isNew(T entity) { // 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) { + } else if (persistenceProvider == PersistenceProvider.ECLIPSELINK) { // EclipseLink always use 1 as seed of integer version if (value.longValue() < 1) { return true;