From 30c5e2f5e0d5378007e1fdfec5e7340949c6bc56 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 3 Sep 2019 16:23:48 +0200 Subject: [PATCH] #162 - Refine declaration of nullable values in DatabaseClient. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DatabaseClient.BindSpec.bind(…) (execute) and DatabaseClient.GenericInsertSpec.value(…) (insert) now consistently accept SettableValue for scalar and absent values. This change allows us to provide a Kotlin extension leveraging reified generics to provide the type of a value even if it is null to construct an appropriate SettableValue for fluent API usage. --- .../data/r2dbc/core/DatabaseClient.java | 32 ++----- .../r2dbc/core/DefaultDatabaseClient.java | 63 +++++-------- .../r2dbc/core/DatabaseClientExtensions.kt | 28 +++++- ...bstractDatabaseClientIntegrationTests.java | 4 +- .../core/DefaultDatabaseClientUnitTests.java | 79 +++++++++++++++- .../core/DatabaseClientExtensionsTests.kt | 91 +++++++++++++++++++ 6 files changed, 229 insertions(+), 68 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java b/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java index 1ae343f..4db79cd 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java @@ -480,10 +480,12 @@ public interface DatabaseClient { interface GenericInsertSpec extends InsertSpec { /** - * Specify a field and non-{@literal null} value to insert. + * Specify a field and non-{@literal null} value to insert. {@code value} can be either a scalar value or + * {@link SettableValue}. * * @param field must not be {@literal null} or empty. - * @param value must not be {@literal null} + * @param value the field value to set, must not be {@literal null}. Can be either a scalar value or + * {@link SettableValue}. */ GenericInsertSpec value(String field, Object value); @@ -492,19 +494,10 @@ public interface DatabaseClient { * * @param field must not be {@literal null} or empty. * @param type must not be {@literal null}. - * @deprecated will be removed soon. Use {@link #nullValue(String)}. */ - @Deprecated default GenericInsertSpec nullValue(String field, Class type) { return value(field, SettableValue.empty(type)); } - - /** - * Specify a {@literal null} value to insert. - * - * @param field must not be {@literal null} or empty. - */ - GenericInsertSpec nullValue(String field); } /** @@ -704,10 +697,11 @@ public interface DatabaseClient { interface BindSpec> { /** - * Bind a non-{@literal null} value to a parameter identified by its {@code index}. + * Bind a non-{@literal null} value to a parameter identified by its {@code index}. {@code value} can be either a + * scalar value or {@link SettableValue}. * * @param index zero based index to bind the parameter to. - * @param value to bind. Must not be {@literal null}. + * @param value must not be {@literal null}. Can be either a scalar value or {@link SettableValue}. */ S bind(int index, Object value); @@ -720,10 +714,11 @@ public interface DatabaseClient { S bindNull(int index, Class type); /** - * Bind a non-{@literal null} value to a parameter identified by its {@code name}. + * Bind a non-{@literal null} value to a parameter identified by its {@code name}. {@code value} can be either a + * scalar value or {@link SettableValue}. * * @param name must not be {@literal null} or empty. - * @param value must not be {@literal null}. + * @param value must not be {@literal null}. Can be either a scalar value or {@link SettableValue}. */ S bind(String name, Object value); @@ -734,12 +729,5 @@ public interface DatabaseClient { * @param type must not be {@literal null}. */ S bindNull(String name, Class type); - - /** - * Bind a bean according to Java {@link java.beans.BeanInfo Beans} using property names. - * - * @param bean must not be {@literal null}. - */ - S bind(Object bean); } } 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 0bc6377..2b2ccb6 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java @@ -15,6 +15,16 @@ */ package org.springframework.data.r2dbc.core; +import io.r2dbc.spi.Connection; +import io.r2dbc.spi.ConnectionFactory; +import io.r2dbc.spi.R2dbcException; +import io.r2dbc.spi.Result; +import io.r2dbc.spi.Row; +import io.r2dbc.spi.RowMetadata; +import io.r2dbc.spi.Statement; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -31,18 +41,9 @@ import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; -import io.r2dbc.spi.Connection; -import io.r2dbc.spi.ConnectionFactory; -import io.r2dbc.spi.R2dbcException; -import io.r2dbc.spi.Result; -import io.r2dbc.spi.Row; -import io.r2dbc.spi.RowMetadata; -import io.r2dbc.spi.Statement; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; import org.springframework.dao.DataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; @@ -383,7 +384,12 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { Assert.notNull(value, () -> String.format("Value at index %d must not be null. Use bindNull(…) instead.", index)); Map byIndex = new LinkedHashMap<>(this.byIndex); - byIndex.put(index, SettableValue.fromOrEmpty(value, value.getClass())); + + if (value instanceof SettableValue) { + byIndex.put(index, (SettableValue) value); + } else { + byIndex.put(index, SettableValue.fromOrEmpty(value, value.getClass())); + } return createInstance(byIndex, this.byName, this.sqlSupplier); } @@ -407,7 +413,12 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { () -> String.format("Value for parameter %s must not be null. Use bindNull(…) instead.", name)); Map byName = new LinkedHashMap<>(this.byName); - byName.put(name, SettableValue.fromOrEmpty(value, value.getClass())); + + if (value instanceof SettableValue) { + byName.put(name, (SettableValue) value); + } else { + byName.put(name, SettableValue.fromOrEmpty(value, value.getClass())); + } return createInstance(this.byIndex, byName, this.sqlSupplier); } @@ -433,13 +444,6 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { Supplier sqlSupplier) { return new ExecuteSpecSupport(byIndex, byName, sqlSupplier); } - - public ExecuteSpecSupport bind(Object bean) { - - Assert.notNull(bean, "Bean must not be null!"); - - throw new UnsupportedOperationException("Implement me!"); - } } /** @@ -510,11 +514,6 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { return (DefaultGenericExecuteSpec) super.bindNull(name, type); } - @Override - public DefaultGenericExecuteSpec bind(Object bean) { - return (DefaultGenericExecuteSpec) super.bind(bean); - } - @Override protected ExecuteSpecSupport createInstance(Map byIndex, Map byName, Supplier sqlSupplier) { @@ -603,11 +602,6 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { return (DefaultTypedExecuteSpec) super.bindNull(name, type); } - @Override - public DefaultTypedExecuteSpec bind(Object bean) { - return (DefaultTypedExecuteSpec) super.bind(bean); - } - @Override protected DefaultTypedExecuteSpec createInstance(Map byIndex, Map byName, Supplier sqlSupplier) { @@ -933,8 +927,6 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { 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 byName = new LinkedHashMap<>(this.byName); @@ -947,17 +939,6 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { return new DefaultGenericInsertSpec<>(this.table, byName, this.mappingFunction); } - @Override - public GenericInsertSpec nullValue(String field) { - - Assert.notNull(field, "Field must not be null!"); - - Map byName = new LinkedHashMap<>(this.byName); - byName.put(field, null); - - return new DefaultGenericInsertSpec<>(this.table, byName, this.mappingFunction); - } - @Override public FetchSpec map(Function mappingFunction) { diff --git a/src/main/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensions.kt b/src/main/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensions.kt index 63e12b2..f52c971 100644 --- a/src/main/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensions.kt +++ b/src/main/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensions.kt @@ -16,6 +16,7 @@ package org.springframework.data.r2dbc.core import kotlinx.coroutines.reactive.awaitFirstOrNull +import org.springframework.data.r2dbc.mapping.SettableValue /** * Coroutines variant of [DatabaseClient.GenericExecuteSpec.then]. @@ -27,7 +28,23 @@ suspend fun DatabaseClient.GenericExecuteSpec.await() { } /** - * Extension for [DatabaseClient.GenericExecuteSpec.as] providing a + * Extension for [DatabaseClient.BindSpec.bind] providing a variant leveraging reified type parameters + * + * @author Mark Paluch + */ +@Suppress("EXTENSION_SHADOWED_BY_MEMBER") +inline fun DatabaseClient.BindSpec<*>.bind(index: Int, value: T?) = bind(index, SettableValue.fromOrEmpty(value, T::class.java)) + +/** + * Extension for [DatabaseClient.BindSpec.bind] providing a variant leveraging reified type parameters + * + * @author Mark Paluch + */ +@Suppress("EXTENSION_SHADOWED_BY_MEMBER") +inline fun DatabaseClient.BindSpec<*>.bind(name: String, value: T?) = bind(name, SettableValue.fromOrEmpty(value, T::class.java)) + +/** + * Extension for [DatabaseClient.GenericExecuteSpec. as] providing a * `asType()` variant. * * @author Sebastien Deleuze @@ -80,6 +97,15 @@ suspend fun DatabaseClient.InsertSpec.await() { inline fun DatabaseClient.InsertIntoSpec.into(): DatabaseClient.TypedInsertSpec = into(T::class.java) +/** + * Extension for [DatabaseClient.GenericInsertSpec.value] providing a variant leveraging reified type parameters + * + * @author Mark Paluch + */ +@Suppress("EXTENSION_SHADOWED_BY_MEMBER") +inline fun DatabaseClient.GenericInsertSpec<*>.value(name: String, value: T?) = value(name, SettableValue.fromOrEmpty(value, T::class.java)) + + /** * Extension for [DatabaseClient.SelectFromSpec.from] providing a * `from()` variant. diff --git a/src/test/java/org/springframework/data/r2dbc/core/AbstractDatabaseClientIntegrationTests.java b/src/test/java/org/springframework/data/r2dbc/core/AbstractDatabaseClientIntegrationTests.java index b8a80dc..656aeff 100644 --- a/src/test/java/org/springframework/data/r2dbc/core/AbstractDatabaseClientIntegrationTests.java +++ b/src/test/java/org/springframework/data/r2dbc/core/AbstractDatabaseClientIntegrationTests.java @@ -172,7 +172,7 @@ public abstract class AbstractDatabaseClientIntegrationTests extends R2dbcIntegr databaseClient.insert().into("legoset")// .value("id", 42055) // .value("name", "SCHAUFELRADBAGGER") // - .nullValue("manual") // + .nullValue("manual", Integer.class) // .fetch() // .rowsUpdated() // .as(StepVerifier::create) // @@ -190,7 +190,7 @@ public abstract class AbstractDatabaseClientIntegrationTests extends R2dbcIntegr databaseClient.insert().into("legoset")// .value("id", 42055) // .value("name", "SCHAUFELRADBAGGER") // - .nullValue("manual") // + .nullValue("manual", Integer.class) // .then() // .as(StepVerifier::create) // .verifyComplete(); diff --git a/src/test/java/org/springframework/data/r2dbc/core/DefaultDatabaseClientUnitTests.java b/src/test/java/org/springframework/data/r2dbc/core/DefaultDatabaseClientUnitTests.java index c0dd2a0..97b9c8f 100644 --- a/src/test/java/org/springframework/data/r2dbc/core/DefaultDatabaseClientUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/core/DefaultDatabaseClientUnitTests.java @@ -34,6 +34,7 @@ import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; import org.springframework.data.r2dbc.dialect.PostgresDialect; +import org.springframework.data.r2dbc.mapping.SettableValue; import org.springframework.data.r2dbc.support.R2dbcExceptionTranslator; /** @@ -118,6 +119,34 @@ public class DefaultDatabaseClientUnitTests { verify(statement).bindNull("$1", String.class); } + @Test // gh-162 + public void executeShouldBindSettableValues() { + + Statement statement = mock(Statement.class); + when(connection.createStatement("SELECT * FROM table WHERE key = $1")).thenReturn(statement); + when(statement.execute()).thenReturn(Mono.empty()); + + DefaultDatabaseClient databaseClient = (DefaultDatabaseClient) DatabaseClient.builder() + .connectionFactory(connectionFactory) + .dataAccessStrategy(new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE)).build(); + + databaseClient.execute("SELECT * FROM table WHERE key = $1") // + .bind(0, SettableValue.empty(String.class)) // + .then() // + .as(StepVerifier::create) // + .verifyComplete(); + + verify(statement).bindNull(0, String.class); + + databaseClient.execute("SELECT * FROM table WHERE key = $1") // + .bind("$1", SettableValue.empty(String.class)) // + .then() // + .as(StepVerifier::create) // + .verifyComplete(); + + verify(statement).bindNull("$1", String.class); + } + @Test // gh-128 public void executeShouldBindNamedNullValues() { @@ -138,7 +167,7 @@ public class DefaultDatabaseClientUnitTests { verify(statement).bindNull(0, String.class); } - @Test // gh-128 + @Test // gh-128, gh-162 public void executeShouldBindValues() { Statement statement = mock(Statement.class); @@ -150,7 +179,7 @@ public class DefaultDatabaseClientUnitTests { .dataAccessStrategy(new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE)).build(); databaseClient.execute("SELECT * FROM table WHERE key = $1") // - .bind(0, "foo") // + .bind(0, SettableValue.from("foo")) // .then() // .as(StepVerifier::create) // .verifyComplete(); @@ -166,6 +195,52 @@ public class DefaultDatabaseClientUnitTests { verify(statement).bind("$1", "foo"); } + @Test // gh-162 + public void insertShouldAcceptNullValues() { + + Statement statement = mock(Statement.class); + when(connection.createStatement("INSERT INTO foo (first, second) VALUES ($1, $2)")).thenReturn(statement); + when(statement.returnGeneratedValues()).thenReturn(statement); + when(statement.execute()).thenReturn(Mono.empty()); + + DefaultDatabaseClient databaseClient = (DefaultDatabaseClient) DatabaseClient.builder() + .connectionFactory(connectionFactory) + .dataAccessStrategy(new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE)).build(); + + databaseClient.insert().into("foo") // + .value("first", "foo") // + .nullValue("second", Integer.class) // + .then() // + .as(StepVerifier::create) // + .verifyComplete(); + + verify(statement).bind(0, "foo"); + verify(statement).bindNull(1, Integer.class); + } + + @Test // gh-162 + public void insertShouldAcceptSettableValue() { + + Statement statement = mock(Statement.class); + when(connection.createStatement("INSERT INTO foo (first, second) VALUES ($1, $2)")).thenReturn(statement); + when(statement.returnGeneratedValues()).thenReturn(statement); + when(statement.execute()).thenReturn(Mono.empty()); + + DefaultDatabaseClient databaseClient = (DefaultDatabaseClient) DatabaseClient.builder() + .connectionFactory(connectionFactory) + .dataAccessStrategy(new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE)).build(); + + databaseClient.insert().into("foo") // + .value("first", SettableValue.from("foo")) // + .value("second", SettableValue.empty(Integer.class)) // + .then() // + .as(StepVerifier::create) // + .verifyComplete(); + + verify(statement).bind(0, "foo"); + verify(statement).bindNull(1, Integer.class); + } + @Test // gh-128 public void executeShouldBindNamedValuesByIndex() { diff --git a/src/test/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensionsTests.kt b/src/test/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensionsTests.kt index 6bea790..58003e3 100644 --- a/src/test/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensionsTests.kt +++ b/src/test/kotlin/org/springframework/data/r2dbc/core/DatabaseClientExtensionsTests.kt @@ -21,6 +21,7 @@ import io.mockk.verify import kotlinx.coroutines.runBlocking import org.assertj.core.api.Assertions.assertThat import org.junit.Test +import org.springframework.data.r2dbc.mapping.SettableValue import reactor.core.publisher.Mono /** @@ -32,6 +33,66 @@ import reactor.core.publisher.Mono */ class DatabaseClientExtensionsTests { + @Test // gh-162 + fun bindByIndexShouldBindValue() { + + val spec = mockk() + every { spec.bind(eq(0), any()) } returns spec + + runBlocking { + spec.bind(0, "foo") + } + + verify { + spec.bind(0, SettableValue.fromOrEmpty("foo", String::class.java)) + } + } + + @Test // gh-162 + fun bindByIndexShouldBindNull() { + + val spec = mockk() + every { spec.bind(eq(0), any()) } returns spec + + runBlocking { + spec.bind(0, null) + } + + verify { + spec.bind(0, SettableValue.empty(String::class.java)) + } + } + + @Test // gh-162 + fun bindByNameShouldBindValue() { + + val spec = mockk() + every { spec.bind(eq("field"), any()) } returns spec + + runBlocking { + spec.bind("field", "foo") + } + + verify { + spec.bind("field", SettableValue.fromOrEmpty("foo", String::class.java)) + } + } + + @Test // gh-162 + fun bindByNameShouldBindNull() { + + val spec = mockk() + every { spec.bind(eq("field"), any()) } returns spec + + runBlocking { + spec.bind("field", null) + } + + verify { + spec.bind("field", SettableValue.empty(String::class.java)) + } + } + @Test // gh-63 fun genericExecuteSpecAwait() { @@ -140,6 +201,36 @@ class DatabaseClientExtensionsTests { } } + @Test // gh-162 + fun insertValueShouldBindValue() { + + val spec = mockk>() + every { spec.value(eq("field"), any()) } returns spec + + runBlocking { + spec.value("field", "foo") + } + + verify { + spec.value("field", SettableValue.fromOrEmpty("foo", String::class.java)) + } + } + + @Test // gh-162 + fun insertValueShouldBindNull() { + + val spec = mockk>() + every { spec.value(eq("field"), any()) } returns spec + + runBlocking { + spec.value("field", null) + } + + verify { + spec.value("field", SettableValue.empty(String::class.java)) + } + } + @Test // gh-122 fun selectFromSpecFrom() {