Skip to content

Commit e246cc4

Browse files
committed
Fix parameter binding of reused named parameters using anonymous bind markers.
We now properly bind values for reused named parameters correctly when using anonymous bind markers (e.g. ? for MySQL). Previously, the subsequent usages of named parameters especially with IN parameters were left not bound. Closes #1306
1 parent 2a8f622 commit e246cc4

File tree

2 files changed

+151
-25
lines changed

2 files changed

+151
-25
lines changed

spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/core/NamedParameterUtils.java

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.TreeMap;
2727

2828
import org.springframework.dao.InvalidDataAccessApiUsageException;
29+
import org.springframework.lang.Nullable;
2930
import org.springframework.r2dbc.core.PreparedOperation;
3031
import org.springframework.r2dbc.core.binding.BindMarker;
3132
import org.springframework.r2dbc.core.binding.BindMarkers;
@@ -436,6 +437,7 @@ NamedParameter getOrCreate(String namedParameter) {
436437
return param;
437438
}
438439

440+
@Nullable
439441
List<NamedParameter> getMarker(String name) {
440442
return this.references.get(name);
441443
}
@@ -499,36 +501,38 @@ private static class ExpandedQuery implements PreparedOperation<String> {
499501
@SuppressWarnings("unchecked")
500502
public void bind(org.springframework.r2dbc.core.binding.BindTarget target, String identifier, Object value) {
501503

502-
List<BindMarker> bindMarkers = getBindMarkers(identifier);
504+
List<List<BindMarker>> bindMarkers = getBindMarkers(identifier);
503505

504506
if (bindMarkers == null) {
505507

506508
target.bind(identifier, value);
507509
return;
508510
}
509511

510-
if (value instanceof Collection) {
511-
Collection<Object> collection = (Collection<Object>) value;
512+
for (List<BindMarker> outer : bindMarkers) {
513+
if (value instanceof Collection) {
514+
Collection<Object> collection = (Collection<Object>) value;
512515

513-
Iterator<Object> iterator = collection.iterator();
514-
Iterator<BindMarker> markers = bindMarkers.iterator();
516+
Iterator<Object> iterator = collection.iterator();
517+
Iterator<BindMarker> markers = outer.iterator();
515518

516-
while (iterator.hasNext()) {
519+
while (iterator.hasNext()) {
517520

518-
Object valueToBind = iterator.next();
521+
Object valueToBind = iterator.next();
519522

520-
if (valueToBind instanceof Object[]) {
521-
Object[] objects = (Object[]) valueToBind;
522-
for (Object object : objects) {
523-
bind(target, markers, object);
523+
if (valueToBind instanceof Object[]) {
524+
Object[] objects = (Object[]) valueToBind;
525+
for (Object object : objects) {
526+
bind(target, markers, object);
527+
}
528+
} else {
529+
bind(target, markers, valueToBind);
524530
}
525-
} else {
526-
bind(target, markers, valueToBind);
527531
}
528-
}
529-
} else {
530-
for (BindMarker bindMarker : bindMarkers) {
531-
bindMarker.bind(target, value);
532+
} else {
533+
for (BindMarker bindMarker : outer) {
534+
bindMarker.bind(target, value);
535+
}
532536
}
533537
}
534538
}
@@ -547,31 +551,33 @@ private void bind(org.springframework.r2dbc.core.binding.BindTarget target, Iter
547551
public void bindNull(org.springframework.r2dbc.core.binding.BindTarget target, String identifier,
548552
Class<?> valueType) {
549553

550-
List<BindMarker> bindMarkers = getBindMarkers(identifier);
554+
List<List<BindMarker>> bindMarkers = getBindMarkers(identifier);
551555

552556
if (bindMarkers == null) {
553557

554558
target.bindNull(identifier, valueType);
555559
return;
556560
}
557561

558-
for (BindMarker bindMarker : bindMarkers) {
559-
bindMarker.bindNull(target, valueType);
562+
for (List<BindMarker> outer : bindMarkers) {
563+
for (BindMarker bindMarker : outer) {
564+
bindMarker.bindNull(target, valueType);
565+
}
560566
}
561567
}
562568

563-
List<BindMarker> getBindMarkers(String identifier) {
569+
@Nullable
570+
List<List<BindMarker>> getBindMarkers(String identifier) {
564571

565572
List<NamedParameters.NamedParameter> parameters = this.parameters.getMarker(identifier);
566573

567574
if (parameters == null) {
568575
return null;
569576
}
570577

571-
List<BindMarker> markers = new ArrayList<>();
572-
578+
List<List<BindMarker>> markers = new ArrayList<>();
573579
for (NamedParameters.NamedParameter parameter : parameters) {
574-
markers.addAll(parameter.placeholders);
580+
markers.add(new ArrayList<>(parameter.placeholders));
575581
}
576582

577583
return markers;
@@ -582,7 +588,6 @@ public String getSource() {
582588
return this.expandedSql;
583589
}
584590

585-
586591
@Override
587592
public void bindTo(BindTarget target) {
588593

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Copyright 2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.r2dbc.core;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
20+
import java.util.ArrayList;
21+
import java.util.Arrays;
22+
import java.util.Collections;
23+
import java.util.HashMap;
24+
import java.util.List;
25+
import java.util.Map;
26+
27+
import org.junit.jupiter.api.Test;
28+
29+
import org.springframework.r2dbc.core.Parameter;
30+
import org.springframework.r2dbc.core.PreparedOperation;
31+
import org.springframework.r2dbc.core.binding.BindMarkersFactory;
32+
import org.springframework.r2dbc.core.binding.BindTarget;
33+
34+
/**
35+
* Unit tests for {@link NamedParameterUtils}.
36+
*
37+
* @author Mark Paluch
38+
*/
39+
class NamedParameterUtilsTests {
40+
41+
@Test // GH-1306
42+
void inCollectionSameParameterNameShouldBindAllAnonymousParameters() {
43+
44+
ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement("select :names AND :names");
45+
PreparedOperation<String> operation = NamedParameterUtils.substituteNamedParameters(parsedSql,
46+
BindMarkersFactory.anonymous("?"),
47+
new MapBindParameterSource(Collections.singletonMap("names", Parameter.from(Arrays.asList("1", "2", "3")))));
48+
49+
List<String> bindings = new ArrayList<>();
50+
51+
operation.bindTo(new BindingCaptor(bindings));
52+
53+
assertThat(operation.get()).isEqualTo("select ?, ?, ? AND ?, ?, ?");
54+
assertThat(bindings).contains("0: 1", "1: 2", "2: 3", "3: 1", "4: 2", "5: 3");
55+
}
56+
57+
@Test // GH-1306
58+
void complexInCollectionSameParameterNameShouldBindAllAnonymousParameters() {
59+
60+
Map<String, Parameter> parameterMap = new HashMap<>();
61+
parameterMap.put("names", Parameter.from(Arrays.asList("1", "2", "3")));
62+
parameterMap.put("hello", Parameter.from("world"));
63+
64+
ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement("select :names AND :hello OR :names");
65+
PreparedOperation<String> operation = NamedParameterUtils.substituteNamedParameters(parsedSql,
66+
BindMarkersFactory.anonymous("?"), new MapBindParameterSource(parameterMap));
67+
68+
List<String> bindings = new ArrayList<>();
69+
70+
operation.bindTo(new BindingCaptor(bindings));
71+
72+
assertThat(operation.get()).isEqualTo("select ?, ?, ? AND ? OR ?, ?, ?");
73+
assertThat(bindings).contains("0: 1", "1: 2", "2: 3", "3: world", "4: 1", "5: 2", "6: 3");
74+
}
75+
76+
@Test // GH-1306
77+
void inCollectionSameParameterNameShouldBindAllNamedParameters() {
78+
79+
ParsedSql parsedSql = NamedParameterUtils.parseSqlStatement("select :names AND :names");
80+
PreparedOperation<String> operation = NamedParameterUtils.substituteNamedParameters(parsedSql,
81+
BindMarkersFactory.indexed("$", 1),
82+
new MapBindParameterSource(Collections.singletonMap("names", Parameter.from(Arrays.asList("1", "2", "3")))));
83+
84+
List<String> bindings = new ArrayList<>();
85+
86+
operation.bindTo(new BindingCaptor(bindings));
87+
88+
assertThat(operation.get()).isEqualTo("select $1, $2, $3 AND $1, $2, $3");
89+
assertThat(bindings).containsOnly("0: 1", "1: 2", "2: 3");
90+
}
91+
92+
static class BindingCaptor implements BindTarget {
93+
94+
private final List<String> bindings;
95+
96+
BindingCaptor(List<String> bindings) {
97+
this.bindings = bindings;
98+
}
99+
100+
@Override
101+
public void bind(String identifier, Object value) {
102+
bindings.add(identifier + ": " + value);
103+
}
104+
105+
@Override
106+
public void bind(int index, Object value) {
107+
bindings.add(index + ": " + value);
108+
}
109+
110+
@Override
111+
public void bindNull(String identifier, Class<?> type) {
112+
113+
}
114+
115+
@Override
116+
public void bindNull(int index, Class<?> type) {
117+
118+
}
119+
120+
}
121+
}

0 commit comments

Comments
 (0)