From a31c39db7a12113b5adcb6fbaa2a92d97f1b3a02 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 8 Oct 2021 11:23:58 +0200 Subject: [PATCH] Polishing. Fix version reference to Spring Data Commons. Avoid stream creation when fetching all/one/first element. Move off deprecated API. Add override comments. Add null guargs. See #2294 Original pull request: #2326. --- pom.xml | 2 +- .../FetchableFluentQueryByExample.java | 93 +++++++++++++++---- .../FetchableFluentQueryByPredicate.java | 91 ++++++++++++++---- .../support/QuerydslJpaPredicateExecutor.java | 13 ++- .../support/SimpleJpaRepository.java | 5 +- 5 files changed, 162 insertions(+), 42 deletions(-) diff --git a/pom.xml b/pom.xml index d91c077f28..e31849e911 100644 --- a/pom.xml +++ b/pom.xml @@ -25,7 +25,7 @@ 5.5.3.Final 8.0.23 42.2.19 - 2.6.0-2228-SNAPSHOT + 2.6.0-SNAPSHOT 0.10.3 org.hibernate diff --git a/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByExample.java b/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByExample.java index 798f4663e5..c187ccd884 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByExample.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByExample.java @@ -15,10 +15,10 @@ */ package org.springframework.data.jpa.repository.support; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.persistence.EntityManager; @@ -37,6 +37,7 @@ import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; import org.springframework.data.support.PageableExecutionUtils; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * Immutable implementation of {@link FetchableFluentQuery} based on Query by {@link Example}. All methods that return a @@ -45,6 +46,7 @@ * @param Domain type * @param Result type * @author Greg Turnquist + * @author Mark Paluch * @since 2.6 */ class FetchableFluentQueryByExample extends FluentQuerySupport implements FetchableFluentQuery { @@ -79,24 +81,39 @@ private FetchableFluentQueryByExample(Example example, Class returnType, S this.escapeCharacter = escapeCharacter; } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#sortBy(org.springframework.data.domain.Sort) + */ @Override public FetchableFluentQuery sortBy(Sort sort) { - return new FetchableFluentQueryByExample(this.example, this.resultType, this.sort.and(sort), this.properties, + Assert.notNull(sort, "Sort must not be null!"); + + return new FetchableFluentQueryByExample<>(this.example, this.resultType, this.sort.and(sort), this.properties, this.finder, this.countOperation, this.existsOperation, this.context, this.entityManager, this.escapeCharacter); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#as(java.lang.Class) + */ @Override 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 FetchableFluentQueryByExample(this.example, resultType, this.sort, this.properties, this.finder, + return new FetchableFluentQueryByExample<>(this.example, resultType, this.sort, this.properties, this.finder, this.countOperation, this.existsOperation, this.context, this.entityManager, this.escapeCharacter); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#project(java.util.Collection) + */ @Override public FetchableFluentQuery project(Collection properties) { @@ -104,62 +121,86 @@ public FetchableFluentQuery project(Collection properties) { this.finder, this.countOperation, this.existsOperation, this.context, this.entityManager, this.escapeCharacter); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#oneValue() + */ @Override public R oneValue() { TypedQuery limitedQuery = this.finder.apply(this.sort); limitedQuery.setMaxResults(2); // Never need more than 2 values - List results = limitedQuery // - .getResultStream() // - .map(getConversionFunction(this.example.getProbeType(), this.resultType)) // - .collect(Collectors.toList()); - ; + List results = limitedQuery.getResultList(); if (results.size() > 1) { throw new IncorrectResultSizeDataAccessException(1); } - return results.isEmpty() ? null : results.get(0); + return results.isEmpty() ? null : getConversionFunction().apply(results.get(0)); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#firstValue() + */ @Override public R firstValue() { TypedQuery limitedQuery = this.finder.apply(this.sort); limitedQuery.setMaxResults(1); // Never need more than 1 value - List results = limitedQuery // - .getResultStream() // - .map(getConversionFunction(this.example.getProbeType(), this.resultType)) // - .collect(Collectors.toList()); + List results = limitedQuery.getResultList(); - return results.isEmpty() ? null : results.get(0); + return results.isEmpty() ? null : getConversionFunction().apply(results.get(0)); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#all() + */ @Override public List all() { - return stream().collect(Collectors.toList()); + + List resultList = this.finder.apply(this.sort).getResultList(); + + return convert(resultList); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#page(org.springframework.data.domain.Pageable) + */ @Override public Page page(Pageable pageable) { return pageable.isUnpaged() ? new PageImpl<>(all()) : readPage(pageable); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#stream() + */ @Override public Stream stream() { return this.finder.apply(this.sort) // .getResultStream() // - .map(getConversionFunction(this.example.getProbeType(), this.resultType)); + .map(getConversionFunction()); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#count() + */ @Override public long count() { return this.countOperation.apply(example); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#exists() + */ @Override public boolean exists() { return this.existsOperation.apply(example); @@ -174,10 +215,24 @@ private Page readPage(Pageable pageable) { pagedQuery.setMaxResults(pageable.getPageSize()); } - List paginatedResults = pagedQuery.getResultStream() // - .map(getConversionFunction(this.example.getProbeType(), this.resultType)) // - .collect(Collectors.toList()); + List paginatedResults = convert(pagedQuery.getResultList()); return PageableExecutionUtils.getPage(paginatedResults, pageable, () -> this.countOperation.apply(this.example)); } + + private List convert(List resultList) { + + Function conversionFunction = getConversionFunction(); + List mapped = new ArrayList<>(resultList.size()); + + for (S s : resultList) { + mapped.add(conversionFunction.apply(s)); + } + return mapped; + } + + private Function getConversionFunction() { + return getConversionFunction(this.example.getProbeType(), this.resultType); + } + } diff --git a/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java b/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java index ba2b0790fc..a5890f568f 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryByPredicate.java @@ -15,11 +15,11 @@ */ package org.springframework.data.jpa.repository.support; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.function.BiFunction; import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.springframework.dao.IncorrectResultSizeDataAccessException; @@ -33,6 +33,7 @@ import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; import org.springframework.data.support.PageableExecutionUtils; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import com.querydsl.core.types.Predicate; import com.querydsl.jpa.JPQLQuery; @@ -44,6 +45,7 @@ * @param Domain type * @param Result type * @author Greg Turnquist + * @author Mark Paluch * @since 2.6 */ class FetchableFluentQueryByPredicate extends FluentQuerySupport implements FetchableFluentQuery { @@ -78,16 +80,27 @@ private FetchableFluentQueryByPredicate(Predicate predicate, Class resultType this.entityType = entityType; } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#sortBy(org.springframework.data.domain.Sort) + */ @Override public FetchableFluentQuery sortBy(Sort sort) { + Assert.notNull(sort, "Sort must not be null!"); + return new FetchableFluentQueryByPredicate<>(this.predicate, this.resultType, this.sort.and(sort), this.properties, this.finder, this.pagedFinder, this.countOperation, this.existsOperation, this.entityType, this.context); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#as(java.lang.Class) + */ @Override 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."); } @@ -96,6 +109,10 @@ public FetchableFluentQuery as(Class resultType) { this.pagedFinder, this.countOperation, this.existsOperation, this.entityType, this.context); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#project(java.util.Collection) + */ @Override public FetchableFluentQuery project(Collection properties) { @@ -104,57 +121,83 @@ public FetchableFluentQuery project(Collection properties) { this.entityType, this.context); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#oneValue() + */ @Override public R oneValue() { - List results = this.finder.apply(this.sort) // + List results = this.finder.apply(this.sort) // .limit(2) // Never need more than 2 values - .stream() // - .map(getConversionFunction(this.entityType, this.resultType)) // - .collect(Collectors.toList()); + .fetch(); if (results.size() > 1) { throw new IncorrectResultSizeDataAccessException(1); } - return results.isEmpty() ? null : results.get(0); + return results.isEmpty() ? null : getConversionFunction().apply(results.get(0)); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#firstValue() + */ @Override public R firstValue() { - List results = this.finder.apply(this.sort) // + List results = this.finder.apply(this.sort) // .limit(1) // Never need more than 1 value - .stream() // - .map(getConversionFunction(this.entityType, this.resultType)) // - .collect(Collectors.toList()); + .fetch(); - return results.isEmpty() ? null : results.get(0); + return results.isEmpty() ? null : getConversionFunction().apply(results.get(0)); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#all() + */ @Override public List all() { - return stream().collect(Collectors.toList()); + + JPQLQuery query = this.finder.apply(this.sort); + return convert(query.fetch()); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#page(org.springframework.data.domain.Pageable) + */ @Override public Page page(Pageable pageable) { return pageable.isUnpaged() ? new PageImpl<>(all()) : readPage(pageable); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#stream() + */ @Override public Stream stream() { return this.finder.apply(this.sort) // .stream() // - .map(getConversionFunction(this.entityType, this.resultType)); + .map(getConversionFunction()); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#count() + */ @Override public long count() { return this.countOperation.apply(this.predicate); } + /* + * (non-Javadoc) + * @see org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery#exists() + */ @Override public boolean exists() { return this.existsOperation.apply(this.predicate); @@ -163,11 +206,25 @@ public boolean exists() { private Page readPage(Pageable pageable) { JPQLQuery pagedQuery = this.pagedFinder.apply(this.sort, pageable); - - List paginatedResults = pagedQuery.stream() // - .map(getConversionFunction(this.entityType, this.resultType)) // - .collect(Collectors.toList()); + List paginatedResults = convert(pagedQuery.fetch()); return PageableExecutionUtils.getPage(paginatedResults, pageable, () -> this.countOperation.apply(this.predicate)); } + + private List convert(List resultList) { + + Function conversionFunction = getConversionFunction(); + List mapped = new ArrayList<>(resultList.size()); + + for (S s : resultList) { + mapped.add(conversionFunction.apply(s)); + } + + return mapped; + } + + private Function getConversionFunction() { + return getConversionFunction(this.entityType, this.resultType); + } + } diff --git a/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java b/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java index 32676777c7..19ecfcf79a 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java @@ -34,7 +34,7 @@ import org.springframework.data.querydsl.QSort; import org.springframework.data.querydsl.QuerydslPredicateExecutor; import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; -import org.springframework.data.repository.support.PageableExecutionUtils; +import org.springframework.data.support.PageableExecutionUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -167,11 +167,16 @@ public Page findAll(Predicate predicate, Pageable pageable) { return PageableExecutionUtils.getPage(query.fetch(), pageable, countQuery::fetchCount); } + /* + * (non-Javadoc) + * @see org.springframework.data.querydsl.QuerydslPredicateExecutor#findBy(com.querydsl.core.types.Predicate, java.util.function.Function) + */ + @SuppressWarnings("unchecked") @Override public R findBy(Predicate predicate, Function, R> queryFunction) { Assert.notNull(predicate, "Predicate must not be null!"); - Assert.notNull(queryFunction, "Function must not be null!"); + Assert.notNull(queryFunction, "Query function must not be null!"); Function> finder = sort -> { JPQLQuery select = createQuery(predicate).select(path); @@ -194,12 +199,12 @@ public R findBy(Predicate predicate, Function fluentQuery = (FetchableFluentQuery) new FetchableFluentQueryByPredicate<>(predicate, + FetchableFluentQueryByPredicate fluentQuery = new FetchableFluentQueryByPredicate<>(predicate, entityInformation.getJavaType(), finder, pagedFinder, this::count, this::exists, this.entityInformation.getJavaType(), new JpaMetamodelMappingContext(Collections.singleton(this.entityManager.getMetamodel()))); - return queryFunction.apply(fluentQuery); + return queryFunction.apply((FetchableFluentQuery) fluentQuery); } /* diff --git a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java index cc2b001a85..b69d43667f 100644 --- a/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java +++ b/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java @@ -58,7 +58,7 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.repository.query.FluentQuery.FetchableFluentQuery; -import org.springframework.data.repository.support.PageableExecutionUtils; +import org.springframework.data.support.PageableExecutionUtils; import org.springframework.data.util.ProxyUtils; import org.springframework.data.util.Streamable; import org.springframework.lang.Nullable; @@ -583,6 +583,9 @@ public Page findAll(Example example, Pageable pageable) { @Override public R findBy(Example example, Function, R> queryFunction) { + Assert.notNull(example, "Sample must not be null!"); + Assert.notNull(queryFunction, "Query function must not be null!"); + Function> finder = sort -> { ExampleSpecification spec = new ExampleSpecification<>(example, escapeCharacter);