From 3aa0912e15f095dbf8ae3f6caf7c8b22c9287141 Mon Sep 17 00:00:00 2001 From: Matthias Herrmann Date: Mon, 20 Jul 2015 08:45:07 +0200 Subject: [PATCH] DATAJPA-763 - Building up joins now considers already registered fetches, too. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QueryUtils.toExpressionRecursively(…) now also checks the provided root for already existing fetches to avoid creating superfluous join. Original pull request: #151. --- .../data/jpa/repository/query/QueryUtils.java | 15 ++++++++++++++- .../query/QueryUtilsIntegrationTests.java | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) 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 1d1513729..2b9d643ba 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 @@ -40,6 +40,7 @@ import javax.persistence.Parameter; import javax.persistence.Query; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.Expression; +import javax.persistence.criteria.Fetch; import javax.persistence.criteria.From; import javax.persistence.criteria.Join; import javax.persistence.criteria.JoinType; @@ -469,7 +470,7 @@ public abstract class QueryUtils { propertyPathModel = from.get(segment).getModel(); } - if (requiresJoin(propertyPathModel, model instanceof PluralAttribute)) { + if (requiresJoin(propertyPathModel, model instanceof PluralAttribute) && !isAlreadyFetched(from, segment)) { Join join = getOrCreateJoin(from, segment); return (Expression) (property.hasNext() ? toExpressionRecursively(join, property.next()) : join); } else { @@ -544,4 +545,16 @@ public abstract class QueryUtils { return from.join(attribute, JoinType.LEFT); } + + + private static boolean isAlreadyFetched(final From from, final String attribute) { + for(final Fetch f : from.getFetches()) { + final boolean sameName = f.getAttribute().getName().equals(attribute); + + if(sameName && f.getJoinType().equals(JoinType.LEFT)) { + return true; + } + } + 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 4f8deaddc..7e6e60c27 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 @@ -31,6 +31,7 @@ import javax.persistence.Persistence; import javax.persistence.PersistenceContext; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.JoinType; import javax.persistence.criteria.Root; import javax.persistence.spi.PersistenceProvider; import javax.persistence.spi.PersistenceProviderResolver; @@ -39,7 +40,9 @@ import javax.persistence.spi.PersistenceProviderResolverHolder; import org.hibernate.ejb.HibernatePersistence; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.data.jpa.domain.sample.Category; import org.springframework.data.jpa.domain.sample.Order; +import org.springframework.data.jpa.domain.sample.Product; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.mapping.PropertyPath; import org.springframework.test.context.ContextConfiguration; @@ -142,6 +145,21 @@ public class QueryUtilsIntegrationTests { } } + /** + * @see DATAJPA-763 + */ + @Test + public void doesNotCreateAJoinForAlreadyFetchedAssociation() { + + final CriteriaBuilder builder = em.getCriteriaBuilder(); + final CriteriaQuery query = builder.createQuery(Category.class); + final Root root = query.from(Category.class); + root.fetch("product", JoinType.LEFT); + + QueryUtils.toExpressionRecursively(root, PropertyPath.from("product", Category.class)); + assertThat(root.getJoins(), is(empty())); + } + protected void assertNoJoinRequestedForOptionalAssociation(Root root) { assertThat(root.getJoins(), is(empty())); }