diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index 0fb78d4b5..630c561ba 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -857,7 +857,7 @@ public abstract class QueryUtils { } /** - * Returns an existing join for the given attribute if one already exists or creates a new one if not. + * Returns an existing (fetch) join for the given attribute if one already exists or creates a new one if not. * * @param from the {@link From} to get the current joins from. * @param attribute the {@link Attribute} to look for in the current joins. @@ -866,6 +866,13 @@ public abstract class QueryUtils { */ private static Join getOrCreateJoin(From from, String attribute, JoinType joinType) { + for (Fetch fetch : from.getFetches()) { + + if (fetch instanceof Join join && join.getAttribute().getName().equals(attribute)) { + return join; + } + } + for (Join join : from.getJoins()) { if (join.getAttribute().getName().equals(attribute)) { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java index e60764854..ce1b95d90 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java @@ -15,11 +15,25 @@ */ package org.springframework.data.jpa.repository.query; +import static org.assertj.core.api.Assertions.*; + +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Path; +import jakarta.persistence.criteria.Root; + +import org.junit.jupiter.api.Test; + +import org.springframework.data.jpa.domain.sample.User; +import org.springframework.data.mapping.PropertyPath; import org.springframework.test.context.ContextConfiguration; /** + * EclipseLink variant of {@link QueryUtilsIntegrationTests}. + * * @author Oliver Gierke * @author Jens Schauder + * @author Mark Paluch */ @ContextConfiguration("classpath:eclipselink.xml") class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests { @@ -28,4 +42,25 @@ class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests { return 1; } + @Test // GH-2756 + @Override + void prefersFetchOverJoin() { + + CriteriaBuilder builder = em.getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(User.class); + Root from = query.from(User.class); + from.fetch("manager"); + from.join("manager"); + + PropertyPath managerFirstname = PropertyPath.from("manager.firstname", User.class); + PropertyPath managerLastname = PropertyPath.from("manager.lastname", User.class); + + QueryUtils.toExpressionRecursively(from, managerLastname); + Path expr = (Path) QueryUtils.toExpressionRecursively(from, managerFirstname); + + assertThat(expr.getParentPath()).hasFieldOrPropertyWithValue("isFetch", true); + assertThat(from.getFetches()).hasSize(1); + assertThat(from.getJoins()).hasSize(1); + } + } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java index d2bf333de..1d4f917a5 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java @@ -32,6 +32,7 @@ import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.From; import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.JoinType; +import jakarta.persistence.criteria.Path; import jakarta.persistence.criteria.Root; import jakarta.persistence.spi.PersistenceProvider; import jakarta.persistence.spi.PersistenceProviderResolver; @@ -91,6 +92,44 @@ class QueryUtilsIntegrationTests { assertThat(from.getJoins()).hasSize(1); } + @Test // GH-2756 + void reusesExistingFetchJoinForExpression() { + + CriteriaBuilder builder = em.getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(User.class); + Root from = query.from(User.class); + from.fetch("manager"); + + PropertyPath managerFirstname = PropertyPath.from("manager.firstname", User.class); + PropertyPath managerLastname = PropertyPath.from("manager.lastname", User.class); + + QueryUtils.toExpressionRecursively(from, managerLastname); + QueryUtils.toExpressionRecursively(from, managerFirstname); + + assertThat(from.getFetches()).hasSize(1); + assertThat(from.getJoins()).isEmpty(); + } + + @Test // GH-2756 + void prefersFetchOverJoin() { + + CriteriaBuilder builder = em.getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(User.class); + Root from = query.from(User.class); + from.fetch("manager"); + from.join("manager"); + + PropertyPath managerFirstname = PropertyPath.from("manager.firstname", User.class); + PropertyPath managerLastname = PropertyPath.from("manager.lastname", User.class); + + QueryUtils.toExpressionRecursively(from, managerLastname); + Path expr = (Path) QueryUtils.toExpressionRecursively(from, managerFirstname); + + assertThat(expr.getParentPath()).hasFieldOrPropertyWithValue("fetched", true); + assertThat(from.getFetches()).hasSize(1); + assertThat(from.getJoins()).hasSize(1); + } + @Test // DATAJPA-401, DATAJPA-1238 void createsJoinForNavigationAcrossOptionalAssociation() {