diff --git a/pom.xml b/pom.xml index d21fe2574c..5006a1a08f 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa-parent - 3.2.3-SNAPSHOT + 3.2.x-3310-SNAPSHOT pom Spring Data JPA Parent diff --git a/spring-data-envers/pom.xml b/spring-data-envers/pom.xml index 3177982e6d..5a5aceea03 100755 --- a/spring-data-envers/pom.xml +++ b/spring-data-envers/pom.xml @@ -5,12 +5,12 @@ org.springframework.data spring-data-envers - 3.2.3-SNAPSHOT + 3.2.x-3310-SNAPSHOT org.springframework.data spring-data-jpa-parent - 3.2.3-SNAPSHOT + 3.2.x-3310-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-distribution/pom.xml b/spring-data-jpa-distribution/pom.xml index 0d81e8df67..f8fe119f79 100644 --- a/spring-data-jpa-distribution/pom.xml +++ b/spring-data-jpa-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-jpa-parent - 3.2.3-SNAPSHOT + 3.2.x-3310-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/pom.xml b/spring-data-jpa/pom.xml index 87a16895d8..21a2c4814e 100644 --- a/spring-data-jpa/pom.xml +++ b/spring-data-jpa/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jpa - 3.2.3-SNAPSHOT + 3.2.x-3310-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.2.3-SNAPSHOT + 3.2.x-3310-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index 4502ac5260..37b17e6a3c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -18,6 +18,8 @@ import jakarta.persistence.EntityManager; import jakarta.persistence.Query; +import java.util.Objects; + import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.QueryRewriter; @@ -28,6 +30,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ConcurrentLruCache; /** * Base class for {@link String} based JPA queries. @@ -40,6 +43,7 @@ * @author Mark Paluch * @author Diego Krupitza * @author Greg Turnquist + * @author Christoph Strobl */ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { @@ -49,6 +53,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { private final SpelExpressionParser parser; private final QueryParameterSetter.QueryMetadataCache metadataCache = new QueryParameterSetter.QueryMetadataCache(); private final QueryRewriter queryRewriter; + private ConcurrentLruCache queryCache = new ConcurrentLruCache<>(16, this::applySorting); /** * Creates a new {@link AbstractStringBasedJpaQuery} from the given {@link JpaQueryMethod}, {@link EntityManager} and @@ -92,12 +97,12 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri @Override public Query doCreateQuery(JpaParametersParameterAccessor accessor) { - String sortedQueryString = QueryEnhancerFactory.forQuery(query) // - .applySorting(accessor.getSort(), query.getAlias()); + Sort sort = accessor.getSort(); + String sortedQueryString = applySortingIfNecessary(query, sort); + ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor); - Query query = createJpaQuery(sortedQueryString, accessor.getSort(), accessor.getPageable(), - processor.getReturnedType()); + Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), processor.getReturnedType()); QueryParameterSetter.QueryMetadata metadata = metadataCache.getMetadata(sortedQueryString, query); @@ -106,6 +111,10 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) { return parameterBinder.get().bindAndPrepare(query, metadata, accessor); } + protected String applySorting(DeclaredQuery query, Sort sort) { + return queryCache.get(new CachableQuery(query, sort)); + } + @Override protected ParameterBinder createBinder() { @@ -179,4 +188,77 @@ protected String potentiallyRewriteQuery(String originalQuery, Sort sort, @Nulla ? queryRewriter.rewrite(originalQuery, pageable) // : queryRewriter.rewrite(originalQuery, sort); } + + String applySorting(CachableQuery cachableQuery) { + + return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery()).applySorting(cachableQuery.getSort(), + cachableQuery.getAlias()); + } + + private String applySortingIfNecessary(DeclaredQuery query, Sort sort) { + + if (sort.isUnsorted()) { + return query.getQueryString(); + } + return applySorting(query, sort); + } + + /** + * Value object with optimized {@link Object#equals(Object)} to cache a query based on its query string and + * {@link Sort sorting}. + * + * @since 3.2.3 + * @author Christoph Strobl + */ + static class CachableQuery { + + private DeclaredQuery declaredQuery; + private final String queryString; + private final Sort sort; + + CachableQuery(DeclaredQuery query, Sort sort) { + + this.declaredQuery = query; + this.queryString = query.getQueryString(); + this.sort = sort; + } + + DeclaredQuery getDeclaredQuery() { + return declaredQuery; + } + + Sort getSort() { + return sort; + } + + String getAlias() { + return declaredQuery.getAlias(); + } + + @Override + public boolean equals(Object o) { + + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + CachableQuery that = (CachableQuery) o; + + if (!Objects.equals(queryString, that.queryString)) { + return false; + } + return Objects.equals(sort, that.sort); + } + + @Override + public int hashCode() { + + int result = queryString != null ? queryString.hashCode() : 0; + result = 31 * result + (sort != null ? sort.hashCode() : 0); + return result; + } + } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java new file mode 100644 index 0000000000..81c9cf82d8 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java @@ -0,0 +1,229 @@ +/* + * Copyright 2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.repository.query; + +import static org.mockito.Mockito.*; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.metamodel.Metamodel; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Supplier; + +import org.assertj.core.api.Assertions; +import org.assertj.core.util.Arrays; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.provider.QueryExtractor; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.jpa.repository.QueryRewriter; +import org.springframework.data.projection.SpelAwareProxyProjectionFactory; +import org.springframework.data.repository.Repository; +import org.springframework.data.repository.core.RepositoryMetadata; +import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; +import org.springframework.data.repository.query.ParametersSource; +import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.data.repository.query.ReturnedType; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.lang.Nullable; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; +import org.springframework.util.ReflectionUtils; + +/** + * @author Christoph Strobl + */ +public class AbstractStringBasedJpaQueryUnitTests { + + @Test // GH-3310 + void shouldNotAttemptToAppendSortIfNoSortArgumentPresent() { + + InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find"); + stringQuery.createQueryWithArguments(); + + stringQuery.neverCalled("applySorting"); + } + + @Test // GH-3310 + void shouldNotAttemptToAppendSortIfSortIndicatesUnsorted() { + + InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find", Sort.class); + stringQuery.createQueryWithArguments(Sort.unsorted()); + + stringQuery.neverCalled("applySorting"); + } + + @Test // GH-3310 + void shouldAppendSortIfSortPresent() { + + InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find", Sort.class); + stringQuery.createQueryWithArguments(Sort.by("name")); + + stringQuery.called("applySorting").times(1); + } + + @Test // GH-3311 + void cachesInvocationBasedOnSortArgument() { + + InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find", Sort.class); + stringQuery.createQueryWithArguments(Sort.by("name")); + stringQuery.called("applySorting").times(1); + + stringQuery.createQueryWithArguments(Sort.by("name")); + stringQuery.called("applySorting").times(1); + + stringQuery.createQueryWithArguments(Sort.by("age")); + stringQuery.called("applySorting").times(2); + } + + interface TestRepo extends Repository { + + @Query("SELECT e FROM Employee e") + Object find(); + + @Query("SELECT e FROM Employee e") + Object find(Sort sort); + } + + static InvocationCapturingStringQueryStub forMethod(Class repository, String method, Class... args) { + + Method respositoryMethod = ReflectionUtils.findMethod(repository, method, args); + RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(repository); + SpelAwareProxyProjectionFactory projectionFactory = Mockito.mock(SpelAwareProxyProjectionFactory.class); + QueryExtractor queryExtractor = Mockito.mock(QueryExtractor.class); + JpaQueryMethod queryMethod = new JpaQueryMethod(respositoryMethod, repositoryMetadata, projectionFactory, + queryExtractor); + + Query query = AnnotatedElementUtils.getMergedAnnotation(respositoryMethod, Query.class); + + return new InvocationCapturingStringQueryStub(respositoryMethod, queryMethod, query.value(), query.countQuery(), + new SpelExpressionParser()); + + } + + static class InvocationCapturingStringQueryStub extends AbstractStringBasedJpaQuery { + + private final Method targetMethod; + private final MultiValueMap capturedArguments = new LinkedMultiValueMap<>(3); + + InvocationCapturingStringQueryStub(Method targetMethod, JpaQueryMethod queryMethod, String queryString, + @Nullable String countQueryString, SpelExpressionParser parser) { + super(queryMethod, new Supplier() { + + @Override + public EntityManager get() { + + EntityManager em = Mockito.mock(EntityManager.class); + + Metamodel meta = mock(Metamodel.class); + when(em.getMetamodel()).thenReturn(meta); + when(em.getDelegate()).thenReturn(new Object()); // some generic jpa + + return em; + } + }.get(), queryString, countQueryString, Mockito.mock(QueryRewriter.class), + Mockito.mock(QueryMethodEvaluationContextProvider.class), parser); + + this.targetMethod = targetMethod; + } + + @Override + protected String applySorting(CachableQuery query) { + + captureInvocation("applySorting", query); + + return super.applySorting(query); + } + + @Override + protected jakarta.persistence.Query createJpaQuery(String queryString, Sort sort, @Nullable Pageable pageable, + ReturnedType returnedType) { + + captureInvocation("createJpaQuery", queryString, sort, pageable, returnedType); + + jakarta.persistence.Query jpaQuery = super.createJpaQuery(queryString, sort, pageable, returnedType); + return jpaQuery == null ? Mockito.mock(jakarta.persistence.Query.class) : jpaQuery; + } + + // --> convenience for tests + + JpaParameters getParameters() { + return new JpaParameters(ParametersSource.of(targetMethod)); + } + + JpaParametersParameterAccessor getParameterAccessor(Object... args) { + return new JpaParametersParameterAccessor(getParameters(), args); + } + + jakarta.persistence.Query createQueryWithArguments(Object... args) { + return doCreateQuery(getParameterAccessor(args)); + } + + // --> capturing methods + + private void captureInvocation(String key, Object... args) { + capturedArguments.add(key, new Arguments(args)); + } + + // --> verification methdos + + int getInvocationCount(String method) { + + List invocations = capturedArguments.get(method); + return invocations != null ? invocations.size() : 0; + } + + public void neverCalled(String method) { + called(method).never(); + } + + public Times called(String method) { + + return (invocationCount -> { + + int actualCount = getInvocationCount(method); + Assertions.assertThat(actualCount) + .withFailMessage( + () -> "Expected %d invocations for %s, but recorded %d".formatted(invocationCount, method, actualCount)) + .isEqualTo(invocationCount); + }); + } + + static class Arguments { + + List values = new ArrayList<>(3); + + public Arguments(Object... values) { + this.values = Arrays.asList(values); + } + } + + interface Times { + + void times(int invocationCount); + + default void never() { + times(0); + } + } + + } +}