From 8c0d01ae8c96014aa588ceb6cd2cb4b8aaa66ddd Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 21 Apr 2020 15:30:09 +0200 Subject: [PATCH] #344 - Polishing. Consider projection properties as SELECT projection. Refactor distinct() into marker method without accepting a boolean flag. Make distinct field final. Update tests. Original pull request: #346. --- .../r2dbc/core/DefaultStatementMapper.java | 10 +++-- .../data/r2dbc/core/StatementMapper.java | 14 +++---- .../repository/query/PartTreeR2dbcQuery.java | 19 ++++++++- .../repository/query/R2dbcQueryCreator.java | 39 ++++++++++++------- ...stractR2dbcRepositoryIntegrationTests.java | 22 +++++++++++ .../query/PartTreeR2dbcQueryUnitTests.java | 16 +++++--- 6 files changed, 90 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java b/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java index a9a10d42..852618c1 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java @@ -86,18 +86,20 @@ class DefaultStatementMapper implements StatementMapper { Table table = selectSpec.getTable(); SelectBuilder.SelectAndFrom selectAndFrom = StatementBuilder.select(getSelectList(selectSpec, entity)); - if(selectSpec.isDistinct()){ + + if (selectSpec.isDistinct()) { selectAndFrom = selectAndFrom.distinct(); } + SelectBuilder.SelectFromAndJoin selectBuilder = selectAndFrom.from(table); BindMarkers bindMarkers = this.dialect.getBindMarkersFactory().create(); Bindings bindings = Bindings.empty(); + CriteriaDefinition criteria = selectSpec.getCriteria(); - if (!selectSpec.getCriteria().isEmpty()) { + if (criteria != null && !criteria.isEmpty()) { - BoundCondition mappedObject = this.updateMapper.getMappedObject(bindMarkers, selectSpec.getCriteria(), table, - entity); + BoundCondition mappedObject = this.updateMapper.getMappedObject(bindMarkers, criteria, table, entity); bindings = mappedObject.getBindings(); selectBuilder.where(mappedObject.getCondition()); diff --git a/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java b/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java index 4041f398..22399e7e 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java @@ -202,7 +202,7 @@ public interface StatementMapper { private final Sort sort; private final long offset; private final int limit; - private boolean distinct = false; + private final boolean distinct; protected SelectSpec(Table table, List projectedFields, List selectList, @Nullable CriteriaDefinition criteria, Sort sort, int limit, long offset, boolean distinct) { @@ -237,8 +237,8 @@ public interface StatementMapper { List projectedFields = Collections.emptyList(); List selectList = Collections.emptyList(); - return new SelectSpec(Table.create(table), projectedFields, selectList, Criteria.empty(), Sort.unsorted(), -1, - -1, false); + return new SelectSpec(Table.create(table), projectedFields, selectList, Criteria.empty(), Sort.unsorted(), -1, -1, + false); } public SelectSpec doWithTable(BiFunction function) { @@ -279,7 +279,8 @@ public interface StatementMapper { List selectList = new ArrayList<>(this.selectList); selectList.addAll(Arrays.asList(expressions)); - return new SelectSpec(this.table, projectedFields, selectList, this.criteria, this.sort, this.limit, this.offset, this.distinct); + return new SelectSpec(this.table, projectedFields, selectList, this.criteria, this.sort, this.limit, this.offset, + this.distinct); } /** @@ -371,12 +372,11 @@ public interface StatementMapper { /** * Associate a result statement distinct with the select and create a new {@link SelectSpec}. * - * @param distinct * @return the {@link SelectSpec}. */ - public SelectSpec distinct(boolean distinct) { + public SelectSpec distinct() { return new SelectSpec(this.table, this.projectedFields, this.selectList, this.criteria, this.sort, limit, - this.offset, distinct); + this.offset, true); } public Table getTable() { diff --git a/src/main/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQuery.java b/src/main/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQuery.java index 4690d245..8c4476dc 100644 --- a/src/main/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQuery.java +++ b/src/main/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQuery.java @@ -15,6 +15,10 @@ */ package org.springframework.data.r2dbc.repository.query; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + import org.springframework.data.domain.Sort; import org.springframework.data.r2dbc.convert.R2dbcConverter; import org.springframework.data.r2dbc.core.DatabaseClient; @@ -23,6 +27,8 @@ import org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy; import org.springframework.data.relational.repository.query.RelationalEntityMetadata; import org.springframework.data.relational.repository.query.RelationalParameterAccessor; import org.springframework.data.relational.repository.query.RelationalParameters; +import org.springframework.data.repository.query.ResultProcessor; +import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.repository.query.parser.PartTree; /** @@ -34,6 +40,7 @@ import org.springframework.data.repository.query.parser.PartTree; */ public class PartTreeR2dbcQuery extends AbstractR2dbcQuery { + private final ResultProcessor processor; private final ReactiveDataAccessStrategy dataAccessStrategy; private final RelationalParameters parameters; private final PartTree tree; @@ -50,6 +57,8 @@ public class PartTreeR2dbcQuery extends AbstractR2dbcQuery { public PartTreeR2dbcQuery(R2dbcQueryMethod method, DatabaseClient databaseClient, R2dbcConverter converter, ReactiveDataAccessStrategy dataAccessStrategy) { super(method, databaseClient, converter); + + this.processor = method.getResultProcessor(); this.dataAccessStrategy = dataAccessStrategy; this.parameters = method.getParameters(); @@ -78,8 +87,16 @@ public class PartTreeR2dbcQuery extends AbstractR2dbcQuery { @Override protected BindableQuery createQuery(RelationalParameterAccessor accessor) { + ReturnedType returnedType = processor.withDynamicProjection(accessor).getReturnedType(); + List projectedProperties = Collections.emptyList(); + + if (returnedType.needsCustomConstruction()) { + projectedProperties = new ArrayList<>(returnedType.getInputProperties()); + } + RelationalEntityMetadata entityMetadata = getQueryMethod().getEntityInformation(); - R2dbcQueryCreator queryCreator = new R2dbcQueryCreator(tree, dataAccessStrategy, entityMetadata, accessor); + R2dbcQueryCreator queryCreator = new R2dbcQueryCreator(tree, dataAccessStrategy, entityMetadata, accessor, + projectedProperties); PreparedOperation preparedQuery = queryCreator.createQuery(getDynamicSort(accessor)); return new PreparedOperationBindableQuery(preparedQuery); diff --git a/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcQueryCreator.java b/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcQueryCreator.java index 6fd5fadb..45ec1ac1 100644 --- a/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcQueryCreator.java +++ b/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcQueryCreator.java @@ -15,6 +15,7 @@ */ package org.springframework.data.r2dbc.repository.query; +import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -32,7 +33,7 @@ import org.springframework.data.relational.repository.query.RelationalParameterA import org.springframework.data.relational.repository.query.RelationalQueryCreator; import org.springframework.data.repository.query.parser.AbstractQueryCreator; import org.springframework.data.repository.query.parser.PartTree; -import org.springframework.util.Assert; +import org.springframework.lang.Nullable; /** * Implementation of {@link AbstractQueryCreator} that creates {@link PreparedOperation} from a {@link PartTree}. @@ -42,34 +43,35 @@ import org.springframework.util.Assert; * @author Mingyuan Wu * @since 1.1 */ -public class R2dbcQueryCreator extends RelationalQueryCreator> { +class R2dbcQueryCreator extends RelationalQueryCreator> { private final PartTree tree; private final RelationalParameterAccessor accessor; private final ReactiveDataAccessStrategy dataAccessStrategy; private final RelationalEntityMetadata entityMetadata; + private final List projectedProperties; /** * Creates new instance of this class with the given {@link PartTree}, {@link ReactiveDataAccessStrategy}, * {@link RelationalEntityMetadata} and {@link RelationalParameterAccessor}. - * + * * @param tree part tree, must not be {@literal null}. * @param dataAccessStrategy data access strategy, must not be {@literal null}. * @param entityMetadata relational entity metadata, must not be {@literal null}. * @param accessor parameter metadata provider, must not be {@literal null}. + * @param projectedProperties properties to project, must not be {@literal null}. */ public R2dbcQueryCreator(PartTree tree, ReactiveDataAccessStrategy dataAccessStrategy, - RelationalEntityMetadata entityMetadata, RelationalParameterAccessor accessor) { + RelationalEntityMetadata entityMetadata, RelationalParameterAccessor accessor, + List projectedProperties) { super(tree, accessor); - Assert.notNull(dataAccessStrategy, "Data access strategy must not be null"); - Assert.notNull(entityMetadata, "Relational entity metadata must not be null"); - this.tree = tree; this.accessor = accessor; this.dataAccessStrategy = dataAccessStrategy; this.entityMetadata = entityMetadata; + this.projectedProperties = projectedProperties; } /** @@ -80,7 +82,7 @@ public class R2dbcQueryCreator extends RelationalQueryCreator complete(Criteria criteria, Sort sort) { + protected PreparedOperation complete(@Nullable Criteria criteria, Sort sort) { StatementMapper statementMapper = dataAccessStrategy.getStatementMapper().forType(entityMetadata.getJavaType()); @@ -91,7 +93,7 @@ public class R2dbcQueryCreator extends RelationalQueryCreator delete(Criteria criteria, StatementMapper statementMapper) { + private PreparedOperation delete(@Nullable Criteria criteria, StatementMapper statementMapper) { StatementMapper.DeleteSpec deleteSpec = statementMapper.createDelete(entityMetadata.getTableName()) .withCriteria(criteria); @@ -99,7 +101,7 @@ public class R2dbcQueryCreator extends RelationalQueryCreator select(Criteria criteria, Sort sort, StatementMapper statementMapper) { + private PreparedOperation select(@Nullable Criteria criteria, Sort sort, StatementMapper statementMapper) { StatementMapper.SelectSpec selectSpec = statementMapper.createSelect(entityMetadata.getTableName()) .withProjection(getSelectProjection()); @@ -123,8 +125,8 @@ public class R2dbcQueryCreator extends RelationalQueryCreator columnNames; - if (tree.isExistsProjection()) { + if (!projectedProperties.isEmpty()) { + + RelationalPersistentEntity entity = entityMetadata.getTableEntity(); + columnNames = new ArrayList<>(projectedProperties.size()); + + for (String projectedProperty : projectedProperties) { + + RelationalPersistentProperty property = entity.getPersistentProperty(projectedProperty); + columnNames.add(property != null ? property.getColumnName() : SqlIdentifier.unquoted(projectedProperty)); + } + + } else if (tree.isExistsProjection()) { columnNames = dataAccessStrategy.getIdentifierColumns(entityMetadata.getJavaType()); } else { columnNames = dataAccessStrategy.getAllColumns(entityMetadata.getJavaType()); diff --git a/src/test/java/org/springframework/data/r2dbc/repository/AbstractR2dbcRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/r2dbc/repository/AbstractR2dbcRepositoryIntegrationTests.java index 12a50883..c6a2f708 100644 --- a/src/test/java/org/springframework/data/r2dbc/repository/AbstractR2dbcRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/r2dbc/repository/AbstractR2dbcRepositoryIntegrationTests.java @@ -156,6 +156,26 @@ public abstract class AbstractR2dbcRepositoryIntegrationTests extends R2dbcInteg }).verifyComplete(); } + @Test // gh-344 + public void shouldFindApplyingDistinctProjection() { + + LegoSet legoSet1 = new LegoSet(null, "SCHAUFELRADBAGGER", 12); + LegoSet legoSet2 = new LegoSet(null, "SCHAUFELRADBAGGER", 13); + + repository.saveAll(Arrays.asList(legoSet1, legoSet2)) // + .as(StepVerifier::create) // + .expectNextCount(2) // + .verifyComplete(); + + repository.findDistinctBy() // + .map(Named::getName) // + .collectList() // + .as(StepVerifier::create) // + .consumeNextWith(actual -> { + assertThat(actual).hasSize(1).contains("SCHAUFELRADBAGGER"); + }).verifyComplete(); + } + @Test // gh-41 public void shouldFindApplyingSimpleTypeProjection() { @@ -282,6 +302,8 @@ public abstract class AbstractR2dbcRepositoryIntegrationTests extends R2dbcInteg Flux findAsProjection(); + Flux findDistinctBy(); + Mono findByManual(int manual); Flux findAllIds(); diff --git a/src/test/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQueryUnitTests.java b/src/test/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQueryUnitTests.java index 2d4b0de9..927f0b97 100644 --- a/src/test/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQueryUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQueryUnitTests.java @@ -614,11 +614,10 @@ public class PartTreeR2dbcQueryUnitTests { dataAccessStrategy); BindableQuery bindableQuery = r2dbcQuery.createQuery(getAccessor(queryMethod, new Object[] { "John" })); - assertThat(bindableQuery.get()) - .isEqualTo("SELECT " + DISTINCT + " " + ALL_FIELDS + " FROM " + TABLE + " WHERE " + TABLE + ".first_name = $1"); + assertThat(bindableQuery.get()).isEqualTo("SELECT " + DISTINCT + " " + TABLE + ".first_name, " + TABLE + + ".foo FROM " + TABLE + " WHERE " + TABLE + ".first_name = $1"); } - private R2dbcQueryMethod getQueryMethod(String methodName, Class... parameterTypes) throws Exception { Method method = UserRepository.class.getMethod(methodName, parameterTypes); return new R2dbcQueryMethod(method, new DefaultRepositoryMetadata(UserRepository.class), @@ -696,8 +695,8 @@ public class PartTreeR2dbcQueryUnitTests { Flux findTop3ByFirstName(String firstName); Mono findFirstByFirstName(String firstName); - - Mono findDistinctByFirstName(String firstName); + + Mono findDistinctByFirstName(String firstName); Mono deleteByFirstName(String firstName); } @@ -713,4 +712,11 @@ public class PartTreeR2dbcQueryUnitTests { private Integer age; private Boolean active; } + + interface UserProjection { + + String getFirstName(); + + String getFoo(); + } }