From 0c174d5dfc9b9eed59eb38a57e922a784b386dc9 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 12 Apr 2017 14:44:51 +0200 Subject: [PATCH] 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. --- .../support/CrudMethodMetadata.java | 6 +- .../CrudMethodMetadataPostProcessor.java | 9 +- .../support/QuerydslJpaRepository.java | 26 +++-- .../support/SimpleJpaRepository.java | 98 +++++++++++++++---- ...raphRepositoryMethodsIntegrationTests.java | 2 +- .../support/SimpleJpaRepositoryUnitTests.java | 3 +- 6 files changed, 109 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadata.java b/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadata.java index d99434a1c..a53966e53 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadata.java +++ b/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 getEntityGraph(); /** * Returns the {@link Method} to be used. diff --git a/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java b/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java index e099fc57f..f24629a01 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java +++ b/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 queryHints; - private final EntityGraph entityGraph; + private final Optional 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 getEntityGraph() { return entityGraph; } 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 96b4e376b..7562a0336 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 @@ -179,7 +179,8 @@ public class QuerydslJpaRepository 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 extends SimpleJpa } LockModeType type = metadata.getLockModeType(); - query = type == null ? query : query.setLockMode(type); - - for (Entry 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 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 hint : hints) { + query.setHint(hint.getKey(), hint.getValue()); + } + + 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 65dad9bd2..83a1b0103 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 @@ -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 LockModeType type = metadata.getLockModeType(); - Map hints = getQueryHints(); + Map 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 getQueryHints() { - - if (metadata.getEntityGraph() == null) { - return metadata.getQueryHints(); - } - - Map hints = new HashMap(); - 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 private void applyQueryHints(Query query) { - for (Entry hint : getQueryHints().entrySet()) { + for (Entry hint : getQueryHints().withFetchGraphs()) { query.setHint(hint.getKey(), hint.getValue()); } } @@ -821,4 +812,77 @@ public class SimpleJpaRepository 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> { + + /** + * 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 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> iterator() { + return asMap().entrySet().iterator(); + } + + @Override + public Map asMap() { + + Map 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; + } + } } 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 0dfbcaac9..d8b028155 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EntityGraphRepositoryMethodsIntegrationTests.java +++ b/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)); 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 4f2ef82fb..179adacec 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 @@ -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));