From 8a1b4365a2d5bdae23bed6618309c3151467dc7a Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 4 Mar 2014 11:20:48 +0100 Subject: [PATCH] DATAJPA-476 - Mitigate spec violations in Hibernate for query creation. Hibernate invalidly returns null for getModel() on its PluralAttribute implementation which causes the necessity for joins not having been detected previously. We now fall back to joining in case we don't find a Model and deal with a PluralAttribute. --- .../data/jpa/repository/query/QueryUtils.java | 23 ++++++---- .../query/QueryUtilsIntegrationTests.java | 42 +++++++++++++++++++ src/test/resources/META-INF/persistence.xml | 12 ++++++ 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index 6628a2c2e..66e8338f8 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -50,6 +50,7 @@ import javax.persistence.metamodel.Attribute; import javax.persistence.metamodel.Attribute.PersistentAttributeType; import javax.persistence.metamodel.Bindable; import javax.persistence.metamodel.ManagedType; +import javax.persistence.metamodel.PluralAttribute; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.domain.Sort; @@ -439,22 +440,25 @@ public abstract class QueryUtils { static Expression toExpressionRecursively(From from, PropertyPath property) { Bindable propertyPathModel = null; - if (from.getModel() instanceof ManagedType) { + Bindable model = from.getModel(); + String segment = property.getSegment(); + + if (model instanceof ManagedType) { /* * Required to keep support for EclipseLink 2.4.x. TODO: Remove once we drop that (probably Dijkstra M1) * See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892 */ - propertyPathModel = (Bindable) ((ManagedType) from.getModel()).getAttribute(property.getSegment()); + propertyPathModel = (Bindable) ((ManagedType) model).getAttribute(segment); } else { - propertyPathModel = from.get(property.getSegment()).getModel(); + propertyPathModel = from.get(segment).getModel(); } - if (requiresJoin(propertyPathModel)) { - Join join = getOrCreateJoin(from, property.getSegment()); + if (requiresJoin(propertyPathModel, model instanceof PluralAttribute)) { + Join join = getOrCreateJoin(from, segment); return (Expression) (property.hasNext() ? toExpressionRecursively(join, property.next()) : join); } else { - Path path = from.get(property.getSegment()); + Path path = from.get(segment); return (Expression) (property.hasNext() ? toExpressionRecursively(path, property.next()) : path); } } @@ -464,9 +468,14 @@ public abstract class QueryUtils { * non-optional association. * * @param propertyPathModel must not be {@literal null}. + * @param for * @return */ - private static boolean requiresJoin(Bindable propertyPathModel) { + private static boolean requiresJoin(Bindable propertyPathModel, boolean forPluralAttribute) { + + if (propertyPathModel == null && forPluralAttribute) { + return true; + } if (!(propertyPathModel instanceof Attribute)) { return false; diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java index cc979cc2f..65a0f0b7c 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java @@ -18,7 +18,14 @@ package org.springframework.data.jpa.repository.query; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import java.util.Set; + +import javax.persistence.Entity; import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Id; +import javax.persistence.OneToMany; +import javax.persistence.Persistence; import javax.persistence.PersistenceContext; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; @@ -105,7 +112,42 @@ public class QueryUtilsIntegrationTests { assertThat(root.getJoins(), hasSize(1)); } + /** + * @see DATAJPA-476 + */ + @Test + public void traversesPluralAttributeCorrectly() { + + EntityManagerFactory entityManagerFactory = Persistence.createEntityManagerFactory("merchant"); + CriteriaBuilder builder = entityManagerFactory.createEntityManager().getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(Merchant.class); + Root root = query.from(Merchant.class); + + QueryUtils.toExpressionRecursively(root, PropertyPath.from("employeesCredentialsUid", Merchant.class)); + } + protected void assertNoJoinRequestedForOptionalAssociation(Root root) { assertThat(root.getJoins(), is(empty())); } + + @Entity + static class Merchant { + + @Id String id; + @OneToMany Set employees; + } + + @Entity + static class Employee { + + @Id String id; + @OneToMany Set credentials; + } + + @Entity + static class Credential { + + @Id String id; + String uid; + } } diff --git a/src/test/resources/META-INF/persistence.xml b/src/test/resources/META-INF/persistence.xml index 4ada165df..a67914334 100644 --- a/src/test/resources/META-INF/persistence.xml +++ b/src/test/resources/META-INF/persistence.xml @@ -55,6 +55,18 @@ + + + org.springframework.data.jpa.domain.sample.User + org.springframework.data.jpa.repository.query.QueryUtilsIntegrationTests$Merchant + org.springframework.data.jpa.repository.query.QueryUtilsIntegrationTests$Employee + org.springframework.data.jpa.repository.query.QueryUtilsIntegrationTests$Credential + true + + + + +