Skip to content

Commit

Permalink
Polishing.
Browse files Browse the repository at this point in the history 
Use FilterFunction instead of nullable fetchSize to avoid unconditional capturing lambdas and improve defaulting.

Add since tag.

See #1652
Original pull request: #1898
  • Loading branch information
@mp911de
mp911de committed Oct 1, 2024
1 parent 96a4121 commit 90b6d8e
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 47 deletions.
8 changes: 5 additions & 3 deletions spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -320,7 +321,8 @@ public <T> Flux<T> select(Query query, Class<T> entityClass) throws DataAccessEx
<T, P extends Publisher<T>> P doSelect(Query query, Class<?> entityClass, SqlIdentifier tableName,
Class<T> returnType, Function<RowsFetchSpec<T>, P> resultHandler, @Nullable Integer fetchSize) {

RowsFetchSpec<T> fetchSpec = doSelect(query, entityClass, tableName, returnType, fetchSize);
RowsFetchSpec<T> fetchSpec = doSelect(query, entityClass, tableName, returnType,
fetchSize != null ? statement -> statement.fetchSize(fetchSize) : Function.identity());

P result = resultHandler.apply(fetchSpec);

Expand All @@ -332,7 +334,7 @@ <T, P extends Publisher<T>> P doSelect(Query query, Class<?> entityClass, SqlIde
}

private <T> RowsFetchSpec<T> doSelect(Query query, Class<?> entityType, SqlIdentifier tableName,
Class<T> returnType, @Nullable Integer fetchSize) {
Class<T> returnType, Function<? super Statement, ? extends Statement> filterFunction) {

StatementMapper statementMapper = dataAccessStrategy.getStatementMapper().forType(entityType);

Expand Down Expand Up @@ -360,7 +362,7 @@ private <T> RowsFetchSpec<T> 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
);
Expand Down
6 changes: 4 additions & 2 deletions ...data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,12 @@ interface SelectWithProjection<T> extends SelectWithQuery<T> {
interface SelectWithQuery<T> extends TerminatingSelect<T> {

/**
* 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<T> withFetchSize(int fetchSize);

Expand Down
4 changes: 0 additions & 4 deletions ...dbc/src/main/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ static class ReactiveSelectSupport<T> implements ReactiveSelect<T> {
private final Class<T> returnType;
private final Query query;
private final @Nullable SqlIdentifier tableName;

private final @Nullable Integer fetchSize;

ReactiveSelectSupport(R2dbcEntityTemplate template, Class<?> domainType, Class<T> returnType, Query query,
Expand Down Expand Up @@ -86,9 +85,6 @@ public <R> SelectWithQuery<R> as(Class<R> returnType) {

@Override
public SelectWithQuery<T> withFetchSize(int fetchSize) {

Assert.notNull(returnType, "FetchSize must not be null");

return new ReactiveSelectSupport<>(template, domainType, returnType, query, tableName, fetchSize);
}

Expand Down
47 changes: 23 additions & 24 deletions ...r2dbc/src/test/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplateUnitTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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()
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -204,7 +203,7 @@ void shouldProjectCountResultWithoutId() {
.expectNext(1L).verifyComplete();
}

@Test // gh-469
@Test // GH-469
void shouldExistsByCriteria() {

MockRowMetadata metadata = MockRowMetadata.builder()
Expand All @@ -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());
Expand All @@ -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()
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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()
Expand All @@ -321,7 +320,7 @@ void shouldUpdateByQuery() {
Parameter.from("Walter"));
}

@Test // gh-220
@Test // GH-220
void shouldDeleteByQuery() {

MockRowMetadata metadata = MockRowMetadata.builder()
Expand All @@ -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() //
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -419,7 +418,7 @@ void shouldSkipDefaultIdValueOnVersionedInsert() {
Parameter.from("bar"));
}

@Test // gh-451
@Test // GH-451
void shouldInsertCorrectlyVersionedAndAudited() {

MockRowMetadata metadata = MockRowMetadata.builder().build();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -504,7 +503,7 @@ void insertShouldInvokeCallback() {
Parameter.from("before-save"));
}

@Test // gh-365
@Test // GH-365
void shouldUpdateVersioned() {

MockRowMetadata metadata = MockRowMetadata.builder().build();
Expand All @@ -526,7 +525,7 @@ void shouldUpdateVersioned() {
Parameter.from(1L));
}

@Test // gh-215
@Test // GH-215
void updateShouldInvokeCallback() {

MockRowMetadata metadata = MockRowMetadata.builder().build();
Expand Down Expand Up @@ -559,7 +558,7 @@ void updateShouldInvokeCallback() {
Parameter.from("before-save"));
}

@Test // gh-637
@Test // GH-637
void insertIncludesInsertOnlyColumns() {

MockRowMetadata metadata = MockRowMetadata.builder().build();
Expand All @@ -578,7 +577,7 @@ void insertIncludesInsertOnlyColumns() {
Parameter.from("insert this"));
}

@Test // gh-637
@Test // GH-637
void updateExcludesInsertOnlyColumns() {

MockRowMetadata metadata = MockRowMetadata.builder().build();
Expand Down
26 changes: 12 additions & 14 deletions ...c/src/test/java/org/springframework/data/r2dbc/core/ReactiveSelectOperationUnitTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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();
Expand All @@ -256,17 +257,14 @@ 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) //
.verifyComplete();

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);
}

Expand Down

0 comments on commit 90b6d8e

Please # to comment.