From 5df9b355eb5d846a746be346604cf8954e95b3cb Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 20 Nov 2018 12:20:36 +0100 Subject: [PATCH] #15 - Incorporate review feedback. Improve Javadoc. Rename parameters for more expressiveness. Replace ASCII filtering with filter Function to move actual filtering into dialects. Original pull request: #19. --- .../data/r2dbc/dialect/BindMarkers.java | 6 +- .../r2dbc/dialect/BindMarkersFactory.java | 63 +++++++++++++++---- .../data/r2dbc/dialect/NamedBindMarkers.java | 50 ++++++--------- .../dialect/IndexedBindMarkersUnitTests.java | 15 +++-- .../dialect/NamedBindMarkersUnitTests.java | 41 +++++++++--- 5 files changed, 114 insertions(+), 61 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkers.java b/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkers.java index 63bb95f49..3d56ed40b 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkers.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkers.java @@ -24,13 +24,13 @@ public interface BindMarkers { BindMarker next(); /** - * Creates a new {@link BindMarker} that accepts a {@code nameHint}. Implementations are allowed to consider/ignore + * Creates a new {@link BindMarker} that accepts a {@code hint}. Implementations are allowed to consider/ignore/filter * the name hint to create more expressive bind markers. * - * @param nameHint an optional name hint. + * @param hint an optional name hint that can be used as part of the bind marker. * @return a new {@link BindMarker}. */ - default BindMarker next(String nameHint) { + default BindMarker next(String hint) { return next(); } } diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkersFactory.java b/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkersFactory.java index 483222551..751d8aeda 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkersFactory.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/BindMarkersFactory.java @@ -1,9 +1,16 @@ package org.springframework.data.r2dbc.dialect; +import java.util.function.Function; + import org.springframework.util.Assert; /** * This class creates new {@link BindMarkers} instances to bind parameter for a specific {@link io.r2dbc.spi.Statement}. + *

+ * Bind markers can be typically represented as placeholder and identifier. Placeholders are used within the query to + * execute so the underlying database system can substitute the placeholder with the actual value. Identifiers are used + * in R2DBC drivers to bind a value to a bind marker. Identifiers are typically a part of an entire bind marker when + * using indexed or named bind markers. * * @author Mark Paluch * @see BindMarkers @@ -20,11 +27,15 @@ public interface BindMarkersFactory { BindMarkers create(); /** - * Create index-based {@link BindMarkers}. + * Create index-based {@link BindMarkers} using indexes to bind parameters. Allow customization of the bind marker + * placeholder {@code prefix} to represent the bind marker as placeholder within the query. * - * @param prefix bind parameter prefix. + * @param prefix bind parameter prefix that is included in {@link BindMarker#getPlaceholder()} but not the actual + * identifier. * @param beginWith the first index to use. * @return a {@link BindMarkersFactory} using {@code prefix} and {@code beginWith}. + * @see io.r2dbc.spi.Statement#bindNull(int, Class) + * @see io.r2dbc.spi.Statement#bind(int, Object) */ static BindMarkersFactory indexed(String prefix, int beginWith) { @@ -33,21 +44,49 @@ public interface BindMarkersFactory { } /** - * Create named {@link BindMarkers}. Named bind markers can support {@link BindMarkers#next(String) name hints}. - * Typically, named markers use name hints. If no namehint is given, named bind markers use a counter to generate - * unique bind markers. + * Create named {@link BindMarkers} using identifiers to bind parameters. Named bind markers can support + * {@link BindMarkers#next(String) name hints}. If no {@link BindMarkers#next(String) hint} is given, named bind + * markers can use a counter or a random value source to generate unique bind markers. + *

+ * Allow customization of the bind marker placeholder {@code prefix} and {@code namePrefix} to represent the bind + * marker as placeholder within the query. + * + * @param prefix bind parameter prefix that is included in {@link BindMarker#getPlaceholder()} but not the actual + * identifier. + * @param namePrefix prefix for bind marker name that is included in {@link BindMarker#getPlaceholder()} and the + * actual identifier. + * @param maxLength maximal length of parameter names when using name hints. + * @return a {@link BindMarkersFactory} using {@code prefix} and {@code beginWith}. + * @see io.r2dbc.spi.Statement#bindNull(Object, Class) + * @see io.r2dbc.spi.Statement#bind(Object, Object) + */ + static BindMarkersFactory named(String prefix, String namePrefix, int maxLength) { + return named(prefix, namePrefix, maxLength, Function.identity()); + } + + /** + * Create named {@link BindMarkers} using identifiers to bind parameters. Named bind markers can support + * {@link BindMarkers#next(String) name hints}. If no {@link BindMarkers#next(String) hint} is given, named bind + * markers can use a counter or a random value source to generate unique bind markers. * - * @param prefix bind parameter prefix. - * @param indexPrefix prefix for bind markers that were created by incrementing a counter to generate a unique bind - * marker. - * @param nameLimit maximal length of parameter names when using name hints. + * @param prefix bind parameter prefix that is included in {@link BindMarker#getPlaceholder()} but not the actual + * identifier. + * @param namePrefix prefix for bind marker name that is included in {@link BindMarker#getPlaceholder()} and the + * actual identifier. + * @param maxLength maximal length of parameter names when using name hints. + * @param hintFilterFunction filter {@link Function} to consider database-specific limitations in bind marker/variable + * names such as ASCII chars only. * @return a {@link BindMarkersFactory} using {@code prefix} and {@code beginWith}. + * @see io.r2dbc.spi.Statement#bindNull(Object, Class) + * @see io.r2dbc.spi.Statement#bind(Object, Object) */ - static BindMarkersFactory named(String prefix, String indexPrefix, int nameLimit) { + static BindMarkersFactory named(String prefix, String namePrefix, int maxLength, + Function hintFilterFunction) { Assert.notNull(prefix, "Prefix must not be null!"); - Assert.notNull(indexPrefix, "Index prefix must not be null!"); + Assert.notNull(namePrefix, "Index prefix must not be null!"); + Assert.notNull(hintFilterFunction, "Hint filter function must not be null!"); - return () -> new NamedBindMarkers(prefix, indexPrefix, nameLimit); + return () -> new NamedBindMarkers(prefix, namePrefix, maxLength, hintFilterFunction); } } diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/NamedBindMarkers.java b/src/main/java/org/springframework/data/r2dbc/dialect/NamedBindMarkers.java index cf42c7795..ff7e57225 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/NamedBindMarkers.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/NamedBindMarkers.java @@ -3,6 +3,7 @@ package org.springframework.data.r2dbc.dialect; import io.r2dbc.spi.Statement; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; +import java.util.function.Function; import org.springframework.util.Assert; @@ -21,17 +22,24 @@ class NamedBindMarkers implements BindMarkers { private final String prefix; - private final String indexPrefix; + private final String namePrefix; private final int nameLimit; - NamedBindMarkers(String prefix, String indexPrefix, int nameLimit) { + private final Function hintFilterFunction; + + NamedBindMarkers(String prefix, String namePrefix, int nameLimit, Function hintFilterFunction) { this.prefix = prefix; - this.indexPrefix = indexPrefix; + this.namePrefix = namePrefix; this.nameLimit = nameLimit; + this.hintFilterFunction = hintFilterFunction; } + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.BindMarkers#next() + */ @Override public BindMarker next() { @@ -40,18 +48,16 @@ class NamedBindMarkers implements BindMarkers { return new NamedBindMarker(prefix + name, name); } + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.BindMarkers#next(java.lang.String) + */ @Override - public BindMarker next(String nameHint) { - - Assert.notNull(nameHint, "Name hint must not be null"); + public BindMarker next(String hint) { - String name = nextName(); - - String filteredNameHint = filter(nameHint); + Assert.notNull(hint, "Name hint must not be null"); - if (!filteredNameHint.isEmpty()) { - name += "_" + filteredNameHint; - } + String name = nextName() + hintFilterFunction.apply(hint); if (name.length() > nameLimit) { name = name.substring(0, nameLimit); @@ -63,25 +69,7 @@ class NamedBindMarkers implements BindMarkers { private String nextName() { int index = COUNTER_INCREMENTER.getAndIncrement(this); - return indexPrefix + index; - } - - private static String filter(CharSequence input) { - - StringBuilder builder = new StringBuilder(); - - for (int i = 0; i < input.length(); i++) { - - char ch = input.charAt(i); - - // ascii letter or digit - if (Character.isLetterOrDigit(ch) && ch < 127) { - builder.append(ch); - } - - } - - return builder.toString(); + return namePrefix + index; } /** diff --git a/src/test/java/org/springframework/data/r2dbc/dialect/IndexedBindMarkersUnitTests.java b/src/test/java/org/springframework/data/r2dbc/dialect/IndexedBindMarkersUnitTests.java index 338a152c2..62e1e82ec 100644 --- a/src/test/java/org/springframework/data/r2dbc/dialect/IndexedBindMarkersUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/dialect/IndexedBindMarkersUnitTests.java @@ -29,13 +29,18 @@ public class IndexedBindMarkersUnitTests { @Test // gh-15 public void nextShouldIncrementBindMarker() { - BindMarkers bindMarkers = BindMarkersFactory.indexed("$", 0).create(); + String[] prefixes = { "$", "?" }; + + for (String prefix : prefixes) { + + BindMarkers bindMarkers = BindMarkersFactory.indexed(prefix, 0).create(); - BindMarker marker1 = bindMarkers.next(); - BindMarker marker2 = bindMarkers.next(); + BindMarker marker1 = bindMarkers.next(); + BindMarker marker2 = bindMarkers.next(); - assertThat(marker1.getPlaceholder()).isEqualTo("$0"); - assertThat(marker2.getPlaceholder()).isEqualTo("$1"); + assertThat(marker1.getPlaceholder()).isEqualTo(prefix + "0"); + assertThat(marker2.getPlaceholder()).isEqualTo(prefix + "1"); + } } @Test // gh-15 diff --git a/src/test/java/org/springframework/data/r2dbc/dialect/NamedBindMarkersUnitTests.java b/src/test/java/org/springframework/data/r2dbc/dialect/NamedBindMarkersUnitTests.java index 7da838574..ca6afcce2 100644 --- a/src/test/java/org/springframework/data/r2dbc/dialect/NamedBindMarkersUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/dialect/NamedBindMarkersUnitTests.java @@ -29,24 +29,46 @@ public class NamedBindMarkersUnitTests { @Test // gh-15 public void nextShouldIncrementBindMarker() { - BindMarkers bindMarkers = BindMarkersFactory.named("@", "p", 32).create(); + String[] prefixes = { "$", "?" }; - BindMarker marker1 = bindMarkers.next(); - BindMarker marker2 = bindMarkers.next(); + for (String prefix : prefixes) { - assertThat(marker1.getPlaceholder()).isEqualTo("@p0"); - assertThat(marker2.getPlaceholder()).isEqualTo("@p1"); + BindMarkers bindMarkers = BindMarkersFactory.named(prefix, "p", 32).create(); + + BindMarker marker1 = bindMarkers.next(); + BindMarker marker2 = bindMarkers.next(); + + assertThat(marker1.getPlaceholder()).isEqualTo(prefix + "p0"); + assertThat(marker2.getPlaceholder()).isEqualTo(prefix + "p1"); + } } @Test // gh-15 public void nextShouldConsiderNameHint() { - BindMarkers bindMarkers = BindMarkersFactory.named("@", "p", 32).create(); + BindMarkers bindMarkers = BindMarkersFactory.named("@", "x", 32).create(); + + BindMarker marker1 = bindMarkers.next("foo1bar"); + BindMarker marker2 = bindMarkers.next(); - BindMarker marker1 = bindMarkers.next("foo.bar?"); + assertThat(marker1.getPlaceholder()).isEqualTo("@x0foo1bar"); + assertThat(marker2.getPlaceholder()).isEqualTo("@x1"); + } + + @Test // gh-15 + public void nextShouldConsiderFilteredNameHint() { + + BindMarkers bindMarkers = BindMarkersFactory.named("@", "p", 32, s -> { + + return s.chars().filter(Character::isAlphabetic) + .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append).toString(); + + }).create(); + + BindMarker marker1 = bindMarkers.next("foo1.bar?"); BindMarker marker2 = bindMarkers.next(); - assertThat(marker1.getPlaceholder()).isEqualTo("@p0_foobar"); + assertThat(marker1.getPlaceholder()).isEqualTo("@p0foobar"); assertThat(marker2.getPlaceholder()).isEqualTo("@p1"); } @@ -57,7 +79,7 @@ public class NamedBindMarkersUnitTests { BindMarker marker1 = bindMarkers.next("123456789"); - assertThat(marker1.getPlaceholder()).isEqualTo("@p0_1234567"); + assertThat(marker1.getPlaceholder()).isEqualTo("@p012345678"); } @Test // gh-15 @@ -82,7 +104,6 @@ public class NamedBindMarkersUnitTests { BindMarkers bindMarkers = BindMarkersFactory.named("@", "p", 32).create(); bindMarkers.next(); // ignore - bindMarkers.next().bindNull(statement, Integer.class); verify(statement).bindNull("p1", Integer.class);