Reject Limit parameter using String-based queries.

Document limitations, add test cases for derived queries.

Closes #1654

Author: mp911de

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java

@@ -117,6 +117,11 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera
 			throw new UnsupportedOperationException(
 					"Page queries are not supported using string-based queries; Offending method: " + queryMethod);
 		}
+
+		if (queryMethod.getParameters().hasLimitParameter()) {
+			throw new UnsupportedOperationException(
+					"Queries with Limit are not supported using string-based queries; Offending method: " + queryMethod);
+		}
 	}
 
 	@Override

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java

@@ -52,6 +52,7 @@
 import org.springframework.data.annotation.Id;
 import org.springframework.data.domain.Example;
 import org.springframework.data.domain.ExampleMatcher;
+import org.springframework.data.domain.Limit;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
@@ -491,6 +492,18 @@ public void pageByNameShouldReturnCorrectResult() {
 		assertThat(repository.findPageByNameContains("a", PageRequest.of(1, 2)).getContent()).hasSize(1);
 	}
 
+	@Test // GH-1654
+	public void selectWithLimitShouldReturnCorrectResult() {
+
+		repository.saveAll(Arrays.asList(new DummyEntity("a1"), new DummyEntity("a2"), new DummyEntity("a3")));
+
+		List<DummyEntity> page = repository.findByNameContains("a", Limit.of(3));
+		assertThat(page).hasSize(3);
+
+		assertThat(repository.findByNameContains("a", Limit.of(2))).hasSize(2);
+		assertThat(repository.findByNameContains("a", Limit.unlimited())).hasSize(3);
+	}
+
 	@Test // GH-774
 	public void sliceByNameShouldReturnCorrectResult() {
 
@@ -1385,6 +1398,8 @@ interface DummyEntityRepository extends CrudRepository<DummyEntity, Long>, Query
 
 		Page<DummyEntity> findPageByNameContains(String name, Pageable pageable);
 
+		List<DummyEntity> findByNameContains(String name, Limit limit);
+
 		Page<DummyProjection> findPageProjectionByName(String name, Pageable pageable);
 
 		Slice<DummyEntity> findSliceByNameContains(String name, Pageable pageable);

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java

@@ -36,6 +36,7 @@
 import org.springframework.dao.DataAccessException;
 import org.springframework.data.convert.ReadingConverter;
 import org.springframework.data.convert.WritingConverter;
+import org.springframework.data.domain.Limit;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Slice;
@@ -51,6 +52,7 @@
 import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
 import org.springframework.data.repository.core.support.PropertiesBasedNamedQueries;
 import org.springframework.data.repository.query.ExtensionAwareQueryMethodEvaluationContextProvider;
+import org.springframework.data.repository.query.Param;
 import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
 import org.springframework.data.spel.spi.EvaluationContextExtension;
 import org.springframework.jdbc.core.ResultSetExtractor;
@@ -191,6 +193,16 @@ void pageQueryNotSupported() {
 						.hasMessageContaining("Page queries are not supported using string-based queries");
 	}
 
+	@Test // GH-1654
+	void limitNotSupported() {
+
+		JdbcQueryMethod queryMethod = createMethod("unsupportedLimitQuery", String.class, Limit.class);
+
+		assertThatThrownBy(
+				() -> new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter, evaluationContextProvider))
+						.isInstanceOf(UnsupportedOperationException.class);
+	}
+
 	@Test // GH-1212
 	void convertsEnumCollectionParameterIntoStringCollectionParameter() {
 
@@ -230,7 +242,6 @@ void appliesConverterToIterable() {
 				.extractParameterSource();
 
 		assertThat(sqlParameterSource.getValue("value")).isEqualTo("one");
-
 	}
 
 	QueryFixture forMethod(String name, Class... paramTypes) {
@@ -337,6 +348,9 @@ interface MyRepository extends Repository<Object, Long> {
 
 		@Query("SELECT * FROM table WHERE c = :#{myext.testValue} AND c2 = :#{myext.doSomething()}")
 		Object findBySpelExpression(Object object);
+
+		@Query("SELECT * FROM person WHERE lastname = $1")
+		Object unsupportedLimitQuery(@Param("lastname") String lastname, Limit limit);
 	}
 
 	@Test // GH-619

spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java

@@ -100,6 +100,21 @@ public StringBasedR2dbcQuery(String query, R2dbcQueryMethod method, R2dbcEntityO
 		this.expressionQuery = ExpressionQuery.create(query);
 		this.binder = new ExpressionEvaluatingParameterBinder(expressionQuery, dataAccessStrategy);
 		this.expressionDependencies = createExpressionDependencies();
+
+		if (method.isSliceQuery()) {
+			throw new UnsupportedOperationException(
+					"Slice queries are not supported using string-based queries; Offending method: " + method);
+		}
+
+		if (method.isPageQuery()) {
+			throw new UnsupportedOperationException(
+					"Page queries are not supported using string-based queries; Offending method: " + method);
+		}
+
+		if (method.getParameters().hasLimitParameter()) {
+			throw new UnsupportedOperationException(
+					"Queries with Limit are not supported using string-based queries; Offending method: " + method);
+		}
 	}
 
 	private ExpressionDependencies createExpressionDependencies() {

spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/AbstractR2dbcRepositoryIntegrationTests.java

@@ -15,14 +15,29 @@
  */
 package org.springframework.data.r2dbc.repository;
 
+import static org.assertj.core.api.Assertions.*;
+
 import io.r2dbc.spi.ConnectionFactory;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Hooks;
+import reactor.core.publisher.Mono;
+import reactor.test.StepVerifier;
+
+import java.util.Arrays;
+import java.util.Map;
+import java.util.Objects;
+import java.util.stream.IntStream;
+
+import javax.sql.DataSource;
+
 import org.assertj.core.api.Condition;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.dao.DataAccessException;
 import org.springframework.data.annotation.Id;
 import org.springframework.data.annotation.PersistenceCreator;
+import org.springframework.data.domain.Limit;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.r2dbc.repository.support.R2dbcRepositoryFactory;
@@ -35,17 +50,6 @@
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.r2dbc.connection.R2dbcTransactionManager;
 import org.springframework.transaction.reactive.TransactionalOperator;
-import reactor.core.publisher.Flux;
-import reactor.core.publisher.Hooks;
-import reactor.core.publisher.Mono;
-import reactor.test.StepVerifier;
-
-import javax.sql.DataSource;
-import java.util.Arrays;
-import java.util.Map;
-import java.util.stream.IntStream;
-
-import static org.assertj.core.api.Assertions.*;
 
 /**
  * Abstract base class for integration tests for {@link LegoSetRepository} using {@link R2dbcRepositoryFactory}.
@@ -143,6 +147,22 @@ void shouldFindItemsByNameContains() {
 				}).verifyComplete();
 	}
 
+	@Test // GH-1654
+	void shouldFindItemsByNameContainsWithLimit() {
+
+		shouldInsertNewItems();
+
+		repository.findByNameContains("F", Limit.of(1)) //
+				.as(StepVerifier::create) //
+				.expectNextCount(1) //
+				.verifyComplete();
+
+		repository.findByNameContains("F", Limit.unlimited()) //
+				.as(StepVerifier::create) //
+				.expectNextCount(2) //
+				.verifyComplete();
+	}
+
 	@Test // GH-475, GH-607
 	void shouldFindApplyingInterfaceProjection() {
 
@@ -423,6 +443,8 @@ interface LegoSetRepository extends ReactiveCrudRepository<LegoSet, Integer> {
 
 		Flux<LegoSet> findByNameContains(String name);
 
+		Flux<LegoSet> findByNameContains(String name, Limit limit);
+
 		Flux<LegoSet> findFirst10By();
 
 		Flux<LegoSet> findAllByOrderByManual(Pageable pageable);
@@ -483,6 +505,7 @@ public static class LegoSet extends Lego implements Buildable {
 		public LegoSet() {
 		}
 
+		@Override
 		public String getName() {
 			return this.name;
 		}
@@ -547,15 +570,15 @@ public String getUnknown() {
 
 		public boolean equals(final Object o) {
 			if (o == this) return true;
-			if (!(o instanceof LegoDto)) return false;
-			final LegoDto other = (LegoDto) o;
+			if (!(o instanceof final LegoDto other))
+				return false;
 			final Object this$name = this.getName();
 			final Object other$name = other.getName();
-			if (this$name == null ? other$name != null : !this$name.equals(other$name)) return false;
+			if (!Objects.equals(this$name, other$name))
+				return false;
 			final Object this$unknown = this.getUnknown();
 			final Object other$unknown = other.getUnknown();
-			if (this$unknown == null ? other$unknown != null : !this$unknown.equals(other$unknown)) return false;
-			return true;
+			return Objects.equals(this$unknown, other$unknown);
 		}
 
 		public int hashCode() {

spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java

@@ -18,11 +18,8 @@
 import static org.assertj.core.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
-import io.r2dbc.spi.R2dbcType;
-import io.r2dbc.spi.test.MockColumnMetadata;
 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;
 
@@ -36,7 +33,7 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
-
+import org.springframework.data.domain.Limit;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.projection.ProjectionFactory;
 import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
@@ -288,8 +285,6 @@ void usesDtoTypeForDtoResultMapping() {
 	@Test // gh-612
 	void selectsSimpleType() {
 
-		MockRowMetadata metadata = MockRowMetadata.builder()
-				.columnMetadata(MockColumnMetadata.builder().name("date").type(R2dbcType.DATE).build()).build();
 		LocalDate value = LocalDate.now();
 		MockResult result = MockResult.builder()
 				.row(MockRow.builder().identified(0, LocalDate.class, value).build()).build();
@@ -309,6 +304,12 @@ void selectsSimpleType() {
 		flux.as(StepVerifier::create).expectNext(value).verifyComplete();
 	}
 
+	@Test // GH-1654
+	void rejectsStringBasedLimitQuery() {
+		assertThatExceptionOfType(UnsupportedOperationException.class)
+				.isThrownBy(() -> getQueryMethod("unsupportedLimitQuery", String.class, Limit.class));
+	}
+
 	private StringBasedR2dbcQuery getQueryMethod(String name, Class<?>... args) {
 
 		Method method = ReflectionUtils.findMethod(SampleRepository.class, name, args);
@@ -366,6 +367,9 @@ private interface SampleRepository extends Repository<Person, String> {
 
 		@Query("SELECT MAX(DATE)")
 		Flux<LocalDate> findAllLocalDates();
+
+		@Query("SELECT * FROM person WHERE lastname = $1")
+		Person unsupportedLimitQuery(@Param("lastname") String lastname, Limit limit);
 	}
 
 	static class PersonDto {
@@ -388,6 +392,6 @@ public String getName() {
 	}
 
 	enum MyEnum {
-		INSTANCE;
+		INSTANCE
 	}
 }

src/main/antora/modules/ROOT/pages/jdbc/query-methods.adoc

@@ -169,6 +169,9 @@ Properties that don't have a matching column in the result will not be set.
 The query is used for populating the aggregate root, embedded entities and one-to-one relationships including arrays of primitive types which get stored and loaded as SQL-array-types.
 Separate queries are generated for maps, lists, sets and arrays of entities.
 
+WARNING: Note that String-based queries do not support pagination nor accept `Sort`, `PageRequest`, and `Limit` as a query parameter as for these queries the query would be required to be rewritten.
+If you want to apply limiting, please express this intent using SQL and bind the appropriate parameters to the query yourself.
+
 Queries may contain SpEL expressions where bind variables are allowed.
 Such a SpEL expression will get replaced with a bind variable and the variable gets bound to the result of the SpEL expression.
 

src/main/antora/modules/ROOT/pages/r2dbc/query-methods.adoc

@@ -182,6 +182,27 @@ Therefore also fields with auditing annotations do not get updated if they don't
 
 Alternatively, you can add custom modifying behavior by using the facilities described in xref:repositories/custom-implementations.adoc[Custom Implementations for Spring Data Repositories].
 
+[[r2dbc.query-methods.at-query]]
+== Using `@Query`
+
+The following example shows how to use `@Query` to declare a query method:
+
+.Declare a query method by using @Query
+[source,java]
+----
+interface UserRepository extends ReactiveCrudRepository<User, Long> {
+
+  @Query("select firstName, lastName from User u where u.emailAddress = :email")
+  Flux<User> findByEmailAddress(@Param("email") String email);
+}
+----
+
+WARNING: Note that String-based queries do not support pagination nor accept `Sort`, `PageRequest`, and `Limit` as a query parameter as for these queries the query would be required to be rewritten.
+If you want to apply limiting, please express this intent using SQL and bind the appropriate parameters to the query yourself.
+
+NOTE: Spring fully supports Java 8’s parameter name discovery based on the `-parameters` compiler flag.
+By using this flag in your build as an alternative to debug information, you can omit the `@Param` annotation for named parameters.
+
 [[r2dbc.repositories.queries.spel]]
 === Queries with SpEL Expressions