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 c3d071399..37aca64bf 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 @@ -43,6 +43,7 @@ import javax.persistence.metamodel.Attribute; import javax.persistence.metamodel.Attribute.PersistentAttributeType; import javax.persistence.metamodel.Bindable; import javax.persistence.metamodel.Bindable.BindableType; +import javax.persistence.metamodel.ManagedType; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; @@ -428,33 +429,42 @@ public abstract class QueryUtils { @SuppressWarnings("unchecked") static Expression toExpressionRecursively(From from, PropertyPath property) { - Path path = from.get(property.getSegment()); + Bindable propertyPathModel = null; + if (from.getModel() instanceof ManagedType) { + /* + * Avoid calling from.get(...) because this triggers the generation of an inner-join instead + * of and outer-join in eclipse-link. + * See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892 + */ + propertyPathModel = (Bindable) ((ManagedType) from.getModel()).getAttribute(property.getSegment()); + } else { + propertyPathModel = (Bindable) from.get(property.getSegment()).getModel(); + } - if (property.isCollection() || isEntityPath(path)) { + if (property.isCollection() || isEntityPath(propertyPathModel)) { Join join = from.join(property.getSegment(), JoinType.LEFT); return (Expression) (property.hasNext() ? toExpressionRecursively((From) join, property.next()) : join); } else { + Path path = from.get(property.getSegment()); return (Expression) (property.hasNext() ? toExpressionRecursively(path, property.next()) : path); } } /** - * Returns whether the given path can be considered referring an entity. + * Returns whether the given {@code propertyPathModel} can be considered referring an entity. * - * @param path must not be {@literal null}. + * @param propertyPathModel must not be {@literal null}. * @return */ - private static boolean isEntityPath(Path path) { + private static boolean isEntityPath(Bindable propertyPathModel) { - Bindable model = path.getModel(); - - if (BindableType.ENTITY_TYPE.equals(model.getBindableType())) { + if (BindableType.ENTITY_TYPE.equals(propertyPathModel.getBindableType())) { return true; } - if (model instanceof Attribute) { + if (propertyPathModel instanceof Attribute) { - Attribute attribute = (Attribute) model; + Attribute attribute = (Attribute) propertyPathModel; if (attribute.isAssociation()) { return true; diff --git a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java index 617107dac..7327b1dcf 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -51,6 +51,16 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi } @Override - public void doesNotDropNullValuesOnPagedSpecificationExecution() { + public void doesNotDropNullValuesOnPagedSpecificationExecution() {} + + /** + * Works with a workaround in QueryUtils.toExpressionRecursively(…). TODO: remove once EclipseLink bug is fixed. + * + * @see DATAJPA-346 + * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892 + */ + @Override + public void shouldGenerateLeftOuterJoinInfindAllWithPaginationAndSortOnNestedPropertyPath() { + super.shouldGenerateLeftOuterJoinInfindAllWithPaginationAndSortOnNestedPropertyPath(); } } diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index 2b162bfe5..a8a41caf9 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -74,12 +74,10 @@ import org.springframework.transaction.annotation.Transactional; @Transactional public class UserRepositoryTests { - @PersistenceContext - EntityManager em; + @PersistenceContext EntityManager em; // CUT - @Autowired - UserRepository repository; + @Autowired UserRepository repository; // Test fixture User firstUser, secondUser, thirdUser, fourthUser; @@ -1000,6 +998,28 @@ public class UserRepositoryTests { assertThat(page, hasItem(firstUser)); } + /** + * @see DATAJPA-346 + */ + @Test + public void shouldGenerateLeftOuterJoinInfindAllWithPaginationAndSortOnNestedPropertyPath() { + + firstUser.setManager(null); + secondUser.setManager(null); + thirdUser.setManager(firstUser); // manager Oliver + fourthUser.setManager(secondUser); // manager Joachim + + flushTestUsers(); + + Page pages = repository.findAll(new PageRequest(0, 4, new Sort(Sort.Direction.ASC, "manager.firstname"))); + assertThat(pages.getSize(), is(4)); + assertThat(pages.getContent().get(0).getManager(), is(nullValue())); + assertThat(pages.getContent().get(1).getManager(), is(nullValue())); + assertThat(pages.getContent().get(2).getManager().getFirstname(), is("Joachim")); + assertThat(pages.getContent().get(3).getManager().getFirstname(), is("Oliver")); + assertThat(pages.getTotalElements(), is(4L)); + } + /** * @see DATAJPA-292 */