Skip to content

Commit 12f228b

Browse files
committed
Convert elements of collection- and array-valued query method parameters.
Query method parameters exposed through Spring Data REST were passed to the backing repository without element conversion when the raw value already was an instance of the declared target type (e.g. a `List`). As a result, a repeated request parameter such as `?colors=RED&colors=GREEN` reached an `in`-style query as a `List<String>` instead of a `List<Color>`, causing the persistence provider to fail. `ReflectionRepositoryInvoker` now also invokes conversion for array- and `Iterable`-typed parameters even when the value already matches the target type, so the raw elements get converted to the declared element type.
1 parent c64558c commit 12f228b

2 files changed

Lines changed: 37 additions & 1 deletion

File tree

src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
* @author Oliver Gierke
4747
* @author Alessandro Nistico
4848
* @author Johannes Englmeier
49+
* @author Seonwoo Jung
4950
* @since 1.10
5051
*/
5152
class ReflectionRepositoryInvoker implements RepositoryInvoker {
@@ -188,7 +189,13 @@ private Object[] prepareParameters(Method method, MultiValueMap<String, ?> rawPa
188189

189190
Object value = unwrapSingleElement(rawParameters.get(parameterName));
190191

191-
result[i] = targetType.isInstance(value) ? value : convert(value, param);
192+
// Collection- and array-valued parameters must be converted even if the value already is an instance of
193+
// the target type (e.g. a List), as the raw elements (typically String) still need to be converted to the
194+
// declared element type (see GH-3502).
195+
boolean elementConversionRequired = value != null
196+
&& (targetType.isArray() || Iterable.class.isAssignableFrom(targetType));
197+
198+
result[i] = (targetType.isInstance(value) && !elementConversionRequired) ? value : convert(value, param);
192199
}
193200
}
194201

src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.junit.jupiter.api.Test;
3131
import org.junit.jupiter.api.extension.ExtendWith;
3232
import org.mockito.AdditionalAnswers;
33+
import org.mockito.ArgumentCaptor;
3334
import org.mockito.junit.jupiter.MockitoExtension;
3435
import org.springframework.core.convert.ConversionFailedException;
3536
import org.springframework.core.convert.ConversionService;
@@ -52,6 +53,7 @@
5253
* Integration tests for {@link ReflectionRepositoryInvoker}.
5354
*
5455
* @author Oliver Gierke
56+
* @author Seonwoo Jung
5557
*/
5658
@ExtendWith(MockitoExtension.class)
5759
class ReflectionRepositoryInvokerUnitTests {
@@ -221,6 +223,24 @@ void translatesCollectionRequestParametersCorrectly() throws Exception {
221223
}
222224
}
223225

226+
@Test // GH-3502
227+
void convertsElementsOfCollectionValuedQueryParameter() throws Exception {
228+
229+
MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
230+
parameters.put("colors", Arrays.asList("RED", "GREEN"));
231+
232+
var method = ColorRepository.class.getMethod("findByColorIn", Collection.class);
233+
var repository = mock(ColorRepository.class);
234+
235+
getInvokerFor(repository, expectInvocationOf(method)).invokeQueryMethod(method, parameters, Pageable.unpaged(),
236+
Sort.unsorted());
237+
238+
var captor = ArgumentCaptor.forClass(Collection.class);
239+
verify(repository).findByColorIn(captor.capture());
240+
241+
assertThat(captor.getValue()).containsExactly(Color.RED, Color.GREEN);
242+
}
243+
224244
@Test // DATACMNS-700
225245
void failedParameterConversionCapturesContext() throws Exception {
226246

@@ -371,4 +391,13 @@ interface DeleteByIdOverrideRepository<T, ID> extends Repository<T, ID> {
371391
}
372392

373393
interface DeleteByIdOverrideSubRepository extends DeleteByIdOverrideRepository<Domain, Long> {}
394+
395+
// GH-3502
396+
enum Color {
397+
RED, GREEN;
398+
}
399+
400+
interface ColorRepository extends Repository<Domain, Long> {
401+
List<Domain> findByColorIn(@Param("colors") Collection<Color> colors);
402+
}
374403
}

0 commit comments

Comments
 (0)