DATAJPA-1087 - Polishing.

Extracted QueryHints and DefaultQueryHints into dedicated files (still package scope). Removed handling of optional CrudMethodMetadata in favour of a null object implementation of QueryHints.

Original pull request: #196.

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

@@ -162,12 +162,12 @@ class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, B
 
 			this.lockModeType = findLockModeType(method);
 			this.queryHints = findQueryHints(method);
-			this.entityGraph = Optional.ofNullable(findEntityGraph(method));
+			this.entityGraph = findEntityGraph(method);
 			this.method = method;
 		}
 
-		private static EntityGraph findEntityGraph(Method method) {
-			return AnnotatedElementUtils.findMergedAnnotation(method, EntityGraph.class);
+		private static Optional<EntityGraph> findEntityGraph(Method method) {
+			return Optional.ofNullable(AnnotatedElementUtils.findMergedAnnotation(method, EntityGraph.class));
 		}
 
 		private static LockModeType findLockModeType(Method method) {

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

@@ -0,0 +1,111 @@
+package org.springframework.data.jpa.repository.support;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
+
+import javax.persistence.EntityManager;
+
+import org.springframework.data.jpa.repository.EntityGraph;
+import org.springframework.data.jpa.repository.query.Jpa21Utils;
+import org.springframework.data.jpa.repository.query.JpaEntityGraph;
+import org.springframework.data.util.Optionals;
+import org.springframework.util.Assert;
+
+/**
+ * Default implementation of {@link QueryHints}.
+ *
+ * @author Christoph Strobl
+ * @author Oliver Gierke
+ * @since 2.0
+ */
+class DefaultQueryHints implements QueryHints {
+
+	private final JpaEntityInformation<?, ?> information;
+	private final CrudMethodMetadata metadata;
+	private final Optional<EntityManager> entityManager;
+
+	/**
+	 * Creates a new {@link DefaultQueryHints} instance for the given {@link JpaEntityInformation},
+	 * {@link CrudMethodMetadata}, {@link EntityManager} and whether to include fetch graphs.
+	 * 
+	 * @param information must not be {@literal null}.
+	 * @param metadata must not be {@literal null}.
+	 * @param entityManager must not be {@literal null}.
+	 * @param includeFetchGraphs
+	 */
+	private DefaultQueryHints(JpaEntityInformation<?, ?> information, CrudMethodMetadata metadata,
+			Optional<EntityManager> entityManager) {
+
+		this.information = information;
+		this.metadata = metadata;
+		this.entityManager = entityManager;
+	}
+
+	/**
+	 * Creates a new {@link QueryHints} instance for the given {@link JpaEntityInformation}, {@link CrudMethodMetadata}
+	 * and {@link EntityManager}.
+	 * 
+	 * @param information must not be {@literal null}.
+	 * @param metadata must not be {@literal null}.
+	 * @param entityManager must not be {@literal null}.
+	 * @return
+	 */
+	public static QueryHints of(JpaEntityInformation<?, ?> information, CrudMethodMetadata metadata) {
+
+		Assert.notNull(information, "JpaEntityInformation must not be null!");
+		Assert.notNull(metadata, "CrudMethodMetadata must not be null!");
+
+		return new DefaultQueryHints(information, metadata, Optional.empty());
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jpa.repository.support.QueryHints#withFetchGraphs()
+	 */
+	@Override
+	public QueryHints withFetchGraphs(EntityManager em) {
+		return new DefaultQueryHints(this.information, this.metadata, Optional.of(em));
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see java.lang.Iterable#iterator()
+	 */
+	@Override
+	public Iterator<Entry<String, Object>> iterator() {
+		return asMap().entrySet().iterator();
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jpa.repository.support.QueryHints#asMap()
+	 */
+	@Override
+	public Map<String, Object> asMap() {
+
+		Map<String, Object> hints = new HashMap<>();
+
+		hints.putAll(metadata.getQueryHints());
+		hints.putAll(getFetchGraphs());
+
+		return hints;
+	}
+
+	private Map<String, Object> getFetchGraphs() {
+
+		return Optionals
+				.mapIfAllPresent(entityManager, metadata.getEntityGraph(),
+						(em, graph) -> Jpa21Utils.tryGetFetchGraphHints(em, getEntityGraph(graph), information.getJavaType()))
+				.orElse(Collections.emptyMap());
+	}
+
+	private JpaEntityGraph getEntityGraph(EntityGraph entityGraph) {
+
+		String fallbackName = information.getEntityName() + "." + metadata.getMethod().getName();
+		return new JpaEntityGraph(entityGraph, fallbackName);
+	}
+}

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

@@ -0,0 +1,72 @@
+package org.springframework.data.jpa.repository.support;
+
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import javax.persistence.EntityManager;
+
+/**
+ * QueryHints provides access to query hints defined via {@link CrudMethodMetadata#getQueryHints()} by default excluding
+ * JPA {@link javax.persistence.EntityGraph}.
+ *
+ * @author Christoph Strobl
+ * @author Oliver Gierke
+ * @since 2.0
+ */
+interface QueryHints extends Iterable<Entry<String, Object>> {
+
+	/**
+	 * Creates and returns a new {@link QueryHints} instance including {@link javax.persistence.EntityGraph}.
+	 *
+	 * @param em must not be {@literal null}.
+	 * @return new instance of {@link QueryHints}.
+	 */
+	QueryHints withFetchGraphs(EntityManager em);
+
+	/**
+	 * Get the query hints as a {@link Map}.
+	 *
+	 * @return never {@literal null}.
+	 */
+	Map<String, Object> asMap();
+
+	/**
+	 * Null object implementation of {@link QueryHints}.
+	 *
+	 * @author Oliver Gierke
+	 * @since 2.0
+	 */
+	static enum NoHints implements QueryHints {
+
+		INSTANCE;
+
+		/* 
+		 * (non-Javadoc)
+		 * @see org.springframework.data.jpa.repository.support.QueryHints#asMap()
+		 */
+		@Override
+		public Map<String, Object> asMap() {
+			return Collections.emptyMap();
+		}
+
+		/* 
+		 * (non-Javadoc)
+		 * @see java.lang.Iterable#iterator()
+		 */
+		@Override
+		public Iterator<Entry<String, Object>> iterator() {
+			return Collections.emptyIterator();
+		}
+
+		/* 
+		 * (non-Javadoc)
+		 * @see org.springframework.data.jpa.repository.support.QueryHints#withFetchGraphs(javax.persistence.EntityManager)
+		 */
+		@Override
+		public QueryHints withFetchGraphs(EntityManager em) {
+			return this;
+		}
+	}
+}

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

@@ -57,6 +57,7 @@ public class QuerydslJpaRepository<T, ID extends Serializable> extends SimpleJpa
 	private final EntityPath<T> path;
 	private final PathBuilder<T> builder;
 	private final Querydsl querydsl;
+	private final EntityManager entityManager;
 
 	/**
 	 * Creates a new {@link QuerydslJpaRepository} from the given domain class and {@link EntityManager}. This will use
@@ -85,6 +86,7 @@ public class QuerydslJpaRepository<T, ID extends Serializable> extends SimpleJpa
 		this.path = resolver.createPath(entityInformation.getJavaType());
 		this.builder = new PathBuilder<T>(path.getType(), path.getMetadata());
 		this.querydsl = new Querydsl(entityManager, builder);
+		this.entityManager = entityManager;
 	}
 
 	/*
@@ -179,7 +181,7 @@ public class QuerydslJpaRepository<T, ID extends Serializable> extends SimpleJpa
 	 */
 	protected JPQLQuery<?> createQuery(Predicate... predicate) {
 
-		AbstractJPAQuery<?, ?> query = doCreateQuery(getQueryHints().withFetchGraphs(), predicate);
+		AbstractJPAQuery<?, ?> query = doCreateQuery(getQueryHints().withFetchGraphs(entityManager), predicate);
 
 		CrudMethodMetadata metadata = getRepositoryMethodMetadata();
 

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

@@ -20,8 +20,6 @@ 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.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -53,9 +51,8 @@ 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;
-import org.springframework.data.jpa.repository.query.JpaEntityGraph;
 import org.springframework.data.jpa.repository.query.QueryUtils;
+import org.springframework.data.jpa.repository.support.QueryHints.NoHints;
 import org.springframework.data.repository.support.PageableExecutionUtils;
 import org.springframework.stereotype.Repository;
 import org.springframework.transaction.annotation.Transactional;
@@ -233,7 +230,7 @@ public class SimpleJpaRepository<T, ID extends Serializable>
 
 		LockModeType type = metadata.getLockModeType();
 
-		Map<String, Object> hints = getQueryHints().withFetchGraphs().asMap();
+		Map<String, Object> hints = getQueryHints().withFetchGraphs(em).asMap();
 
 		return Optional.ofNullable(type == null ? em.find(domainType, id, hints) : em.find(domainType, id, type, hints));
 	}
@@ -245,13 +242,7 @@ public class SimpleJpaRepository<T, ID extends Serializable>
 	 * @return
 	 */
 	protected QueryHints getQueryHints() {
-		return new QueryHintsImpl(metadata);
-	}
-
-	private JpaEntityGraph getEntityGraph(EntityGraph entityGraph) {
-
-		String fallbackName = this.entityInformation.getEntityName() + "." + metadata.getMethod().getName();
-		return new JpaEntityGraph(entityGraph, fallbackName);
+		return metadata == null ? NoHints.INSTANCE : DefaultQueryHints.of(entityInformation, metadata);
 	}
 
 	/* 
@@ -724,7 +715,7 @@ public class SimpleJpaRepository<T, ID extends Serializable>
 
 	private void applyQueryHints(Query query) {
 
-		for (Entry<String, Object> hint : getQueryHints().withFetchGraphs()) {
+		for (Entry<String, Object> hint : getQueryHints().withFetchGraphs(em)) {
 			query.setHint(hint.getKey(), hint.getValue());
 		}
 	}
@@ -812,77 +803,4 @@ 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;
-		}
-	}
 }