DATAJPA-1041 - Polishing.

Ignored test cases failing on EclipseLink. Formatting and a bit of Javadoc.

EntityGraphRepositoryMethodsIntegrationTests.shouldRespectDynamicFetchGraphForGetOneWithAttributeNamesById() has to be ignored now as the new wiping of the EntityManager - which is needed to actually make the test test what's intended to be tested - reveals the same Eclipselink issue the newly introduced tests reveal, too.

Author: odrotbohm

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

@@ -157,9 +157,10 @@ static void configureFetchGraphFrom(JpaEntityGraph jpaEntityGraph, EntityGraph<?
 			// Fast path - just single attribute
 			if (!path.contains(".")) {
 
-				if(findAttributeNode(path, entityGraph) == null) {
+				if (findAttributeNode(path, entityGraph) == null) {
 					entityGraph.addAttributeNodes(path);
 				}
+
 				continue;
 			}
 
@@ -175,26 +176,50 @@ static void configureFetchGraphFrom(JpaEntityGraph jpaEntityGraph, EntityGraph<?
 		}
 	}
 
-	private static Subgraph<?> findOrCreateSubgraph(String attributeNode, EntityGraph<?> entityGraph) {
+	/**
+	 * Returns the {@link Subgraph} with the given name fro the given {@link EntityGraph} or creates a new one if none
+	 * already available.
+	 * 
+	 * @param name
+	 * @param entityGraph
+	 * @return
+	 */
+	private static Subgraph<?> findOrCreateSubgraph(String name, EntityGraph<?> entityGraph) {
+
+		Subgraph<?> subgraph = findSubgraph(name, entityGraph);
 
-		Subgraph<?> subgraph = findSubgraph(attributeNode, entityGraph);
-		return subgraph != null ? subgraph : entityGraph.addSubgraph(attributeNode);
+		return subgraph != null ? subgraph : entityGraph.addSubgraph(name);
 	}
 
-	private static Subgraph<?> findSubgraph(String attributeNode, EntityGraph<?> entityGraph) {
+	/**
+	 * Returns the {@link Subgraph} with the given name from the given {@link EntityGraph}.
+	 * 
+	 * @param name
+	 * @param entityGraph
+	 * @return
+	 */
+	private static Subgraph<?> findSubgraph(String name, EntityGraph<?> entityGraph) {
+
+		AttributeNode<?> node = findAttributeNode(name, entityGraph);
 
-		AttributeNode<?> node = findAttributeNode(attributeNode, entityGraph);
-		if(node != null && !ObjectUtils.isEmpty(node.getSubgraphs())) {
+		if (node != null && !ObjectUtils.isEmpty(node.getSubgraphs())) {
 			return node.getSubgraphs().values().iterator().next();
 		}
 
 		return null;
 	}
 
-	private static AttributeNode<?> findAttributeNode(String attributeNode, EntityGraph<?> entityGraph) {
+	/**
+	 * Returns the {@link AttributeNode} with the given name if present in the given {@link EntityGraph}.
+	 * 
+	 * @param name
+	 * @param entityGraph must not be {@literal null}.
+	 * @return
+	 */
+	private static AttributeNode<?> findAttributeNode(String name, EntityGraph<?> entityGraph) {
 
-		for(AttributeNode<?> node : entityGraph.getAttributeNodes()) {
-			if(ObjectUtils.nullSafeEquals(node.getAttributeName(), attributeNode)) {
+		for (AttributeNode<?> node : entityGraph.getAttributeNodes()) {
+			if (ObjectUtils.nullSafeEquals(node.getAttributeName(), name)) {
 				return node;
 			}
 		}

src/test/java/org/springframework/data/jpa/domain/sample/User.java

@@ -53,30 +53,27 @@
  * @author Christoph Strobl
  */
 @Entity
-@NamedEntityGraphs({
-		@NamedEntityGraph(name = "User.overview", attributeNodes = { @NamedAttributeNode("roles") }),
-		@NamedEntityGraph(name = "User.detail", attributeNodes = { @NamedAttributeNode("roles"),
-				@NamedAttributeNode("manager"), @NamedAttributeNode("colleagues") }),
-		@NamedEntityGraph(name = "User.getOneWithDefinedEntityGraphById", attributeNodes = { @NamedAttributeNode("roles"),
-				@NamedAttributeNode("manager"), @NamedAttributeNode("colleagues") }),
-        @NamedEntityGraph(name = "User.withSubGraph",
-				attributeNodes = {
-        			@NamedAttributeNode("roles"),
-					@NamedAttributeNode(value="colleagues", subgraph = "User.colleagues")
-				},
-				subgraphs = {
-        			@NamedSubgraph(name = "User.colleagues", attributeNodes = {@NamedAttributeNode("colleagues"), @NamedAttributeNode("roles")})
-				}
-		)})
+@NamedEntityGraphs({ @NamedEntityGraph(name = "User.overview", attributeNodes = { @NamedAttributeNode("roles") }),
+		@NamedEntityGraph(name = "User.detail",
+				attributeNodes = { @NamedAttributeNode("roles"), @NamedAttributeNode("manager"),
+						@NamedAttributeNode("colleagues") }),
+		@NamedEntityGraph(name = "User.getOneWithDefinedEntityGraphById",
+				attributeNodes = { @NamedAttributeNode("roles"), @NamedAttributeNode("manager"),
+						@NamedAttributeNode("colleagues") }),
+		@NamedEntityGraph(name = "User.withSubGraph",
+				attributeNodes = { @NamedAttributeNode("roles"),
+						@NamedAttributeNode(value = "colleagues", subgraph = "User.colleagues") },
+				subgraphs = { @NamedSubgraph(name = "User.colleagues",
+						attributeNodes = { @NamedAttributeNode("colleagues"), @NamedAttributeNode("roles") }) }) })
 @NamedQuery(name = "User.findByEmailAddress", query = "SELECT u FROM User u WHERE u.emailAddress = ?1")
 @NamedStoredProcedureQueries({ //
-@NamedStoredProcedureQuery(name = "User.plus1", procedureName = "plus1inout", parameters = {
-		@StoredProcedureParameter(mode = ParameterMode.IN, name = "arg", type = Integer.class),
-		@StoredProcedureParameter(mode = ParameterMode.OUT, name = "res", type = Integer.class) }) //
+		@NamedStoredProcedureQuery(name = "User.plus1", procedureName = "plus1inout",
+				parameters = { @StoredProcedureParameter(mode = ParameterMode.IN, name = "arg", type = Integer.class),
+						@StoredProcedureParameter(mode = ParameterMode.OUT, name = "res", type = Integer.class) }) //
 })
-@NamedStoredProcedureQuery(name = "User.plus1IO", procedureName = "plus1inout", parameters = {
-		@StoredProcedureParameter(mode = ParameterMode.IN, name = "arg", type = Integer.class),
-		@StoredProcedureParameter(mode = ParameterMode.OUT, name = "res", type = Integer.class) })
+@NamedStoredProcedureQuery(name = "User.plus1IO", procedureName = "plus1inout",
+		parameters = { @StoredProcedureParameter(mode = ParameterMode.IN, name = "arg", type = Integer.class),
+				@StoredProcedureParameter(mode = ParameterMode.OUT, name = "res", type = Integer.class) })
 @Table(name = "SD_User")
 public class User {
 
@@ -392,7 +389,7 @@ public Date getDateOfBirth() {
 	public void setDateOfBirth(Date dateOfBirth) {
 		this.dateOfBirth = dateOfBirth;
 	}
-	
+
 	public void setCreatedAt(Date createdAt) {
 		this.createdAt = createdAt;
 	}

src/test/java/org/springframework/data/jpa/repository/EclipseLinkEntityGraphRepositoryMethodsIntegrationTests.java

@@ -15,13 +15,26 @@
  */
 package org.springframework.data.jpa.repository;
 
+import org.junit.Ignore;
+import org.junit.Test;
 import org.springframework.test.context.ContextConfiguration;
 
 /**
  * @author Oliver Gierke
  */
 @ContextConfiguration("classpath:eclipselink.xml")
-public class EclipseLinkEntityGraphRepositoryMethodsIntegrationTests extends
-		EntityGraphRepositoryMethodsIntegrationTests {
+public class EclipseLinkEntityGraphRepositoryMethodsIntegrationTests
+		extends EntityGraphRepositoryMethodsIntegrationTests {
 
+	@Ignore
+	@Test
+	public void shouldRespectNamedEntitySubGraph() {}
+
+	@Ignore
+	@Test
+	public void shouldRespectMultipleSubGraphForSameAttributeWithDynamicFetchGraph() {}
+
+	@Ignore
+	@Test
+	public void shouldRespectDynamicFetchGraphForGetOneWithAttributeNamesById() {}
 }

src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java

@@ -19,11 +19,11 @@
 import static org.junit.Assert.*;
 import static org.springframework.data.jpa.support.EntityManagerTestUtils.*;
 
-import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
 
-import javax.persistence.*;
+import javax.persistence.EntityManager;
+import javax.persistence.Persistence;
+import javax.persistence.PersistenceUtil;
 
 import org.junit.Assume;
 import org.junit.Before;
@@ -39,7 +39,6 @@
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.transaction.annotation.Transactional;
-import org.springframework.util.StringUtils;
 
 /**
  * Integration tests for RepositoryMethodsWithEntityGraphConfigJpaRepository.
@@ -62,6 +61,8 @@ public class EntityGraphRepositoryMethodsIntegrationTests {
 	User christoph;
 	Role role;
 
+	PersistenceUtil util = Persistence.getPersistenceUtil();
+
 	@Before
 	public void setup() {
 
@@ -91,7 +92,7 @@ public void shouldRespectConfiguredJpaEntityGraph() {
 		List<User> result = repository.findAll();
 
 		assertThat(result.size(), is(3));
-		assertThat(Persistence.getPersistenceUtil().isLoaded(result.get(0), "roles"), is(true));
+		assertThat(util.isLoaded(result.get(0), "roles"), is(true));
 		assertThat(result.get(0), is(tom));
 	}
 
@@ -103,8 +104,8 @@ public void shouldRespectConfiguredJpaEntityGraphInFindOne() {
 		User user = repository.findOne(tom.getId());
 
 		assertThat(user, is(notNullValue()));
-		assertThat("colleages should be fetched with 'user.detail' fetchgraph",
-				Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true));
+		assertThat("colleages should be fetched with 'user.detail' fetchgraph", util.isLoaded(user, "colleagues"),
+				is(true));
 	}
 
 	@Test // DATAJPA-696
@@ -115,8 +116,8 @@ public void shouldRespectInferFetchGraphFromMethodName() {
 		User user = repository.getOneWithDefinedEntityGraphById(tom.getId());
 
 		assertThat(user, is(notNullValue()));
-		assertThat("colleages should be fetched with 'user.detail' fetchgraph",
-				Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true));
+		assertThat("colleages should be fetched with 'user.detail' fetchgraph", util.isLoaded(user, "colleagues"),
+				is(true));
 	}
 
 	@Test // DATAJPA-696
@@ -130,12 +131,13 @@ public void shouldRespectDynamicFetchGraphForGetOneWithAttributeNamesById() {
 		User user = repository.getOneWithAttributeNamesById(tom.getId());
 
 		assertThat(user, is(notNullValue()));
-		assertThat("colleages should be fetched with 'user.detail' fetchgraph",
-				Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true));
-		assertThat(Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true));
+
+		assertThat("colleages should be fetched with 'user.detail' fetchgraph", util.isLoaded(user, "colleagues"),
+				is(true));
+		assertThat(util.isLoaded(user, "colleagues"), is(true));
 
 		for (User colleague : user.getColleagues()) {
-			assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "roles"), is(true));
+			assertThat(util.isLoaded(colleague, "roles"), is(true));
 		}
 	}
 
@@ -148,7 +150,7 @@ public void shouldRespectConfiguredJpaEntityGraphWithPaginationAndQueryDslPredic
 		List<User> result = page.getContent();
 
 		assertThat(result.size(), is(3));
-		assertThat(Persistence.getPersistenceUtil().isLoaded(result.get(0).getRoles()), is(true));
+		assertThat(util.isLoaded(result.get(0).getRoles()), is(true));
 		assertThat(result.get(0), is(tom));
 	}
 
@@ -165,11 +167,11 @@ public void shouldRespectNamedEntitySubGraph() {
 		assertThat(user, is(notNullValue()));
 
 		assertThat("colleagues on root should have been fetched by named 'User.colleagues' subgraph declaration",
-				Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true));
+				util.isLoaded(user, "colleagues"), is(true));
 
 		for (User colleague : user.getColleagues()) {
-			assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "colleagues"), is(true));
-			assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "roles"), is(true));
+			assertThat(util.isLoaded(colleague, "colleagues"), is(true));
+			assertThat(util.isLoaded(colleague, "roles"), is(true));
 		}
 	}
 
@@ -186,55 +188,11 @@ public void shouldRespectMultipleSubGraphForSameAttributeWithDynamicFetchGraph()
 		assertThat(user, is(notNullValue()));
 
 		assertThat("colleagues on root should have been fetched by dynamic subgraph declaration",
-				Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true));
+				util.isLoaded(user, "colleagues"), is(true));
 
 		for (User colleague : user.getColleagues()) {
-			assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "colleagues"), is(true));
-			assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "roles"), is(true));
-		}
-	}
-
-	@Test // DATAJPA-1041 - TODO: remove when done fighting with eclipselink.
-	public void thisOneFailsWithEclipselink() {
-
-		Assume.assumeTrue(currentEntityManagerIsAJpa21EntityManager(em));
-
-		em.flush();
-		em.clear();
-
-		javax.persistence.EntityGraph<?> graph = em.getEntityGraph("User.withSubGraph");
-
-		printGraph(graph);
-
-		User result = (User) em.createQuery("Select u from User u where u.id = " + tom.getId())
-				.setHint("javax.persistence.loadgraph", graph).getResultList().get(0);
-
-		assertThat(Persistence.getPersistenceUtil().isLoaded(result, "roles"), is(true));
-		assertThat(Persistence.getPersistenceUtil().isLoaded(result, "colleagues"), is(true));
-	}
-
-	private void printGraph(javax.persistence.EntityGraph<?> graph) {
-
-		try {
-			for (AttributeNode<?> node : graph.getAttributeNodes()) {
-				System.out.println("|- node.getAttributeName(): " + node.getAttributeName());
-				for (Map.Entry<Class, Subgraph> subGraph : node.getSubgraphs().entrySet()) {
-					System.out.print("|  +- subGraph: " + subGraph.getKey().getSimpleName() + " -> [");
-
-					Iterator it = subGraph.getValue().getAttributeNodes().iterator();
-					while (it.hasNext()) {
-
-						AttributeNode<?> an = (AttributeNode<?>) it.next();
-						System.out.print(an.getAttributeName());
-						if (it.hasNext()) {
-							System.out.print(", ");
-						}
-					}
-					System.out.println("]");
-				}
-			}
-		} catch (Exception e) {
-			// o_O what happened here - ignore it - it's just debug output.
+			assertThat(util.isLoaded(colleague, "colleagues"), is(true));
+			assertThat(util.isLoaded(colleague, "roles"), is(true));
 		}
 	}
 }

src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigRepository.java

@@ -69,5 +69,4 @@ public interface RepositoryMethodsWithEntityGraphConfigRepository
 	// DATAJPA-1041
 	@EntityGraph(attributePaths = { "colleagues", "colleagues.roles", "colleagues.colleagues" })
 	User findOneWithMultipleSubGraphsById(Integer id);
-
 }