#105 - Move named parameter resolution to ReactiveDataAccessStrategy.

Named parameter resolution is now provided as part of ReactiveDataAccessStrategy. This allows us to hide implementation internals (bind markers). DatabaseClient allows configuration whether to use named parameter expansion.

Detailed configuration of named parameter support is now moved to DefaultReactiveDataAccessStrategy.

Original pull request: #105.

Author: mp911de

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

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.r2dbc.core;
 
+import org.springframework.data.util.Streamable;
 import org.springframework.lang.Nullable;
 
 /**
@@ -58,4 +59,11 @@ public interface BindParameterSource {
 	default Class<?> getType(String paramName) {
 		return Object.class;
 	}
+
+	/**
+	 * Returns parameter names of the underlying parameter source.
+	 *
+	 * @return parameter names of the underlying parameter source.
+	 */
+	Streamable<String> getParameterNames();
 }

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

@@ -157,14 +157,14 @@ interface Builder {
 		Builder dataAccessStrategy(ReactiveDataAccessStrategy accessStrategy);
 
 		/**
-		 * Configures {@link NamedParameterExpander}.
+		 * Configures whether to use named parameter expansion. Defaults to {@literal true}.
 		 *
-		 * @param expander must not be {@literal null}.
+		 * @param enabled {@literal true} to use named parameter expansion. {@literal false} to disable named parameter
+		 *          expansion.
 		 * @return {@code this} {@link Builder}.
-		 * @see NamedParameterExpander#enabled()
-		 * @see NamedParameterExpander#disabled()
+		 * @see NamedParameterExpander
 		 */
-		Builder namedParameters(NamedParameterExpander expander);
+		Builder namedParameters(boolean enabled);
 
 		/**
 		 * Configures a {@link Consumer} to configure this builder.

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

@@ -79,13 +79,12 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor {
 
 	private final ReactiveDataAccessStrategy dataAccessStrategy;
 
-	private final NamedParameterExpander namedParameters;
+	private final boolean namedParameters;
 
 	private final DefaultDatabaseClientBuilder builder;
 
 	DefaultDatabaseClient(ConnectionFactory connector, R2dbcExceptionTranslator exceptionTranslator,
-			ReactiveDataAccessStrategy dataAccessStrategy, NamedParameterExpander namedParameters,
-			DefaultDatabaseClientBuilder builder) {
+			ReactiveDataAccessStrategy dataAccessStrategy, boolean namedParameters, DefaultDatabaseClientBuilder builder) {
 
 		this.connector = connector;
 		this.exceptionTranslator = exceptionTranslator;
@@ -284,6 +283,18 @@ protected DefaultGenericExecuteSpec createGenericExecuteSpec(Supplier<String> sq
 		return new DefaultGenericExecuteSpec(sqlSupplier);
 	}
 
+	private static void bindByName(Statement statement, Map<String, SettableValue> byName) {
+
+		byName.forEach((name, o) -> {
+
+			if (o.getValue() != null) {
+				statement.bind(name, o.getValue());
+			} else {
+				statement.bindNull(name, o.getType());
+			}
+		});
+	}
+
 	private static void bindByIndex(Statement statement, Map<Integer, SettableValue> byIndex) {
 
 		byIndex.forEach((i, o) -> {
@@ -353,27 +364,28 @@ <T> FetchSpec<T> exchange(Supplier<String> sqlSupplier, BiFunction<Row, RowMetad
 					return statement;
 				}
 
-				BindableOperation operation = namedParameters.expand(sql, dataAccessStrategy.getBindMarkersFactory(),
-						new MapBindParameterSource(this.byName));
+				if (namedParameters) {
 
-				String expanded = getRequiredSql(operation);
-				if (logger.isTraceEnabled()) {
-					logger.trace("Expanded SQL [" + expanded + "]");
-				}
+					PreparedOperation<?> operation = dataAccessStrategy.processNamedParameters(sql, this.byName);
 
-				Statement statement = it.createStatement(expanded);
-				BindTarget bindTarget = new StatementWrapper(statement);
+					String expanded = getRequiredSql(operation);
+					if (logger.isTraceEnabled()) {
+						logger.trace("Expanded SQL [" + expanded + "]");
+					}
 
-				this.byName.forEach((name, o) -> {
+					Statement statement = it.createStatement(expanded);
+					BindTarget bindTarget = new StatementWrapper(statement);
 
-					if (o.getValue() != null) {
-						operation.bind(bindTarget, name, o.getValue());
-					} else {
-						operation.bindNull(bindTarget, name, o.getType());
-					}
-				});
+					operation.bindTo(bindTarget);
+					bindByIndex(statement, this.byIndex);
+
+					return statement;
+				}
+
+				Statement statement = it.createStatement(sql);
 
 				bindByIndex(statement, this.byIndex);
+				bindByName(statement, this.byName);
 
 				return statement;
 			};

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

@@ -36,9 +36,12 @@
 class DefaultDatabaseClientBuilder implements DatabaseClient.Builder {
 
 	private @Nullable ConnectionFactory connectionFactory;
+
 	private @Nullable R2dbcExceptionTranslator exceptionTranslator;
+
 	private ReactiveDataAccessStrategy accessStrategy;
-	private NamedParameterExpander namedParameters;
+
+	private boolean namedParameters = true;
 
 	DefaultDatabaseClientBuilder() {}
 
@@ -93,14 +96,12 @@ public Builder dataAccessStrategy(ReactiveDataAccessStrategy accessStrategy) {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.r2dbc.function.DatabaseClient.Builder#namedParameters(org.springframework.data.r2dbc.function.NamedParameterExpander)
+	 * @see org.springframework.data.r2dbc.function.DatabaseClient.Builder#namedParameters(boolean)
 	 */
 	@Override
-	public Builder namedParameters(NamedParameterExpander expander) {
-
-		Assert.notNull(expander, "NamedParameterExpander must not be null!");
+	public Builder namedParameters(boolean enabled) {
 
-		this.namedParameters = expander;
+		this.namedParameters = enabled;
 		return this;
 	}
 
@@ -125,19 +126,12 @@ public DatabaseClient build() {
 			accessStrategy = new DefaultReactiveDataAccessStrategy(dialect);
 		}
 
-		NamedParameterExpander namedParameters = this.namedParameters;
-
-		if (namedParameters == null) {
-			namedParameters = NamedParameterExpander.enabled();
-		}
-
 		return doBuild(this.connectionFactory, exceptionTranslator, accessStrategy, namedParameters,
 				new DefaultDatabaseClientBuilder(this));
 	}
 
 	protected DatabaseClient doBuild(ConnectionFactory connector, R2dbcExceptionTranslator exceptionTranslator,
-			ReactiveDataAccessStrategy accessStrategy, NamedParameterExpander namedParameters,
-			DefaultDatabaseClientBuilder builder) {
+			ReactiveDataAccessStrategy accessStrategy, boolean namedParameters, DefaultDatabaseClientBuilder builder) {
 		return new DefaultDatabaseClient(connector, exceptionTranslator, accessStrategy, namedParameters, builder);
 	}
 

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

@@ -22,6 +22,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.function.BiFunction;
 
 import org.springframework.dao.InvalidDataAccessResourceUsageException;
@@ -31,7 +32,6 @@
 import org.springframework.data.r2dbc.convert.MappingR2dbcConverter;
 import org.springframework.data.r2dbc.convert.R2dbcConverter;
 import org.springframework.data.r2dbc.convert.R2dbcCustomConversions;
-import org.springframework.data.r2dbc.dialect.BindMarkersFactory;
 import org.springframework.data.r2dbc.dialect.R2dbcDialect;
 import org.springframework.data.r2dbc.mapping.OutboundRow;
 import org.springframework.data.r2dbc.mapping.SettableValue;
@@ -57,6 +57,7 @@ public class DefaultReactiveDataAccessStrategy implements ReactiveDataAccessStra
 	private final UpdateMapper updateMapper;
 	private final MappingContext<RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> mappingContext;
 	private final StatementMapper statementMapper;
+	private final NamedParameterExpander expander;
 
 	/**
 	 * Creates a new {@link DefaultReactiveDataAccessStrategy} given {@link R2dbcDialect} and optional
@@ -81,7 +82,7 @@ public DefaultReactiveDataAccessStrategy(R2dbcDialect dialect, Collection<?> con
 		this(dialect, createConverter(dialect, converters));
 	}
 
-	private static R2dbcConverter createConverter(R2dbcDialect dialect, Collection<?> converters) {
+	public static R2dbcConverter createConverter(R2dbcDialect dialect, Collection<?> converters) {
 
 		Assert.notNull(dialect, "Dialect must not be null");
 		Assert.notNull(converters, "Converters must not be null");
@@ -101,11 +102,24 @@ private static R2dbcConverter createConverter(R2dbcDialect dialect, Collection<?
 	 * @param dialect the {@link R2dbcDialect} to use.
 	 * @param converter must not be {@literal null}.
 	 */
-	@SuppressWarnings("unchecked")
 	public DefaultReactiveDataAccessStrategy(R2dbcDialect dialect, R2dbcConverter converter) {
+		this(dialect, converter, new NamedParameterExpander());
+	}
+
+	/**
+	 * Creates a new {@link DefaultReactiveDataAccessStrategy} given {@link R2dbcDialect} and {@link R2dbcConverter}.
+	 *
+	 * @param dialect the {@link R2dbcDialect} to use.
+	 * @param converter must not be {@literal null}.
+	 * @param expander must not be {@literal null}.
+	 */
+	@SuppressWarnings("unchecked")
+	public DefaultReactiveDataAccessStrategy(R2dbcDialect dialect, R2dbcConverter converter,
+			NamedParameterExpander expander) {
 
 		Assert.notNull(dialect, "Dialect must not be null");
 		Assert.notNull(converter, "RelationalConverter must not be null");
+		Assert.notNull(expander, "NamedParameterExpander must not be null");
 
 		this.converter = converter;
 		this.updateMapper = new UpdateMapper(converter);
@@ -116,6 +130,7 @@ public DefaultReactiveDataAccessStrategy(R2dbcDialect dialect, R2dbcConverter co
 		RenderContextFactory factory = new RenderContextFactory(dialect);
 		this.statementMapper = new DefaultStatementMapper(dialect, factory.createRenderContext(), this.updateMapper,
 				this.mappingContext);
+		this.expander = expander;
 	}
 
 	/*
@@ -215,29 +230,29 @@ public <T> BiFunction<Row, RowMetadata, T> getRowMapper(Class<T> typeToRead) {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getTableName(java.lang.Class)
+	 * @see org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy#processNamedParameters(java.lang.String, java.util.Map)
 	 */
 	@Override
-	public String getTableName(Class<?> type) {
-		return getRequiredPersistentEntity(type).getTableName();
+	public PreparedOperation<?> processNamedParameters(String query, Map<String, SettableValue> bindings) {
+		return this.expander.expand(query, this.dialect.getBindMarkersFactory(), new MapBindParameterSource(bindings));
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getStatementMapper()
+	 * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getTableName(java.lang.Class)
 	 */
 	@Override
-	public StatementMapper getStatementMapper() {
-		return this.statementMapper;
+	public String getTableName(Class<?> type) {
+		return getRequiredPersistentEntity(type).getTableName();
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getBindMarkersFactory()
+	 * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getStatementMapper()
 	 */
 	@Override
-	public BindMarkersFactory getBindMarkersFactory() {
-		return this.dialect.getBindMarkersFactory();
+	public StatementMapper getStatementMapper() {
+		return this.statementMapper;
 	}
 
 	/*

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

@@ -39,7 +39,7 @@
 class DefaultTransactionalDatabaseClient extends DefaultDatabaseClient implements TransactionalDatabaseClient {
 
 	DefaultTransactionalDatabaseClient(ConnectionFactory connector, R2dbcExceptionTranslator exceptionTranslator,
-			ReactiveDataAccessStrategy dataAccessStrategy, NamedParameterExpander namedParameters,
+			ReactiveDataAccessStrategy dataAccessStrategy, boolean namedParameters,
 			DefaultDatabaseClientBuilder builder) {
 		super(connector, exceptionTranslator, dataAccessStrategy, namedParameters, builder);
 	}

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

@@ -70,11 +70,11 @@ public TransactionalDatabaseClient.Builder dataAccessStrategy(ReactiveDataAccess
 	}
 
 	/* (non-Javadoc)
-	 * @see org.springframework.data.r2dbc.function.DefaultDatabaseClientBuilder#dataAccessStrategy(org.springframework.data.r2dbc.function.NamedParameterSupport)
+	 * @see org.springframework.data.r2dbc.function.DefaultDatabaseClientBuilder#dataAccessStrategy(boolean)
 	 */
 	@Override
-	public TransactionalDatabaseClient.Builder namedParameters(NamedParameterExpander expander) {
-		super.namedParameters(expander);
+	public TransactionalDatabaseClient.Builder namedParameters(boolean enabled) {
+		super.namedParameters(enabled);
 		return this;
 	}
 
@@ -97,7 +97,7 @@ public TransactionalDatabaseClient build() {
 
 	@Override
 	protected DatabaseClient doBuild(ConnectionFactory connector, R2dbcExceptionTranslator exceptionTranslator,
-			ReactiveDataAccessStrategy accessStrategy, NamedParameterExpander namedParameters,
+			ReactiveDataAccessStrategy accessStrategy, boolean namedParameters,
 			DefaultDatabaseClientBuilder builder) {
 		return new DefaultTransactionalDatabaseClient(connector, exceptionTranslator, accessStrategy, namedParameters,
 				builder);