DATAJPA-346 - EclipseLink specific workaround for joins in QueryUtils.

Added fix in QueryUtils to work around an EclipseLinks specialty to add strict joins on a call to root.get(…) even if a later root.join(…, JoinType.LEFT) should trump this.

We needed the first call to examine the metamodel of the path obtained to decide whether to join at all in next steps. We now work around this issue by doing a lot of ugly type checking and casting on the Metamodel directly.

We filed https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892 to maybe let EclipseLink improve at that point.

Original pull request: #30.

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 <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property) {
 
-		Path<Object> 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<Object, Object> join = from.join(property.getSegment(), JoinType.LEFT);
 			return (Expression<T>) (property.hasNext() ? toExpressionRecursively((From<?, ?>) join, property.next()) : join);
 		} else {
+			Path<Object> path = from.get(property.getSegment());
 			return (Expression<T>) (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;

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();
 	}
 }

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<User> 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
 	 */