From 7e817acca6449e724a803603ac6d8267f26027ac Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Wed, 19 Feb 2020 12:02:48 +0100
Subject: [PATCH] #310 - Fix bind marker reuse when expanding collection
 arguments using named parameters.

We now correctly reuse bind markers for named parameter substitution when using collection arguments to create dynamic argument lists. Previously, we allocated a new bind marker for each item in the collection which left the parameters intended for reuse unbound.
---
 .../data/r2dbc/core/NamedParameterUtils.java  | 18 ++++---
 .../core/NamedParameterUtilsUnitTests.java    | 48 +++++++++++++++++++
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/springframework/data/r2dbc/core/NamedParameterUtils.java b/src/main/java/org/springframework/data/r2dbc/core/NamedParameterUtils.java
index 19cf9d1..301aecc 100644
--- a/src/main/java/org/springframework/data/r2dbc/core/NamedParameterUtils.java
+++ b/src/main/java/org/springframework/data/r2dbc/core/NamedParameterUtils.java
@@ -31,7 +31,6 @@ import org.springframework.data.r2dbc.dialect.BindMarkers;
 import org.springframework.data.r2dbc.dialect.BindMarkersFactory;
 import org.springframework.data.r2dbc.dialect.BindTarget;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 
 /**
  * Helper methods for named parameter parsing.
@@ -287,6 +286,7 @@ abstract class NamedParameterUtils {
 
 					Iterator<?> entryIter = ((Collection<?>) value).iterator();
 					int k = 0;
+					int counter = 0;
 					while (entryIter.hasNext()) {
 						if (k > 0) {
 							actualSql.append(", ");
@@ -300,11 +300,13 @@ abstract class NamedParameterUtils {
 								if (m > 0) {
 									actualSql.append(", ");
 								}
-								actualSql.append(marker.addPlaceholder());
+								actualSql.append(marker.getPlaceholder(counter));
+								counter++;
 							}
 							actualSql.append(')');
 						} else {
-							actualSql.append(marker.addPlaceholder());
+							actualSql.append(marker.getPlaceholder(counter));
+							counter++;
 						}
 
 					}
@@ -459,12 +461,16 @@ abstract class NamedParameterUtils {
 			}
 
 			String getPlaceholder() {
+				return getPlaceholder(0);
+			}
 
-				if (this.placeholders.isEmpty()) {
-					return addPlaceholder();
+			String getPlaceholder(int counter) {
+
+				while (counter + 1 > this.placeholders.size()) {
+					addPlaceholder();
 				}
 
-				return this.placeholders.get(0).getPlaceholder();
+				return this.placeholders.get(counter).getPlaceholder();
 			}
 		}
 	}
diff --git a/src/test/java/org/springframework/data/r2dbc/core/NamedParameterUtilsUnitTests.java b/src/test/java/org/springframework/data/r2dbc/core/NamedParameterUtilsUnitTests.java
index 387f5f1..84ddb94 100644
--- a/src/test/java/org/springframework/data/r2dbc/core/NamedParameterUtilsUnitTests.java
+++ b/src/test/java/org/springframework/data/r2dbc/core/NamedParameterUtilsUnitTests.java
@@ -31,6 +31,8 @@ import org.springframework.data.r2dbc.dialect.BindTarget;
 import org.springframework.data.r2dbc.dialect.PostgresDialect;
 import org.springframework.data.r2dbc.dialect.SqlServerDialect;
 import org.springframework.data.r2dbc.mapping.SettableValue;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
 
 /**
  * Unit tests for {@link NamedParameterUtils}.
@@ -335,6 +337,52 @@ public class NamedParameterUtilsUnitTests {
 		});
 	}
 
+	@Test // gh-310
+	public void multipleEqualCollectionParameterReferencesBindsValueOnce() {
+
+		String sql = "SELECT * FROM person where name IN (:ids) or lastname IN (:ids)";
+
+		BindMarkersFactory factory = BindMarkersFactory.indexed("$", 0);
+
+		MultiValueMap<Integer, Object> bindings = new LinkedMultiValueMap<>();
+
+		PreparedOperation<String> operation = NamedParameterUtils.substituteNamedParameters(sql, factory,
+				new MapBindParameterSource(
+						Collections.singletonMap("ids", SettableValue.from(Arrays.asList("foo", "bar", "baz")))));
+
+		assertThat(operation.toQuery())
+				.isEqualTo("SELECT * FROM person where name IN ($0, $1, $2) or lastname IN ($0, $1, $2)");
+
+		operation.bindTo(new BindTarget() {
+			@Override
+			public void bind(String identifier, Object value) {
+				throw new UnsupportedOperationException();
+			}
+
+			@Override
+			public void bind(int index, Object value) {
+				assertThat(index).isIn(0, 1, 2);
+				assertThat(value).isIn("foo", "bar", "baz");
+
+				bindings.add(index, value);
+			}
+
+			@Override
+			public void bindNull(String identifier, Class<?> type) {
+				throw new UnsupportedOperationException();
+			}
+
+			@Override
+			public void bindNull(int index, Class<?> type) {
+				throw new UnsupportedOperationException();
+			}
+		});
+
+		assertThat(bindings).containsEntry(0, Collections.singletonList("foo")) //
+				.containsEntry(1, Collections.singletonList("bar")) //
+				.containsEntry(2, Collections.singletonList("baz"));
+	}
+
 	@Test // gh-138
 	public void multipleEqualParameterReferencesForAnonymousMarkersBindsValueMultipleTimes() {