From d07aeae81450237f49b069894248c8da5814828a Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Mon, 9 Mar 2015 18:27:56 +0100 Subject: [PATCH] =?UTF-8?q?DATAJPA-689=20-=20Allow=20@EntityGraph=20on=20C?= =?UTF-8?q?rudRepository.findOne(=E2=80=A6).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now honor @EntityGraph definitions on CrudRepository.findOne(…) which was previously only the case for methods that created a Query explicitly. Extracted tryGetFetchGraphHints(…) method from tryConfigureFetchGraph(…) method in Jpa21Utils to allow EntityGraph hints to be used in SimpleJpaRepository.findOne(…). Construction of query hints from context information in SimpleJpaRepository is now performed via the getQueryHints(…) method. Adjusted QueryDslJpaRepository to use query hints as well. Added unit and integration tests to verify that @EntityGraph information is propagated to findOne executions. Original pull request: #137. --- .../repository/query/AbstractJpaQuery.java | 10 +++-- .../data/jpa/repository/query/Jpa21Utils.java | 24 ++++++------ .../support/QueryDslJpaRepository.java | 21 +--------- .../support/SimpleJpaRepository.java | 38 ++++++++++++++++--- ...raphRepositoryMethodsIntegrationTests.java | 24 ++++++++++++ .../query/JpaQueryMethodUnitTests.java | 21 +++++++++- ...odsWithEntityGraphConfigJpaRepository.java | 14 +++++-- .../support/SimpleJpaRepositoryUnitTests.java | 31 +++++++++++++-- 8 files changed, 134 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index ebd806324..41269876e 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-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,6 +15,8 @@ */ package org.springframework.data.jpa.repository.query; +import java.util.Map; + import javax.persistence.EntityManager; import javax.persistence.LockModeType; import javax.persistence.Query; @@ -180,10 +182,10 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { Assert.notNull(query, "Query must not be null!"); Assert.notNull(method, "JpaQueryMethod must not be null!"); - JpaEntityGraph entityGraph = method.getEntityGraph(); + Map hints = Jpa21Utils.tryGetFetchGraphHints(em, method.getEntityGraph()); - if (entityGraph != null) { - Jpa21Utils.tryConfigureFetchGraph(em, query, entityGraph); + for (Map.Entry hint : hints.entrySet()) { + query.setHint(hint.getKey(), hint.getValue()); } return query; 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 d4b929d55..63793443a 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 @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-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. @@ -16,6 +16,8 @@ package org.springframework.data.jpa.repository.query; import java.lang.reflect.Method; +import java.util.Collections; +import java.util.Map; import javax.persistence.EntityGraph; import javax.persistence.EntityManager; @@ -52,27 +54,27 @@ public class Jpa21Utils { } /** - * Adds a JPA 2.1 fetch-graph or load-graph hint to the given {@link Query} if running under JPA 2.1. + * Returns a {@link Map} with hints for a JPA 2.1 fetch-graph or load-graph if running under JPA 2.1. * - * @see JPA 2.1 Specfication 3.7.4 - Use of Entity Graphs in find and query operations P.117 - * @param em must not be {@literal null}. - * @param query must not be {@literal null}. - * @param entityGraph can be {@literal null}. + * @param em must not be {@literal null} + * @param query must not be {@literal null} + * @param entityGraph can be {@literal null} + * @return a {@code Map} with the hints or an empty {@code Map} if no hints were found + * @since 1.8 */ - public static T tryConfigureFetchGraph(EntityManager em, T query, JpaEntityGraph entityGraph) { + public static Map tryGetFetchGraphHints(EntityManager em, JpaEntityGraph entityGraph) { if (entityGraph == null) { - return query; + return Collections.emptyMap(); } EntityGraph graph = tryGetFetchGraph(em, entityGraph); if (graph == null) { - return query; + return Collections.emptyMap(); } - query.setHint(entityGraph.getType().getKey(), graph); - return query; + return Collections. singletonMap(entityGraph.getType().getKey(), graph); } /** diff --git a/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java index 9c8e0d585..cbe350e86 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/QueryDslJpaRepository.java @@ -20,7 +20,6 @@ import java.util.Collections; import java.util.List; import java.util.Map.Entry; -import javax.persistence.EntityGraph; import javax.persistence.EntityManager; import javax.persistence.LockModeType; @@ -28,8 +27,6 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; -import org.springframework.data.jpa.repository.query.Jpa21Utils; -import org.springframework.data.jpa.repository.query.JpaEntityGraph; import org.springframework.data.querydsl.EntityPathResolver; import org.springframework.data.querydsl.QSort; import org.springframework.data.querydsl.QueryDslPredicateExecutor; @@ -57,7 +54,6 @@ public class QueryDslJpaRepository extends SimpleJpa private final EntityPath path; private final PathBuilder builder; private final Querydsl querydsl; - private final EntityManager em; /** * Creates a new {@link QueryDslJpaRepository} from the given domain class and {@link EntityManager}. This will use @@ -82,7 +78,6 @@ public class QueryDslJpaRepository extends SimpleJpa EntityPathResolver resolver) { super(entityInformation, entityManager); - this.em = entityManager; this.path = resolver.createPath(entityInformation.getJavaType()); this.builder = new PathBuilder(path.getType(), path.getMetadata()); this.querydsl = new Querydsl(entityManager, builder); @@ -185,24 +180,10 @@ public class QueryDslJpaRepository extends SimpleJpa LockModeType type = metadata.getLockModeType(); query = type == null ? query : query.setLockMode(type); - for (Entry hint : metadata.getQueryHints().entrySet()) { + for (Entry hint : getQueryHints().entrySet()) { query.setHint(hint.getKey(), hint.getValue()); } - JpaEntityGraph jpaEntityGraph = metadata.getEntityGraph(); - - if (jpaEntityGraph == null) { - return query; - } - - EntityGraph entityGraph = Jpa21Utils.tryGetFetchGraph(em, jpaEntityGraph); - - if (entityGraph == null) { - return query; - } - - query.setHint(jpaEntityGraph.getType().getKey(), entityGraph); - return query; } diff --git a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index c4d919c36..8f6797395 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-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. @@ -20,6 +20,7 @@ import static org.springframework.data.jpa.repository.query.QueryUtils.*; import java.io.Serializable; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -28,6 +29,7 @@ import javax.persistence.EntityManager; import javax.persistence.LockModeType; import javax.persistence.NoResultException; import javax.persistence.Parameter; +import javax.persistence.Query; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; @@ -43,6 +45,7 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.Specification; import org.springframework.data.jpa.provider.PersistenceProvider; +import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.jpa.repository.query.Jpa21Utils; @@ -225,11 +228,31 @@ public class SimpleJpaRepository implements JpaRepos } LockModeType type = metadata.getLockModeType(); - Map hints = metadata.getQueryHints(); + + Map hints = getQueryHints(); return type == null ? em.find(domainType, id, hints) : em.find(domainType, id, type, hints); } + /** + * Returns a {@link Map} with the query hints based on the current {@link CrudMethodMetadata} and potential + * {@link EntityGraph} information. + * + * @return + */ + protected Map getQueryHints() { + + if (metadata.getEntityGraph() == null) { + return metadata.getQueryHints(); + } + + Map hints = new HashMap(); + hints.putAll(metadata.getQueryHints()); + hints.putAll(Jpa21Utils.tryGetFetchGraphHints(em, metadata.getEntityGraph())); + + return hints; + } + /* * (non-Javadoc) * @see org.springframework.data.jpa.repository.JpaRepository#getOne(java.io.Serializable) @@ -567,11 +590,16 @@ public class SimpleJpaRepository implements JpaRepos LockModeType type = metadata.getLockModeType(); TypedQuery toReturn = type == null ? query : query.setLockMode(type); - for (Entry hint : metadata.getQueryHints().entrySet()) { + applyQueryHints(toReturn); + + return toReturn; + } + + private void applyQueryHints(Query query) { + + for (Entry hint : getQueryHints().entrySet()) { query.setHint(hint.getKey(), hint.getValue()); } - - return Jpa21Utils.tryConfigureFetchGraph(em, toReturn, metadata.getEntityGraph()); } /** 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 5742fb5ab..9dc459f30 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java @@ -48,12 +48,15 @@ public class EntityGraphRepositoryMethodsIntegrationTests { @Autowired RepositoryMethodsWithEntityGraphConfigJpaRepository repository; User tom; + User olli; Role role; @Before public void setup() { tom = new User("Thomas", "Darimont", "tdarimont@example.org"); + olli = new User("Oliver", "Gierke", "ogierke@example.org"); + role = new Role("Developer"); em.persist(role); tom.getRoles().add(role); @@ -75,4 +78,25 @@ public class EntityGraphRepositoryMethodsIntegrationTests { assertThat(Persistence.getPersistenceUtil().isLoaded(result.get(0).getRoles()), is(true)); assertThat(result.get(0), is(tom)); } + + /** + * @see DATAJPA-689 + */ + @Test + public void shouldRespectConfiguredJpaEntityGraphInFindOne() { + + Assume.assumeTrue(currentEntityManagerIsAJpa21EntityManager(em)); + + olli = repository.save(olli); + tom.getColleagues().add(olli); + tom = repository.save(tom); + + em.flush(); + + User user = repository.findOne(tom.getId()); + + assertThat(user, is(notNullValue())); + assertThat("colleages should be fetched with 'user.detail' fetchgraph", + Persistence.getPersistenceUtil().isLoaded(user.getColleagues()), is(true)); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java index f948142c2..7bd7ebfe3 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryMethodUnitTests.java @@ -345,6 +345,19 @@ public class JpaQueryMethodUnitTests { assertThat(method.getEntityGraph().getType(), is(EntityGraphType.FETCH)); } + /** + * @see DATAJPA-689 + */ + @Test + public void shouldFindEntityGraphAnnotationOnOverriddenSimpleJpaRepositoryMethodFindOne() throws Exception { + + JpaQueryMethod method = new JpaQueryMethod(JpaRepositoryOverride.class.getMethod("findOne"), metadata, extractor); + + assertThat(method.getEntityGraph(), is(notNullValue())); + assertThat(method.getEntityGraph().getName(), is("User.detail")); + assertThat(method.getEntityGraph().getType(), is(EntityGraphType.FETCH)); + } + /** * Interface to define invalid repository methods for testing. * @@ -414,7 +427,13 @@ public class JpaQueryMethodUnitTests { */ @Override @EntityGraph("User.detail") - public List findAll(); + List findAll(); + + /** + * DATAJPA-689 + */ + @EntityGraph("User.detail") + User findOne(); } @Lock(LockModeType.OPTIMISTIC_FORCE_INCREMENT) diff --git a/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigJpaRepository.java b/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigJpaRepository.java index 206cdf65a..a10c65a8e 100644 --- a/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigJpaRepository.java +++ b/src/test/java/org/springframework/data/jpa/repository/sample/RepositoryMethodsWithEntityGraphConfigJpaRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-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. @@ -23,16 +23,22 @@ import org.springframework.data.jpa.repository.EntityGraph.EntityGraphType; import org.springframework.data.jpa.repository.JpaRepository; /** - * Custom repository interface that customizes the fetching behavior of querys of well known repository interface methods via {@link EntityGraph} - * annotation. + * Custom repository interface that customizes the fetching behavior of querys of well known repository interface + * methods via {@link EntityGraph} annotation. * * @author Thomas Darimont */ -public interface RepositoryMethodsWithEntityGraphConfigJpaRepository extends JpaRepository { +public interface RepositoryMethodsWithEntityGraphConfigJpaRepository extends JpaRepository { /** * Should find all users. */ @EntityGraph(type = EntityGraphType.LOAD, value = "User.overview") List findAll(); + + /** + * Should fetch all user details + */ + @EntityGraph(type = EntityGraphType.FETCH, value = "User.detail") + User findOne(Integer id); } diff --git a/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java index ecf07b181..070f5a1c1 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2014 the original author or authors. + * Copyright 2011-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,8 +15,10 @@ */ package org.springframework.data.jpa.repository.support; +import static java.util.Collections.*; import static org.mockito.Mockito.*; +import javax.persistence.EntityGraph; import javax.persistence.EntityManager; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; @@ -30,16 +32,19 @@ import org.mockito.runners.MockitoJUnitRunner; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.data.domain.PageRequest; import org.springframework.data.jpa.domain.sample.User; +import org.springframework.data.jpa.repository.EntityGraph.EntityGraphType; +import org.springframework.data.jpa.repository.query.JpaEntityGraph; /** * Unit tests for {@link SimpleJpaRepository}. * * @author Oliver Gierke + * @author Thomas Darimont */ @RunWith(MockitoJUnitRunner.class) public class SimpleJpaRepositoryUnitTests { - SimpleJpaRepository repo; + SimpleJpaRepository repo; @Mock EntityManager em; @Mock CriteriaBuilder builder; @@ -49,6 +54,7 @@ public class SimpleJpaRepositoryUnitTests { @Mock TypedQuery countQuery; @Mock JpaEntityInformation information; @Mock CrudMethodMetadata metadata; + @Mock EntityGraph entityGraph; @Before public void setUp() { @@ -64,7 +70,7 @@ public class SimpleJpaRepositoryUnitTests { when(em.createQuery(criteriaQuery)).thenReturn(query); when(em.createQuery(countCriteriaQuery)).thenReturn(countQuery); - repo = new SimpleJpaRepository(information, em); + repo = new SimpleJpaRepository(information, em); repo.setRepositoryMethodMetadata(metadata); } @@ -86,6 +92,23 @@ public class SimpleJpaRepositoryUnitTests { @Test(expected = EmptyResultDataAccessException.class) public void throwsExceptionIfEntityToDeleteDoesNotExist() { - repo.delete(4711L); + repo.delete(4711); + } + + /** + * @see DATAJPA-689 + */ + @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) + public void shouldPropagateConfiguredEntityGraphToFindOne() { + + String entityGraphName = "User.detail"; + when(metadata.getEntityGraph()).thenReturn(new JpaEntityGraph(entityGraphName, EntityGraphType.LOAD)); + when(em.getEntityGraph(entityGraphName)).thenReturn((EntityGraph) entityGraph); + + Integer id = 0; + repo.findOne(id); + + verify(em).find(User.class, id, singletonMap(EntityGraphType.LOAD.getKey(), (Object) entityGraph)); } }