DATAJPA-1087 - Apply query hints to count queries for Querydsl but leave out fetch graphs.

We now make sure to apply query hints also to count queries created via Querydsl Predicates but avoid applying potential fetch graphs.

Original pull request: #196.

src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadata.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2015 the original author or authors.
+ * Copyright 2011-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.
@@ -17,6 +17,7 @@ package org.springframework.data.jpa.repository.support;
 
 import java.lang.reflect.Method;
 import java.util.Map;
+import java.util.Optional;
 
 import javax.persistence.LockModeType;
 
@@ -28,6 +29,7 @@ import org.springframework.data.jpa.repository.EntityGraph;
  * 
  * @author Oliver Gierke
  * @author Thomas Darimont
+ * @author Christoph Strobl
  */
 public interface CrudMethodMetadata {
 
@@ -51,7 +53,7 @@ public interface CrudMethodMetadata {
 	 * @return
 	 * @since 1.9
 	 */
-	EntityGraph getEntityGraph();
+	Optional<EntityGraph> getEntityGraph();
 
 	/**
 	 * Returns the {@link Method} to be used.

src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2016 the original author or authors.
+ * Copyright 2011-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.
@@ -19,6 +19,7 @@ import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -147,7 +148,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B
 
 		private final LockModeType lockModeType;
 		private final Map<String, Object> queryHints;
-		private final EntityGraph entityGraph;
+		private final Optional<EntityGraph> entityGraph;
 		private final Method method;
 
 		/**
@@ -161,7 +162,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B
 
 			this.lockModeType = findLockModeType(method);
 			this.queryHints = findQueryHints(method);
-			this.entityGraph = findEntityGraph(method);
+			this.entityGraph = Optional.ofNullable(findEntityGraph(method));
 			this.method = method;
 		}
 
@@ -219,7 +220,7 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B
 		 * @see org.springframework.data.jpa.repository.support.CrudMethodMetadata#getEntityGraph()
 		 */
 		@Override
-		public EntityGraph getEntityGraph() {
+		public Optional<EntityGraph> getEntityGraph() {
 			return entityGraph;
 		}
 

src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaRepository.java

@@ -179,7 +179,8 @@ public class QuerydslJpaRepository<T, ID extends Serializable> extends SimpleJpa
 	 */
 	protected JPQLQuery<?> createQuery(Predicate... predicate) {
 
-		AbstractJPAQuery<?, ?> query = querydsl.createQuery(path).where(predicate);
+		AbstractJPAQuery<?, ?> query = doCreateQuery(getQueryHints().withFetchGraphs(), predicate);
+
 		CrudMethodMetadata metadata = getRepositoryMethodMetadata();
 
 		if (metadata == null) {
@@ -187,13 +188,7 @@ public class QuerydslJpaRepository<T, ID extends Serializable> extends SimpleJpa
 		}
 
 		LockModeType type = metadata.getLockModeType();
-		query = type == null ? query : query.setLockMode(type);
-
-		for (Entry<String, Object> hint : getQueryHints().entrySet()) {
-			query.setHint(hint.getKey(), hint.getValue());
-		}
-
-		return query;
+		return type == null ? query : query.setLockMode(type);
 	}
 
 	/**
@@ -202,8 +197,19 @@ public class QuerydslJpaRepository<T, ID extends Serializable> extends SimpleJpa
 	 * @param predicate, can be {@literal null}.
 	 * @return the Querydsl count {@link JPQLQuery}.
 	 */
-	protected JPQLQuery<?> createCountQuery(Predicate predicate) {
-		return querydsl.createQuery(path).where(predicate);
+	protected JPQLQuery<?> createCountQuery(Predicate... predicate) {
+		return doCreateQuery(getQueryHints(), predicate);
+	}
+
+	private AbstractJPAQuery<?, ?> doCreateQuery(QueryHints hints, Predicate... predicate) {
+
+		AbstractJPAQuery<?, ?> query = querydsl.createQuery(path).where(predicate);
+
+		for (Entry<String, Object> hint : hints) {
+			query.setHint(hint.getKey(), hint.getValue());
+		}
+
+		return query;
 	}
 
 	/**

src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java

@@ -21,6 +21,7 @@ import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -232,35 +233,25 @@ public class SimpleJpaRepository<T, ID extends Serializable>
 
 		LockModeType type = metadata.getLockModeType();
 
-		Map<String, Object> hints = getQueryHints();
+		Map<String, Object> hints = getQueryHints().withFetchGraphs().asMap();
 
 		return Optional.ofNullable(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
+	 * Returns {@link QueryHints} with the query hints based on the current {@link CrudMethodMetadata} and potential
 	 * {@link EntityGraph} information.
 	 * 
 	 * @return
 	 */
-	protected Map<String, Object> getQueryHints() {
-
-		if (metadata.getEntityGraph() == null) {
-			return metadata.getQueryHints();
-		}
-
-		Map<String, Object> hints = new HashMap<String, Object>();
-		hints.putAll(metadata.getQueryHints());
-
-		hints.putAll(Jpa21Utils.tryGetFetchGraphHints(em, getEntityGraph(), getDomainClass()));
-
-		return hints;
+	protected QueryHints getQueryHints() {
+		return new QueryHintsImpl(metadata);
 	}
 
-	private JpaEntityGraph getEntityGraph() {
+	private JpaEntityGraph getEntityGraph(EntityGraph entityGraph) {
 
 		String fallbackName = this.entityInformation.getEntityName() + "." + metadata.getMethod().getName();
-		return new JpaEntityGraph(metadata.getEntityGraph(), fallbackName);
+		return new JpaEntityGraph(entityGraph, fallbackName);
 	}
 
 	/* 
@@ -733,7 +724,7 @@ public class SimpleJpaRepository<T, ID extends Serializable>
 
 	private void applyQueryHints(Query query) {
 
-		for (Entry<String, Object> hint : getQueryHints().entrySet()) {
+		for (Entry<String, Object> hint : getQueryHints().withFetchGraphs()) {
 			query.setHint(hint.getKey(), hint.getValue());
 		}
 	}
@@ -821,4 +812,77 @@ public class SimpleJpaRepository<T, ID extends Serializable>
 			return QueryByExamplePredicateBuilder.getPredicate(root, cb, example);
 		}
 	}
+
+	/**
+	 * QueryHints provides access to query hints defined via {@link CrudMethodMetadata#getQueryHints()} by default
+	 * excluding JPA {@link javax.persistence.EntityGraph}.
+	 *
+	 * @author Christoph Strobl
+	 * @since 2.0
+	 */
+	public interface QueryHints extends Iterable<Map.Entry<String, Object>> {
+
+		/**
+		 * Creates and returns a new {@link QueryHints} instance including {@link javax.persistence.EntityGraph}.
+		 *
+		 * @return new instance of {@link QueryHints}.
+		 */
+		QueryHints withFetchGraphs();
+
+		/**
+		 * Get the query hints as a {@link Map}.
+		 *
+		 * @return never {@literal null}.
+		 */
+		Map<String, Object> asMap();
+	}
+
+	/**
+	 * Default implementation of {@link QueryHints}.
+	 *
+	 * @author Christoph Strobl
+	 * @since 2.0
+	 */
+	private class QueryHintsImpl implements QueryHints {
+
+		final boolean includeFetchGraphs;
+		final CrudMethodMetadata metadata;
+
+		private QueryHintsImpl(CrudMethodMetadata metadata) {
+			this(metadata, false);
+		}
+
+		private QueryHintsImpl(CrudMethodMetadata metadata, boolean includeFetchGraphs) {
+			this.metadata = metadata;
+			this.includeFetchGraphs = includeFetchGraphs;
+		}
+
+		@Override
+		public QueryHints withFetchGraphs() {
+			return new QueryHintsImpl(this.metadata, true);
+		}
+
+		@Override
+		public Iterator<Entry<String, Object>> iterator() {
+			return asMap().entrySet().iterator();
+		}
+
+		@Override
+		public Map<String, Object> asMap() {
+
+			Map<String, Object> hints = new HashMap<>();
+
+			if (metadata != null) {
+
+				hints.putAll(metadata.getQueryHints());
+
+				if (includeFetchGraphs) {
+					metadata.getEntityGraph().ifPresent(entityGraph -> hints
+							.putAll(Jpa21Utils.tryGetFetchGraphHints(em, getEntityGraph(entityGraph), getDomainClass())));
+				}
+			}
+
+			return hints;
+		}
+	}
 }

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

@@ -147,7 +147,7 @@ public class EntityGraphRepositoryMethodsIntegrationTests {
 		}
 	}
 
-	@Test // DATAJPA-790
+	@Test // DATAJPA-790, DATAJPA-1087
 	public void shouldRespectConfiguredJpaEntityGraphWithPaginationAndQueryDslPredicates() {
 
 		Assume.assumeTrue(currentEntityManagerIsAJpa21EntityManager(em));

src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java

@@ -20,6 +20,7 @@ import static org.mockito.Mockito.*;
 
 import java.io.Serializable;
 import java.util.Arrays;
+import java.util.Optional;
 
 import javax.persistence.EntityGraph;
 import javax.persistence.EntityManager;
@@ -121,7 +122,7 @@ public class SimpleJpaRepositoryUnitTests {
 		String entityGraphName = "User.detail";
 		when(entityGraphAnnotation.value()).thenReturn(entityGraphName);
 		when(entityGraphAnnotation.type()).thenReturn(EntityGraphType.LOAD);
-		when(metadata.getEntityGraph()).thenReturn(entityGraphAnnotation);
+		when(metadata.getEntityGraph()).thenReturn(Optional.of(entityGraphAnnotation));
 		when(em.getEntityGraph(entityGraphName)).thenReturn((EntityGraph) entityGraph);
 		when(information.getEntityName()).thenReturn("User");
 		when(metadata.getMethod()).thenReturn(CrudRepository.class.getMethod("findOne", Serializable.class));