From fee3aad37209d128e7d0493f7f47a421d747d918 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 21 Feb 2018 11:39:30 +0100 Subject: [PATCH] DATAJPA-1105 - Fixed support for nested IdClass with non mapped non primitive type. When obtaining a multipart id from an entity we have to check for each part of the id if it is another entity. Before this change that check was done be checking if the type is a primitive or wrapper type, which fails to properly classify e.g. String. This caused us to try to get an id from a String instance which of course failed because String is not a managed type. Now we make the necessary distinction based on JpaMetamodel.isJpaManaged(type). Changes in the persistence.xml beyond adding entities for tests are required to make the tests work again with EclipseLink. This didn't cause problems in the past because all tests that actually access the database and use the changed persistence context are disabled for EclipseLink. The new test had to be get disabled for EclipseLink though, due to another bug in EclipseLink which prevents the usage of inner classes in @IdClass annotations. See also: https://bugs.eclipse.org/bugs/show_bug.cgi?id=531528 Original pull request: #251. --- .../JpaMetamodelEntityInformation.java | 7 ++- ...odelEntityInformationIntegrationTests.java | 9 +++ ...odelEntityInformationIntegrationTests.java | 55 +++++++++++++++++++ src/test/resources/META-INF/persistence.xml | 20 +++++++ 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java b/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java index 6225023f5..4d898a919 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java @@ -35,10 +35,10 @@ import javax.persistence.metamodel.Type.PersistenceType; import org.springframework.beans.BeanWrapper; import org.springframework.beans.BeanWrapperImpl; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.data.jpa.util.JpaMetamodel; import org.springframework.data.util.DirectFieldAccessFallbackBeanWrapper; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; /** * Implementation of {@link org.springframework.data.repository.core.EntityInformation} that uses JPA {@link Metamodel} @@ -48,6 +48,7 @@ import org.springframework.util.ClassUtils; * @author Thomas Darimont * @author Christoph Strobl * @author Mark Paluch + * @author Jens Schauder */ public class JpaMetamodelEntityInformation extends JpaEntityInformationSupport { @@ -319,10 +320,12 @@ public class JpaMetamodelEntityInformation extends JpaEntityInformationSu extends DirectFieldAccessFallbackBeanWrapper { private final Metamodel metamodel; + private final JpaMetamodel jpaMetamodel; IdentifierDerivingDirectFieldAccessFallbackBeanWrapper(Class type, Metamodel metamodel) { super(type); this.metamodel = metamodel; + this.jpaMetamodel = new JpaMetamodel(metamodel); } /** @@ -374,7 +377,7 @@ public class JpaMetamodelEntityInformation extends JpaEntityInformationSu Class idPropertyValueType = idPropertyValue.getClass(); - if (ClassUtils.isPrimitiveOrWrapper(idPropertyValueType)) { + if (!jpaMetamodel.isJpaManaged(idPropertyValueType)) { return idPropertyValue; } diff --git a/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java index df1897361..e1e2bf760 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/EclipseLinkJpaMetamodelEntityInformationIntegrationTests.java @@ -28,6 +28,7 @@ import org.springframework.test.context.ContextConfiguration; * EclipseLink execution for {@link JpaMetamodelEntityInformationIntegrationTests}. * * @author Oliver Gierke + * @author Jens Schauder */ @ContextConfiguration("classpath:eclipselink.xml") public class EclipseLinkJpaMetamodelEntityInformationIntegrationTests @@ -69,6 +70,14 @@ public class EclipseLinkJpaMetamodelEntityInformationIntegrationTests super.detectsVersionPropertyOnMappedSuperClass(); } + /** + * Ignored due to https://bugs.eclipse.org/bugs/show_bug.cgi?id=531528 EclipseLink doesn't support + * {@link javax.persistence.IdClass} referencing inner classes. + */ + @Override + @Ignore + public void correctlyDeterminesIdValueForNestedIdClassesWithNonPrimitiveNonManagedType() {} + @Override protected String getMetadadataPersitenceUnitName() { return "metadata_el"; diff --git a/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java index d1f9d0093..db8a6fda1 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformationIntegrationTests.java @@ -19,6 +19,8 @@ import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.springframework.data.jpa.repository.support.JpaEntityInformationSupport.*; +import lombok.Data; + import java.io.Serializable; import java.sql.Timestamp; import java.util.Date; @@ -42,6 +44,7 @@ import org.springframework.test.util.ReflectionTestUtils; * @author Oliver Gierke * @author Thomas Darimont * @author Christoph Strobl + * @author Jens Schauder */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration({ "classpath:infrastructure.xml" }) @@ -258,6 +261,26 @@ public class JpaMetamodelEntityInformationIntegrationTests { assertThat(ReflectionTestUtils.getField(information, "versionAttribute"), is(notNullValue())); } + @Test // DATAJPA-1105 + public void correctlyDeterminesIdValueForNestedIdClassesWithNonPrimitiveNonManagedType() { + + EntityManagerFactory emf = Persistence.createEntityManagerFactory(getMetadadataPersitenceUnitName()); + EntityManager em = emf.createEntityManager(); + + JpaEntityInformation information = getEntityInformation(EntityWithNestedIdClass.class, + em); + + EntityWithNestedIdClass entity = new EntityWithNestedIdClass(); + entity.id = 23L; + entity.reference = new EntityWithIdClass(); + entity.reference.id1 = "one"; + entity.reference.id2 = "two"; + + Object id = information.getId(entity); + + assertThat(id, is(notNullValue())); + } + protected String getMetadadataPersitenceUnitName() { return "metadata"; } @@ -283,4 +306,36 @@ public class JpaMetamodelEntityInformationIntegrationTests { public static class Sample extends Identifiable { } + + @Entity + @Access(AccessType.FIELD) + @IdClass(EntityWithNestedIdClassPK.class) + public static class EntityWithNestedIdClass { + + @Id Long id; + @Id @ManyToOne private EntityWithIdClass reference; + } + + @Entity + @Access(AccessType.FIELD) + @IdClass(EntityWithIdClassPK.class) + public static class EntityWithIdClass { + + @Id String id1; + @Id String id2; + } + + @Data + public static class EntityWithIdClassPK implements Serializable { + + String id1; + String id2; + } + + @Data + public static class EntityWithNestedIdClassPK implements Serializable { + + Long id; + EntityWithIdClassPK reference; + } } diff --git a/src/test/resources/META-INF/persistence.xml b/src/test/resources/META-INF/persistence.xml index eae1b2ecc..435b4f0df 100644 --- a/src/test/resources/META-INF/persistence.xml +++ b/src/test/resources/META-INF/persistence.xml @@ -94,6 +94,12 @@ org.springframework.data.jpa.domain.sample.MailUser org.springframework.data.jpa.domain.sample.User org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformationIntegrationTests$Sample + + org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformationIntegrationTests$EntityWithNestedIdClass + + + org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformationIntegrationTests$EntityWithIdClass + true @@ -107,8 +113,22 @@ org.springframework.data.jpa.domain.sample.MailUser org.springframework.data.jpa.domain.sample.User org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformationIntegrationTests$Sample + + org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformationIntegrationTests$EntityWithNestedIdClass + + + org.springframework.data.jpa.repository.support.JpaMetamodelEntityInformationIntegrationTests$EntityWithIdClass + org.springframework.data.jpa.domain.sample.Dummy true + + + + + + + + org.apache.openjpa.persistence.PersistenceProviderImpl