From cbe78769c279f04dfe788995d86818bc330c177f Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 23 Sep 2024 14:51:29 +0200 Subject: [PATCH 01/10] Prepare issue branch. --- pom.xml | 2 +- spring-data-envers/pom.xml | 4 ++-- spring-data-jpa-distribution/pom.xml | 2 +- spring-data-jpa/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 1847f4d2c4..de79e1d08f 100755 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa-parent - 3.5.0-SNAPSHOT + 3.5.0-3076-SNAPSHOT pom Spring Data JPA Parent diff --git a/spring-data-envers/pom.xml b/spring-data-envers/pom.xml index 5dcbbb69fd..b38ec85271 100755 --- a/spring-data-envers/pom.xml +++ b/spring-data-envers/pom.xml @@ -5,12 +5,12 @@ org.springframework.data spring-data-envers - 3.5.0-SNAPSHOT + 3.5.0-3076-SNAPSHOT org.springframework.data spring-data-jpa-parent - 3.5.0-SNAPSHOT + 3.5.0-3076-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-distribution/pom.xml b/spring-data-jpa-distribution/pom.xml index 38a234cb71..4fb349b87d 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.5.0-SNAPSHOT + 3.5.0-3076-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/pom.xml b/spring-data-jpa/pom.xml index c6d9301c02..a33e4f9f2d 100644 --- a/spring-data-jpa/pom.xml +++ b/spring-data-jpa/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jpa - 3.5.0-SNAPSHOT + 3.5.0-3076-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.5.0-SNAPSHOT + 3.5.0-3076-SNAPSHOT ../pom.xml From e0348c8122d5bb8df5c82bc56824fd3daf727839 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 13 Aug 2024 14:25:08 +0200 Subject: [PATCH 02/10] Rewrite string-queries to use constructor expressions when return type is DTO. We now rewrite String-based JPA queries to use constructor expressions when either selecting the entity or selecting individual properties. We do not rewrite queries that already use constructor expressions. Closes #3076 --- .../repository/query/AbstractJpaQuery.java | 3 +- .../query/AbstractStringBasedJpaQuery.java | 63 +++++++--- .../query/DefaultQueryEnhancer.java | 6 + .../DtoProjectionTransformerDelegate.java | 73 ++++++++++++ .../query/EqlSortedQueryTransformer.java | 25 +++- .../query/HqlSortedQueryTransformer.java | 24 ++++ .../query/JSqlParserQueryEnhancer.java | 6 + .../repository/query/JpaQueryEnhancer.java | 24 +++- .../query/JpqlSortedQueryTransformer.java | 32 +++++ .../jpa/repository/query/QueryEnhancer.java | 4 + .../jpa/repository/query/QueryTokens.java | 1 + .../repository/query/QueryTransformers.java | 2 +- .../repository/UserRepositoryFinderTests.java | 20 +++- .../AbstractStringBasedJpaQueryUnitTests.java | 8 +- .../EqlDtoQueryTransformerUnitTests.java | 112 ++++++++++++++++++ .../HqlDtoQueryTransformerUnitTests.java | 112 ++++++++++++++++++ .../JpqlDtoQueryTransformerUnitTests.java | 112 ++++++++++++++++++ .../query/NativeJpaQueryUnitTests.java | 15 +-- .../jpa/repository/sample/UserRepository.java | 10 ++ 19 files changed, 615 insertions(+), 37 deletions(-) create mode 100644 spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlDtoQueryTransformerUnitTests.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlDtoQueryTransformerUnitTests.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index bbde6d9414..909053364b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -283,7 +283,8 @@ protected Class getTypeToRead(ReturnedType returnedType) { return null; } - return returnedType.isProjecting() && !getMetamodel().isJpaManaged(returnedType.getReturnedType()) // + return returnedType.isProjecting() && returnedType.getReturnedType().isInterface() + && !getMetamodel().isJpaManaged(returnedType.getReturnedType()) // ? Tuple.class // : null; } 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 0d257fe5a2..16ab1d21a5 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 @@ -99,10 +99,13 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri }); this.queryRewriter = queryRewriter; + ReturnedType returnedType = method.getResultProcessor().getReturnedType(); JpaParameters parameters = method.getParameters(); - if (parameters.hasPageableParameter() || parameters.hasSortParameter()) { + if ((parameters.hasPageableParameter() || parameters.hasSortParameter()) && !parameters.hasDynamicProjection()) { this.querySortRewriter = new CachingQuerySortRewriter(); + } else if (returnedType.isProjecting() && !returnedType.getReturnedType().isInterface()) { + this.querySortRewriter = new ProjectingSortRewriter(); } else { this.querySortRewriter = NoOpQuerySortRewriter.INSTANCE; } @@ -115,9 +118,8 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri public Query doCreateQuery(JpaParametersParameterAccessor accessor) { Sort sort = accessor.getSort(); - String sortedQueryString = getSortedQueryString(sort); - ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor); + String sortedQueryString = getSortedQueryString(sort, processor.getReturnedType()); Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), processor.getReturnedType()); @@ -128,8 +130,8 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) { return parameterBinder.get().bindAndPrepare(query, metadata, accessor); } - String getSortedQueryString(Sort sort) { - return querySortRewriter.getSorted(query, sort); + String getSortedQueryString(Sort sort, ReturnedType returnedType) { + return querySortRewriter.getSorted(query, sort, returnedType); } @Override @@ -211,24 +213,25 @@ protected String potentiallyRewriteQuery(String originalQuery, Sort sort, @Nulla String applySorting(CachableQuery cachableQuery) { - return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery()).applySorting(cachableQuery.getSort(), - cachableQuery.getAlias()); + return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery()).rewrite(cachableQuery.getSort(), + cachableQuery.getReturnedType()); } /** * Query Sort Rewriter interface. */ interface QuerySortRewriter { - String getSorted(DeclaredQuery query, Sort sort); + String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedType); } /** * No-op query rewriter. */ enum NoOpQuerySortRewriter implements QuerySortRewriter { + INSTANCE; - public String getSorted(DeclaredQuery query, Sort sort) { + public String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedType) { if (sort.isSorted()) { throw new UnsupportedOperationException("NoOpQueryCache does not support sorting"); @@ -238,6 +241,25 @@ public String getSorted(DeclaredQuery query, Sort sort) { } } + static class ProjectingSortRewriter implements QuerySortRewriter { + + private volatile String cachedQueryString; + + public String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedType) { + + if (sort.isSorted()) { + throw new UnsupportedOperationException("NoOpQueryCache does not support sorting"); + } + + String cachedQueryString = this.cachedQueryString; + if (cachedQueryString == null) { + this.cachedQueryString = cachedQueryString = QueryEnhancerFactory.forQuery(query).rewrite(sort, returnedType); + } + + return cachedQueryString; + } + } + /** * Caching variant of {@link QuerySortRewriter}. */ @@ -246,14 +268,22 @@ class CachingQuerySortRewriter implements QuerySortRewriter { private final ConcurrentLruCache queryCache = new ConcurrentLruCache<>(16, AbstractStringBasedJpaQuery.this::applySorting); + private volatile String cachedQueryString; + @Override - public String getSorted(DeclaredQuery query, Sort sort) { + public String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedType) { if (sort.isUnsorted()) { - return query.getQueryString(); + + String cachedQueryString = this.cachedQueryString; + if (cachedQueryString == null) { + this.cachedQueryString = cachedQueryString = queryCache.get(new CachableQuery(query, sort, returnedType)); + } + + return cachedQueryString; } - return queryCache.get(new CachableQuery(query, sort)); + return queryCache.get(new CachableQuery(query, sort, returnedType)); } } @@ -269,12 +299,14 @@ static class CachableQuery { private final DeclaredQuery declaredQuery; private final String queryString; private final Sort sort; + private final ReturnedType returnedType; - CachableQuery(DeclaredQuery query, Sort sort) { + CachableQuery(DeclaredQuery query, Sort sort, ReturnedType returnedType) { this.declaredQuery = query; this.queryString = query.getQueryString(); this.sort = sort; + this.returnedType = returnedType; } DeclaredQuery getDeclaredQuery() { @@ -285,9 +317,8 @@ Sort getSort() { return sort; } - @Nullable - String getAlias() { - return declaredQuery.getAlias(); + public ReturnedType getReturnedType() { + return returnedType; } @Override diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java index d8c5bb4a50..07ef7d15be 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java @@ -18,6 +18,7 @@ import java.util.Set; import org.springframework.data.domain.Sort; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; /** @@ -52,6 +53,11 @@ public String applySorting(Sort sort, @Nullable String alias) { return QueryUtils.applySorting(this.query.getQueryString(), sort, alias); } + @Override + public String rewrite(Sort sort, ReturnedType returnedType) { + return QueryUtils.applySorting(this.query.getQueryString(), sort, alias); + } + @Override public String createCountQueryFor(@Nullable String countProjection) { return QueryUtils.createCountQueryFor(this.query.getQueryString(), countProjection, this.query.isNativeQuery()); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java new file mode 100644 index 0000000000..558575f15c --- /dev/null +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java @@ -0,0 +1,73 @@ +/* + * 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.springframework.data.jpa.repository.query.QueryTokens.*; + +import org.springframework.data.repository.query.ReturnedType; + +/** + * HQL Query Transformer that rewrites the query using constructor expressions. + *

+ * Query rewriting from a plain property/object selection towards constructor expression only works if either: + *

    + *
  • The query selects its primary alias ({@code SELECT p FROM Person p})
    • + *
  • The query specifies a property list ({@code SELECT p.foo, p.bar FROM Person p})
    • + *
+ * + * @author Mark Paluch + */ +class DtoProjectionTransformerDelegate { + + private final ReturnedType returnedType; + + public DtoProjectionTransformerDelegate(ReturnedType returnedType) { + this.returnedType = returnedType; + } + + public QueryTokenStream transformSelectionList(QueryTokenStream selectionList) { + + if (!returnedType.isProjecting() || selectionList.stream().anyMatch(it -> it.equals(TOKEN_NEW))) { + return selectionList; + } + + QueryRenderer.QueryRendererBuilder builder = QueryRenderer.builder(); + builder.append(QueryTokens.TOKEN_NEW); + builder.append(QueryTokens.token(returnedType.getReturnedType().getName())); + builder.append(QueryTokens.TOKEN_OPEN_PAREN); + + // assume the selection points to the document + if (selectionList.size() == 1) { + + builder.appendInline(QueryTokenStream.concat(returnedType.getInputProperties(), property -> { + + QueryRenderer.QueryRendererBuilder prop = QueryRenderer.builder(); + prop.append(QueryTokens.token(selectionList.getFirst().value())); + prop.append(QueryTokens.TOKEN_DOT); + prop.append(QueryTokens.token(property)); + + return prop.build(); + }, QueryTokens.TOKEN_COMMA)); + + } else { + builder.appendInline(selectionList); + } + + builder.append(QueryTokens.TOKEN_CLOSE_PAREN); + + return builder.build(); + } +} diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java index ed14e9afdf..edd906f07f 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java @@ -21,6 +21,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -40,13 +41,15 @@ class EqlSortedQueryTransformer extends EqlQueryRenderer { private final JpaQueryTransformerSupport transformerSupport = new JpaQueryTransformerSupport(); private final Sort sort; private final @Nullable String primaryFromAlias; + private final @Nullable DtoProjectionTransformerDelegate dtoDelegate; - EqlSortedQueryTransformer(Sort sort, @Nullable String primaryFromAlias) { + EqlSortedQueryTransformer(Sort sort, @Nullable String primaryFromAlias, @Nullable ReturnedType returnedType) { Assert.notNull(sort, "Sort must not be null"); this.sort = sort; this.primaryFromAlias = primaryFromAlias; + this.dtoDelegate = returnedType == null ? null : new DtoProjectionTransformerDelegate(returnedType); } @Override @@ -80,6 +83,26 @@ public QueryRendererBuilder visitSelect_statement(EqlParser.Select_statementCont return builder; } + @Override + public QueryTokenStream visitSelect_clause(EqlParser.Select_clauseContext ctx) { + + if (dtoDelegate == null) { + return super.visitSelect_clause(ctx); + } + + QueryRendererBuilder builder = QueryRenderer.builder(); + + builder.append(QueryTokens.expression(ctx.SELECT())); + + if (ctx.DISTINCT() != null) { + builder.append(QueryTokens.expression(ctx.DISTINCT())); + } + + QueryTokenStream tokenStream = QueryTokenStream.concat(ctx.select_item(), this::visit, TOKEN_COMMA); + + return builder.append(dtoDelegate.transformSelectionList(tokenStream)); + } + private void doVisitOrderBy(QueryRendererBuilder builder, EqlParser.Select_statementContext ctx, Sort sort) { if (ctx.orderby_clause() != null) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java index b6b8853f93..9953b3e6c1 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java @@ -21,6 +21,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -38,6 +39,7 @@ class HqlSortedQueryTransformer extends HqlQueryRenderer { private final JpaQueryTransformerSupport transformerSupport = new JpaQueryTransformerSupport(); private final Sort sort; private final @Nullable String primaryFromAlias; + private final @Nullable DtoProjectionTransformerDelegate dtoDelegate; HqlSortedQueryTransformer(Sort sort, @Nullable String primaryFromAlias) { @@ -45,6 +47,16 @@ class HqlSortedQueryTransformer extends HqlQueryRenderer { this.sort = sort; this.primaryFromAlias = primaryFromAlias; + this.dtoDelegate = null; + } + + HqlSortedQueryTransformer(Sort sort, @Nullable String primaryFromAlias, @Nullable ReturnedType returnedType) { + + Assert.notNull(sort, "Sort must not be null"); + + this.sort = sort; + this.primaryFromAlias = primaryFromAlias; + this.dtoDelegate = returnedType == null ? null : new DtoProjectionTransformerDelegate(returnedType); } @Override @@ -81,6 +93,18 @@ public QueryRendererBuilder visitOrderedQuery(HqlParser.OrderedQueryContext ctx) return visitOrderedQuery(ctx, this.sort); } + @Override + public QueryTokenStream visitSelectionList(HqlParser.SelectionListContext ctx) { + + QueryTokenStream tokenStream = super.visitSelectionList(ctx); + + if (dtoDelegate != null && !isSubquery(ctx)) { + return dtoDelegate.transformSelectionList(tokenStream); + } + + return tokenStream; + } + @Override public QueryTokenStream visitJoinPath(HqlParser.JoinPathContext ctx) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java index 37ec06e12f..a8a0ef19c8 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java @@ -50,6 +50,7 @@ import java.util.StringJoiner; import org.springframework.data.domain.Sort; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; @@ -300,6 +301,11 @@ public String applySorting(Sort sort) { return applySorting(sort, detectAlias()); } + @Override + public String rewrite(Sort sort, ReturnedType returnedType) { + return applySorting(sort, primaryAlias); + } + @Override public String applySorting(Sort sort, @Nullable String alias) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java index c677b2efcc..ad0597983a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java @@ -30,6 +30,7 @@ import org.antlr.v4.runtime.atn.PredictionMode; import org.antlr.v4.runtime.tree.ParseTreeVisitor; import org.springframework.data.domain.Sort; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -48,12 +49,12 @@ class JpaQueryEnhancer implements QueryEnhancer { private final ParserRuleContext context; private final ParsedQueryIntrospector introspector; private final String projection; - private final BiFunction> sortFunction; + private final SortedQueryRewriteFunction sortFunction; private final BiFunction> countQueryFunction; JpaQueryEnhancer(ParserRuleContext context, ParsedQueryIntrospector introspector, - @Nullable BiFunction> sortFunction, - @Nullable BiFunction> countQueryFunction) { + SortedQueryRewriteFunction sortFunction, + BiFunction> countQueryFunction) { this.context = context; this.introspector = introspector; @@ -136,6 +137,10 @@ public static JpaQueryEnhancer forEql(DeclaredQuery query) { return EqlQueryParser.parseQuery(query.getQueryString()); } + ParserRuleContext getContext() { + return context; + } + /** * Checks if the select clause has a new constructor instantiation in the JPA query. * @@ -191,7 +196,12 @@ public DeclaredQuery getQuery() { */ @Override public String applySorting(Sort sort) { - return QueryRenderer.TokenRenderer.render(sortFunction.apply(sort, detectAlias()).visit(context)); + return QueryRenderer.TokenRenderer.render(sortFunction.apply(sort, detectAlias(), null).visit(context)); + } + + @Override + public String rewrite(Sort sort, ReturnedType returnedType) { + return QueryRenderer.TokenRenderer.render(sortFunction.apply(sort, detectAlias(), returnedType).visit(context)); } /** @@ -308,4 +318,10 @@ public static JpqlQueryParser parseQuery(String query) throws BadJpqlGrammarExce return new JpqlQueryParser(query); } } + + interface SortedQueryRewriteFunction { + + ParseTreeVisitor apply(Sort sort, String primaryAlias, @Nullable ReturnedType returnedType); + + } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlSortedQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlSortedQueryTransformer.java index a545171bbf..2539322498 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlSortedQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlSortedQueryTransformer.java @@ -21,6 +21,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -38,6 +39,7 @@ class JpqlSortedQueryTransformer extends JpqlQueryRenderer { private final JpaQueryTransformerSupport transformerSupport = new JpaQueryTransformerSupport(); private final Sort sort; private final @Nullable String primaryFromAlias; + private final @Nullable DtoProjectionTransformerDelegate dtoDelegate; JpqlSortedQueryTransformer(Sort sort, @Nullable String primaryFromAlias) { @@ -45,6 +47,16 @@ class JpqlSortedQueryTransformer extends JpqlQueryRenderer { this.sort = sort; this.primaryFromAlias = primaryFromAlias; + this.dtoDelegate = null; + } + + JpqlSortedQueryTransformer(Sort sort, @Nullable String primaryFromAlias, @Nullable ReturnedType returnedType) { + + Assert.notNull(sort, "Sort must not be null"); + + this.sort = sort; + this.primaryFromAlias = primaryFromAlias; + this.dtoDelegate = returnedType == null ? null : new DtoProjectionTransformerDelegate(returnedType); } @Override @@ -72,6 +84,26 @@ public QueryTokenStream visitSelect_statement(JpqlParser.Select_statementContext return builder; } + @Override + public QueryTokenStream visitSelect_clause(JpqlParser.Select_clauseContext ctx) { + + if (dtoDelegate == null) { + return super.visitSelect_clause(ctx); + } + + QueryRendererBuilder builder = QueryRenderer.builder(); + + builder.append(QueryTokens.expression(ctx.SELECT())); + + if (ctx.DISTINCT() != null) { + builder.append(QueryTokens.expression(ctx.DISTINCT())); + } + + QueryTokenStream tokenStream = QueryTokenStream.concat(ctx.select_item(), this::visit, TOKEN_COMMA); + + return builder.append(dtoDelegate.transformSelectionList(tokenStream)); + } + private void doVisitOrderBy(QueryRendererBuilder builder, JpqlParser.Select_statementContext ctx) { if (ctx.orderby_clause() != null) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java index 55c168a4f5..ba0dfcf658 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java @@ -18,6 +18,7 @@ import java.util.Set; import org.springframework.data.domain.Sort; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; /** @@ -85,6 +86,8 @@ public interface QueryEnhancer { @Deprecated String applySorting(Sort sort, @Nullable String alias); + String rewrite(Sort sort, ReturnedType returnedType); + /** * Creates a count projected query from the given original query. * @@ -101,4 +104,5 @@ default String createCountQueryFor() { * @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}. */ String createCountQueryFor(@Nullable String countProjection); + } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTokens.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTokens.java index 80df0b3300..cc339f4adf 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTokens.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTokens.java @@ -38,6 +38,7 @@ class QueryTokens { static final QueryToken TOKEN_EQUALS = token(" = "); static final QueryToken TOKEN_OPEN_PAREN = token("("); static final QueryToken TOKEN_CLOSE_PAREN = token(")"); + static final QueryToken TOKEN_NEW = expression("new"); static final QueryToken TOKEN_ORDER_BY = expression("order by"); static final QueryToken TOKEN_LOWER_FUNC = token("lower("); static final QueryToken TOKEN_SELECT_COUNT = token("select count("); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java index 46bdc36003..b8f06d1368 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryTransformers.java @@ -61,7 +61,7 @@ static CountSelectionTokenStream create(QueryTokenStream selection) { token = QueryTokens.token(token.value()); } - if (!containsNew && token.value().contains("new")) { + if (!containsNew && token.equals(TOKEN_NEW)) { containsNew = true; } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java index fdf3330ca5..d887cc51bd 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java @@ -378,7 +378,7 @@ void executesNamedQueryWithConstructorExpression() { } @Test // DATAJPA-1713, GH-2008 - public void selectProjectionWithSubselect() { + void selectProjectionWithSubselect() { List dtos = userRepository.findProjectionBySubselect(); @@ -405,4 +405,22 @@ void findByNegatingSimplePropertyUsingMixedNullNonNullArgument() { result = userRepository.findUserByLastname(carter.getLastname()); assertThat(result).containsExactly(carter); } + + @Test // GH-3076 + void dtoProjectionShouldApplyConstructorExpressionRewriting() { + + List dtos = userRepository.findRecordProjection(); + + assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) // + .contains("Dave", "Carter", "Oliver August"); + } + + @Test // GH-3076 + void dtoMultiselectProjectionShouldApplyConstructorExpressionRewriting() { + + List dtos = userRepository.findMultiselectRecordProjection(); + + assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) // + .contains("Dave", "Carter", "Oliver August"); + } } 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 index b2e6ba4fce..9611cd11e2 100644 --- 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 @@ -52,6 +52,7 @@ * Unit tests for {@link AbstractStringBasedJpaQuery}. * * @author Christoph Strobl + * @author Mark Paluch */ class AbstractStringBasedJpaQueryUnitTests { @@ -64,13 +65,14 @@ void shouldNotAttemptToAppendSortIfNoSortArgumentPresent() { stringQuery.neverCalled("applySorting"); } - @Test // GH-3310 - void shouldNotAttemptToAppendSortIfSortIndicatesUnsorted() { + @Test // GH-3310, GH-3076 + void shouldRunQueryRewriterOnce() { InvocationCapturingStringQueryStub stringQuery = forMethod(TestRepo.class, "find", Sort.class); stringQuery.createQueryWithArguments(Sort.unsorted()); + stringQuery.createQueryWithArguments(Sort.unsorted()); - stringQuery.neverCalled("applySorting"); + stringQuery.called("applySorting").times(1); } @Test // GH-3310 diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlDtoQueryTransformerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlDtoQueryTransformerUnitTests.java new file mode 100644 index 0000000000..4e0a7ead47 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlDtoQueryTransformerUnitTests.java @@ -0,0 +1,112 @@ +/* + * 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.assertj.core.api.Assertions.*; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.provider.PersistenceProvider; +import org.springframework.data.projection.SpelAwareProxyProjectionFactory; +import org.springframework.data.repository.Repository; +import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; + +/** + * Unit tests for {@link DtoProjectionTransformerDelegate}. + * + * @author Mark Paluch + */ +class EqlDtoQueryTransformerUnitTests { + + JpaQueryMethod method = getMethod("dtoProjection"); + EqlSortedQueryTransformer transformer = new EqlSortedQueryTransformer(Sort.unsorted(), null, + method.getResultProcessor().getReturnedType()); + + @Test // GH-3076 + void shouldTranslateSingleProjectionToDto() { + + JpaQueryEnhancer.EqlQueryParser parser = JpaQueryEnhancer.EqlQueryParser.parseQuery("SELECT p from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "SELECT new org.springframework.data.jpa.repository.query.EqlDtoQueryTransformerUnitTests$MyRecord(p.foo, p.bar) from Person p"); + } + + @Test // GH-3076 + void shouldRewriteQueriesWithSubselect() { + + JpaQueryEnhancer.EqlQueryParser parser = JpaQueryEnhancer.EqlQueryParser + .parseQuery("select u from User u left outer join u.roles r where r in (select r from Role r)"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "select new org.springframework.data.jpa.repository.query.EqlDtoQueryTransformerUnitTests$MyRecord(u.foo, u.bar) from User u left outer join u.roles r where r in (select r from Role r)"); + } + + @Test // GH-3076 + void shouldNotTranslateConstructorExpressionQuery() { + + JpaQueryEnhancer.EqlQueryParser parser = JpaQueryEnhancer.EqlQueryParser + .parseQuery("SELECT NEW String(p) from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo("SELECT NEW String(p) from Person p"); + } + + @Test + void shouldTranslatePropertySelectionToDto() { + + JpaQueryEnhancer.EqlQueryParser parser = JpaQueryEnhancer.EqlQueryParser + .parseQuery("SELECT p.foo, p.bar, sum(p.age) from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "SELECT new org.springframework.data.jpa.repository.query.EqlDtoQueryTransformerUnitTests$MyRecord(p.foo, p.bar, sum(p.age)) from Person p"); + } + + private JpaQueryMethod getMethod(String name, Class... parameterTypes) { + + try { + Method method = MyRepo.class.getMethod(name, parameterTypes); + PersistenceProvider persistenceProvider = PersistenceProvider.HIBERNATE; + + return new JpaQueryMethod(method, new DefaultRepositoryMetadata(MyRepo.class), + new SpelAwareProxyProjectionFactory(), persistenceProvider); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + + interface MyRepo extends Repository { + + MyRecord dtoProjection(); + } + + record Person(String id) { + + } + + record MyRecord(String foo, String bar) { + + } +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlDtoQueryTransformerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlDtoQueryTransformerUnitTests.java new file mode 100644 index 0000000000..425d68fba1 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlDtoQueryTransformerUnitTests.java @@ -0,0 +1,112 @@ +/* + * 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.assertj.core.api.Assertions.*; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.provider.PersistenceProvider; +import org.springframework.data.projection.SpelAwareProxyProjectionFactory; +import org.springframework.data.repository.Repository; +import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; + +/** + * Unit tests for {@link DtoProjectionTransformerDelegate}. + * + * @author Mark Paluch + */ +class HqlDtoQueryTransformerUnitTests { + + JpaQueryMethod method = getMethod("dtoProjection"); + HqlSortedQueryTransformer transformer = new HqlSortedQueryTransformer(Sort.unsorted(), null, + method.getResultProcessor().getReturnedType()); + + @Test // GH-3076 + void shouldTranslateSingleProjectionToDto() { + + JpaQueryEnhancer.HqlQueryParser parser = JpaQueryEnhancer.HqlQueryParser.parseQuery("SELECT p from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "SELECT new org.springframework.data.jpa.repository.query.HqlDtoQueryTransformerUnitTests$MyRecord(p.foo, p.bar) from Person p"); + } + + @Test // GH-3076 + void shouldRewriteQueriesWithSubselect() { + + JpaQueryEnhancer.HqlQueryParser parser = JpaQueryEnhancer.HqlQueryParser + .parseQuery("select u from User u left outer join u.roles r where r in (select r from Role r)"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "select new org.springframework.data.jpa.repository.query.HqlDtoQueryTransformerUnitTests$MyRecord(u.foo, u.bar) from User u left outer join u.roles r where r in (select r from Role r)"); + } + + @Test // GH-3076 + void shouldNotTranslateConstructorExpressionQuery() { + + JpaQueryEnhancer.HqlQueryParser parser = JpaQueryEnhancer.HqlQueryParser + .parseQuery("SELECT NEW String(p) from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo("SELECT NEW String(p) from Person p"); + } + + @Test + void shouldTranslatePropertySelectionToDto() { + + JpaQueryEnhancer.HqlQueryParser parser = JpaQueryEnhancer.HqlQueryParser + .parseQuery("SELECT p.foo, p.bar, sum(p.age) from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "SELECT new org.springframework.data.jpa.repository.query.HqlDtoQueryTransformerUnitTests$MyRecord(p.foo, p.bar, sum(p.age)) from Person p"); + } + + private JpaQueryMethod getMethod(String name, Class... parameterTypes) { + + try { + Method method = MyRepo.class.getMethod(name, parameterTypes); + PersistenceProvider persistenceProvider = PersistenceProvider.HIBERNATE; + + return new JpaQueryMethod(method, new DefaultRepositoryMetadata(MyRepo.class), + new SpelAwareProxyProjectionFactory(), persistenceProvider); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + + interface MyRepo extends Repository { + + MyRecord dtoProjection(); + } + + record Person(String id) { + + } + + record MyRecord(String foo, String bar) { + + } +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java new file mode 100644 index 0000000000..de6b22c90e --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java @@ -0,0 +1,112 @@ +/* + * 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.assertj.core.api.Assertions.*; + +import java.lang.reflect.Method; + +import org.junit.jupiter.api.Test; + +import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.provider.PersistenceProvider; +import org.springframework.data.projection.SpelAwareProxyProjectionFactory; +import org.springframework.data.repository.Repository; +import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; + +/** + * Unit tests for {@link DtoProjectionTransformerDelegate}. + * + * @author Mark Paluch + */ +class JpqlDtoQueryTransformerUnitTests { + + JpaQueryMethod method = getMethod("dtoProjection"); + JpqlSortedQueryTransformer transformer = new JpqlSortedQueryTransformer(Sort.unsorted(), null, + method.getResultProcessor().getReturnedType()); + + @Test // GH-3076 + void shouldTranslateSingleProjectionToDto() { + + JpaQueryEnhancer.JpqlQueryParser parser = JpaQueryEnhancer.JpqlQueryParser.parseQuery("SELECT p from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "SELECT new org.springframework.data.jpa.repository.query.JpqlDtoQueryTransformerUnitTests$MyRecord(p.foo, p.bar) from Person p"); + } + + @Test // GH-3076 + void shouldRewriteQueriesWithSubselect() { + + JpaQueryEnhancer.JpqlQueryParser parser = JpaQueryEnhancer.JpqlQueryParser + .parseQuery("select u from User u left outer join u.roles r where r in (select r from Role r)"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "select new org.springframework.data.jpa.repository.query.JpqlDtoQueryTransformerUnitTests$MyRecord(u.foo, u.bar) from User u left outer join u.roles r where r in (select r from Role r)"); + } + + @Test // GH-3076 + void shouldNotTranslateConstructorExpressionQuery() { + + JpaQueryEnhancer.JpqlQueryParser parser = JpaQueryEnhancer.JpqlQueryParser + .parseQuery("SELECT NEW String(p) from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo("SELECT NEW String(p) from Person p"); + } + + @Test + void shouldTranslatePropertySelectionToDto() { + + JpaQueryEnhancer.JpqlQueryParser parser = JpaQueryEnhancer.JpqlQueryParser + .parseQuery("SELECT p.foo, p.bar, sum(p.age) from Person p"); + + QueryTokenStream visit = transformer.visit(parser.getContext()); + + assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( + "SELECT new org.springframework.data.jpa.repository.query.JpqlDtoQueryTransformerUnitTests$MyRecord(p.foo, p.bar, sum(p.age)) from Person p"); + } + + private JpaQueryMethod getMethod(String name, Class... parameterTypes) { + + try { + Method method = MyRepo.class.getMethod(name, parameterTypes); + PersistenceProvider persistenceProvider = PersistenceProvider.HIBERNATE; + + return new JpaQueryMethod(method, new DefaultRepositoryMetadata(MyRepo.class), + new SpelAwareProxyProjectionFactory(), persistenceProvider); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + + interface MyRepo extends Repository { + + MyRecord dtoProjection(); + } + + record Person(String id) { + + } + + record MyRecord(String foo, String bar) { + + } +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java index 7a9cf35d1f..9522bf4859 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java @@ -65,15 +65,8 @@ void setUp() { @Test // GH-3546 void shouldApplySorting() { - NativeJpaQuery query = getQuery(TestRepo.class, "find", Sort.class); - String sql = query.getSortedQueryString(Sort.by("foo", "bar")); - - assertThat(sql).isEqualTo("SELECT e FROM Employee e order by e.foo asc, e.bar asc"); - } - - private NativeJpaQuery getQuery(Class repository, String method, Class... args) { - Method respositoryMethod = ReflectionUtils.findMethod(repository, method, args); - RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(repository); + Method respositoryMethod = ReflectionUtils.findMethod(TestRepo.class, "find", Sort.class); + RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(TestRepo.class); SpelAwareProxyProjectionFactory projectionFactory = mock(SpelAwareProxyProjectionFactory.class); QueryExtractor queryExtractor = mock(QueryExtractor.class); JpaQueryMethod queryMethod = new JpaQueryMethod(respositoryMethod, repositoryMetadata, projectionFactory, @@ -83,7 +76,9 @@ private NativeJpaQuery getQuery(Class repository, String method, Class... NativeJpaQuery query = new NativeJpaQuery(queryMethod, em, annotation.value(), annotation.countQuery(), QueryRewriter.IdentityQueryRewriter.INSTANCE, ValueExpressionDelegate.create()); - return query; + String sql = query.getSortedQueryString(Sort.by("foo", "bar"), queryMethod.getResultProcessor().getReturnedType()); + + assertThat(sql).isEqualTo("SELECT e FROM Employee e order by e.foo asc, e.bar asc"); } interface TestRepo extends Repository { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index 76896ece7a..ac6a2ea9b1 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -721,6 +721,12 @@ List findAllAndSortByFunctionResultNamedParameter(@Param("namedParameter @Query("select u from User u where u.firstname >= (select Min(u0.firstname) from User u0)") List findProjectionBySubselect(); + @Query("select u from User u") + List findRecordProjection(); + + @Query("select u.firstname, u.lastname from User u") + List findMultiselectRecordProjection(); + Window findBy(OffsetScrollPosition position); interface RolesAndFirstname { @@ -747,4 +753,8 @@ interface IdOnly { int getId(); } + record UserExcerpt(String firstname, String lastname) { + + } + } From c8bc94dc5f6806353413f768132e586e7b99a35e Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 23 Sep 2024 16:17:40 +0200 Subject: [PATCH 03/10] Add documentation. See #3076 --- .../ROOT/pages/repositories/projections.adoc | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc index 00a6a8c8ef..c55be85997 100644 --- a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc +++ b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc @@ -36,6 +36,41 @@ When using <> with JPQL, you must use (Note the usage of a FQDN for the DTO type!) This JPQL expression can be used in `@Query` annotations as well where you define any named queries. As a workaround you may use named queries with `ResultSetMapping` or the Hibernate-specific javadoc:{hibernatejavadocurl}org.hibernate.query.ResultListTransformer[] +===== DTO Projection JPQL Query Rewriting + +JPQL queries allow selection of the root object, individual properties, and DTO objects through constructor expressions. +Using a constructor expression can quickly add a lot of text to a query and make it difficult to read the actual query. +Spring Data JPA can support you with your JPQL queries by introducing constructor expressions for your convenience. + +Consider the following queries: + +.Projection Queries +==== +[source,java] +---- +interface UserRepository extends Repository { + + @Query("SELECT u FROM USER u") <1> + List findByLastname(String lastname); + + @Query("SELECT u.firstname, u.lastname FROM USER u") <2> + List findMultipleColumnsByLastname(String lastname); +} + +record UserDto(String firstname, String lastname){} +---- + +<1> Selection of the top-level entity. +This query gets rewritten to `SELECT new UserDto(u.firstname, u.lastname) FROM USER u`. +<2> Multi-select of `firstname` and `lastname` properties. +This query gets rewritten to `SELECT new UserDto(u.firstname, u.lastname) FROM USER u`. +==== + +Repository query methods that return a DTO projection type (a Java type outside the domain type hierarchy) are subject for query rewriting. +If an `@Query`-annotated query already uses constructor expressions, then Spring Data backs off and doesn't apply DTO constructor expression rewriting. + +Make sure that your DTO types provide an all-args constructor for the projection, otherwise the query will fail. + ==== Native Queries When using <>, their usage requires slightly more consideration depending on your : From 6fe1b2ba38a058f5809cad97f98660254705ba0b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 24 Sep 2024 12:18:03 +0200 Subject: [PATCH 04/10] Add support for class-based DTOs for Fluent API. Also, interface-based projections now use Tuple queries to consistently use tuple-based queries. Closes #2327 --- .../repository/query/AbstractJpaQuery.java | 2 +- .../data/jpa/repository/query/QueryUtils.java | 3 +- .../FetchableFluentQueryByPredicate.java | 147 ++++++++++++++---- .../FetchableFluentQueryBySpecification.java | 25 +-- .../support/FluentQuerySupport.java | 19 ++- .../jpa/repository/support/JakartaTuple.java | 85 ++++++++++ .../JpaMetamodelEntityInformation.java | 8 +- .../data/jpa/repository/support/Querydsl.java | 6 +- .../support/QuerydslJpaPredicateExecutor.java | 10 +- .../support/SimpleJpaRepository.java | 71 ++++++++- .../support/SpringDataJpaQuery.java | 92 +++++++++++ .../jpa/repository/UserRepositoryTests.java | 126 ++++++++------- ...chableFluentQueryByPredicateUnitTests.java | 9 +- ...QuerydslJpaPredicateExecutorUnitTests.java | 42 ++--- 14 files changed, 487 insertions(+), 158 deletions(-) create mode 100644 spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java create mode 100644 spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index 909053364b..f797284c5a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -305,7 +305,7 @@ protected Class getTypeToRead(ReturnedType returnedType) { */ protected abstract Query doCreateCountQuery(JpaParametersParameterAccessor accessor); - static class TupleConverter implements Converter { + public static class TupleConverter implements Converter { private final ReturnedType type; diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index 3c06c7079b..ad812a5e84 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -769,7 +769,8 @@ static Expression toExpressionRecursively(From from, PropertyPath p return toExpressionRecursively(from, property, false); } - static Expression toExpressionRecursively(From from, PropertyPath property, boolean isForSelection) { + public static Expression toExpressionRecursively(From from, PropertyPath property, + boolean isForSelection) { return toExpressionRecursively(from, property, isForSelection, false); } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java index 9ed0a0ce3e..335b6e33bb 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java @@ -16,17 +16,18 @@ package org.springframework.data.jpa.repository.support; import jakarta.persistence.EntityManager; -import jakarta.persistence.Query; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Stream; import org.springframework.dao.IncorrectResultSizeDataAccessException; +import org.springframework.data.domain.KeysetScrollPosition; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; @@ -36,10 +37,18 @@ import org.springframework.data.jpa.repository.query.ScrollDelegate; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.support.PageableExecutionUtils; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import com.querydsl.core.types.EntityPath; +import com.querydsl.core.types.Expression; +import com.querydsl.core.types.ExpressionBase; import com.querydsl.core.types.Predicate; +import com.querydsl.core.types.Visitor; +import com.querydsl.core.types.dsl.PathBuilder; +import com.querydsl.jpa.JPQLSerializer; import com.querydsl.jpa.impl.AbstractJPAQuery; /** @@ -57,33 +66,41 @@ */ class FetchableFluentQueryByPredicate extends FluentQuerySupport implements FetchableFluentQuery { + private final EntityPath entityPath; + private final JpaEntityInformation entityInformation; + private final ScrollQueryFactory> scrollQueryFactory; private final Predicate predicate; private final Function> finder; - private final PredicateScrollDelegate scroll; private final BiFunction> pagedFinder; private final Function countOperation; private final Function existsOperation; private final EntityManager entityManager; - FetchableFluentQueryByPredicate(Predicate predicate, Class entityType, - Function> finder, PredicateScrollDelegate scroll, + FetchableFluentQueryByPredicate(EntityPath entityPath, Predicate predicate, + JpaEntityInformation entityInformation, Function> finder, + ScrollQueryFactory> scrollQueryFactory, BiFunction> pagedFinder, Function countOperation, Function existsOperation, EntityManager entityManager, ProjectionFactory projectionFactory) { - this(predicate, entityType, (Class) entityType, Sort.unsorted(), 0, Collections.emptySet(), finder, scroll, + this(entityPath, predicate, entityInformation, (Class) entityInformation.getJavaType(), Sort.unsorted(), 0, + Collections.emptySet(), finder, scrollQueryFactory, pagedFinder, countOperation, existsOperation, entityManager, projectionFactory); } - private FetchableFluentQueryByPredicate(Predicate predicate, Class entityType, Class resultType, Sort sort, - int limit, Collection properties, Function> finder, - PredicateScrollDelegate scroll, BiFunction> pagedFinder, + private FetchableFluentQueryByPredicate(EntityPath entityPath, Predicate predicate, + JpaEntityInformation entityInformation, Class resultType, Sort sort, int limit, + Collection properties, Function> finder, + ScrollQueryFactory> scrollQueryFactory, + BiFunction> pagedFinder, Function countOperation, Function existsOperation, EntityManager entityManager, ProjectionFactory projectionFactory) { - super(resultType, sort, limit, properties, entityType, projectionFactory); + super(resultType, sort, limit, properties, entityInformation.getJavaType(), projectionFactory); + this.entityInformation = entityInformation; + this.entityPath = entityPath; this.predicate = predicate; this.finder = finder; - this.scroll = scroll; + this.scrollQueryFactory = scrollQueryFactory; this.pagedFinder = pagedFinder; this.countOperation = countOperation; this.existsOperation = existsOperation; @@ -95,8 +112,9 @@ public FetchableFluentQuery sortBy(Sort sort) { Assert.notNull(sort, "Sort must not be null"); - return new FetchableFluentQueryByPredicate<>(predicate, entityType, resultType, this.sort.and(sort), limit, - properties, finder, scroll, pagedFinder, countOperation, existsOperation, entityManager, projectionFactory); + return new FetchableFluentQueryByPredicate<>(entityPath, predicate, entityInformation, resultType, + this.sort.and(sort), limit, properties, finder, scrollQueryFactory, pagedFinder, countOperation, + existsOperation, entityManager, projectionFactory); } @Override @@ -104,8 +122,9 @@ public FetchableFluentQuery limit(int limit) { Assert.isTrue(limit >= 0, "Limit must not be negative"); - return new FetchableFluentQueryByPredicate<>(predicate, entityType, resultType, sort, limit, properties, finder, - scroll, pagedFinder, countOperation, existsOperation, entityManager, projectionFactory); + return new FetchableFluentQueryByPredicate<>(entityPath, predicate, entityInformation, resultType, sort, limit, + properties, finder, scrollQueryFactory, pagedFinder, countOperation, existsOperation, entityManager, + projectionFactory); } @Override @@ -113,19 +132,17 @@ public FetchableFluentQuery as(Class resultType) { Assert.notNull(resultType, "Projection target type must not be null"); - if (!resultType.isInterface()) { - throw new UnsupportedOperationException("Class-based DTOs are not yet supported."); - } - - return new FetchableFluentQueryByPredicate<>(predicate, entityType, resultType, sort, limit, properties, finder, - scroll, pagedFinder, countOperation, existsOperation, entityManager, projectionFactory); + return new FetchableFluentQueryByPredicate<>(entityPath, predicate, entityInformation, resultType, sort, limit, + properties, finder, scrollQueryFactory, pagedFinder, countOperation, existsOperation, entityManager, + projectionFactory); } @Override public FetchableFluentQuery project(Collection properties) { - return new FetchableFluentQueryByPredicate<>(predicate, entityType, resultType, sort, limit, - mergeProperties(properties), finder, scroll, pagedFinder, countOperation, existsOperation, entityManager, + return new FetchableFluentQueryByPredicate<>(entityPath, predicate, entityInformation, resultType, sort, limit, + mergeProperties(properties), finder, scrollQueryFactory, pagedFinder, countOperation, existsOperation, + entityManager, projectionFactory); } @@ -163,7 +180,8 @@ public Window scroll(ScrollPosition scrollPosition) { Assert.notNull(scrollPosition, "ScrollPosition must not be null"); - return scroll.scroll(sort, limit, scrollPosition).map(getConversionFunction()); + return new PredicateScrollDelegate<>(scrollQueryFactory, entityInformation) + .scroll(returnedType, sort, limit, scrollPosition).map(getConversionFunction()); } @Override @@ -192,6 +210,35 @@ public boolean exists() { private AbstractJPAQuery createSortedAndProjectedQuery() { AbstractJPAQuery query = finder.apply(sort); + applyQuerySettings(this.returnedType, this.limit, query, null); + return query; + } + + private void applyQuerySettings(ReturnedType returnedType, int limit, AbstractJPAQuery query, + @Nullable ScrollPosition scrollPosition) { + + List inputProperties = returnedType.getInputProperties(); + + if (returnedType.needsCustomConstruction() && !inputProperties.isEmpty()) { + + Collection requiredSelection; + if (scrollPosition instanceof KeysetScrollPosition && returnedType.getReturnedType().isInterface()) { + requiredSelection = new LinkedHashSet<>(inputProperties); + sort.forEach(it -> requiredSelection.add(it.getProperty())); + entityInformation.getIdAttributeNames().forEach(requiredSelection::add); + } else { + requiredSelection = inputProperties; + } + + PathBuilder builder = new PathBuilder<>(entityPath.getType(), entityPath.getMetadata()); + Expression[] projection = requiredSelection.stream().map(builder::get).toArray(Expression[]::new); + + if (returnedType.getReturnedType().isInterface()) { + query.select(new JakartaTuple(projection)); + } else { + query.select(new DtoProjection(returnedType.getReturnedType(), projection)); + } + } if (!properties.isEmpty()) { query.setHint(EntityGraphFactory.HINT, EntityGraphFactory.create(entityManager, entityType, properties)); @@ -200,8 +247,6 @@ public boolean exists() { if (limit != 0) { query.limit(limit); } - - return query; } private Page readPage(Pageable pageable) { @@ -233,23 +278,57 @@ private Function getConversionFunction() { return getConversionFunction(entityType, resultType); } - static class PredicateScrollDelegate extends ScrollDelegate { + class PredicateScrollDelegate extends ScrollDelegate { - private final ScrollQueryFactory scrollFunction; + private final ScrollQueryFactory> scrollFunction; - PredicateScrollDelegate(ScrollQueryFactory scrollQueryFactory, JpaEntityInformation entity) { + PredicateScrollDelegate(ScrollQueryFactory> scrollQueryFactory, + JpaEntityInformation entity) { super(entity); this.scrollFunction = scrollQueryFactory; } - public Window scroll(Sort sort, int limit, ScrollPosition scrollPosition) { + public Window scroll(ReturnedType returnedType, Sort sort, int limit, ScrollPosition scrollPosition) { - Query query = scrollFunction.createQuery(sort, scrollPosition); - if (limit > 0) { - query = query.setMaxResults(limit); - } - return scroll(query, sort, scrollPosition); + AbstractJPAQuery query = scrollFunction.createQuery(returnedType, sort, scrollPosition); + + applyQuerySettings(returnedType, limit, query, scrollPosition); + + return scroll(query.createQuery(), sort, scrollPosition); } } + private static class DtoProjection extends ExpressionBase { + + private final Expression[] projection; + + public DtoProjection(Class resultType, Expression[] projection) { + super(resultType); + this.projection = projection; + } + + @SuppressWarnings("unchecked") + @Override + public R accept(Visitor v, @Nullable C context) { + + if (v instanceof JPQLSerializer s) { + + s.append("new ").append(getType().getName()).append("("); + boolean first = true; + for (Expression expression : projection) { + if (first) { + first = false; + } else { + s.append(", "); + } + + expression.accept(v, context); + } + + s.append(")"); + } + + return (R) this; + } + } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java index 08659af984..78601566d3 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java @@ -23,6 +23,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Stream; @@ -38,6 +39,7 @@ import org.springframework.data.jpa.support.PageableUtils; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.repository.query.FluentQuery; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.support.PageableExecutionUtils; import org.springframework.util.Assert; @@ -55,13 +57,14 @@ class FetchableFluentQueryBySpecification extends FluentQuerySupport implements FluentQuery.FetchableFluentQuery { private final Specification spec; - private final Function> finder; + private final BiFunction> finder; private final SpecificationScrollDelegate scroll; private final Function, Long> countOperation; private final Function, Boolean> existsOperation; private final EntityManager entityManager; - FetchableFluentQueryBySpecification(Specification spec, Class entityType, Function> finder, + FetchableFluentQueryBySpecification(Specification spec, Class entityType, + BiFunction> finder, SpecificationScrollDelegate scrollDelegate, Function, Long> countOperation, Function, Boolean> existsOperation, EntityManager entityManager, ProjectionFactory projectionFactory) { @@ -70,7 +73,7 @@ class FetchableFluentQueryBySpecification extends FluentQuerySupport } private FetchableFluentQueryBySpecification(Specification spec, Class entityType, Class resultType, - Sort sort, int limit, Collection properties, Function> finder, + Sort sort, int limit, Collection properties, BiFunction> finder, SpecificationScrollDelegate scrollDelegate, Function, Long> countOperation, Function, Boolean> existsOperation, EntityManager entityManager, ProjectionFactory projectionFactory) { @@ -106,9 +109,6 @@ public FetchableFluentQuery limit(int limit) { public FetchableFluentQuery as(Class resultType) { Assert.notNull(resultType, "Projection target type must not be null"); - if (!resultType.isInterface()) { - throw new UnsupportedOperationException("Class-based DTOs are not yet supported."); - } return new FetchableFluentQueryBySpecification<>(spec, entityType, resultType, sort, limit, properties, finder, scroll, countOperation, existsOperation, entityManager, projectionFactory); @@ -155,7 +155,7 @@ public Window scroll(ScrollPosition scrollPosition) { Assert.notNull(scrollPosition, "ScrollPosition must not be null"); - return scroll.scroll(sort, limit, scrollPosition).map(getConversionFunction()); + return scroll.scroll(returnedType, sort, limit, scrollPosition).map(getConversionFunction()); } @Override @@ -183,7 +183,7 @@ public boolean exists() { private TypedQuery createSortedAndProjectedQuery() { - TypedQuery query = finder.apply(sort); + TypedQuery query = finder.apply(returnedType, sort); if (!properties.isEmpty()) { query.setHint(EntityGraphFactory.HINT, EntityGraphFactory.create(entityManager, entityType, properties)); @@ -227,16 +227,17 @@ private Function getConversionFunction() { static class SpecificationScrollDelegate extends ScrollDelegate { - private final ScrollQueryFactory scrollFunction; + private final ScrollQueryFactory> scrollFunction; - SpecificationScrollDelegate(ScrollQueryFactory scrollQueryFactory, JpaEntityInformation entity) { + SpecificationScrollDelegate(ScrollQueryFactory> scrollQueryFactory, + JpaEntityInformation entity) { super(entity); this.scrollFunction = scrollQueryFactory; } - public Window scroll(Sort sort, int limit, ScrollPosition scrollPosition) { + public Window scroll(ReturnedType returnedType, Sort sort, int limit, ScrollPosition scrollPosition) { - Query query = scrollFunction.createQuery(sort, scrollPosition); + Query query = scrollFunction.createQuery(returnedType, sort, scrollPosition); if (limit > 0) { query = query.setMaxResults(limit); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FluentQuerySupport.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FluentQuerySupport.java index 5917a119f5..668d4b536b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FluentQuerySupport.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FluentQuerySupport.java @@ -15,8 +15,6 @@ */ package org.springframework.data.jpa.repository.support; -import jakarta.persistence.Query; - import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -26,7 +24,9 @@ import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.repository.query.AbstractJpaQuery; import org.springframework.data.projection.ProjectionFactory; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; /** @@ -41,6 +41,7 @@ */ abstract class FluentQuerySupport { + protected final ReturnedType returnedType; protected final Class resultType; protected final Sort sort; protected final int limit; @@ -51,6 +52,7 @@ abstract class FluentQuerySupport { FluentQuerySupport(Class resultType, Sort sort, int limit, @Nullable Collection properties, Class entityType, ProjectionFactory projectionFactory) { + this.returnedType = ReturnedType.of(resultType, entityType, projectionFactory); this.resultType = resultType; this.sort = sort; this.limit = limit; @@ -80,15 +82,20 @@ final Function getConversionFunction(Class inputType, Class tar return (Function) Function.identity(); } - if (targetType.isInterface()) { - return o -> projectionFactory.createProjection(targetType, o); + if (returnedType.isProjecting()) { + + AbstractJpaQuery.TupleConverter tupleConverter = new AbstractJpaQuery.TupleConverter(returnedType); + + if (resultType.isInterface()) { + return o -> projectionFactory.createProjection(targetType, tupleConverter.convert(o)); + } } return o -> DefaultConversionService.getSharedInstance().convert(o, targetType); } - interface ScrollQueryFactory { - Query createQuery(Sort sort, ScrollPosition scrollPosition); + interface ScrollQueryFactory { + Q createQuery(ReturnedType returnedType, Sort sort, ScrollPosition scrollPosition); } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java new file mode 100644 index 0000000000..cf6c29fdea --- /dev/null +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java @@ -0,0 +1,85 @@ +/* + * 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.support; + +import jakarta.persistence.Tuple; + +import java.util.Arrays; +import java.util.List; + +import com.querydsl.core.types.Expression; +import com.querydsl.core.types.ExpressionBase; +import com.querydsl.core.types.ExpressionUtils; +import com.querydsl.core.types.FactoryExpression; +import com.querydsl.core.types.Ops; +import com.querydsl.core.types.Path; +import com.querydsl.core.types.Projections; +import com.querydsl.core.types.Visitor; +import com.querydsl.jpa.JPQLSerializer; + +class JakartaTuple extends ExpressionBase { + + private final List> args; + + /** + * Create a new JakartaTuple instance + * + * @param args + */ + protected JakartaTuple(Expression... args) { + this(Arrays.asList(args)); + } + + /** + * Create a new JakartaTuple instance + * + * @param args + */ + protected JakartaTuple(List> args) { + super(Tuple.class); + this.args = args.stream().map(it -> { + + if (it instanceof Path p) { + return ExpressionUtils.operation(p.getType(), Ops.ALIAS, p, p); + } + + return it; + }).toList(); + } + + @Override + public R accept(Visitor v, C context) { + + if (v instanceof JPQLSerializer) { + return Projections.tuple(args).accept(v, context); + } + + return (R) this; + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } else if (obj instanceof FactoryExpression) { + FactoryExpression c = (FactoryExpression) obj; + return args.equals(c.getArgs()) && getType().equals(c.getType()); + } else { + return false; + } + } + +} diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java index 9979fc773b..9f25baa47d 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java @@ -161,7 +161,9 @@ public ID getId(T entity) { return (ID) t.get(idMetadata.getSimpleIdAttribute().getName()); } - return (ID) persistenceUnitUtil.getIdentifier(entity); + if (getJavaType().isInstance(entity)) { + return (ID) persistenceUnitUtil.getIdentifier(entity); + } } // otherwise, check if the complex id type has any partially filled fields @@ -172,6 +174,10 @@ public ID getId(T entity) { Object propertyValue = entityWrapper.getPropertyValue(attribute.getName()); + if (idMetadata.hasSimpleId()) { + return (ID) propertyValue; + } + if (propertyValue != null) { partialIdValueFound = true; } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java index 0ade24f133..b3faa0fb5c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java @@ -78,9 +78,9 @@ public Querydsl(EntityManager em, PathBuilder builder) { public AbstractJPAQuery> createQuery() { return switch (provider) { - case ECLIPSELINK -> new JPAQuery<>(em, EclipseLinkTemplates.DEFAULT); - case HIBERNATE -> new JPAQuery<>(em, HQLTemplates.DEFAULT); - default -> new JPAQuery<>(em); + case ECLIPSELINK -> new SpringDataJpaQuery<>(em, EclipseLinkTemplates.DEFAULT); + case HIBERNATE -> new SpringDataJpaQuery<>(em, HQLTemplates.DEFAULT); + default -> new SpringDataJpaQuery<>(em); }; } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java index c16f95c0a1..a2abcb9e38 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java @@ -34,7 +34,6 @@ import org.springframework.data.jpa.repository.query.KeysetScrollDelegate; import org.springframework.data.jpa.repository.query.KeysetScrollDelegate.QueryStrategy; import org.springframework.data.jpa.repository.query.KeysetScrollSpecification; -import org.springframework.data.jpa.repository.support.FetchableFluentQueryByPredicate.PredicateScrollDelegate; import org.springframework.data.jpa.repository.support.FluentQuerySupport.ScrollQueryFactory; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; @@ -188,7 +187,7 @@ public R findBy(Predicate predicate, Function { + ScrollQueryFactory> scroll = (returnedType, sort, scrollPosition) -> { Predicate predicateToUse = predicate; @@ -214,7 +213,7 @@ public R findBy(Predicate predicate, Function> pagedFinder = (sort, pageable) -> { @@ -229,10 +228,11 @@ public R findBy(Predicate predicate, Function fluentQuery = new FetchableFluentQueryByPredicate<>( // + path, predicate, // - this.entityInformation.getJavaType(), // + this.entityInformation, // finder, // - new PredicateScrollDelegate<>(scroll, entityInformation), // + scroll, // pagedFinder, // this::count, // this::exists, // diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index bf2faea2f4..80d2325076 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -30,16 +30,19 @@ import jakarta.persistence.criteria.Path; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; +import jakarta.persistence.criteria.Selection; import java.io.Serial; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Function; import org.springframework.data.domain.Example; @@ -48,6 +51,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.convert.QueryByExamplePredicateBuilder; import org.springframework.data.jpa.domain.Specification; @@ -60,9 +64,11 @@ import org.springframework.data.jpa.repository.support.FluentQuerySupport.ScrollQueryFactory; import org.springframework.data.jpa.repository.support.QueryHints.NoHints; import org.springframework.data.jpa.support.PageableUtils; +import org.springframework.data.mapping.PropertyPath; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.support.PageableExecutionUtils; import org.springframework.data.util.ProxyUtils; import org.springframework.data.util.Streamable; @@ -107,7 +113,7 @@ public class SimpleJpaRepository implements JpaRepositoryImplementation entityInformation, EntityM this.entityInformation = entityInformation; this.entityManager = entityManager; this.provider = PersistenceProvider.fromEntityManager(entityManager); + this.projectionFactory = new SpelAwareProxyProjectionFactory(); } /** @@ -502,7 +509,7 @@ private R doFindBy(Specification spec, Class domainClass, Assert.notNull(spec, "Specification must not be null"); Assert.notNull(queryFunction, "Query function must not be null"); - ScrollQueryFactory scrollFunction = (sort, scrollPosition) -> { + ScrollQueryFactory> scrollFunction = (returnedType, sort, scrollPosition) -> { Specification specToUse = spec; @@ -512,7 +519,7 @@ private R doFindBy(Specification spec, Class domainClass, specToUse = specToUse.and(keysetSpec); } - TypedQuery query = getQuery(specToUse, domainClass, sort); + TypedQuery query = getQuery(returnedType, specToUse, domainClass, sort, scrollPosition); if (scrollPosition instanceof OffsetScrollPosition offset) { if (!offset.isInitial()) { @@ -523,7 +530,8 @@ private R doFindBy(Specification spec, Class domainClass, return query; }; - Function> finder = sort -> getQuery(spec, domainClass, sort); + BiFunction> finder = (returnedType, sort) -> getQuery(returnedType, spec, + domainClass, sort, null); SpecificationScrollDelegate scrollDelegate = new SpecificationScrollDelegate<>(scrollFunction, entityInformation); @@ -745,12 +753,63 @@ protected TypedQuery getQuery(@Nullable Specification spec, Sort sort) { * @param sort must not be {@literal null}. */ protected TypedQuery getQuery(@Nullable Specification spec, Class domainClass, Sort sort) { + return getQuery(ReturnedType.of(domainClass, domainClass, projectionFactory), spec, domainClass, sort, null); + } + + /** + * Creates a {@link TypedQuery} for the given {@link Specification} and {@link Sort}. + * + * @param returnedType must not be {@literal null}. + * @param spec can be {@literal null}. + * @param domainClass must not be {@literal null}. + * @param sort must not be {@literal null}. + */ + private TypedQuery getQuery(ReturnedType returnedType, @Nullable Specification spec, + Class domainClass, Sort sort, @Nullable ScrollPosition scrollPosition) { CriteriaBuilder builder = entityManager.getCriteriaBuilder(); - CriteriaQuery query = builder.createQuery(domainClass); + CriteriaQuery query; + + List inputProperties = returnedType.getInputProperties(); + + if (returnedType.needsCustomConstruction() && !inputProperties.isEmpty()) { + query = (CriteriaQuery) (returnedType.getReturnedType().isInterface() ? builder.createTupleQuery() + : builder.createQuery(returnedType.getReturnedType())); + } else { + query = builder.createQuery(domainClass); + } Root root = applySpecificationToCriteria(spec, domainClass, query); - query.select(root); + + if (returnedType.needsCustomConstruction() && !inputProperties.isEmpty()) { + + Collection requiredSelection; + + if (scrollPosition instanceof KeysetScrollPosition && returnedType.getReturnedType().isInterface()) { + requiredSelection = new LinkedHashSet<>(inputProperties); + sort.stream().map(Sort.Order::getProperty).forEach(requiredSelection::add); + entityInformation.getIdAttributeNames().forEach(requiredSelection::add); + } else { + requiredSelection = inputProperties; + } + + List> selections = new ArrayList<>(); + + for (String property : requiredSelection) { + + PropertyPath path = PropertyPath.from(property, returnedType.getDomainType()); + selections.add(QueryUtils.toExpressionRecursively(root, path, true).alias(property)); + } + + Class typeToRead = returnedType.getReturnedType(); + + query = typeToRead.isInterface() // + ? query.multiselect(selections) // + : query.select((Selection) builder.construct(typeToRead, // + selections.toArray(new Selection[0]))); + } else { + query.select(root); + } if (sort.isSorted()) { query.orderBy(toOrders(sort, root, builder)); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java new file mode 100644 index 0000000000..5d747dea4c --- /dev/null +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java @@ -0,0 +1,92 @@ +/* + * 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.support; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.Query; +import jakarta.persistence.Tuple; + +import java.util.Map; + +import org.springframework.lang.Nullable; + +import com.querydsl.core.QueryModifiers; +import com.querydsl.core.types.Expression; +import com.querydsl.core.types.FactoryExpression; +import com.querydsl.jpa.JPQLSerializer; +import com.querydsl.jpa.JPQLTemplates; +import com.querydsl.jpa.impl.JPAQuery; +import com.querydsl.jpa.impl.JPAUtil; + +/** + * @author Mark Paluch + */ +class SpringDataJpaQuery extends JPAQuery { + + public SpringDataJpaQuery(EntityManager em) { + super(em); + } + + public SpringDataJpaQuery(EntityManager em, JPQLTemplates templates) { + super(em, templates); + } + + protected Query createQuery(@Nullable QueryModifiers modifiers, boolean forCount) { + + JPQLSerializer serializer = serialize(forCount); + String queryString = serializer.toString(); + logQuery(queryString); + + Query query = getMetadata().getProjection() instanceof JakartaTuple + ? entityManager.createQuery(queryString, Tuple.class) + : entityManager.createQuery(queryString); + + JPAUtil.setConstants(query, serializer.getConstants(), getMetadata().getParams()); + if (modifiers != null && modifiers.isRestricting()) { + Integer limit = modifiers.getLimitAsInteger(); + Integer offset = modifiers.getOffsetAsInteger(); + if (limit != null) { + query.setMaxResults(limit); + } + if (offset != null) { + query.setFirstResult(offset); + } + } + if (lockMode != null) { + query.setLockMode(lockMode); + } + if (flushMode != null) { + query.setFlushMode(flushMode); + } + + for (Map.Entry entry : hints.entrySet()) { + query.setHint(entry.getKey(), entry.getValue()); + } + + // set transformer, if necessary and possible + Expression projection = getMetadata().getProjection(); + this.projection = null; // necessary when query is reused + + if (!forCount && projection instanceof FactoryExpression) { + if (!queryHandler.transform(query, (FactoryExpression) projection)) { + this.projection = (FactoryExpression) projection; + } + } + + return query; + } + +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index caf40e1d99..e23f383fb1 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -1409,6 +1409,57 @@ void scrollByPredicateKeysetBackward() { assertThat(previousWindow.hasNext()).isFalse(); } + @Test // GH-2327 + void scrollByPredicateKeysetWithInterfaceProjection() { + + User jane1 = new User("Jane", "Doe", "jane@doe1.com"); + User jane2 = new User("Jane", "Doe", "jane@doe2.com"); + User john1 = new User("John", "Doe", "john@doe1.com"); + User john2 = new User("John", "Doe", "john@doe2.com"); + + repository.saveAllAndFlush(Arrays.asList(john1, john2, jane1, jane2)); + + Window firstWindow = repository.findBy(QUser.user.firstname.startsWith("J"), + q -> q.limit(1).sortBy(Sort.by("firstname", "emailAddress")).as(UserProjectionInterfaceBased.class) + .scroll(ScrollPosition.keyset())); + + assertThat(firstWindow.getContent()).extracting(UserProjectionInterfaceBased::getFirstname) + .containsOnly(jane1.getFirstname()); + assertThat(firstWindow.hasNext()).isTrue(); + + Window nextWindow = repository.findBy(QUser.user.firstname.startsWith("J"), + q -> q.limit(2).sortBy(Sort.by("firstname", "emailAddress")).as(UserProjectionInterfaceBased.class) + .scroll(firstWindow.positionAt(0))); + + assertThat(nextWindow.getContent()).extracting(UserProjectionInterfaceBased::getFirstname) + .containsExactly(jane2.getFirstname(), john1.getFirstname()); + assertThat(nextWindow.hasNext()).isTrue(); + } + + @Test // GH-2327 + void scrollByPredicateKeysetWithDtoProjection() { + + User jane1 = new User("Jane", "Doe", "jane@doe1.com"); + User jane2 = new User("Jane", "Doe", "jane@doe2.com"); + User john1 = new User("John", "Doe", "john@doe1.com"); + User john2 = new User("John", "Doe", "john@doe2.com"); + + repository.saveAllAndFlush(Arrays.asList(john1, john2, jane1, jane2)); + + Window firstWindow = repository.findBy(QUser.user.firstname.startsWith("J"), + q -> q.limit(1).sortBy(Sort.by("firstname", "emailAddress")).as(UserDto.class).scroll(ScrollPosition.keyset())); + + assertThat(firstWindow.getContent()).extracting(UserDto::firstname).containsOnly(jane1.getFirstname()); + assertThat(firstWindow.hasNext()).isTrue(); + + Window nextWindow = repository.findBy(QUser.user.firstname.startsWith("J"), q -> q.limit(2) + .sortBy(Sort.by("firstname", "emailAddress")).as(UserDto.class).scroll(firstWindow.positionAt(0))); + + assertThat(nextWindow.getContent()).extracting(UserDto::firstname).containsExactly(jane2.getFirstname(), + john1.getFirstname()); + assertThat(nextWindow.hasNext()).isTrue(); + } + @Test // GH-2878 void scrollByPartTreeKeysetBackward() { @@ -2556,40 +2607,6 @@ void findByFluentExampleWithSortedInterfaceBasedProjection() { .containsExactlyInAnyOrder(thirdUser.getFirstname(), firstUser.getFirstname(), fourthUser.getFirstname()); } - @Test // GH-2294 - void fluentExamplesWithClassBasedDtosNotYetSupported() { - - class UserDto { - String firstname; - - public UserDto() {} - - public String getFirstname() { - return this.firstname; - } - - public void setFirstname(String firstname) { - this.firstname = firstname; - } - - public String toString() { - return "UserDto(firstname=" + this.getFirstname() + ")"; - } - } - - assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> { - - User prototype = new User(); - prototype.setFirstname("v"); - - repository.findBy( - of(prototype, - matching().withIgnorePaths("age", "createdAt", "active").withMatcher("firstname", - GenericPropertyMatcher::contains)), // - q -> q.as(UserDto.class).sortBy(Sort.by("firstname")).all()); - }); - } - @Test // GH-2294 void countByFluentExample() { @@ -2691,6 +2708,17 @@ void findByFluentSpecificationWithInterfaceBasedProjection() { .containsExactlyInAnyOrder(firstUser.getFirstname(), thirdUser.getFirstname(), fourthUser.getFirstname()); } + @Test // GH-2327 + void findByFluentSpecificationWithDtoProjection() { + + flushTestUsers(); + + List users = repository.findBy(userHasFirstnameLike("v"), q -> q.as(UserDto.class).all()); + + assertThat(users).extracting(UserDto::firstname).containsExactlyInAnyOrder(firstUser.getFirstname(), + thirdUser.getFirstname(), fourthUser.getFirstname()); + } + @Test // GH-2274 void findByFluentSpecificationWithSimplePropertyPathsDoesntLoadUnrequestedPaths() { @@ -2801,32 +2829,6 @@ void findByFluentSpecificationWithSortedInterfaceBasedProjection() { .containsExactlyInAnyOrder(thirdUser.getFirstname(), firstUser.getFirstname(), fourthUser.getFirstname()); } - @Test // GH-2274 - void fluentSpecificationWithClassBasedDtosNotYetSupported() { - - class UserDto { - String firstname; - - public UserDto() {} - - public String getFirstname() { - return this.firstname; - } - - public void setFirstname(String firstname) { - this.firstname = firstname; - } - - public String toString() { - return "UserDto(firstname=" + this.getFirstname() + ")"; - } - } - - assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> { - repository.findBy(userHasFirstnameLike("v"), q -> q.as(UserDto.class).sortBy(Sort.by("firstname")).all()); - }); - } - @Test // GH-2274 void countByFluentSpecification() { @@ -3455,6 +3457,10 @@ private interface UserProjectionInterfaceBased { String getFirstname(); } + record UserDto(Integer id, String firstname, String lastname, String emailAddress) { + + } + private interface UserProjectionUsingSpEL { @Value("#{@greetingsFrom.groot(target.firstname)}") diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicateUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicateUnitTests.java index f2c5e9f00c..8d2b159c79 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicateUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicateUnitTests.java @@ -20,6 +20,8 @@ import org.junit.jupiter.api.Test; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; +import org.springframework.data.jpa.domain.sample.User; +import org.springframework.data.projection.SpelAwareProxyProjectionFactory; /** * Unit tests for {@link FetchableFluentQueryByPredicate}. @@ -32,10 +34,13 @@ class FetchableFluentQueryByPredicateUnitTests { @SuppressWarnings({ "rawtypes", "unchecked" }) void multipleSortBy() { + JpaEntityInformationSupport entityInformation = new JpaEntityInformationSupportUnitTests.DummyJpaEntityInformation( + User.class); + Sort s1 = Sort.by(Order.by("s1")); Sort s2 = Sort.by(Order.by("s2")); - FetchableFluentQueryByPredicate f = new FetchableFluentQueryByPredicate(null, null, null, null, null, null, null, - null, null); + FetchableFluentQueryByPredicate f = new FetchableFluentQueryByPredicate(null, null, entityInformation, null, null, + null, null, null, null, new SpelAwareProxyProjectionFactory()); f = (FetchableFluentQueryByPredicate) f.sortBy(s1).sortBy(s2); assertThat(f.sort).isEqualTo(s1.and(s2)); } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutorUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutorUnitTests.java index 7962695b6a..b7995dec45 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutorUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutorUnitTests.java @@ -23,13 +23,13 @@ import java.sql.Date; import java.time.LocalDate; import java.util.List; -import java.util.Set; import java.util.stream.Stream; import org.hibernate.LazyInitializationException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; + import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; @@ -403,6 +403,16 @@ void findByFluentPredicateWithInterfaceBasedProjection() { .containsExactlyInAnyOrder(dave.getFirstname(), oliver.getFirstname()); } + @Test // GH-2327 + void findByFluentPredicateWithDtoProjection() { + + List users = predicateExecutor.findBy(user.firstname.contains("v"), + q -> q.as(UserProjectionDto.class).all()); + + assertThat(users).extracting(UserProjectionDto::firstname).containsExactlyInAnyOrder(dave.getFirstname(), + oliver.getFirstname()); + } + @Test // GH-2294 void findByFluentPredicateWithSortedInterfaceBasedProjection() { @@ -435,31 +445,6 @@ void existsByFluentPredicate() { assertThat(exists).isTrue(); } - @Test // GH-2294 - void fluentExamplesWithClassBasedDtosNotYetSupported() { - - class UserDto { - String firstname; - - public UserDto() {} - - public String getFirstname() { - return this.firstname; - } - - public void setFirstname(String firstname) { - this.firstname = firstname; - } - - public String toString() { - return "UserDto(firstname=" + this.getFirstname() + ")"; - } - } - - assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> predicateExecutor - .findBy(user.firstname.contains("v"), q -> q.as(UserDto.class).sortBy(Sort.by("firstname")).all())); - } - @Test // GH-2329 void findByFluentPredicateWithSimplePropertyPathsDoesntLoadUnrequestedPaths() { @@ -534,6 +519,9 @@ private interface UserProjectionInterfaceBased { String getFirstname(); - Set getRoles(); + String getLastname(); + } + + public record UserProjectionDto(String firstname, String lastname) { } } From cb368dbad4a81d948f6e008d71d4d62f4fc533e4 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 25 Sep 2024 09:44:40 +0200 Subject: [PATCH 05/10] Polishing. Refactor code duplications. See #2327 --- .../query/JpaKeysetScrollQueryCreator.java | 8 ++----- .../query/KeysetScrollDelegate.java | 22 +++++++++++++++++++ .../FetchableFluentQueryByPredicate.java | 8 +++---- .../support/SimpleJpaRepository.java | 10 ++++----- .../jpa/repository/UserRepositoryTests.java | 10 +++++++++ 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaKeysetScrollQueryCreator.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaKeysetScrollQueryCreator.java index 25e7c25ca9..9d767d004f 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaKeysetScrollQueryCreator.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaKeysetScrollQueryCreator.java @@ -21,8 +21,6 @@ import jakarta.persistence.criteria.Root; import java.util.Collection; -import java.util.LinkedHashSet; -import java.util.Set; import org.springframework.data.domain.KeysetScrollPosition; import org.springframework.data.domain.Sort; @@ -77,9 +75,7 @@ Collection getRequiredSelection(Sort sort, ReturnedType returnedType) { Sort sortToUse = KeysetScrollSpecification.createSort(scrollPosition, sort, entityInformation); - Set selection = new LinkedHashSet<>(returnedType.getInputProperties()); - sortToUse.forEach(it -> selection.add(it.getProperty())); - - return selection; + return KeysetScrollDelegate.getProjectionInputProperties(entityInformation, returnedType.getInputProperties(), + sortToUse); } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java index 2942fa0bce..66b9245b60 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java @@ -16,7 +16,9 @@ package org.springframework.data.jpa.repository.query; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -24,6 +26,7 @@ import org.springframework.data.domain.ScrollPosition.Direction; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; +import org.springframework.data.jpa.repository.support.JpaEntityInformation; import org.springframework.lang.Nullable; /** @@ -47,6 +50,25 @@ public static KeysetScrollDelegate of(Direction direction) { return direction == Direction.FORWARD ? FORWARD : REVERSE; } + /** + * Return a collection of property names required to construct a keyset selection query that include all keyset and + * identifier properties required to resume keyset scrolling. + * + * @param entity the underlying entity. + * @param projectionProperties projection property names. + * @param sort sort properties. + * @return a collection of property names required to construct a keyset selection query + */ + public static Collection getProjectionInputProperties(JpaEntityInformation entity, + Collection projectionProperties, Sort sort) { + + Collection properties = new LinkedHashSet<>(projectionProperties); + sort.forEach(it -> properties.add(it.getProperty())); + properties.addAll(entity.getIdAttributeNames()); + + return properties; + } + @Nullable public P createPredicate(KeysetScrollPosition keyset, Sort sort, QueryStrategy strategy) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java index 335b6e33bb..2e9a1fbb22 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.LinkedHashSet; import java.util.List; import java.util.function.BiFunction; import java.util.function.Function; @@ -34,6 +33,7 @@ import org.springframework.data.domain.ScrollPosition; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Window; +import org.springframework.data.jpa.repository.query.KeysetScrollDelegate; import org.springframework.data.jpa.repository.query.ScrollDelegate; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; @@ -219,13 +219,11 @@ private void applyQuerySettings(ReturnedType returnedType, int limit, AbstractJP List inputProperties = returnedType.getInputProperties(); - if (returnedType.needsCustomConstruction() && !inputProperties.isEmpty()) { + if (returnedType.needsCustomConstruction()) { Collection requiredSelection; if (scrollPosition instanceof KeysetScrollPosition && returnedType.getReturnedType().isInterface()) { - requiredSelection = new LinkedHashSet<>(inputProperties); - sort.forEach(it -> requiredSelection.add(it.getProperty())); - entityInformation.getIdAttributeNames().forEach(requiredSelection::add); + requiredSelection = KeysetScrollDelegate.getProjectionInputProperties(entityInformation, inputProperties, sort); } else { requiredSelection = inputProperties; } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index 80d2325076..99d3199c62 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -37,7 +37,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -58,6 +57,7 @@ import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.query.EscapeCharacter; +import org.springframework.data.jpa.repository.query.KeysetScrollDelegate; import org.springframework.data.jpa.repository.query.KeysetScrollSpecification; import org.springframework.data.jpa.repository.query.QueryUtils; import org.springframework.data.jpa.repository.support.FetchableFluentQueryBySpecification.SpecificationScrollDelegate; @@ -772,7 +772,7 @@ private TypedQuery getQuery(ReturnedType returnedType, @Nullabl List inputProperties = returnedType.getInputProperties(); - if (returnedType.needsCustomConstruction() && !inputProperties.isEmpty()) { + if (returnedType.needsCustomConstruction()) { query = (CriteriaQuery) (returnedType.getReturnedType().isInterface() ? builder.createTupleQuery() : builder.createQuery(returnedType.getReturnedType())); } else { @@ -781,14 +781,12 @@ private TypedQuery getQuery(ReturnedType returnedType, @Nullabl Root root = applySpecificationToCriteria(spec, domainClass, query); - if (returnedType.needsCustomConstruction() && !inputProperties.isEmpty()) { + if (returnedType.needsCustomConstruction()) { Collection requiredSelection; if (scrollPosition instanceof KeysetScrollPosition && returnedType.getReturnedType().isInterface()) { - requiredSelection = new LinkedHashSet<>(inputProperties); - sort.stream().map(Sort.Order::getProperty).forEach(requiredSelection::add); - entityInformation.getIdAttributeNames().forEach(requiredSelection::add); + requiredSelection = KeysetScrollDelegate.getProjectionInputProperties(entityInformation, inputProperties, sort); } else { requiredSelection = inputProperties; } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index e23f383fb1..c03f8132c1 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -2959,6 +2959,16 @@ void dynamicProjectionReturningList() { assertThat(users).hasSize(1); } + @Test // GH-2327 + void dynamicOpenProjectionReturningList() { + + flushTestUsers(); + + List users = repository.findAsListByFirstnameLike("%O%", UserProjectionUsingSpEL.class); + + assertThat(users).hasSize(1); + } + @Test // DATAJPA-1179 void duplicateSpelsWorkAsIntended() { From 5e800b0d9cb956a0b8938115f7a36d09c1cb1db8 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 22 Nov 2024 11:59:31 +0100 Subject: [PATCH 06/10] Polishing. There's a difference in what the query needs to look like using dto vs. interface projections where the former does not allow column aliases and the latter requires them. See #2327 --- .../repository/query/AbstractJpaQuery.java | 21 ++++ .../query/AbstractStringBasedJpaQuery.java | 12 +- .../EclipseLinkUserRepositoryFinderTests.java | 4 + .../repository/UserRepositoryFinderTests.java | 112 +++++++++++++++--- .../JpqlDtoQueryTransformerUnitTests.java | 20 ++++ .../jpa/repository/sample/UserRepository.java | 31 +++++ .../ROOT/pages/repositories/projections.adoc | 7 ++ 7 files changed, 190 insertions(+), 17 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index f797284c5a..7b3d225e9f 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -23,6 +23,8 @@ import jakarta.persistence.TupleElement; import jakarta.persistence.TypedQuery; +import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -51,6 +53,7 @@ import org.springframework.jdbc.support.JdbcUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; /** * Abstract base class to implement {@link RepositoryQuery}s. @@ -353,6 +356,24 @@ public Object convert(Object source) { } } + if(type.isProjecting() && !type.getReturnedType().isInterface() && !type.getInputProperties().isEmpty()) { + List ctorArgs = new ArrayList<>(type.getInputProperties().size()); + type.getInputProperties().forEach(it -> { + ctorArgs.add(tuple.get(it)); + }); + try { + return type.getReturnedType().getConstructor(ctorArgs.stream().map(Object::getClass).toArray(Class[]::new)).newInstance(ctorArgs.toArray()); + } catch (InstantiationException e) { + throw new RuntimeException(e); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } catch (InvocationTargetException e) { + throw new RuntimeException(e); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + return new TupleBackedMap(tupleWrapper.apply(tuple)); } 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 16ab1d21a5..7b89245661 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 @@ -119,7 +119,13 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) { Sort sort = accessor.getSort(); ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor); - String sortedQueryString = getSortedQueryString(sort, processor.getReturnedType()); + + String sortedQueryString = null; + if(querySortRewriter.equals(NoOpQuerySortRewriter.INSTANCE) && accessor.findDynamicProjection() != null && !accessor.findDynamicProjection().isInterface()) { + sortedQueryString = getSortedQueryString(new ProjectingSortRewriter(), query, sort, processor.getReturnedType()); + } else { + sortedQueryString = getSortedQueryString(sort, processor.getReturnedType()); + } Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), processor.getReturnedType()); @@ -134,6 +140,10 @@ String getSortedQueryString(Sort sort, ReturnedType returnedType) { return querySortRewriter.getSorted(query, sort, returnedType); } + private static String getSortedQueryString(QuerySortRewriter rewriter, DeclaredQuery query, Sort sort, ReturnedType returnedType) { + return rewriter.getSorted(query, sort, returnedType); + } + @Override protected ParameterBinder createBinder() { return createBinder(query); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/EclipseLinkUserRepositoryFinderTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/EclipseLinkUserRepositoryFinderTests.java index 99343adb99..18e0570de8 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/EclipseLinkUserRepositoryFinderTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/EclipseLinkUserRepositoryFinderTests.java @@ -36,4 +36,8 @@ void executesNotInQueryCorrectly() {} @Override void executesInKeywordForPageCorrectly() {} + @Disabled + @Override + void rawMapProjectionWithEntityAndAggregatedValue() {} + } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java index d887cc51bd..09281ed623 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryFinderTests.java @@ -15,18 +15,24 @@ */ package org.springframework.data.jpa.repository; -import static org.assertj.core.api.Assertions.*; -import static org.springframework.data.domain.Sort.Direction.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.springframework.data.domain.Sort.Direction.ASC; +import static org.springframework.data.domain.Sort.Direction.DESC; import jakarta.persistence.EntityManager; import java.util.Arrays; import java.util.List; +import java.util.Map; +import org.assertj.core.data.Offset; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Limit; @@ -45,6 +51,9 @@ import org.springframework.data.jpa.repository.sample.UserRepository.IdOnly; import org.springframework.data.jpa.repository.sample.UserRepository.NameOnly; import org.springframework.data.jpa.repository.sample.UserRepository.RolesAndFirstname; +import org.springframework.data.jpa.repository.sample.UserRepository.UserExcerpt; +import org.springframework.data.jpa.repository.sample.UserRepository.UserRoleCountDtoProjection; +import org.springframework.data.jpa.repository.sample.UserRepository.UserRoleCountInterfaceProjection; import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -247,9 +256,9 @@ void executesQueryWithLimitAndScrollPosition() { @Test // GH-3409 void executesWindowQueryWithPageable() { - Window first = userRepository.findByLastnameOrderByFirstname("Matthews", PageRequest.of(0,1)); + Window first = userRepository.findByLastnameOrderByFirstname("Matthews", PageRequest.of(0, 1)); - Window next = userRepository.findByLastnameOrderByFirstname("Matthews", PageRequest.of(1,1)); + Window next = userRepository.findByLastnameOrderByFirstname("Matthews", PageRequest.of(1, 1)); assertThat(first).containsExactly(dave); assertThat(next).containsExactly(oliver); @@ -406,21 +415,92 @@ void findByNegatingSimplePropertyUsingMixedNullNonNullArgument() { assertThat(result).containsExactly(carter); } - @Test // GH-3076 - void dtoProjectionShouldApplyConstructorExpressionRewriting() { + @Test // GH-3076 + void dtoProjectionShouldApplyConstructorExpressionRewriting() { - List dtos = userRepository.findRecordProjection(); + List dtos = userRepository.findRecordProjection(); - assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) // - .contains("Dave", "Carter", "Oliver August"); - } + assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) // + .contains("Dave", "Carter", "Oliver August"); + } + + @Test // GH-3076 + void dtoMultiselectProjectionShouldApplyConstructorExpressionRewriting() { + + List dtos = userRepository.findMultiselectRecordProjection(); + + assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) // + .contains("Dave", "Carter", "Oliver August"); + } + + @Test // GH-3076 + void dynamicDtoProjection() { + + List dtos = userRepository.findRecordProjection(UserExcerpt.class); + + assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) // + .contains("Dave", "Carter", "Oliver August"); + } + + @Test // GH-3076 + void dtoProjectionWithEntityAndAggregatedValue() { + + Map musicians = Map.of(carter.getFirstname(), carter, dave.getFirstname(), dave, + oliver.getFirstname(), oliver); + + assertThat(userRepository.dtoProjectionEntityAndAggregatedValue()).allSatisfy(projection -> { + assertThat(projection.user()).isIn(musicians.values()); + assertThat(projection.roleCount()).isCloseTo(musicians.get(projection.user().getFirstname()).getRoles().size(), + Offset.offset(0L)); + }); + } - @Test // GH-3076 - void dtoMultiselectProjectionShouldApplyConstructorExpressionRewriting() { + @Test // GH-3076 + void interfaceProjectionWithEntityAndAggregatedValue() { - List dtos = userRepository.findMultiselectRecordProjection(); + Map musicians = Map.of(carter.getFirstname(), carter, dave.getFirstname(), dave, + oliver.getFirstname(), oliver); + + assertThat(userRepository.interfaceProjectionEntityAndAggregatedValue()).allSatisfy(projection -> { + assertThat(projection.getUser()).isIn(musicians.values()); + assertThat(projection.getRoleCount()) + .isCloseTo(musicians.get(projection.getUser().getFirstname()).getRoles().size(), Offset.offset(0L)); + }); + } + + @Test // GH-3076 + void rawMapProjectionWithEntityAndAggregatedValue() { + + Map musicians = Map.of(carter.getFirstname(), carter, dave.getFirstname(), dave, + oliver.getFirstname(), oliver); + + assertThat(userRepository.rawMapProjectionEntityAndAggregatedValue()).allSatisfy(projection -> { + assertThat(projection.get("user")).isIn(musicians.values()); + assertThat(projection).containsKey("roleCount"); + }); + } + + @Test // GH-3076 + void dtoProjectionWithEntityAndAggregatedValueWithPageable() { + + Map musicians = Map.of(carter.getFirstname(), carter, dave.getFirstname(), dave, + oliver.getFirstname(), oliver); + + assertThat( + userRepository.dtoProjectionEntityAndAggregatedValue(PageRequest.of(0, 10).withSort(Sort.by("firstname")))) + .allSatisfy(projection -> { + assertThat(projection.user()).isIn(musicians.values()); + assertThat(projection.roleCount()) + .isCloseTo(musicians.get(projection.user().getFirstname()).getRoles().size(), Offset.offset(0L)); + }); + } + + @ParameterizedTest // GH-3076 + @ValueSource(classes = { UserRoleCountDtoProjection.class, UserRoleCountInterfaceProjection.class }) + void dynamicProjectionWithEntityAndAggregated(Class resultType) { + + assertThat(userRepository.findMultiselectRecordDynamicProjection(resultType)).hasSize(3) + .hasOnlyElementsOfType(resultType); + } - assertThat(dtos).flatExtracting(UserRepository.UserExcerpt::firstname) // - .contains("Dave", "Carter", "Oliver August"); - } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java index de6b22c90e..ab13c719a1 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlDtoQueryTransformerUnitTests.java @@ -49,6 +49,21 @@ void shouldTranslateSingleProjectionToDto() { "SELECT new org.springframework.data.jpa.repository.query.JpqlDtoQueryTransformerUnitTests$MyRecord(p.foo, p.bar) from Person p"); } +// @Test // GH-3076 +// void xxx() { +// +// JpaQueryMethod method = getMethod("dtoProjection2"); +// JpqlSortedQueryTransformer transformer = new JpqlSortedQueryTransformer(Sort.unsorted(), null, +// method.getResultProcessor().getReturnedType()); +// +// JpaQueryEnhancer.JpqlQueryParser parser = JpaQueryEnhancer.JpqlQueryParser.parseQuery("select u.foo, u.bar, count(r) from User u left outer join u.role r group by u"); +// +// QueryTokenStream visit = transformer.visit(parser.getContext()); +// +// assertThat(QueryRenderer.TokenRenderer.render(visit)).isEqualTo( +// "select new org.springframework.data.jpa.repository.query.JpqlDtoQueryTransformerUnitTests$MyRecord2(u.foo, u.bar, count(r)) from User u left outer join u.role r group by u"); +// } + @Test // GH-3076 void shouldRewriteQueriesWithSubselect() { @@ -100,6 +115,7 @@ private JpaQueryMethod getMethod(String name, Class... parameterTypes) { interface MyRepo extends Repository { MyRecord dtoProjection(); + MyRecord2 dtoProjection2(); } record Person(String id) { @@ -109,4 +125,8 @@ record Person(String id) { record MyRecord(String foo, String bar) { } + + record MyRecord2(String foo, String bar, Integer count) { + + } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index ac6a2ea9b1..1676f886a5 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -18,6 +18,8 @@ import jakarta.persistence.EntityManager; import jakarta.persistence.QueryHint; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.Collection; import java.util.Date; import java.util.List; @@ -724,11 +726,33 @@ List findAllAndSortByFunctionResultNamedParameter(@Param("namedParameter @Query("select u from User u") List findRecordProjection(); + @Query("select u from User u") + List findRecordProjection(Class projectionType); + @Query("select u.firstname, u.lastname from User u") List findMultiselectRecordProjection(); + @UserRoleCountProjectingQuery + List dtoProjectionEntityAndAggregatedValue(); + + @UserRoleCountProjectingQuery + Page dtoProjectionEntityAndAggregatedValue(PageRequest page); + + @Query("select u as user, count(r) as roleCount from User u left outer join u.roles r group by u") + List interfaceProjectionEntityAndAggregatedValue(); + + @Query("select u as user, count(r) as roleCount from User u left outer join u.roles r group by u") + List> rawMapProjectionEntityAndAggregatedValue(); + + @UserRoleCountProjectingQuery + List findMultiselectRecordDynamicProjection(Class projectionType); + Window findBy(OffsetScrollPosition position); + @Retention(RetentionPolicy.RUNTIME) + @Query("select u, count(r) from User u left outer join u.roles r group by u") + @interface UserRoleCountProjectingQuery {} + interface RolesAndFirstname { String getFirstname(); @@ -757,4 +781,11 @@ record UserExcerpt(String firstname, String lastname) { } + record UserRoleCountDtoProjection(User user, Long roleCount) {} + + interface UserRoleCountInterfaceProjection { + User getUser(); + Long getRoleCount(); + } + } diff --git a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc index c55be85997..8ac41c7d51 100644 --- a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc +++ b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc @@ -66,6 +66,13 @@ This query gets rewritten to `SELECT new UserDto(u.firstname, u.lastname) FROM U This query gets rewritten to `SELECT new UserDto(u.firstname, u.lastname) FROM USER u`. ==== +[WARNING] +==== +JPQL constructor expressions must not contain aliases for selected columns. +While `SELECT u as user, count(u.roles) as roleCount FROM USER u ...` is a valid usecase for interface based projections that rely on column names from the returned `Tuple`, the same construct is invalid when requesting a DTO where it needs to be `SELECT u, count(u.roles) FROM USER u ...`. + +Some persistence providers may be lenient about this, others not. +==== + Repository query methods that return a DTO projection type (a Java type outside the domain type hierarchy) are subject for query rewriting. If an `@Query`-annotated query already uses constructor expressions, then Spring Data backs off and doesn't apply DTO constructor expression rewriting. From f3cedc360723e001f7d311d8bce2a9a5bdd975d1 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 3 Dec 2024 09:10:55 +0100 Subject: [PATCH 07/10] Polishing. Simplify Querydsl templates retrieva and String query caching. Update documentation. Skip selection list rewriting if the returned type is an interface. Encapsulate rewrite information for Query. Reformat code. See #2327 --- .../repository/query/AbstractJpaQuery.java | 60 ++++++++++++------- .../query/AbstractStringBasedJpaQuery.java | 52 +++++++--------- .../query/DefaultQueryEnhancer.java | 5 +- .../query/DefaultQueryRewriteInformation.java | 37 ++++++++++++ .../DtoProjectionTransformerDelegate.java | 4 +- .../query/JSqlParserQueryEnhancer.java | 10 ++-- .../repository/query/JpaQueryEnhancer.java | 12 +++- .../jpa/repository/query/QueryEnhancer.java | 30 +++++++++- .../FetchableFluentQueryByPredicate.java | 15 ++--- .../jpa/repository/support/JakartaTuple.java | 17 +++++- .../data/jpa/repository/support/Querydsl.java | 36 +++++++---- .../support/SpringDataJpaQuery.java | 4 ++ .../ROOT/pages/repositories/projections.adoc | 2 +- 13 files changed, 197 insertions(+), 87 deletions(-) create mode 100644 spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryRewriteInformation.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index 7b3d225e9f..4914beb4d7 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -23,8 +23,6 @@ import jakarta.persistence.TupleElement; import jakarta.persistence.TypedQuery; -import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -34,6 +32,7 @@ import java.util.function.UnaryOperator; import java.util.stream.Collectors; +import org.springframework.beans.BeanUtils; import org.springframework.core.convert.converter.Converter; import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.jpa.repository.EntityGraph; @@ -46,6 +45,8 @@ import org.springframework.data.jpa.repository.query.JpaQueryExecution.StreamExecution; import org.springframework.data.jpa.repository.support.QueryHints; import org.springframework.data.jpa.util.JpaMetamodel; +import org.springframework.data.mapping.PreferredConstructor; +import org.springframework.data.mapping.model.PreferredConstructorDiscoverer; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; @@ -53,7 +54,7 @@ import org.springframework.jdbc.support.JdbcUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; /** * Abstract base class to implement {@link RepositoryQuery}s. @@ -288,8 +289,8 @@ protected Class getTypeToRead(ReturnedType returnedType) { return returnedType.isProjecting() && returnedType.getReturnedType().isInterface() && !getMetamodel().isJpaManaged(returnedType.getReturnedType()) // - ? Tuple.class // - : null; + ? Tuple.class // + : null; } /** @@ -314,6 +315,10 @@ public static class TupleConverter implements Converter { private final UnaryOperator tupleWrapper; + private final boolean dtoProjection; + + private final @Nullable PreferredConstructor preferredConstructor; + /** * Creates a new {@link TupleConverter} for the given {@link ReturnedType}. * @@ -336,6 +341,14 @@ public TupleConverter(ReturnedType type, boolean nativeQuery) { this.type = type; this.tupleWrapper = nativeQuery ? FallbackTupleWrapper::new : UnaryOperator.identity(); + this.dtoProjection = type.isProjecting() && !type.getReturnedType().isInterface() + && !type.getInputProperties().isEmpty(); + + if (this.dtoProjection) { + this.preferredConstructor = PreferredConstructorDiscoverer.discover(String.class); + } else { + this.preferredConstructor = null; + } } @Override @@ -356,23 +369,26 @@ public Object convert(Object source) { } } - if(type.isProjecting() && !type.getReturnedType().isInterface() && !type.getInputProperties().isEmpty()) { - List ctorArgs = new ArrayList<>(type.getInputProperties().size()); - type.getInputProperties().forEach(it -> { - ctorArgs.add(tuple.get(it)); - }); - try { - return type.getReturnedType().getConstructor(ctorArgs.stream().map(Object::getClass).toArray(Class[]::new)).newInstance(ctorArgs.toArray()); - } catch (InstantiationException e) { - throw new RuntimeException(e); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } catch (InvocationTargetException e) { - throw new RuntimeException(e); - } catch (NoSuchMethodException e) { - throw new RuntimeException(e); - } - } + if (dtoProjection) { + + Object[] ctorArgs = new Object[type.getInputProperties().size()]; + + for (int i = 0; i < type.getInputProperties().size(); i++) { + ctorArgs[i] = tuple.get(i); + } + + try { + + if (preferredConstructor.getParameterCount() == ctorArgs.length) { + return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs); + } + + return BeanUtils.instantiateClass(type.getReturnedType() + .getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class[]::new)), ctorArgs); + } catch (ReflectiveOperationException e) { + ReflectionUtils.handleReflectionException(e); + } + } return new TupleBackedMap(tupleWrapper.apply(tuple)); } 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 7b89245661..a85af07219 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 @@ -69,8 +69,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { * @param valueExpressionDelegate must not be {@literal null}. */ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, String queryString, - @Nullable String countQueryString, QueryRewriter queryRewriter, - ValueExpressionDelegate valueExpressionDelegate) { + @Nullable String countQueryString, QueryRewriter queryRewriter, ValueExpressionDelegate valueExpressionDelegate) { super(method, em); @@ -99,15 +98,17 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri }); this.queryRewriter = queryRewriter; - ReturnedType returnedType = method.getResultProcessor().getReturnedType(); JpaParameters parameters = method.getParameters(); - if ((parameters.hasPageableParameter() || parameters.hasSortParameter()) && !parameters.hasDynamicProjection()) { - this.querySortRewriter = new CachingQuerySortRewriter(); - } else if (returnedType.isProjecting() && !returnedType.getReturnedType().isInterface()) { - this.querySortRewriter = new ProjectingSortRewriter(); + + if (parameters.hasDynamicProjection()) { + this.querySortRewriter = SimpleQuerySortRewriter.INSTANCE; } else { - this.querySortRewriter = NoOpQuerySortRewriter.INSTANCE; + if (parameters.hasPageableParameter() || parameters.hasSortParameter()) { + this.querySortRewriter = new CachingQuerySortRewriter(); + } else { + this.querySortRewriter = new UnsortedCachingQuerySortRewriter(); + } } Assert.isTrue(method.isNativeQuery() || !query.usesJdbcStyleParameters(), @@ -119,19 +120,13 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) { Sort sort = accessor.getSort(); ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor); - - String sortedQueryString = null; - if(querySortRewriter.equals(NoOpQuerySortRewriter.INSTANCE) && accessor.findDynamicProjection() != null && !accessor.findDynamicProjection().isInterface()) { - sortedQueryString = getSortedQueryString(new ProjectingSortRewriter(), query, sort, processor.getReturnedType()); - } else { - sortedQueryString = getSortedQueryString(sort, processor.getReturnedType()); - } - - Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), processor.getReturnedType()); + ReturnedType returnedType = processor.getReturnedType(); + String sortedQueryString = getSortedQueryString(sort, returnedType); + Query query = createJpaQuery(sortedQueryString, sort, accessor.getPageable(), returnedType); QueryParameterSetter.QueryMetadata metadata = metadataCache.getMetadata(sortedQueryString, query); - // it is ok to reuse the binding contained in the ParameterBinder although we create a new query String because the + // it is ok to reuse the binding contained in the ParameterBinder, although we create a new query String because the // parameters in the query do not change. return parameterBinder.get().bindAndPrepare(query, metadata, accessor); } @@ -140,10 +135,6 @@ String getSortedQueryString(Sort sort, ReturnedType returnedType) { return querySortRewriter.getSorted(query, sort, returnedType); } - private static String getSortedQueryString(QuerySortRewriter rewriter, DeclaredQuery query, Sort sort, ReturnedType returnedType) { - return rewriter.getSorted(query, sort, returnedType); - } - @Override protected ParameterBinder createBinder() { return createBinder(query); @@ -223,8 +214,8 @@ protected String potentiallyRewriteQuery(String originalQuery, Sort sort, @Nulla String applySorting(CachableQuery cachableQuery) { - return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery()).rewrite(cachableQuery.getSort(), - cachableQuery.getReturnedType()); + return QueryEnhancerFactory.forQuery(cachableQuery.getDeclaredQuery()) + .rewrite(new DefaultQueryRewriteInformation(cachableQuery.getSort(), cachableQuery.getReturnedType())); } /** @@ -237,21 +228,17 @@ interface QuerySortRewriter { /** * No-op query rewriter. */ - enum NoOpQuerySortRewriter implements QuerySortRewriter { + enum SimpleQuerySortRewriter implements QuerySortRewriter { INSTANCE; public String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedType) { - if (sort.isSorted()) { - throw new UnsupportedOperationException("NoOpQueryCache does not support sorting"); - } - - return query.getQueryString(); + return QueryEnhancerFactory.forQuery(query).rewrite(new DefaultQueryRewriteInformation(sort, returnedType)); } } - static class ProjectingSortRewriter implements QuerySortRewriter { + static class UnsortedCachingQuerySortRewriter implements QuerySortRewriter { private volatile String cachedQueryString; @@ -263,7 +250,8 @@ public String getSorted(DeclaredQuery query, Sort sort, ReturnedType returnedTyp String cachedQueryString = this.cachedQueryString; if (cachedQueryString == null) { - this.cachedQueryString = cachedQueryString = QueryEnhancerFactory.forQuery(query).rewrite(sort, returnedType); + this.cachedQueryString = cachedQueryString = QueryEnhancerFactory.forQuery(query) + .rewrite(new DefaultQueryRewriteInformation(sort, returnedType)); } return cachedQueryString; diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java index 07ef7d15be..8ec778fb70 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java @@ -18,7 +18,6 @@ import java.util.Set; import org.springframework.data.domain.Sort; -import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; /** @@ -54,8 +53,8 @@ public String applySorting(Sort sort, @Nullable String alias) { } @Override - public String rewrite(Sort sort, ReturnedType returnedType) { - return QueryUtils.applySorting(this.query.getQueryString(), sort, alias); + public String rewrite(QueryRewriteInformation rewriteInformation) { + return QueryUtils.applySorting(this.query.getQueryString(), rewriteInformation.getSort(), alias); } @Override diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryRewriteInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryRewriteInformation.java new file mode 100644 index 0000000000..ee17ca3f04 --- /dev/null +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryRewriteInformation.java @@ -0,0 +1,37 @@ +/* + * 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 org.springframework.data.domain.Sort; +import org.springframework.data.repository.query.ReturnedType; + +/** + * Default {@link org.springframework.data.jpa.repository.query.QueryEnhancer.QueryRewriteInformation} implementation. + * + * @author Mark Paluch + */ +record DefaultQueryRewriteInformation(Sort sort, + ReturnedType returnedType) implements QueryEnhancer.QueryRewriteInformation { + @Override + public Sort getSort() { + return sort(); + } + + @Override + public ReturnedType getReturnedType() { + return returnedType(); + } +} diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java index 558575f15c..c87a5d63de 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DtoProjectionTransformerDelegate.java @@ -29,6 +29,7 @@ * * * @author Mark Paluch + * @since 3.5 */ class DtoProjectionTransformerDelegate { @@ -40,7 +41,8 @@ public DtoProjectionTransformerDelegate(ReturnedType returnedType) { public QueryTokenStream transformSelectionList(QueryTokenStream selectionList) { - if (!returnedType.isProjecting() || selectionList.stream().anyMatch(it -> it.equals(TOKEN_NEW))) { + if (!returnedType.isProjecting() || returnedType.getReturnedType().isInterface() + || selectionList.stream().anyMatch(it -> it.equals(TOKEN_NEW))) { return selectionList; } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java index a8a0ef19c8..f9ebe1efa7 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java @@ -50,7 +50,6 @@ import java.util.StringJoiner; import org.springframework.data.domain.Sort; -import org.springframework.data.repository.query.ReturnedType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; @@ -298,17 +297,20 @@ public DeclaredQuery getQuery() { @Override public String applySorting(Sort sort) { - return applySorting(sort, detectAlias()); + return doApplySorting(sort, detectAlias()); } @Override - public String rewrite(Sort sort, ReturnedType returnedType) { - return applySorting(sort, primaryAlias); + public String rewrite(QueryRewriteInformation rewriteInformation) { + return doApplySorting(rewriteInformation.getSort(), primaryAlias); } @Override public String applySorting(Sort sort, @Nullable String alias) { + return doApplySorting(sort, alias); + } + private String doApplySorting(Sort sort, @Nullable String alias) { String queryString = query.getQueryString(); Assert.hasText(queryString, "Query must not be null or empty"); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java index ad0597983a..f5ed753c97 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java @@ -200,8 +200,9 @@ public String applySorting(Sort sort) { } @Override - public String rewrite(Sort sort, ReturnedType returnedType) { - return QueryRenderer.TokenRenderer.render(sortFunction.apply(sort, detectAlias(), returnedType).visit(context)); + public String rewrite(QueryRewriteInformation rewriteInformation) { + return QueryRenderer.TokenRenderer.render(sortFunction + .apply(rewriteInformation.getSort(), detectAlias(), rewriteInformation.getReturnedType()).visit(context)); } /** @@ -319,6 +320,13 @@ public static JpqlQueryParser parseQuery(String query) throws BadJpqlGrammarExce } } + /** + * Functional interface to rewrite a query considering {@link Sort} and {@link ReturnedType}. The function returns a + * visitor object that can visit the parsed query tree. + * + * @since 3.5 + */ + @FunctionalInterface interface SortedQueryRewriteFunction { ParseTreeVisitor apply(Sort sort, String primaryAlias, @Nullable ReturnedType returnedType); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java index ba0dfcf658..0995dd8700 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java @@ -82,11 +82,19 @@ public interface QueryEnhancer { * @param sort the sort specification to apply. * @param alias the alias to be used in the order by clause. May be {@literal null} or empty. * @return the modified query string. + * @deprecated since 3.5, use {@link #rewrite(Sort, ReturnedType)} instead. */ - @Deprecated + @Deprecated(since = "3.5", forRemoval = true) String applySorting(Sort sort, @Nullable String alias); - String rewrite(Sort sort, ReturnedType returnedType); + /** + * Rewrite the query to include sorting and apply {@link ReturnedType} customizations. + * + * @param rewriteInformation the rewrite information to apply. + * @return the modified query string. + * @since 3.5 + */ + String rewrite(QueryRewriteInformation rewriteInformation); /** * Creates a count projected query from the given original query. @@ -105,4 +113,22 @@ default String createCountQueryFor() { */ String createCountQueryFor(@Nullable String countProjection); + /** + * Interface to describe the information needed to rewrite a query. + * + * @since 3.5 + */ + interface QueryRewriteInformation { + + /** + * @return the sort specification to apply. + */ + Sort getSort(); + + /** + * @return type expected to be returned by the query. + */ + ReturnedType getReturnedType(); + } + } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java index 2e9a1fbb22..4302c63650 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java @@ -83,17 +83,16 @@ class FetchableFluentQueryByPredicate extends FluentQuerySupport imp BiFunction> pagedFinder, Function countOperation, Function existsOperation, EntityManager entityManager, ProjectionFactory projectionFactory) { this(entityPath, predicate, entityInformation, (Class) entityInformation.getJavaType(), Sort.unsorted(), 0, - Collections.emptySet(), finder, scrollQueryFactory, - pagedFinder, countOperation, existsOperation, entityManager, projectionFactory); + Collections.emptySet(), finder, scrollQueryFactory, pagedFinder, countOperation, existsOperation, entityManager, + projectionFactory); } private FetchableFluentQueryByPredicate(EntityPath entityPath, Predicate predicate, JpaEntityInformation entityInformation, Class resultType, Sort sort, int limit, Collection properties, Function> finder, ScrollQueryFactory> scrollQueryFactory, - BiFunction> pagedFinder, - Function countOperation, Function existsOperation, - EntityManager entityManager, ProjectionFactory projectionFactory) { + BiFunction> pagedFinder, Function countOperation, + Function existsOperation, EntityManager entityManager, ProjectionFactory projectionFactory) { super(resultType, sort, limit, properties, entityInformation.getJavaType(), projectionFactory); this.entityInformation = entityInformation; @@ -142,8 +141,7 @@ public FetchableFluentQuery project(Collection properties) { return new FetchableFluentQueryByPredicate<>(entityPath, predicate, entityInformation, resultType, sort, limit, mergeProperties(properties), finder, scrollQueryFactory, pagedFinder, countOperation, existsOperation, - entityManager, - projectionFactory); + entityManager, projectionFactory); } @Override @@ -296,6 +294,9 @@ public Window scroll(ReturnedType returnedType, Sort sort, int limit, ScrollP } } + /** + * @since 3.5 + */ private static class DtoProjection extends ExpressionBase { private final Expression[] projection; diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java index cf6c29fdea..1f1a18ac44 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JakartaTuple.java @@ -20,6 +20,8 @@ import java.util.Arrays; import java.util.List; +import org.springframework.lang.Nullable; + import com.querydsl.core.types.Expression; import com.querydsl.core.types.ExpressionBase; import com.querydsl.core.types.ExpressionUtils; @@ -30,6 +32,15 @@ import com.querydsl.core.types.Visitor; import com.querydsl.jpa.JPQLSerializer; +/** + * Expression based on a {@link Tuple}. It's a simplified variant of {@link com.querydsl.core.types.QTuple} without + * being a {@link com.querydsl.core.types.FactoryExpressionBase} as we do not want Querydsl to instantiate any tuples. + * JPA is doing that for us. + * + * @author Mark Paluch + * @since 3.5 + */ +@SuppressWarnings("unchecked") class JakartaTuple extends ExpressionBase { private final List> args; @@ -61,7 +72,8 @@ protected JakartaTuple(List> args) { } @Override - public R accept(Visitor v, C context) { + @Nullable + public R accept(Visitor v, @Nullable C context) { if (v instanceof JPQLSerializer) { return Projections.tuple(args).accept(v, context); @@ -74,8 +86,7 @@ public R accept(Visitor v, C context) { public boolean equals(Object obj) { if (obj == this) { return true; - } else if (obj instanceof FactoryExpression) { - FactoryExpression c = (FactoryExpression) obj; + } else if (obj instanceof FactoryExpression c) { return args.equals(c.getArgs()) && getType().equals(c.getType()); } else { return false; diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java index b3faa0fb5c..4e8ef371d2 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java @@ -25,6 +25,7 @@ import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.querydsl.QSort; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import com.querydsl.core.types.EntityPath; @@ -37,6 +38,7 @@ import com.querydsl.jpa.EclipseLinkTemplates; import com.querydsl.jpa.HQLTemplates; import com.querydsl.jpa.JPQLQuery; +import com.querydsl.jpa.JPQLTemplates; import com.querydsl.jpa.impl.AbstractJPAQuery; import com.querydsl.jpa.impl.JPAQuery; @@ -77,11 +79,25 @@ public Querydsl(EntityManager em, PathBuilder builder) { */ public AbstractJPAQuery> createQuery() { - return switch (provider) { - case ECLIPSELINK -> new SpringDataJpaQuery<>(em, EclipseLinkTemplates.DEFAULT); - case HIBERNATE -> new SpringDataJpaQuery<>(em, HQLTemplates.DEFAULT); - default -> new SpringDataJpaQuery<>(em); - }; + JPQLTemplates templates = getTemplates(); + return templates != null ? new SpringDataJpaQuery<>(em, templates) : new SpringDataJpaQuery<>(em); + } + + /** + * Obtains the {@link JPQLTemplates} for the configured {@link EntityManager}. Can return {@literal null} to use the + * default templates. + * + * @return the {@link JPQLTemplates} for the configured {@link EntityManager} or {@literal null} to use the default. + * @since 3.5 + */ + @Nullable + public JPQLTemplates getTemplates() { + + return switch (provider) { + case ECLIPSELINK -> EclipseLinkTemplates.DEFAULT; + case HIBERNATE -> HQLTemplates.DEFAULT; + default -> JPQLTemplates.DEFAULT; + }; } /** @@ -198,11 +214,11 @@ private NullHandling toQueryDslNullHandling(org.springframework.data.domain.Sort Assert.notNull(nullHandling, "NullHandling must not be null"); - return switch (nullHandling) { - case NULLS_FIRST -> NullHandling.NullsFirst; - case NULLS_LAST -> NullHandling.NullsLast; - default -> NullHandling.Default; - }; + return switch (nullHandling) { + case NULLS_FIRST -> NullHandling.NullsFirst; + case NULLS_LAST -> NullHandling.NullsLast; + default -> NullHandling.Default; + }; } /** diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java index 5d747dea4c..5eba7dd36d 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SpringDataJpaQuery.java @@ -32,7 +32,11 @@ import com.querydsl.jpa.impl.JPAUtil; /** + * Customized String-Query implementation that specifically routes tuple query creation to + * {@code EntityManager#createQuery(queryString, Tuple.class)}. + * * @author Mark Paluch + * @since 3.5 */ class SpringDataJpaQuery extends JPAQuery { diff --git a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc index 8ac41c7d51..95366d30aa 100644 --- a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc +++ b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc @@ -69,7 +69,7 @@ This query gets rewritten to `SELECT new UserDto(u.firstname, u.lastname) FROM U [WARNING] ==== JPQL constructor expressions must not contain aliases for selected columns. -While `SELECT u as user, count(u.roles) as roleCount FROM USER u ...` is a valid usecase for interface based projections that rely on column names from the returned `Tuple`, the same construct is invalid when requesting a DTO where it needs to be `SELECT u, count(u.roles) FROM USER u ...`. + +While `SELECT u as user, count(u.roles) as roleCount FROM USER u …` is a valid query for interface-based projections that rely on column names from the returned `Tuple`, the same construct is invalid when requesting a DTO where it needs to be `SELECT u, count(u.roles) FROM USER u …`. + Some persistence providers may be lenient about this, others not. ==== From 4c1c3e814cf992d53651af4fdc8eb5ab91fc29f6 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 4 Dec 2024 10:21:12 +0100 Subject: [PATCH 08/10] Polishing. Fix reference in javadoc --- .../data/jpa/repository/query/QueryEnhancer.java | 2 +- .../antora/modules/ROOT/pages/repositories/projections.adoc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java index 0995dd8700..3697c22980 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java @@ -82,7 +82,7 @@ public interface QueryEnhancer { * @param sort the sort specification to apply. * @param alias the alias to be used in the order by clause. May be {@literal null} or empty. * @return the modified query string. - * @deprecated since 3.5, use {@link #rewrite(Sort, ReturnedType)} instead. + * @deprecated since 3.5, use {@link #rewrite(QueryRewriteInformation)} instead. */ @Deprecated(since = "3.5", forRemoval = true) String applySorting(Sort sort, @Nullable String alias); diff --git a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc index 95366d30aa..c5e113c8f4 100644 --- a/src/main/antora/modules/ROOT/pages/repositories/projections.adoc +++ b/src/main/antora/modules/ROOT/pages/repositories/projections.adoc @@ -68,7 +68,7 @@ This query gets rewritten to `SELECT new UserDto(u.firstname, u.lastname) FROM U [WARNING] ==== -JPQL constructor expressions must not contain aliases for selected columns. +JPQL constructor expressions must not contain aliases for selected columns and query rewriting will not remove them for you. While `SELECT u as user, count(u.roles) as roleCount FROM USER u …` is a valid query for interface-based projections that rely on column names from the returned `Tuple`, the same construct is invalid when requesting a DTO where it needs to be `SELECT u, count(u.roles) FROM USER u …`. + Some persistence providers may be lenient about this, others not. ==== From 79ab5ade5f23d95155b9b44957d0793dfa419a33 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 5 Dec 2024 13:20:40 +0100 Subject: [PATCH 09/10] Fix potential NPE in TupleConverter --- .../repository/query/AbstractJpaQuery.java | 53 +++++++++++++++++-- .../query/TupleConverterUnitTests.java | 25 +++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index 4914beb4d7..ec90ced24b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -23,6 +23,7 @@ import jakarta.persistence.TupleElement; import jakarta.persistence.TypedQuery; +import java.lang.reflect.Constructor; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -54,6 +55,7 @@ import org.springframework.jdbc.support.JdbcUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; /** @@ -345,7 +347,7 @@ public TupleConverter(ReturnedType type, boolean nativeQuery) { && !type.getInputProperties().isEmpty(); if (this.dtoProjection) { - this.preferredConstructor = PreferredConstructorDiscoverer.discover(String.class); + this.preferredConstructor = PreferredConstructorDiscoverer.discover(type.getReturnedType()); } else { this.preferredConstructor = null; } @@ -373,18 +375,35 @@ public Object convert(Object source) { Object[] ctorArgs = new Object[type.getInputProperties().size()]; + boolean containsNullValue = false; for (int i = 0; i < type.getInputProperties().size(); i++) { - ctorArgs[i] = tuple.get(i); + Object value = tuple.get(i); + ctorArgs[i] = value; + if (!containsNullValue && value == null) { + containsNullValue = true; + } } try { - if (preferredConstructor.getParameterCount() == ctorArgs.length) { + if (preferredConstructor != null && preferredConstructor.getParameterCount() == ctorArgs.length) { return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs); } - return BeanUtils.instantiateClass(type.getReturnedType() - .getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class[]::new)), ctorArgs); + Constructor ctor = null; + + if (!containsNullValue) { // let's seem if we have an argument type match + ctor = type.getReturnedType() + .getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class[]::new)); + } + + if (ctor == null) { // let's see if there's more general constructor we could use that accepts our args + ctor = findFirstMatchingConstructor(ctorArgs); + } + + if (ctor != null) { + return BeanUtils.instantiateClass(ctor, ctorArgs); + } } catch (ReflectiveOperationException e) { ReflectionUtils.handleReflectionException(e); } @@ -393,6 +412,30 @@ public Object convert(Object source) { return new TupleBackedMap(tupleWrapper.apply(tuple)); } + @Nullable + private Constructor findFirstMatchingConstructor(Object[] ctorArgs) { + + for (Constructor ctor : type.getReturnedType().getDeclaredConstructors()) { + + if (ctor.getParameterCount() == ctorArgs.length) { + boolean itsAMatch = true; + for (int i = 0; i < ctor.getParameterCount(); i++) { + if (ctorArgs[i] == null) { + continue; + } + if (!ClassUtils.isAssignable(ctor.getParameterTypes()[i], ctorArgs[i].getClass())) { + itsAMatch = false; + break; + } + } + if (itsAMatch) { + return ctor; + } + } + } + return null; + } + /** * A {@link Map} implementation which delegates all calls to a {@link Tuple}. Depending on the provided * {@link Tuple} implementation it might return the same value for various keys of which only one will appear in the diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java index 13dc550fb0..e16ebaafe2 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java @@ -37,6 +37,7 @@ import org.springframework.data.jpa.repository.query.AbstractJpaQuery.TupleConverter; import org.springframework.data.projection.ProjectionFactory; +import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; @@ -113,6 +114,16 @@ void findsValuesForAllVariantsSupportedByTheTuple() { softly.assertAll(); } + @Test // GH-3076 + void dealsWithNullsInArgumetents() { + + ReturnedType returnedType = ReturnedType.of(WithPC.class, DomainType.class, new SpelAwareProxyProjectionFactory()); + + when(tuple.get(eq(0))).thenReturn("one"); + when(tuple.get(eq(1))).thenReturn(null); + new TupleConverter(returnedType).convert(tuple); + } + interface SampleRepository extends CrudRepository { String someMethod(); } @@ -177,4 +188,18 @@ public String getAlias() { } } } + + static class DomainType { + String one, two, three; + } + + static class WithPC { + String one; + String two; + + public WithPC(String one, String two) { + this.one = one; + this.two = two; + } + } } From c30f009b5f26c513661aff3cde3216ae8cc31697 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 9 Dec 2024 15:59:18 +0100 Subject: [PATCH 10/10] Polishing. Introduce constructor compatibility check by checking assingable types. --- .../repository/query/AbstractJpaQuery.java | 123 +++++++++++------- .../query/TupleConverterUnitTests.java | 118 ++++++++++++++++- 2 files changed, 188 insertions(+), 53 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index ec90ced24b..772b1d7032 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -24,6 +24,7 @@ import jakarta.persistence.TypedQuery; import java.lang.reflect.Constructor; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -34,6 +35,7 @@ import java.util.stream.Collectors; import org.springframework.beans.BeanUtils; +import org.springframework.core.MethodParameter; import org.springframework.core.convert.converter.Converter; import org.springframework.data.jpa.provider.PersistenceProvider; import org.springframework.data.jpa.repository.EntityGraph; @@ -56,7 +58,6 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; -import org.springframework.util.ReflectionUtils; /** * Abstract base class to implement {@link RepositoryQuery}s. @@ -347,7 +348,7 @@ public TupleConverter(ReturnedType type, boolean nativeQuery) { && !type.getInputProperties().isEmpty(); if (this.dtoProjection) { - this.preferredConstructor = PreferredConstructorDiscoverer.discover(type.getReturnedType()); + this.preferredConstructor = PreferredConstructorDiscoverer.discover(type.getReturnedType()); } else { this.preferredConstructor = null; } @@ -373,67 +374,97 @@ public Object convert(Object source) { if (dtoProjection) { - Object[] ctorArgs = new Object[type.getInputProperties().size()]; + Object[] ctorArgs = new Object[elements.size()]; + for (int i = 0; i < ctorArgs.length; i++) { + ctorArgs[i] = tuple.get(i); + } + + List> argTypes = getArgumentTypes(ctorArgs); - boolean containsNullValue = false; - for (int i = 0; i < type.getInputProperties().size(); i++) { - Object value = tuple.get(i); - ctorArgs[i] = value; - if (!containsNullValue && value == null) { - containsNullValue = true; - } + if (preferredConstructor != null && isConstructorCompatible(preferredConstructor.getConstructor(), argTypes)) { + return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs); } - try { + return BeanUtils.instantiateClass(getFirstMatchingConstructor(ctorArgs, argTypes), ctorArgs); + } - if (preferredConstructor != null && preferredConstructor.getParameterCount() == ctorArgs.length) { - return BeanUtils.instantiateClass(preferredConstructor.getConstructor(), ctorArgs); - } + return new TupleBackedMap(tupleWrapper.apply(tuple)); + } - Constructor ctor = null; + private Constructor getFirstMatchingConstructor(Object[] ctorArgs, List> argTypes) { - if (!containsNullValue) { // let's seem if we have an argument type match - ctor = type.getReturnedType() - .getConstructor(Arrays.stream(ctorArgs).map(Object::getClass).toArray(Class[]::new)); - } + for (Constructor ctor : type.getReturnedType().getDeclaredConstructors()) { - if (ctor == null) { // let's see if there's more general constructor we could use that accepts our args - ctor = findFirstMatchingConstructor(ctorArgs); - } + if (ctor.getParameterCount() != ctorArgs.length) { + continue; + } - if (ctor != null) { - return BeanUtils.instantiateClass(ctor, ctorArgs); - } - } catch (ReflectiveOperationException e) { - ReflectionUtils.handleReflectionException(e); + if (isConstructorCompatible(ctor, argTypes)) { + return ctor; } } - return new TupleBackedMap(tupleWrapper.apply(tuple)); + throw new IllegalStateException(String.format( + "Cannot find compatible constructor for DTO projection '%s' accepting '%s'", type.getReturnedType().getName(), + argTypes.stream().map(Class::getName).collect(Collectors.joining(", ")))); } - @Nullable - private Constructor findFirstMatchingConstructor(Object[] ctorArgs) { + private static List> getArgumentTypes(Object[] ctorArgs) { + List> argTypes = new ArrayList<>(ctorArgs.length); - for (Constructor ctor : type.getReturnedType().getDeclaredConstructors()) { + for (Object ctorArg : ctorArgs) { + argTypes.add(ctorArg == null ? Void.class : ctorArg.getClass()); + } + return argTypes; + } - if (ctor.getParameterCount() == ctorArgs.length) { - boolean itsAMatch = true; - for (int i = 0; i < ctor.getParameterCount(); i++) { - if (ctorArgs[i] == null) { - continue; - } - if (!ClassUtils.isAssignable(ctor.getParameterTypes()[i], ctorArgs[i].getClass())) { - itsAMatch = false; - break; - } - } - if (itsAMatch) { - return ctor; - } + public static boolean isConstructorCompatible(Constructor constructor, List> argumentTypes) { + + if (constructor.getParameterCount() != argumentTypes.size()) { + return false; + } + + for (int i = 0; i < argumentTypes.size(); i++) { + + MethodParameter methodParameter = MethodParameter.forExecutable(constructor, i); + Class argumentType = argumentTypes.get(i); + + if (!areAssignmentCompatible(methodParameter.getParameterType(), argumentType)) { + return false; } } - return null; + return true; + } + + private static boolean areAssignmentCompatible(Class to, Class from) { + + if (from == Void.class && !to.isPrimitive()) { + // treat Void as the bottom type, the class of null + return true; + } + + if (to.isPrimitive()) { + + if (to == Short.TYPE) { + return from == Character.class || from == Byte.class; + } + + if (to == Integer.TYPE) { + return from == Short.class || from == Character.class || from == Byte.class; + } + + if (to == Long.TYPE) { + return from == Integer.class || from == Short.class || from == Character.class || from == Byte.class; + } + + if (to == Double.TYPE) { + return from == Float.class; + } + + return ClassUtils.isAssignable(to, from); + } + + return ClassUtils.isAssignable(to, from); } /** diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java index e16ebaafe2..8682012fa4 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/TupleConverterUnitTests.java @@ -18,14 +18,14 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; +import jakarta.persistence.Tuple; +import jakarta.persistence.TupleElement; + import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; -import jakarta.persistence.Tuple; -import jakarta.persistence.TupleElement; - import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -49,6 +49,8 @@ * * @author Oliver Gierke * @author Jens Schauder + * @author Christoph Strobl + * @author Mark Paluch * @soundtrack James Bay - Let it go (Chaos and the Calm) */ @ExtendWith(MockitoExtension.class) @@ -115,13 +117,88 @@ void findsValuesForAllVariantsSupportedByTheTuple() { } @Test // GH-3076 - void dealsWithNullsInArgumetents() { + void dealsWithNullsInArguments() { ReturnedType returnedType = ReturnedType.of(WithPC.class, DomainType.class, new SpelAwareProxyProjectionFactory()); + doReturn(List.of(element, element, element)).when(tuple).getElements(); + when(tuple.get(eq(0))).thenReturn("one"); + when(tuple.get(eq(1))).thenReturn(null); + when(tuple.get(eq(2))).thenReturn(1); + + Object result = new TupleConverter(returnedType).convert(tuple); + assertThat(result).isInstanceOf(WithPC.class); + } + + @Test // GH-3076 + void fallsBackToCompatibleConstructor() { + + ReturnedType returnedType = spy( + ReturnedType.of(MultipleConstructors.class, DomainType.class, new SpelAwareProxyProjectionFactory())); + when(returnedType.isProjecting()).thenReturn(true); + when(returnedType.getInputProperties()).thenReturn(Arrays.asList("one", "two", "three")); + + doReturn(List.of(element, element, element)).when(tuple).getElements(); + when(tuple.get(eq(0))).thenReturn("one"); + when(tuple.get(eq(1))).thenReturn(null); + when(tuple.get(eq(2))).thenReturn(2); + + MultipleConstructors result = (MultipleConstructors) new TupleConverter(returnedType).convert(tuple); + + assertThat(result.one).isEqualTo("one"); + assertThat(result.two).isNull(); + assertThat(result.three).isEqualTo(2); + + reset(tuple); + + doReturn(List.of(element, element, element)).when(tuple).getElements(); + when(tuple.get(eq(0))).thenReturn("one"); + when(tuple.get(eq(1))).thenReturn(null); + when(tuple.get(eq(2))).thenReturn('a'); + + result = (MultipleConstructors) new TupleConverter(returnedType).convert(tuple); + + assertThat(result.one).isEqualTo("one"); + assertThat(result.two).isNull(); + assertThat(result.three).isEqualTo(97); + } + + @Test // GH-3076 + void acceptsConstructorWithCastableType() { + + ReturnedType returnedType = spy( + ReturnedType.of(MultipleConstructors.class, DomainType.class, new SpelAwareProxyProjectionFactory())); + when(returnedType.isProjecting()).thenReturn(true); + when(returnedType.getInputProperties()).thenReturn(Arrays.asList("one", "two", "three", "four")); + + doReturn(List.of(element, element, element, element)).when(tuple).getElements(); when(tuple.get(eq(0))).thenReturn("one"); when(tuple.get(eq(1))).thenReturn(null); - new TupleConverter(returnedType).convert(tuple); + when(tuple.get(eq(2))).thenReturn((byte) 2); + when(tuple.get(eq(3))).thenReturn(2.1f); + + MultipleConstructors result = (MultipleConstructors) new TupleConverter(returnedType).convert(tuple); + + assertThat(result.one).isEqualTo("one"); + assertThat(result.two).isNull(); + assertThat(result.three).isEqualTo(2); + assertThat(result.four).isEqualTo(2, offset(0.1d)); + } + + @Test // GH-3076 + void failsForNonResolvableConstructor() { + + ReturnedType returnedType = spy( + ReturnedType.of(MultipleConstructors.class, DomainType.class, new SpelAwareProxyProjectionFactory())); + when(returnedType.isProjecting()).thenReturn(true); + when(returnedType.getInputProperties()).thenReturn(Arrays.asList("one", "two")); + + doReturn(List.of(element, element)).when(tuple).getElements(); + when(tuple.get(eq(0))).thenReturn(1); + when(tuple.get(eq(1))).thenReturn(null); + + assertThatIllegalStateException().isThrownBy(() -> new TupleConverter(returnedType).convert(tuple)) + .withMessageContaining("Cannot find compatible constructor for DTO projection"); } interface SampleRepository extends CrudRepository { @@ -193,13 +270,40 @@ static class DomainType { String one, two, three; } - static class WithPC { + static class WithPC { String one; String two; + long three; - public WithPC(String one, String two) { + public WithPC(String one, String two, long three) { this.one = one; this.two = two; + this.three = three; } } + + static class MultipleConstructors { + String one; + String two; + long three; + double four; + + public MultipleConstructors(String one) { + this.one = one; + } + + public MultipleConstructors(String one, String two, long three) { + this.one = one; + this.two = two; + this.three = three; + } + + public MultipleConstructors(String one, String two, short three, double four) { + this.one = one; + this.two = two; + this.three = three; + this.four = four; + } + + } }