#61 - Move Conversion-related functionality to MappingR2dbcConverter.

All conversion functionality is now pulled together into MappingR2dbcConverter.

Introduce OutboundRow to provide mapping between column names and settable values. Remove identifier from SettableValue.

Original pull request: #62.

Author: mp911de

src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java

@@ -30,6 +30,7 @@
 import org.springframework.data.r2dbc.function.DatabaseClient;
 import org.springframework.data.r2dbc.function.DefaultReactiveDataAccessStrategy;
 import org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy;
+import org.springframework.data.r2dbc.function.convert.MappingR2dbcConverter;
 import org.springframework.data.r2dbc.function.convert.R2dbcCustomConversions;
 import org.springframework.data.r2dbc.support.R2dbcExceptionTranslator;
 import org.springframework.data.r2dbc.support.SqlErrorCodeR2dbcExceptionTranslator;
@@ -118,8 +119,8 @@ public RelationalMappingContext r2dbcMappingContext(Optional<NamingStrategy> nam
 	}
 
 	/**
-	 * Creates a {@link ReactiveDataAccessStrategy} using the configured {@link #r2dbcMappingContext(Optional, R2dbcCustomConversions)}
-	 * RelationalMappingContext}.
+	 * Creates a {@link ReactiveDataAccessStrategy} using the configured
+	 * {@link #r2dbcMappingContext(Optional, R2dbcCustomConversions)} RelationalMappingContext}.
 	 *
 	 * @param mappingContext the configured {@link RelationalMappingContext}.
 	 * @param r2dbcCustomConversions customized R2DBC conversions.
@@ -134,7 +135,7 @@ public ReactiveDataAccessStrategy reactiveDataAccessStrategy(RelationalMappingCo
 
 		Assert.notNull(mappingContext, "MappingContext must not be null!");
 
-		BasicRelationalConverter converter = new BasicRelationalConverter(mappingContext, r2dbcCustomConversions);
+		MappingR2dbcConverter converter = new MappingR2dbcConverter(mappingContext, r2dbcCustomConversions);
 
 		return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), converter);
 	}

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

@@ -45,12 +45,12 @@ public interface BindableOperation extends QueryOperation {
 	 * @see Statement#bind
 	 * @see Statement#bindNull
 	 */
-	default void bind(Statement statement, SettableValue value) {
+	default void bind(Statement statement, String identifier, SettableValue value) {
 
 		if (value.getValue() == null) {
-			bindNull(statement, value.getIdentifier().toString(), value.getType());
+			bindNull(statement, identifier, value.getType());
 		} else {
-			bind(statement, value.getIdentifier().toString(), value.getValue());
+			bind(statement, identifier, value.getValue());
 		}
 	}
 

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

@@ -47,12 +47,14 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.reactivestreams.Publisher;
+
 import org.springframework.dao.DataAccessException;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.r2dbc.UncategorizedR2dbcException;
 import org.springframework.data.r2dbc.function.connectionfactory.ConnectionProxy;
 import org.springframework.data.r2dbc.function.convert.ColumnMapRowMapper;
+import org.springframework.data.r2dbc.function.convert.OutboundRow;
 import org.springframework.data.r2dbc.function.convert.SettableValue;
 import org.springframework.data.r2dbc.support.R2dbcExceptionTranslator;
 import org.springframework.jdbc.core.SqlProvider;
@@ -365,26 +367,30 @@ <T> FetchSpec<T> exchange(String sql, BiFunction<Row, RowMetadata, T> mappingFun
 
 		public ExecuteSpecSupport bind(int index, Object value) {
 
+			Assert.notNull(value, () -> String.format("Value at index %d must not be null. Use bindNull(…) instead.", index));
+
 			Map<Integer, SettableValue> byIndex = new LinkedHashMap<>(this.byIndex);
-			byIndex.put(index, new SettableValue(index, value, null));
+			byIndex.put(index, new SettableValue(value, value.getClass()));
 
 			return createInstance(byIndex, this.byName, this.sqlSupplier);
 		}
 
 		public ExecuteSpecSupport bindNull(int index, Class<?> type) {
 
 			Map<Integer, SettableValue> byIndex = new LinkedHashMap<>(this.byIndex);
-			byIndex.put(index, new SettableValue(index, null, type));
+			byIndex.put(index, new SettableValue(null, type));
 
 			return createInstance(byIndex, this.byName, this.sqlSupplier);
 		}
 
 		public ExecuteSpecSupport bind(String name, Object value) {
 
 			Assert.hasText(name, "Parameter name must not be null or empty!");
+			Assert.notNull(value,
+					() -> String.format("Value for parameter %s must not be null. Use bindNull(…) instead.", name));
 
 			Map<String, SettableValue> byName = new LinkedHashMap<>(this.byName);
-			byName.put(name, new SettableValue(name, value, null));
+			byName.put(name, new SettableValue(value, value.getClass()));
 
 			return createInstance(this.byIndex, byName, this.sqlSupplier);
 		}
@@ -394,7 +400,7 @@ public ExecuteSpecSupport bindNull(String name, Class<?> type) {
 			Assert.hasText(name, "Parameter name must not be null or empty!");
 
 			Map<String, SettableValue> byName = new LinkedHashMap<>(this.byName);
-			byName.put(name, new SettableValue(name, null, type));
+			byName.put(name, new SettableValue(null, type));
 
 			return createInstance(this.byIndex, byName, this.sqlSupplier);
 		}
@@ -832,9 +838,11 @@ class DefaultGenericInsertSpec<T> implements GenericInsertSpec<T> {
 		public GenericInsertSpec value(String field, Object value) {
 
 			Assert.notNull(field, "Field must not be null!");
+			Assert.notNull(value,
+					() -> String.format("Value for field %s must not be null. Use nullValue(…) instead.", field));
 
 			Map<String, SettableValue> byName = new LinkedHashMap<>(this.byName);
-			byName.put(field, new SettableValue(field, value, null));
+			byName.put(field, new SettableValue(value, value.getClass()));
 
 			return new DefaultGenericInsertSpec<>(this.table, byName, this.mappingFunction);
 		}
@@ -845,7 +853,7 @@ public GenericInsertSpec nullValue(String field, Class<?> type) {
 			Assert.notNull(field, "Field must not be null!");
 
 			Map<String, SettableValue> byName = new LinkedHashMap<>(this.byName);
-			byName.put(field, new SettableValue(field, null, type));
+			byName.put(field, new SettableValue(null, type));
 
 			return new DefaultGenericInsertSpec<>(this.table, byName, this.mappingFunction);
 		}
@@ -885,7 +893,7 @@ private <R> FetchSpec<R> exchange(BiFunction<Row, RowMetadata, R> mappingFunctio
 
 				Statement statement = it.createStatement(sql).returnGeneratedValues();
 
-				byName.forEach((k, v) -> bindableInsert.bind(statement, v));
+				byName.forEach((k, v) -> bindableInsert.bind(statement, k, v));
 
 				return statement;
 			};
@@ -989,12 +997,16 @@ public Mono<Integer> rowsUpdated() {
 
 		private <MR> FetchSpec<MR> exchange(Object toInsert, BiFunction<Row, RowMetadata, MR> mappingFunction) {
 
-			List<SettableValue> insertValues = dataAccessStrategy.getValuesToInsert(toInsert);
+			OutboundRow outboundRow = dataAccessStrategy.getOutboundRow(toInsert);
+
 			Set<String> columns = new LinkedHashSet<>();
 
-			for (SettableValue insertValue : insertValues) {
-				columns.add(insertValue.getIdentifier().toString());
-			}
+			outboundRow.forEach((k, v) -> {
+
+				if (v.hasValue()) {
+					columns.add(k);
+				}
+			});
 
 			BindableOperation bindableInsert = dataAccessStrategy.insertAndReturnGeneratedKeys(table, columns);
 
@@ -1008,9 +1020,11 @@ private <MR> FetchSpec<MR> exchange(Object toInsert, BiFunction<Row, RowMetadata
 
 				Statement statement = it.createStatement(sql).returnGeneratedValues();
 
-				for (SettableValue settable : insertValues) {
-					bindableInsert.bind(statement, settable);
-				}
+				outboundRow.forEach((k, v) -> {
+					if (v.hasValue()) {
+						bindableInsert.bind(statement, k, v);
+					}
+				});
 
 				return statement;
 			};