#341 - Polishing.

Consider modifying indicators in query execution. Consider absence of Criteria for delete and update queries. Add test.

Reformat code, update documentation.

Original pull request: #345.

src/main/asciidoc/reference/r2dbc-repositories.adoc

@@ -234,10 +234,27 @@ The following table shows the keywords that are supported for query methods:
 === Modifying Queries
 
 The previous sections describe how to declare queries to access a given entity or collection of entities.
-You can add custom modifying behavior by using the facilities described in <<repositories.custom-implementations,Custom Implementations for Spring Data Repositories>>.
-As this approach is feasible for comprehensive custom functionality, you can modify queries that only need parameter binding by annotating the query method with `@Modifying`, as shown in the following example:
+Using keywords from the preceding table can be used in conjunction with `delete…By` or `remove…By` to create derived queries that delete matching rows.
 
-Declaring manipulating queries
+.`Delete…By` Query
+====
+[source,java]
+----
+interface ReactivePersonRepository extends ReactiveSortingRepository<Person, String> {
+
+  Mono<Integer> deleteByLastname(String lastname);            <1>
+
+  Mono<Void> deletePersonByLastname(String lastname);         <2>
+
+  Mono<Boolean> deletePersonByLastname(String lastname);      <3>
+}
+----
+<1> Using a return type of `Mono<Integer>` returns the number of affected rows.
+<2> Using `Void` just reports whether the rows were successfully deleted without emitting a result value.
+<3> Using `Boolean` reports whether at least one row was removed.
+====
+
+As this approach is feasible for comprehensive custom functionality, you can modify queries that only need parameter binding by annotating the query method with `@Modifying`, as shown in the following example:
 
 ====
 [source,java]
@@ -256,6 +273,8 @@ The result of a modifying query can be:
 The `@Modifying` annotation is only relevant in combination with the `@Query` annotation.
 Derived custom methods do not require this annotation.
 
+Alternatively, you can add custom modifying behavior by using the facilities described in <<repositories.custom-implementations,Custom Implementations for Spring Data Repositories>>.
+
 [[r2dbc.repositories.queries.spel]]
 === Queries with SpEL Expressions
 

src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java

@@ -28,6 +28,7 @@ import org.springframework.data.r2dbc.query.BoundCondition;
 import org.springframework.data.r2dbc.query.UpdateMapper;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
+import org.springframework.data.relational.core.query.CriteriaDefinition;
 import org.springframework.data.relational.core.sql.*;
 import org.springframework.data.relational.core.sql.InsertBuilder.InsertValuesWithBuild;
 import org.springframework.data.relational.core.sql.render.RenderContext;
@@ -204,10 +205,10 @@ class DefaultStatementMapper implements StatementMapper {
 
 		Update update;
 
-		if (!updateSpec.getCriteria().isEmpty()) {
+		CriteriaDefinition criteria = updateSpec.getCriteria();
+		if (criteria != null && !criteria.isEmpty()) {
 
-			BoundCondition boundCondition = this.updateMapper.getMappedObject(bindMarkers, updateSpec.getCriteria(), table,
-					entity);
+			BoundCondition boundCondition = this.updateMapper.getMappedObject(bindMarkers, criteria, table, entity);
 
 			bindings = bindings.and(boundCondition.getBindings());
 			update = updateBuilder.where(boundCondition.getCondition()).build();
@@ -247,7 +248,9 @@ class DefaultStatementMapper implements StatementMapper {
 		Bindings bindings = Bindings.empty();
 
 		Delete delete;
-		if (!deleteSpec.getCriteria().isEmpty()) {
+		CriteriaDefinition criteria = deleteSpec.getCriteria();
+
+		if (criteria != null && !criteria.isEmpty()) {
 
 			BoundCondition boundCondition = this.updateMapper.getMappedObject(bindMarkers, deleteSpec.getCriteria(), table,
 					entity);

src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java

@@ -198,7 +198,7 @@ public interface StatementMapper {
 		private final Table table;
 		private final List<String> projectedFields;
 		private final List<Expression> selectList;
-		private final CriteriaDefinition criteria;
+		private final @Nullable CriteriaDefinition criteria;
 		private final Sort sort;
 		private final long offset;
 		private final int limit;
@@ -383,6 +383,7 @@ public interface StatementMapper {
 			return Collections.unmodifiableList(selectList);
 		}
 
+		@Nullable
 		public CriteriaDefinition getCriteria() {
 			return this.criteria;
 		}
@@ -475,12 +476,11 @@ public interface StatementMapper {
 	class UpdateSpec {
 
 		private final SqlIdentifier table;
-		@Nullable private final org.springframework.data.relational.core.query.Update update;
-
-		private final CriteriaDefinition criteria;
+		private final @Nullable org.springframework.data.relational.core.query.Update update;
+		private final @Nullable CriteriaDefinition criteria;
 
 		protected UpdateSpec(SqlIdentifier table, @Nullable org.springframework.data.relational.core.query.Update update,
-				CriteriaDefinition criteria) {
+				@Nullable CriteriaDefinition criteria) {
 
 			this.table = table;
 			this.update = update;
@@ -527,6 +527,7 @@ public interface StatementMapper {
 			return this.update;
 		}
 
+		@Nullable
 		public CriteriaDefinition getCriteria() {
 			return this.criteria;
 		}
@@ -538,8 +539,7 @@ public interface StatementMapper {
 	class DeleteSpec {
 
 		private final SqlIdentifier table;
-
-		private final CriteriaDefinition criteria;
+		private final @Nullable CriteriaDefinition criteria;
 
 		protected DeleteSpec(SqlIdentifier table, CriteriaDefinition criteria) {
 			this.table = table;
@@ -581,6 +581,7 @@ public interface StatementMapper {
 			return this.table;
 		}
 
+		@Nullable
 		public CriteriaDefinition getCriteria() {
 			return this.criteria;
 		}

src/main/java/org/springframework/data/r2dbc/repository/query/AbstractR2dbcQuery.java

@@ -20,7 +20,6 @@ import reactor.core.publisher.Mono;
 
 import org.reactivestreams.Publisher;
 
-import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.mapping.model.EntityInstantiators;
 import org.springframework.data.r2dbc.convert.R2dbcConverter;
 import org.springframework.data.r2dbc.core.DatabaseClient;
@@ -109,7 +108,7 @@ public abstract class AbstractR2dbcQuery implements RepositoryQuery {
 
 		SqlIdentifier tableName = method.getEntityInformation().getTableName();
 
-		R2dbcQueryExecution execution = getExecution(processor.getReturnedType(),
+		R2dbcQueryExecution execution = new ResultProcessingExecution(getExecutionToWrap(processor.getReturnedType()),
 				new ResultProcessingConverter(processor, converter.getMappingContext(), instantiators));
 
 		return execution.execute(fetchSpec, processor.getReturnedType().getDomainType(), tableName);
@@ -122,20 +121,9 @@ public abstract class AbstractR2dbcQuery implements RepositoryQuery {
 		return returnedType.isProjecting() ? returnedType.getDomainType() : returnedType.getReturnedType();
 	}
 
-	/**
-	 * Returns the execution instance to use.
-	 *
-	 * @param returnedType must not be {@literal null}.
-	 * @param resultProcessing must not be {@literal null}.
-	 * @return
-	 */
-	private R2dbcQueryExecution getExecution(ReturnedType returnedType, Converter<Object, Object> resultProcessing) {
-		return new ResultProcessingExecution(getExecutionToWrap(returnedType), resultProcessing);
-	}
-
 	private R2dbcQueryExecution getExecutionToWrap(ReturnedType returnedType) {
 
-		if (method.isModifyingQuery()) {
+		if (isModifyingQuery()) {
 
 			if (Boolean.class.isAssignableFrom(returnedType.getReturnedType())) {
 				return (q, t, c) -> q.rowsUpdated().map(integer -> integer > 0);
@@ -162,6 +150,14 @@ public abstract class AbstractR2dbcQuery implements RepositoryQuery {
 		return (q, t, c) -> q.one();
 	}
 
+	/**
+	 * Returns whether this query is a modifying one.
+	 * 
+	 * @return
+	 * @since 1.1
+	 */
+	protected abstract boolean isModifyingQuery();
+
 	/**
 	 * Creates a {@link BindableQuery} instance using the given {@link ParameterAccessor}
 	 *

src/main/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQuery.java

@@ -29,6 +29,7 @@ import org.springframework.data.repository.query.parser.PartTree;
  * An {@link AbstractR2dbcQuery} implementation based on a {@link PartTree}.
  *
  * @author Roman Chigvintsev
+ * @author Mark Paluch
  * @since 1.1
  */
 public class PartTreeR2dbcQuery extends AbstractR2dbcQuery {
@@ -61,6 +62,19 @@ public class PartTreeR2dbcQuery extends AbstractR2dbcQuery {
 		}
 	}
 
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.r2dbc.repository.query.AbstractR2dbcQuery#isModifyingQuery()
+	 */
+	@Override
+	protected boolean isModifyingQuery() {
+		return this.tree.isDelete();
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.r2dbc.repository.query.AbstractR2dbcQuery#createQuery(org.springframework.data.relational.repository.query.RelationalParameterAccessor)
+	 */
 	@Override
 	protected BindableQuery createQuery(RelationalParameterAccessor accessor) {
 

src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcQueryCreator.java

@@ -83,10 +83,24 @@ public class R2dbcQueryCreator extends RelationalQueryCreator<PreparedOperation<
 	protected PreparedOperation<?> complete(Criteria criteria, Sort sort) {
 
 		StatementMapper statementMapper = dataAccessStrategy.getStatementMapper().forType(entityMetadata.getJavaType());
-		if(tree.isDelete()){
-			StatementMapper.DeleteSpec deleteSpec = statementMapper.createDelete(entityMetadata.getTableName()).withCriteria(criteria);
-			return statementMapper.getMappedObject(deleteSpec);
+
+		if (tree.isDelete()) {
+			return delete(criteria, statementMapper);
 		}
+
+		return select(criteria, sort, statementMapper);
+	}
+
+	private PreparedOperation<?> delete(Criteria criteria, StatementMapper statementMapper) {
+
+		StatementMapper.DeleteSpec deleteSpec = statementMapper.createDelete(entityMetadata.getTableName())
+				.withCriteria(criteria);
+
+		return statementMapper.getMappedObject(deleteSpec);
+	}
+
+	private PreparedOperation<?> select(Criteria criteria, Sort sort, StatementMapper statementMapper) {
+
 		StatementMapper.SelectSpec selectSpec = statementMapper.createSelect(entityMetadata.getTableName())
 				.withProjection(getSelectProjection());
 

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

@@ -76,7 +76,17 @@ public class StringBasedR2dbcQuery extends AbstractR2dbcQuery {
 		this.binder = new ExpressionEvaluatingParameterBinder(expressionParser, evaluationContextProvider, expressionQuery);
 	}
 
-	/* (non-Javadoc)
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.r2dbc.repository.query.AbstractR2dbcQuery#isModifyingQuery()
+	 */
+	@Override
+	protected boolean isModifyingQuery() {
+		return getQueryMethod().isModifyingQuery();
+	}
+
+	/* 
+	 * (non-Javadoc)
 	 * @see org.springframework.data.r2dbc.repository.query.AbstractR2dbcQuery#createQuery(org.springframework.data.relational.repository.query.RelationalParameterAccessor)
 	 */
 	@Override
@@ -95,5 +105,4 @@ public class StringBasedR2dbcQuery extends AbstractR2dbcQuery {
 			}
 		};
 	}
-
 }

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

@@ -229,6 +229,20 @@ public abstract class AbstractR2dbcRepositoryIntegrationTests extends R2dbcInteg
 				.verifyComplete();
 	}
 
+	@Test // gh-341
+	public void shouldDeleteAll() {
+
+		shouldInsertNewItems();
+
+		repository.deleteAllBy() //
+				.as(StepVerifier::create) //
+				.verifyComplete();
+
+		repository.findAll() //
+				.as(StepVerifier::create) //
+				.verifyComplete();
+	}
+
 	@Test
 	public void shouldInsertItemsTransactional() {
 
@@ -272,6 +286,8 @@ public abstract class AbstractR2dbcRepositoryIntegrationTests extends R2dbcInteg
 
 		Flux<Integer> findAllIds();
 
+		Mono<Void> deleteAllBy();
+
 		@Query("DELETE from legoset where manual = :manual")
 		Mono<Void> deleteAllByManual(int manual);
 	}

src/test/java/org/springframework/data/r2dbc/repository/query/PartTreeR2dbcQueryUnitTests.java

@@ -55,6 +55,7 @@ import org.springframework.data.repository.core.support.DefaultRepositoryMetadat
  *
  * @author Roman Chigvintsev
  * @author Mark Paluch
+ * @author Mingyuan Wu
  */
 @RunWith(MockitoJUnitRunner.class)
 public class PartTreeR2dbcQueryUnitTests {
@@ -594,13 +595,14 @@ public class PartTreeR2dbcQueryUnitTests {
 
 	@Test // gh-341
 	public void createsQueryToDeleteByFirstName() throws Exception {
+
 		R2dbcQueryMethod queryMethod = getQueryMethod("deleteByFirstName", String.class);
 		PartTreeR2dbcQuery r2dbcQuery = new PartTreeR2dbcQuery(queryMethod, databaseClient, r2dbcConverter,
 				dataAccessStrategy);
 		RelationalParametersParameterAccessor accessor = getAccessor(queryMethod, new Object[] { "John" });
 		BindableQuery bindableQuery = r2dbcQuery.createQuery(accessor);
-		String expectedSql = "DELETE FROM "+ TABLE + " WHERE " + TABLE + ".first_name = $1" ;
-		assertThat(bindableQuery.get()).isEqualTo(expectedSql);
+
+		assertThat(bindableQuery.get()).isEqualTo("DELETE FROM " + TABLE + " WHERE " + TABLE + ".first_name = $1");
 	}
 
 	private R2dbcQueryMethod getQueryMethod(String methodName, Class<?>... parameterTypes) throws Exception {