diff --git a/src/main/java/org/springframework/data/jpa/repository/query/Jpa21Utils.java b/src/main/java/org/springframework/data/jpa/repository/query/Jpa21Utils.java index e9a674799..c57fa311a 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/Jpa21Utils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/Jpa21Utils.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import javax.persistence.AttributeNode; import javax.persistence.EntityGraph; import javax.persistence.EntityManager; import javax.persistence.Query; @@ -28,6 +29,7 @@ import javax.persistence.Subgraph; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; @@ -36,6 +38,7 @@ import org.springframework.util.StringUtils; * * @author Thomas Darimont * @author Oliver Gierke + * @author Christoph Strobl * @since 1.6 */ public class Jpa21Utils { @@ -153,7 +156,10 @@ public class Jpa21Utils { // Fast path - just single attribute if (!path.contains(".")) { - entityGraph.addAttributeNodes(path); + + if(findAttributeNode(path, entityGraph) == null) { + entityGraph.addAttributeNodes(path); + } continue; } @@ -162,10 +168,37 @@ public class Jpa21Utils { Subgraph parent = null; for (int c = 0; c < pathComponents.length - 1; c++) { - parent = c == 0 ? entityGraph.addSubgraph(pathComponents[c]) : parent.addSubgraph(pathComponents[c]); + parent = c == 0 ? findOrCreateSubgraph(pathComponents[c], entityGraph) : parent.addSubgraph(pathComponents[c]); } parent.addAttributeNodes(pathComponents[pathComponents.length - 1]); } } + + private static Subgraph findOrCreateSubgraph(String attributeNode, EntityGraph entityGraph) { + + Subgraph subgraph = findSubgraph(attributeNode, entityGraph); + return subgraph != null ? subgraph : entityGraph.addSubgraph(attributeNode); + } + + private static Subgraph findSubgraph(String attributeNode, EntityGraph entityGraph) { + + AttributeNode node = findAttributeNode(attributeNode, entityGraph); + if(node != null && !ObjectUtils.isEmpty(node.getSubgraphs())) { + return node.getSubgraphs().values().iterator().next(); + } + + return null; + } + + private static AttributeNode findAttributeNode(String attributeNode, EntityGraph entityGraph) { + + for(AttributeNode node : entityGraph.getAttributeNodes()) { + if(ObjectUtils.nullSafeEquals(node.getAttributeName(), attributeNode)) { + return node; + } + } + + return null; + } } diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/User.java b/src/test/java/org/springframework/data/jpa/domain/sample/User.java index a1968272a..e5e09a169 100644 --- a/src/test/java/org/springframework/data/jpa/domain/sample/User.java +++ b/src/test/java/org/springframework/data/jpa/domain/sample/User.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2015 the original author or authors. + * Copyright 2008-2017 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. @@ -37,6 +37,7 @@ import javax.persistence.NamedEntityGraphs; import javax.persistence.NamedQuery; import javax.persistence.NamedStoredProcedureQueries; import javax.persistence.NamedStoredProcedureQuery; +import javax.persistence.NamedSubgraph; import javax.persistence.ParameterMode; import javax.persistence.StoredProcedureParameter; import javax.persistence.Table; @@ -49,6 +50,7 @@ import javax.persistence.TemporalType; * * @author Oliver Gierke * @author Thomas Darimont + * @author Christoph Strobl */ @Entity @NamedEntityGraphs({ @@ -56,7 +58,16 @@ import javax.persistence.TemporalType; @NamedEntityGraph(name = "User.detail", attributeNodes = { @NamedAttributeNode("roles"), @NamedAttributeNode("manager"), @NamedAttributeNode("colleagues") }), @NamedEntityGraph(name = "User.getOneWithDefinedEntityGraphById", attributeNodes = { @NamedAttributeNode("roles"), - @NamedAttributeNode("manager"), @NamedAttributeNode("colleagues") }) }) + @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 = { diff --git a/src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java index a2afe7734..45e3ef21d 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java @@ -19,10 +19,11 @@ import static org.hamcrest.Matchers.*; 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.EntityManager; -import javax.persistence.Persistence; +import javax.persistence.*; import org.junit.Assume; import org.junit.Before; @@ -38,6 +39,7 @@ import org.springframework.data.jpa.repository.sample.RepositoryMethodsWithEntit 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. @@ -45,6 +47,7 @@ import org.springframework.transaction.annotation.Transactional; * @author Thomas Darimont * @author Oliver Gierke * @author Jocelyn Ntakpe + * @author Christoph Strobl */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:config/namespace-autoconfig-context.xml") @@ -56,6 +59,7 @@ public class EntityGraphRepositoryMethodsIntegrationTests { User tom; User ollie; + User christoph; Role role; @Before @@ -63,6 +67,7 @@ public class EntityGraphRepositoryMethodsIntegrationTests { tom = new User("Thomas", "Darimont", "tdarimont@example.org"); ollie = new User("Oliver", "Gierke", "ogierke@example.org"); + christoph = new User("Christoph", "Strobl", "cstrobl@example.org"); role = new Role("Developer"); em.persist(role); @@ -71,6 +76,11 @@ public class EntityGraphRepositoryMethodsIntegrationTests { ollie = repository.save(ollie); tom.getColleagues().add(ollie); + + christoph = repository.save(christoph); + + ollie.getColleagues().add(christoph); + repository.save(ollie); } @Test // DATAJPA-612 @@ -80,8 +90,8 @@ public class EntityGraphRepositoryMethodsIntegrationTests { List result = repository.findAll(); - assertThat(result.size(), is(2)); - assertThat(Persistence.getPersistenceUtil().isLoaded(result.get(0).getRoles()), is(true)); + assertThat(result.size(), is(3)); + assertThat(Persistence.getPersistenceUtil().isLoaded(result.get(0), "roles"), is(true)); assertThat(result.get(0), is(tom)); } @@ -94,7 +104,7 @@ public class EntityGraphRepositoryMethodsIntegrationTests { assertThat(user, is(notNullValue())); assertThat("colleages should be fetched with 'user.detail' fetchgraph", - Persistence.getPersistenceUtil().isLoaded(user.getColleagues()), is(true)); + Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true)); } @Test // DATAJPA-696 @@ -106,7 +116,7 @@ public class EntityGraphRepositoryMethodsIntegrationTests { assertThat(user, is(notNullValue())); assertThat("colleages should be fetched with 'user.detail' fetchgraph", - Persistence.getPersistenceUtil().isLoaded(user.getColleagues()), is(true)); + Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true)); } @Test // DATAJPA-696 @@ -114,11 +124,19 @@ public class EntityGraphRepositoryMethodsIntegrationTests { Assume.assumeTrue(currentEntityManagerIsAJpa21EntityManager(em)); + em.flush(); + em.clear(); + User user = repository.getOneWithAttributeNamesById(tom.getId()); assertThat(user, is(notNullValue())); assertThat("colleages should be fetched with 'user.detail' fetchgraph", - Persistence.getPersistenceUtil().isLoaded(user.getColleagues()), is(true)); + Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true)); + assertThat(Persistence.getPersistenceUtil().isLoaded(user, "colleagues"), is(true)); + + for (User colleague : user.getColleagues()) { + assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "roles"), is(true)); + } } @Test // DATAJPA-790 @@ -129,8 +147,94 @@ public class EntityGraphRepositoryMethodsIntegrationTests { Page page = repository.findAll(QUser.user.firstname.isNotNull(), new PageRequest(0, 100)); List result = page.getContent(); - assertThat(result.size(), is(2)); + assertThat(result.size(), is(3)); assertThat(Persistence.getPersistenceUtil().isLoaded(result.get(0).getRoles()), is(true)); assertThat(result.get(0), is(tom)); } + + @Test // DATAJPA-1041 + public void shouldRespectNamedEntitySubGraph() { + + Assume.assumeTrue(currentEntityManagerIsAJpa21EntityManager(em)); + + em.flush(); + em.clear(); + + User user = repository.findOneWithMultipleSubGraphsUsingNamedEntityGraphById(tom.getId()); + + 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)); + + for (User colleague : user.getColleagues()) { + assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "colleagues"), is(true)); + assertThat(Persistence.getPersistenceUtil().isLoaded(colleague, "roles"), is(true)); + } + } + + @Test // DATAJPA-1041 + public void shouldRespectMultipleSubGraphForSameAttributeWithDynamicFetchGraph() { + + Assume.assumeTrue(currentEntityManagerIsAJpa21EntityManager(em)); + + em.flush(); + em.clear(); + + User user = repository.findOneWithMultipleSubGraphsById(tom.getId()); + + assertThat(user, is(notNullValue())); + + assertThat("colleagues on root should have been fetched by dynamic subgraph declaration", + Persistence.getPersistenceUtil().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 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. + } + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigRepository.java index b2a461a51..f7ea9c14e 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigRepository.java @@ -33,6 +33,7 @@ import com.querydsl.core.types.Predicate; * * @author Thomas Darimont * @author Jocelyn Ntakpe + * @author Christoph Strobl */ public interface RepositoryMethodsWithEntityGraphConfigRepository extends CrudRepository, QueryDslPredicateExecutor { @@ -60,4 +61,13 @@ public interface RepositoryMethodsWithEntityGraphConfigRepository // DATAJPA-790 @EntityGraph("User.detail") Page findAll(Predicate predicate, Pageable pageable); + + // DATAJPA-1041 + @EntityGraph(type = EntityGraphType.FETCH, value = "User.withSubGraph") + User findOneWithMultipleSubGraphsUsingNamedEntityGraphById(Integer id); + + // DATAJPA-1041 + @EntityGraph(attributePaths = { "colleagues", "colleagues.roles", "colleagues.colleagues" }) + User findOneWithMultipleSubGraphsById(Integer id); + }