From 714dec3c94eb7d3afdb14740ca280dd9f45eaaf3 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 19cf9d10..301aecc4 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 387f5f13..84ddb94a 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() {