From 8828ebb11d9f7bb9383316e7d2c9731ea8b8e629 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 17 Feb 2020 12:48:47 +0100 Subject: [PATCH] #305 - Consistently apply registered converters. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now apply registered write converters to bindable values that are bound via bind(…) or provided through the Criteria/Update API. --- .../r2dbc/convert/MappingR2dbcConverter.java | 14 +++ .../data/r2dbc/convert/R2dbcConverter.java | 10 ++ .../r2dbc/core/DefaultDatabaseClient.java | 23 ++-- .../DefaultReactiveDataAccessStrategy.java | 9 ++ .../core/ReactiveDataAccessStrategy.java | 9 ++ .../data/r2dbc/query/QueryMapper.java | 23 +++- .../MySqlDatabaseClientIntegrationTests.java | 89 ++++++++++++++ .../core/ReactiveDataAccessStrategyTests.java | 116 ++++++++++++++++++ 8 files changed, 280 insertions(+), 13 deletions(-) create mode 100644 src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java diff --git a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java index 7f92d928..857f244f 100644 --- a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java +++ b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java @@ -522,6 +522,20 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R return value; } + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.convert.R2dbcConverter#getTargetType(Class) + */ + @Override + public Class getTargetType(Class valueType) { + + Optional> writeTarget = getConversions().getCustomWriteTarget(valueType); + + return writeTarget.orElseGet(() -> { + return Enum.class.isAssignableFrom(valueType) ? String.class : valueType; + }); + } + // ---------------------------------- // Id handling // ---------------------------------- diff --git a/src/main/java/org/springframework/data/r2dbc/convert/R2dbcConverter.java b/src/main/java/org/springframework/data/r2dbc/convert/R2dbcConverter.java index 0637ccf6..d6ab1a2b 100644 --- a/src/main/java/org/springframework/data/r2dbc/convert/R2dbcConverter.java +++ b/src/main/java/org/springframework/data/r2dbc/convert/R2dbcConverter.java @@ -63,6 +63,15 @@ public interface R2dbcConverter */ Object getArrayValue(ArrayColumns arrayColumns, RelationalPersistentProperty property, Object value); + /** + * Return the target type for a value considering registered converters. + * + * @param valueType must not be {@literal null}. + * @return + * @since 1.1 + */ + Class getTargetType(Class valueType); + /** * Returns a {@link java.util.function.Function} that populates the id property of the {@code object} from a * {@link Row}. @@ -81,4 +90,5 @@ public interface R2dbcConverter * @return */ R read(Class type, Row source, RowMetadata metadata); + } diff --git a/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java b/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java index 1e332bcf..7dc37bfe 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java @@ -292,26 +292,29 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { return new DefaultGenericExecuteSpec(sqlSupplier); } - private static void bindByName(Statement statement, Map byName) { + private void bindByName(Statement statement, Map byName) { byName.forEach((name, o) -> { - if (o.getValue() != null) { - statement.bind(name, o.getValue()); + SettableValue converted = dataAccessStrategy.getBindValue(o); + if (converted.getValue() != null) { + + statement.bind(name, converted.getValue()); } else { - statement.bindNull(name, o.getType()); + statement.bindNull(name, converted.getType()); } }); } - private static void bindByIndex(Statement statement, Map byIndex) { + private void bindByIndex(Statement statement, Map byIndex) { byIndex.forEach((i, o) -> { - if (o.getValue() != null) { - statement.bind(i, o.getValue()); + SettableValue converted = dataAccessStrategy.getBindValue(o); + if (converted.getValue() != null) { + statement.bind(i, converted.getValue()); } else { - statement.bindNull(i, o.getType()); + statement.bindNull(i, converted.getType()); } }); } @@ -366,12 +369,12 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { if (byName.containsKey(name)) { remainderByName.remove(name); - return byName.get(name); + return dataAccessStrategy.getBindValue(byName.get(name)); } if (byIndex.containsKey(index)) { remainderByIndex.remove(index); - return byIndex.get(index); + return dataAccessStrategy.getBindValue(byIndex.get(index)); } return null; diff --git a/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java b/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java index f21439b2..ed3628ec 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java @@ -270,6 +270,15 @@ public class DefaultReactiveDataAccessStrategy implements ReactiveDataAccessStra actualType); } + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getBindValue(SettableValue) + */ + @Override + public SettableValue getBindValue(SettableValue value) { + return this.updateMapper.getBindValue(value); + } + /* * (non-Javadoc) * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getRowMapper(java.lang.Class) diff --git a/src/main/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategy.java b/src/main/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategy.java index a8a9df9a..8c65fa54 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategy.java @@ -58,6 +58,15 @@ public interface ReactiveDataAccessStrategy { */ OutboundRow getOutboundRow(Object object); + /** + * Return a potentially converted {@link SettableValue} for strategies that support type conversion. + * + * @param value must not be {@literal null}. + * @return + * @since 1.1 + */ + SettableValue getBindValue(SettableValue value); + /** * Returns a {@link BiFunction row mapping function} to map {@link Row rows} to {@code T}. * diff --git a/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java b/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java index 8dac3094..8170646f 100644 --- a/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java @@ -252,6 +252,21 @@ public class QueryMapper { return createCondition(column, mappedValue, typeHint, bindings, criteria.getComparator()); } + /** + * Potentially convert the {@link SettableValue}. + * + * @param value + * @return + */ + public SettableValue getBindValue(SettableValue value) { + + if (value.isEmpty()) { + return SettableValue.empty(converter.getTargetType(value.getType())); + } + + return SettableValue.from(convertValue(value.getValue(), ClassTypeInformation.OBJECT)); + } + @Nullable protected Object convertValue(@Nullable Object value, TypeInformation typeInformation) { @@ -264,13 +279,15 @@ public class QueryMapper { List mapped = new ArrayList<>(); for (Object o : (Iterable) value) { - mapped.add(this.converter.writeValue(o, typeInformation.getActualType())); + mapped.add(convertValue(o, typeInformation.getActualType() != null ? typeInformation.getRequiredActualType() + : ClassTypeInformation.OBJECT)); } + return mapped; } - if (typeInformation.getType().isAssignableFrom(value.getClass()) - || (typeInformation.getType().isArray() && value.getClass().isArray())) { + if (value.getClass().isArray() + && (ClassTypeInformation.OBJECT.equals(typeInformation) || typeInformation.isCollectionLike())) { return value; } diff --git a/src/test/java/org/springframework/data/r2dbc/core/MySqlDatabaseClientIntegrationTests.java b/src/test/java/org/springframework/data/r2dbc/core/MySqlDatabaseClientIntegrationTests.java index 463c143d..7c582171 100644 --- a/src/test/java/org/springframework/data/r2dbc/core/MySqlDatabaseClientIntegrationTests.java +++ b/src/test/java/org/springframework/data/r2dbc/core/MySqlDatabaseClientIntegrationTests.java @@ -21,13 +21,23 @@ import io.r2dbc.spi.ConnectionFactory; import lombok.Data; import reactor.test.StepVerifier; +import java.util.Arrays; +import java.util.Collections; +import java.util.UUID; + import javax.sql.DataSource; import org.junit.ClassRule; import org.junit.Ignore; import org.junit.Test; +import org.springframework.core.convert.converter.Converter; import org.springframework.dao.DataAccessException; +import org.springframework.data.annotation.Id; +import org.springframework.data.convert.ReadingConverter; +import org.springframework.data.convert.WritingConverter; +import org.springframework.data.r2dbc.dialect.MySqlDialect; +import org.springframework.data.r2dbc.query.Criteria; import org.springframework.data.r2dbc.testing.ExternalDatabase; import org.springframework.data.r2dbc.testing.MySqlTestSupport; import org.springframework.data.relational.core.mapping.Table; @@ -84,6 +94,58 @@ public class MySqlDatabaseClientIntegrationTests extends AbstractDatabaseClientI .verifyComplete(); } + @Test // gh-305 + public void shouldApplyCustomConverters() { + + ConnectionFactory connectionFactory = createConnectionFactory(); + JdbcTemplate jdbc = createJdbcTemplate(createDataSource()); + ReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(MySqlDialect.INSTANCE, + Arrays.asList(UuidToStringConverter.INSTANCE, StringToUuidConverter.INSTANCE)); + + try { + jdbc.execute("DROP TABLE uuid_type"); + } catch (DataAccessException e) {} + jdbc.execute("CREATE TABLE uuid_type (id varchar(255), uuid_value varchar(255))"); + + UuidType uuidType = new UuidType(); + uuidType.setId(UUID.randomUUID()); + uuidType.setUuidValue(UUID.randomUUID()); + + DatabaseClient databaseClient = DatabaseClient.builder().connectionFactory(connectionFactory) + .dataAccessStrategy(strategy).build(); + + databaseClient.insert().into(UuidType.class).using(uuidType).then() // + .as(StepVerifier::create) // + .verifyComplete(); + + databaseClient.select().from(UuidType.class).matching(Criteria.where("id").is(uuidType.getId())) // + .fetch().first() // + .as(StepVerifier::create) // + .consumeNextWith(actual -> assertThat(actual.getUuidValue()).isEqualTo(uuidType.getUuidValue())) // + .verifyComplete(); + + uuidType.setUuidValue(null); + databaseClient.update().table(UuidType.class).using(uuidType).then() // + .as(StepVerifier::create) // + .verifyComplete(); + + databaseClient.execute("SELECT * FROM uuid_type WHERE id = ?") // + .bind(0, uuidType.getId()) // + .as(UuidType.class) // + .fetch().first() // + .as(StepVerifier::create) // + .consumeNextWith(actual -> assertThat(actual.getUuidValue()).isNull()) // + .verifyComplete(); + + databaseClient.execute("SELECT * FROM uuid_type WHERE id in (:ids)") // + .bind("ids", Collections.singleton(uuidType.getId())) // + .as(UuidType.class) // + .fetch().first() // + .as(StepVerifier::create) // + .consumeNextWith(actual -> assertThat(actual.getUuidValue()).isNull()) // + .verifyComplete(); + } + @Ignore("https://github.com/mirromutth/r2dbc-mysql/issues/62") @Test @Override @@ -96,7 +158,34 @@ public class MySqlDatabaseClientIntegrationTests extends AbstractDatabaseClientI int id; boolean flag1; boolean flag2; + } + @Table("uuid_type") + @Data + static class UuidType { + + @Id UUID id; + UUID uuidValue; + } + + @WritingConverter + enum UuidToStringConverter implements Converter { + INSTANCE; + + @Override + public String convert(UUID uuid) { + return uuid.toString(); + } + } + + @ReadingConverter + enum StringToUuidConverter implements Converter { + INSTANCE; + + @Override + public UUID convert(String value) { + return UUID.fromString(value); + } } } diff --git a/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java b/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java new file mode 100644 index 00000000..bd819d0f --- /dev/null +++ b/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java @@ -0,0 +1,116 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.r2dbc.core; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.util.Arrays; +import java.util.UUID; + +import org.junit.Test; + +import org.springframework.core.convert.converter.Converter; +import org.springframework.data.convert.ReadingConverter; +import org.springframework.data.convert.WritingConverter; +import org.springframework.data.r2dbc.dialect.BindTarget; +import org.springframework.data.r2dbc.dialect.MySqlDialect; +import org.springframework.data.r2dbc.mapping.SettableValue; +import org.springframework.data.r2dbc.query.Criteria; +import org.springframework.data.r2dbc.query.Update; + +/** + * Unit tests for {@link ReactiveDataAccessStrategy}. + * + * @author Mark Paluch + */ +public class ReactiveDataAccessStrategyTests { + + BindTarget bindTarget = mock(BindTarget.class); + + ReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(MySqlDialect.INSTANCE, + Arrays.asList(UuidToStringConverter.INSTANCE, StringToUuidConverter.INSTANCE)); + + @Test // gh-305 + public void shouldConvertSettableValue() { + + UUID value = UUID.randomUUID(); + + assertThat(strategy.getBindValue(SettableValue.from(value))).isEqualTo(SettableValue.from(value.toString())); + assertThat(strategy.getBindValue(SettableValue.from(Condition.New))).isEqualTo(SettableValue.from("New")); + } + + @Test // gh-305 + public void shouldConvertEmptySettableValue() { + + assertThat(strategy.getBindValue(SettableValue.empty(UUID.class))).isEqualTo(SettableValue.empty(String.class)); + assertThat(strategy.getBindValue(SettableValue.empty(Condition.class))) + .isEqualTo(SettableValue.empty(String.class)); + } + + @Test // gh-305 + public void shouldConvertCriteria() { + + UUID value = UUID.randomUUID(); + + StatementMapper mapper = strategy.getStatementMapper(); + StatementMapper.SelectSpec spec = mapper.createSelect("foo").withProjection("*") + .withCriteria(Criteria.where("id").is(value)); + + PreparedOperation mappedObject = mapper.getMappedObject(spec); + mappedObject.bindTo(bindTarget); + + verify(bindTarget).bind(0, value.toString()); + } + + @Test // gh-305 + public void shouldConvertAssignment() { + + UUID value = UUID.randomUUID(); + + StatementMapper mapper = strategy.getStatementMapper(); + StatementMapper.UpdateSpec update = mapper.createUpdate("foo", Update.update("id", value)); + + PreparedOperation mappedObject = mapper.getMappedObject(update); + mappedObject.bindTo(bindTarget); + + verify(bindTarget).bind(0, value.toString()); + } + + @WritingConverter + enum UuidToStringConverter implements Converter { + INSTANCE; + + @Override + public String convert(UUID uuid) { + return uuid.toString(); + } + } + + @ReadingConverter + enum StringToUuidConverter implements Converter { + INSTANCE; + + @Override + public UUID convert(String value) { + return UUID.fromString(value); + } + } + + enum Condition { + New, Used + } +}