DATAJPA-763 - Polishing.

Reworked integration test to make it work on EclipseLink, too. Simply checking for new no joins doesn't work here as it even adds joins for plain attribute traversals using root.get(…). We're now using Mockito to verify the expected behavior on the interaction level instead of the Root's state.

Removed superfluous finals. Added some Javadoc where necessary.

Original pull request: #151.

src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

@@ -546,15 +546,24 @@ public abstract class QueryUtils {
 		return from.join(attribute, JoinType.LEFT);
 	}
 
+	/**
+	 * Return whether the given {@link From} contains a fetch declaration for the attribute with the given name.
+	 * 
+	 * @param from the {@link From} to check for fetches.
+	 * @param attribute the attribute name to check.
+	 * @return
+	 */
+	private static boolean isAlreadyFetched(From<?, ?> from, String attribute) {
 
-	private static boolean isAlreadyFetched(final From<?, ?> from, final String attribute) {
-		for(final Fetch<?, ?> f : from.getFetches()) {
-			final boolean sameName = f.getAttribute().getName().equals(attribute);
+		for (Fetch<?, ?> f : from.getFetches()) {
 
-			if(sameName && f.getJoinType().equals(JoinType.LEFT)) {
+			boolean sameName = f.getAttribute().getName().equals(attribute);
+
+			if (sameName && f.getJoinType().equals(JoinType.LEFT)) {
 				return true;
 			}
 		}
+
 		return false;
 	}
 }

src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,35 +15,10 @@
  */
 package org.springframework.data.jpa.repository.query;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-
-import java.util.Set;
-
-import javax.persistence.criteria.Join;
-import javax.persistence.criteria.JoinType;
-import javax.persistence.criteria.Root;
-
-import org.springframework.data.jpa.domain.sample.Order;
 import org.springframework.test.context.ContextConfiguration;
 
 /**
  * @author Oliver Gierke
  */
 @ContextConfiguration("classpath:eclipselink.xml")
-public class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests {
-
-	/**
-	 * Required as EclipseLink generates an inner join for plain association traversal.
-	 */
-	@Override
-	protected void assertNoJoinRequestedForOptionalAssociation(Root<Order> root) {
-
-		Set<Join<Order, ?>> joins = root.getJoins();
-		assertThat(joins, hasSize(1));
-
-		Join<Order, ?> join = joins.iterator().next();
-		assertThat(join.getAttribute().getName(), is("manager"));
-		assertThat(join.getJoinType(), is(JoinType.INNER));
-	}
-}
+public class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests {}

src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java

@@ -17,8 +17,10 @@ package org.springframework.data.jpa.repository.query;
 
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
@@ -40,9 +42,9 @@ import javax.persistence.spi.PersistenceProviderResolverHolder;
 import org.hibernate.ejb.HibernatePersistence;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 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;
@@ -149,19 +151,22 @@ public class QueryUtilsIntegrationTests {
 	 * @see DATAJPA-763
 	 */
 	@Test
+	@SuppressWarnings("unchecked")
 	public void doesNotCreateAJoinForAlreadyFetchedAssociation() {
 
-		final CriteriaBuilder builder = em.getCriteriaBuilder();
-		final CriteriaQuery<Category> query = builder.createQuery(Category.class);
-		final Root<Category> root = query.from(Category.class);
-		root.fetch("product", JoinType.LEFT);
+		CriteriaBuilder builder = em.getCriteriaBuilder();
+		CriteriaQuery<Category> query = builder.createQuery(Category.class);
 
-		QueryUtils.toExpressionRecursively(root, PropertyPath.from("product", Category.class));
-		assertThat(root.getJoins(), is(empty()));
-	}
+		Root<Category> root = query.from(Category.class);
 
-	protected void assertNoJoinRequestedForOptionalAssociation(Root<Order> root) {
-		assertThat(root.getJoins(), is(empty()));
+		Root<Category> mock = Mockito.mock(Root.class);
+		doReturn(root.getModel()).when(mock).getModel();
+		doReturn(Collections.singleton(root.fetch("product", JoinType.LEFT))).when(mock).getFetches();
+
+		QueryUtils.toExpressionRecursively(mock, PropertyPath.from("product", Category.class));
+
+		verify(mock, times(1)).get("product");
+		verify(mock, times(0)).join(Mockito.eq("product"), Mockito.any(JoinType.class));
 	}
 
 	@Entity