From 90b6d8e8a8ee77e7400ba990d3fb9d145451cf7d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 1 Oct 2024 10:17:39 +0200 Subject: [PATCH] Polishing. Use FilterFunction instead of nullable fetchSize to avoid unconditional capturing lambdas and improve defaulting. Add since tag. See #1652 Original pull request: #1898 --- .../data/r2dbc/core/R2dbcEntityTemplate.java | 8 ++-- .../r2dbc/core/ReactiveSelectOperation.java | 6 ++- .../core/ReactiveSelectOperationSupport.java | 4 -- .../core/R2dbcEntityTemplateUnitTests.java | 47 +++++++++---------- .../ReactiveSelectOperationUnitTests.java | 26 +++++----- 5 files changed, 44 insertions(+), 47 deletions(-) diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java index 467c7782f1f..2b1252d6d77 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java @@ -18,6 +18,7 @@ import io.r2dbc.spi.ConnectionFactory; import io.r2dbc.spi.Row; import io.r2dbc.spi.RowMetadata; +import io.r2dbc.spi.Statement; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -320,7 +321,8 @@ public Flux select(Query query, Class entityClass) throws DataAccessEx > P doSelect(Query query, Class entityClass, SqlIdentifier tableName, Class returnType, Function, P> resultHandler, @Nullable Integer fetchSize) { - RowsFetchSpec fetchSpec = doSelect(query, entityClass, tableName, returnType, fetchSize); + RowsFetchSpec fetchSpec = doSelect(query, entityClass, tableName, returnType, + fetchSize != null ? statement -> statement.fetchSize(fetchSize) : Function.identity()); P result = resultHandler.apply(fetchSpec); @@ -332,7 +334,7 @@ > P doSelect(Query query, Class entityClass, SqlIde } private RowsFetchSpec doSelect(Query query, Class entityType, SqlIdentifier tableName, - Class returnType, @Nullable Integer fetchSize) { + Class returnType, Function filterFunction) { StatementMapper statementMapper = dataAccessStrategy.getStatementMapper().forType(entityType); @@ -360,7 +362,7 @@ private RowsFetchSpec doSelect(Query query, Class entityType, SqlIdent PreparedOperation operation = statementMapper.getMappedObject(selectSpec); return getRowsFetchSpec( - databaseClient.sql(operation).filter((statement) -> statement.fetchSize(Optional.ofNullable(fetchSize).orElse(0))), + databaseClient.sql(operation).filter(filterFunction), entityType, returnType ); diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperation.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperation.java index ae1b9dbed7a..8e24c75efd5 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperation.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperation.java @@ -118,10 +118,12 @@ interface SelectWithProjection extends SelectWithQuery { interface SelectWithQuery extends TerminatingSelect { /** - * Specifies the fetch size for this query + * Specifies the fetch size for this query. * * @param fetchSize - * @return + * @return new instance of {@link SelectWithQuery}. + * @since 3.4 + * @see io.r2dbc.spi.Statement#fetchSize(int) */ SelectWithQuery withFetchSize(int fetchSize); diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationSupport.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationSupport.java index d841a1c5ec9..f09781b2292 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationSupport.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationSupport.java @@ -54,7 +54,6 @@ static class ReactiveSelectSupport implements ReactiveSelect { private final Class returnType; private final Query query; private final @Nullable SqlIdentifier tableName; - private final @Nullable Integer fetchSize; ReactiveSelectSupport(R2dbcEntityTemplate template, Class domainType, Class returnType, Query query, @@ -86,9 +85,6 @@ public SelectWithQuery as(Class returnType) { @Override public SelectWithQuery withFetchSize(int fetchSize) { - - Assert.notNull(returnType, "FetchSize must not be null"); - return new ReactiveSelectSupport<>(template, domainType, returnType, query, tableName, fetchSize); } diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplateUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplateUnitTests.java index fd1e9a6db99..e654859168e 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplateUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplateUnitTests.java @@ -99,7 +99,7 @@ void before() { new MappingR2dbcConverter(new R2dbcMappingContext(), conversions)); } - @Test // gh-220 + @Test // GH-220 void shouldCountBy() { MockResult result = MockResult.builder().row(MockRow.builder().identified(0, Long.class, 1L).build()).build(); @@ -117,8 +117,7 @@ void shouldCountBy() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("Walter")); } - @Test - // GH-1690 + @Test // GH-1690 void shouldApplyInterfaceProjection() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -165,7 +164,7 @@ void shouldProjectEntityUsingInheritedInterface() { assertThat(statement.getSql()).isEqualTo("SELECT foo.* FROM foo WHERE foo.THE_NAME = $1"); } - @Test // gh-469 + @Test // GH-469 void shouldProjectExistsResult() { MockResult result = MockResult.builder().row(MockRow.builder().identified(0, Object.class, null).build()).build(); @@ -180,7 +179,7 @@ void shouldProjectExistsResult() { .verifyComplete(); } - @Test // gh-1310 + @Test // GH-1310 void shouldProjectExistsResultWithoutId() { MockResult result = MockResult.builder().row(MockRow.builder().identified(0, Object.class, null).build()).build(); @@ -192,7 +191,7 @@ void shouldProjectExistsResultWithoutId() { .expectNext(true).verifyComplete(); } - @Test // gh-1310 + @Test // GH-1310 void shouldProjectCountResultWithoutId() { MockResult result = MockResult.builder().row(MockRow.builder().identified(0, Long.class, 1L).build()).build(); @@ -204,7 +203,7 @@ void shouldProjectCountResultWithoutId() { .expectNext(1L).verifyComplete(); } - @Test // gh-469 + @Test // GH-469 void shouldExistsByCriteria() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -224,7 +223,7 @@ void shouldExistsByCriteria() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("Walter")); } - @Test // gh-220 + @Test // GH-220 void shouldSelectByCriteria() { recorder.addStubbing(s -> s.startsWith("SELECT"), Collections.emptyList()); @@ -240,7 +239,7 @@ void shouldSelectByCriteria() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("Walter")); } - @Test // gh-215 + @Test // GH-215 void selectShouldInvokeCallback() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -266,7 +265,7 @@ void selectShouldInvokeCallback() { assertThat(callback.getValues()).hasSize(1); } - @Test // gh-220 + @Test // GH-220 void shouldSelectOne() { recorder.addStubbing(s -> s.startsWith("SELECT"), Collections.emptyList()); @@ -282,7 +281,7 @@ void shouldSelectOne() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("Walter")); } - @Test // gh-220, gh-758 + @Test // GH-220, GH-758 void shouldSelectOneDoNotOverrideExistingLimit() { recorder.addStubbing(s -> s.startsWith("SELECT"), Collections.emptyList()); @@ -299,7 +298,7 @@ void shouldSelectOneDoNotOverrideExistingLimit() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("Walter")); } - @Test // gh-220 + @Test // GH-220 void shouldUpdateByQuery() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -321,7 +320,7 @@ void shouldUpdateByQuery() { Parameter.from("Walter")); } - @Test // gh-220 + @Test // GH-220 void shouldDeleteByQuery() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -341,7 +340,7 @@ void shouldDeleteByQuery() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("Walter")); } - @Test // gh-220 + @Test // GH-220 void shouldDeleteEntity() { Person person = Person.empty() // @@ -358,7 +357,7 @@ void shouldDeleteEntity() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("Walter")); } - @Test // gh-365 + @Test // GH-365 void shouldInsertVersioned() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -379,7 +378,7 @@ void shouldInsertVersioned() { Parameter.from(1L)); } - @Test // gh-557, gh-402 + @Test // GH-557, GH-402 void shouldSkipDefaultIdValueOnInsert() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -397,7 +396,7 @@ void shouldSkipDefaultIdValueOnInsert() { assertThat(statement.getBindings()).hasSize(1).containsEntry(0, Parameter.from("bar")); } - @Test // gh-557, gh-402 + @Test // GH-557, GH-402 void shouldSkipDefaultIdValueOnVersionedInsert() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -419,7 +418,7 @@ void shouldSkipDefaultIdValueOnVersionedInsert() { Parameter.from("bar")); } - @Test // gh-451 + @Test // GH-451 void shouldInsertCorrectlyVersionedAndAudited() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -447,7 +446,7 @@ void shouldInsertCorrectlyVersionedAndAudited() { "INSERT INTO with_auditing_and_optimistic_locking (version, name, created_date, last_modified_date) VALUES ($1, $2, $3, $4)"); } - @Test // gh-451 + @Test // GH-451 void shouldUpdateCorrectlyVersionedAndAudited() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -476,7 +475,7 @@ void shouldUpdateCorrectlyVersionedAndAudited() { "UPDATE with_auditing_and_optimistic_locking SET version = $1, name = $2, created_date = $3, last_modified_date = $4"); } - @Test // gh-215 + @Test // GH-215 void insertShouldInvokeCallback() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -504,7 +503,7 @@ void insertShouldInvokeCallback() { Parameter.from("before-save")); } - @Test // gh-365 + @Test // GH-365 void shouldUpdateVersioned() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -526,7 +525,7 @@ void shouldUpdateVersioned() { Parameter.from(1L)); } - @Test // gh-215 + @Test // GH-215 void updateShouldInvokeCallback() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -559,7 +558,7 @@ void updateShouldInvokeCallback() { Parameter.from("before-save")); } - @Test // gh-637 + @Test // GH-637 void insertIncludesInsertOnlyColumns() { MockRowMetadata metadata = MockRowMetadata.builder().build(); @@ -578,7 +577,7 @@ void insertIncludesInsertOnlyColumns() { Parameter.from("insert this")); } - @Test // gh-637 + @Test // GH-637 void updateExcludesInsertOnlyColumns() { MockRowMetadata metadata = MockRowMetadata.builder().build(); diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationUnitTests.java index 6524356467c..f09bdf00de1 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationUnitTests.java @@ -24,11 +24,11 @@ import io.r2dbc.spi.test.MockResult; import io.r2dbc.spi.test.MockRow; import io.r2dbc.spi.test.MockRowMetadata; -import reactor.core.publisher.Flux; import reactor.test.StepVerifier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; + import org.springframework.data.annotation.Id; import org.springframework.data.r2dbc.dialect.PostgresDialect; import org.springframework.data.r2dbc.testing.StatementRecorder; @@ -56,7 +56,7 @@ void before() { entityTemplate = new R2dbcEntityTemplate(client, new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE)); } - @Test // gh-220 + @Test // GH-220 void shouldSelectAll() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -80,7 +80,7 @@ void shouldSelectAll() { .isEqualTo("SELECT person.* FROM person WHERE person.THE_NAME = $1 LIMIT 10 OFFSET 20"); } - @Test // gh-220 + @Test // GH-220 void shouldSelectAs() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -128,7 +128,7 @@ void shouldSelectAsWithColumnName() { assertThat(statement.getSql()).isEqualTo("SELECT person.id, person.a_different_name FROM person WHERE person.THE_NAME = $1"); } - @Test // gh-220 + @Test // GH-220 void shouldSelectFromTable() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -152,7 +152,7 @@ void shouldSelectFromTable() { assertThat(statement.getSql()).isEqualTo("SELECT the_table.* FROM the_table WHERE the_table.THE_NAME = $1"); } - @Test // gh-220 + @Test // GH-220 void shouldSelectFirst() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -175,7 +175,7 @@ void shouldSelectFirst() { assertThat(statement.getSql()).isEqualTo("SELECT person.* FROM person WHERE person.THE_NAME = $1 LIMIT 1"); } - @Test // gh-220 + @Test // GH-220 void shouldSelectOne() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -198,7 +198,7 @@ void shouldSelectOne() { assertThat(statement.getSql()).isEqualTo("SELECT person.* FROM person WHERE person.THE_NAME = $1 LIMIT 2"); } - @Test // gh-220 + @Test // GH-220 void shouldSelectExists() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -221,7 +221,7 @@ void shouldSelectExists() { assertThat(statement.getSql()).isEqualTo("SELECT 1 FROM person WHERE person.THE_NAME = $1 LIMIT 1"); } - @Test // gh-220 + @Test // GH-220 void shouldSelectCount() { MockRowMetadata metadata = MockRowMetadata.builder() @@ -244,8 +244,9 @@ void shouldSelectCount() { assertThat(statement.getSql()).isEqualTo("SELECT COUNT(*) FROM person WHERE person.THE_NAME = $1"); } - @Test // gh-1652 - void shouldBeAbleToProvideFetchSize() { + @Test // GH-1652 + void shouldConsiderFetchSize() { + MockRowMetadata metadata = MockRowMetadata.builder() .columnMetadata(MockColumnMetadata.builder().name("id").type(R2dbcType.INTEGER).build()) .build(); @@ -256,8 +257,7 @@ void shouldBeAbleToProvideFetchSize() { recorder.addStubbing(s -> s.startsWith("SELECT"), result); entityTemplate.select(Person.class) // - .withFetchSize(10) - .matching(query(where("name").is("Walter")).limit(10).offset(20)) // + .withFetchSize(10) // .all() // .as(StepVerifier::create) // .expectNextCount(1) // @@ -265,8 +265,6 @@ void shouldBeAbleToProvideFetchSize() { StatementRecorder.RecordedStatement statement = recorder.getCreatedStatement(s -> s.startsWith("SELECT")); - assertThat(statement.getSql()) - .isEqualTo("SELECT person.* FROM person WHERE person.THE_NAME = $1 LIMIT 10 OFFSET 20"); assertThat(statement.getFetchSize()).isEqualTo(10); }