From a235e8a071fe564bb7fbf6f43c9b8eb05332b81d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 14 Dec 2021 14:45:22 +0100 Subject: [PATCH] Unwrap Parameter objects when used within PreparedOperation. We now unwrap Parameter objects containing type and value for bind parameters when binding these from within a PreparedOperation to a statement. PreparedOperation objects are expected to use low-level R2DBC API (bind, bindNull) instead of using the Parameter abstraction. Previously, we tried to bind Parameter objects to R2DBC Statements and that has failed as drivers cannot encode Spring's Pararameter type. Closes #694 --- .../data/r2dbc/dialect/BindTargetBinder.java | 64 +++++++++++++++++++ .../ExpressionEvaluatingParameterBinder.java | 29 ++------- .../query/StringBasedR2dbcQuery.java | 6 +- ...stgresR2dbcRepositoryIntegrationTests.java | 2 +- .../query/StringBasedR2dbcQueryUnitTests.java | 7 +- 5 files changed, 78 insertions(+), 30 deletions(-) create mode 100644 src/main/java/org/springframework/data/r2dbc/dialect/BindTargetBinder.java diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/BindTargetBinder.java b/src/main/java/org/springframework/data/r2dbc/dialect/BindTargetBinder.java new file mode 100644 index 0000000..d8b076a --- /dev/null +++ b/src/main/java/org/springframework/data/r2dbc/dialect/BindTargetBinder.java @@ -0,0 +1,64 @@ +/* + * Copyright 2021 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.dialect; + +import org.springframework.r2dbc.core.Parameter; +import org.springframework.r2dbc.core.binding.BindTarget; + +/** + * Utility to bind {@link Parameter} to a {@link BindTarget}. Mainly used within the framework. + * + * @author Mark Paluch + * @since 1.4.1 + */ +public final class BindTargetBinder { + + private final BindTarget target; + + public BindTargetBinder(BindTarget target) { + this.target = target; + } + + /** + * Bind a {@link Parameter} by name. + * + * @param name must not be {@literal null}. + * @param parameter must not be {@literal null}. + */ + public void bind(String name, Parameter parameter) { + Object value = parameter.getValue(); + if (value == null) { + target.bindNull(name, parameter.getType()); + } else { + target.bind(name, value); + } + } + + /** + * Bind a {@link Parameter} by index. + * + * @param index must not be {@literal null}. + * @param parameter must not be {@literal null}. + */ + public void bind(int index, Parameter parameter) { + Object value = parameter.getValue(); + if (value == null) { + target.bindNull(index, parameter.getType()); + } else { + target.bind(index, parameter.getValue()); + } + } +} diff --git a/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java b/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java index 1df069d..f895326 100644 --- a/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java +++ b/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java @@ -23,6 +23,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; import org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy; +import org.springframework.data.r2dbc.dialect.BindTargetBinder; import org.springframework.data.relational.repository.query.RelationalParameterAccessor; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.Parameters; @@ -75,12 +76,13 @@ class ExpressionEvaluatingParameterBinder { private void bindExpressions(BindTarget bindSpec, R2dbcSpELExpressionEvaluator evaluator) { + BindTargetBinder binder = new BindTargetBinder(bindSpec); for (ParameterBinding binding : expressionQuery.getBindings()) { org.springframework.r2dbc.core.Parameter valueForBinding = getBindValue( evaluator.evaluate(binding.getExpression())); - bind(bindSpec, binding.getParameterName(), valueForBinding); + binder.bind(binding.getParameterName(), valueForBinding); } } @@ -89,6 +91,7 @@ class ExpressionEvaluatingParameterBinder { int bindingIndex = 0; + BindTargetBinder binder = new BindTargetBinder(bindSpec); for (Parameter bindableParameter : bindableParameters) { Optional name = bindableParameter.getName(); @@ -102,7 +105,7 @@ class ExpressionEvaluatingParameterBinder { org.springframework.r2dbc.core.Parameter parameter = getBindValue(values, bindableParameter); if (!parameter.isEmpty() || hasBindableNullValue) { - bind(bindSpec, name.get(), parameter); + binder.bind(name.get(), parameter); } // skip unused named parameters if there is SpEL @@ -111,7 +114,7 @@ class ExpressionEvaluatingParameterBinder { org.springframework.r2dbc.core.Parameter parameter = getBindValue(values, bindableParameter); if (!parameter.isEmpty() || hasBindableNullValue) { - bind(bindSpec, bindingIndex++, parameter); + binder.bind(bindingIndex++, parameter); } } } @@ -125,27 +128,7 @@ class ExpressionEvaluatingParameterBinder { return dataAccessStrategy.getBindValue(parameter); } - private static void bind(BindTarget spec, String name, - org.springframework.r2dbc.core.Parameter parameter) { - Object value = parameter.getValue(); - if (value == null) { - spec.bindNull(name, parameter.getType()); - } else { - spec.bind(name, value); - } - } - - private static void bind(BindTarget spec, int index, - org.springframework.r2dbc.core.Parameter parameter) { - - Object value = parameter.getValue(); - if (value == null) { - spec.bindNull(index, parameter.getType()); - } else { - spec.bind(index, value); - } - } private org.springframework.r2dbc.core.Parameter getBindValue(org.springframework.r2dbc.core.Parameter bindValue) { return dataAccessStrategy.getBindValue(bindValue); diff --git a/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java b/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java index bc5fd30..7548326 100644 --- a/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java +++ b/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java @@ -27,6 +27,7 @@ import org.jetbrains.annotations.NotNull; import org.springframework.data.r2dbc.convert.R2dbcConverter; import org.springframework.data.r2dbc.core.R2dbcEntityOperations; import org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy; +import org.springframework.data.r2dbc.dialect.BindTargetBinder; import org.springframework.data.r2dbc.mapping.SettableValue; import org.springframework.data.r2dbc.repository.Query; import org.springframework.data.relational.repository.query.RelationalParameterAccessor; @@ -221,10 +222,11 @@ public class StringBasedR2dbcQuery extends AbstractR2dbcQuery { @Override public void bindTo(BindTarget target) { + BindTargetBinder binder = new BindTargetBinder(target); expanded.bindTo(target); - remainderByName.forEach(target::bind); - remainderByIndex.forEach(target::bind); + remainderByName.forEach(binder::bind); + remainderByIndex.forEach(binder::bind); } @Override diff --git a/src/test/java/org/springframework/data/r2dbc/repository/PostgresR2dbcRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/r2dbc/repository/PostgresR2dbcRepositoryIntegrationTests.java index e7b40d8..8991a5b 100644 --- a/src/test/java/org/springframework/data/r2dbc/repository/PostgresR2dbcRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/r2dbc/repository/PostgresR2dbcRepositoryIntegrationTests.java @@ -126,7 +126,7 @@ public class PostgresR2dbcRepositoryIntegrationTests extends AbstractR2dbcReposi Flux findAsProjection(); @Override - @Query("SELECT * FROM legoset WHERE manual = :manual") + @Query("SELECT * FROM legoset WHERE manual = $1") Mono findByManual(int manual); @Override diff --git a/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java b/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java index dbf7217..f55e90f 100644 --- a/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java @@ -56,7 +56,6 @@ import org.springframework.data.repository.query.Param; import org.springframework.data.repository.query.ReactiveQueryMethodEvaluationContextProvider; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.r2dbc.core.DatabaseClient; -import org.springframework.r2dbc.core.Parameter; import org.springframework.r2dbc.core.PreparedOperation; import org.springframework.r2dbc.core.binding.BindTarget; import org.springframework.util.ReflectionUtils; @@ -102,7 +101,7 @@ public class StringBasedR2dbcQueryUnitTests { assertThat(stringQuery.get()).isEqualTo("SELECT * FROM person WHERE lastname = $1"); stringQuery.bindTo(bindTarget); - verify(bindTarget).bind(0, Parameter.from("White")); + verify(bindTarget).bind(0, "White"); } @Test // gh-164 @@ -116,7 +115,7 @@ public class StringBasedR2dbcQueryUnitTests { assertThat(stringQuery.get()).isEqualTo("SELECT * FROM person WHERE lastname = $1"); stringQuery.bindTo(bindTarget); - verify(bindTarget).bind(0, Parameter.from("White")); + verify(bindTarget).bind(0, "White"); } @Test @@ -144,7 +143,7 @@ public class StringBasedR2dbcQueryUnitTests { assertThat(stringQuery.get()).isEqualTo("SELECT * FROM person WHERE lastname = @lastname"); stringQuery.bindTo(bindTarget); - verify(bindTarget).bind("lastname", Parameter.from("White")); + verify(bindTarget).bind("lastname", "White"); } @Test