DATACMNS-1482 - Properly convert collections when collection type matches but element type doesn't.
Various fast returns and the use of Class instead of TypeDescriptor led to e.g. List<BigDecimal> not getting properly converted to List<Integer> leading to unexpected ClassCastExceptions when the collection elements where accessed.
This commit is contained in:
committed by
Oliver Drotbohm
parent
8f8048ecd6
commit
7bc27ca934
@@ -34,6 +34,7 @@ import org.springframework.lang.Nullable;
|
|||||||
*
|
*
|
||||||
* @author Oliver Gierke
|
* @author Oliver Gierke
|
||||||
* @author Mark Paluch
|
* @author Mark Paluch
|
||||||
|
* @author Jens Schauder
|
||||||
*/
|
*/
|
||||||
class QueryExecutionResultHandler {
|
class QueryExecutionResultHandler {
|
||||||
|
|
||||||
@@ -63,10 +64,6 @@ class QueryExecutionResultHandler {
|
|||||||
@Nullable
|
@Nullable
|
||||||
public Object postProcessInvocationResult(@Nullable Object result, Method method) {
|
public Object postProcessInvocationResult(@Nullable Object result, Method method) {
|
||||||
|
|
||||||
if (method.getReturnType().isInstance(result)) {
|
|
||||||
return result;
|
|
||||||
}
|
|
||||||
|
|
||||||
MethodParameter parameter = new MethodParameter(method, -1);
|
MethodParameter parameter = new MethodParameter(method, -1);
|
||||||
|
|
||||||
return postProcessInvocationResult(result, 0, parameter);
|
return postProcessInvocationResult(result, 0, parameter);
|
||||||
@@ -91,20 +88,8 @@ class QueryExecutionResultHandler {
|
|||||||
|
|
||||||
Class<?> expectedReturnType = returnTypeDescriptor.getType();
|
Class<?> expectedReturnType = returnTypeDescriptor.getType();
|
||||||
|
|
||||||
// Early return if the raw value matches
|
|
||||||
|
|
||||||
if (result != null && expectedReturnType.isInstance(result)) {
|
|
||||||
return result;
|
|
||||||
}
|
|
||||||
|
|
||||||
result = unwrapOptional(result);
|
result = unwrapOptional(result);
|
||||||
|
|
||||||
// Early return if the unrwapped value matches
|
|
||||||
|
|
||||||
if (result != null && expectedReturnType.isInstance(result)) {
|
|
||||||
return result;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (QueryExecutionConverters.supports(expectedReturnType)) {
|
if (QueryExecutionConverters.supports(expectedReturnType)) {
|
||||||
|
|
||||||
// For a wrapper type, try nested resolution first
|
// For a wrapper type, try nested resolution first
|
||||||
@@ -132,8 +117,8 @@ class QueryExecutionResultHandler {
|
|||||||
return ReactiveWrapperConverters.toWrapper(result, expectedReturnType);
|
return ReactiveWrapperConverters.toWrapper(result, expectedReturnType);
|
||||||
}
|
}
|
||||||
|
|
||||||
return conversionService.canConvert(result.getClass(), expectedReturnType)
|
return conversionService.canConvert(TypeDescriptor.forObject(result), returnTypeDescriptor)
|
||||||
? conversionService.convert(result, expectedReturnType)
|
? conversionService.convert(result, returnTypeDescriptor)
|
||||||
: result;
|
: result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -15,6 +15,7 @@
|
|||||||
*/
|
*/
|
||||||
package org.springframework.data.repository.core.support;
|
package org.springframework.data.repository.core.support;
|
||||||
|
|
||||||
|
import static java.util.Arrays.*;
|
||||||
import static org.assertj.core.api.Assertions.*;
|
import static org.assertj.core.api.Assertions.*;
|
||||||
|
|
||||||
import io.vavr.control.Try;
|
import io.vavr.control.Try;
|
||||||
@@ -26,6 +27,7 @@ import rx.Observable;
|
|||||||
import rx.Single;
|
import rx.Single;
|
||||||
|
|
||||||
import java.lang.reflect.Method;
|
import java.lang.reflect.Method;
|
||||||
|
import java.math.BigDecimal;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
@@ -34,6 +36,7 @@ import java.util.Optional;
|
|||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
|
import org.assertj.core.api.SoftAssertions;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
import org.reactivestreams.Publisher;
|
import org.reactivestreams.Publisher;
|
||||||
import org.springframework.dao.InvalidDataAccessApiUsageException;
|
import org.springframework.dao.InvalidDataAccessApiUsageException;
|
||||||
@@ -45,6 +48,7 @@ import org.springframework.data.util.Streamable;
|
|||||||
*
|
*
|
||||||
* @author Oliver Gierke
|
* @author Oliver Gierke
|
||||||
* @author Mark Paluch
|
* @author Mark Paluch
|
||||||
|
* @author Jens Schauder
|
||||||
*/
|
*/
|
||||||
public class QueryExecutionResultHandlerUnitTests {
|
public class QueryExecutionResultHandlerUnitTests {
|
||||||
|
|
||||||
@@ -354,7 +358,7 @@ public class QueryExecutionResultHandlerUnitTests {
|
|||||||
@SuppressWarnings("unchecked")
|
@SuppressWarnings("unchecked")
|
||||||
public void convertsIterableIntoStreamable() throws Exception {
|
public void convertsIterableIntoStreamable() throws Exception {
|
||||||
|
|
||||||
Iterable<?> source = Arrays.asList(new Object());
|
Iterable<?> source = asList(new Object());
|
||||||
|
|
||||||
Object result = handler.postProcessInvocationResult(source, getMethod("streamable"));
|
Object result = handler.postProcessInvocationResult(source, getMethod("streamable"));
|
||||||
|
|
||||||
@@ -372,6 +376,22 @@ public class QueryExecutionResultHandlerUnitTests {
|
|||||||
assertThat(result).isInstanceOfSatisfying(Option.class, it -> assertThat(it.get()).isEqualTo(entity));
|
assertThat(result).isInstanceOfSatisfying(Option.class, it -> assertThat(it.get()).isEqualTo(entity));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test // DATACMNS-1482
|
||||||
|
public void nestedConversion() throws Exception {
|
||||||
|
|
||||||
|
Object result = handler.postProcessInvocationResult(asList(BigDecimal.ZERO, BigDecimal.ONE),
|
||||||
|
getMethod("listOfInteger"));
|
||||||
|
|
||||||
|
assertThat(result).isInstanceOf(List.class);
|
||||||
|
|
||||||
|
List list = (List) result;
|
||||||
|
SoftAssertions.assertSoftly(s -> {
|
||||||
|
// for making the test failure more obvious:
|
||||||
|
(list).forEach(v -> s.assertThat(v).isInstanceOf(Integer.class));
|
||||||
|
s.assertThat(list).containsExactly(0, 1);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
private static Method getMethod(String methodName) throws Exception {
|
private static Method getMethod(String methodName) throws Exception {
|
||||||
return Sample.class.getMethod(methodName);
|
return Sample.class.getMethod(methodName);
|
||||||
}
|
}
|
||||||
@@ -406,6 +426,8 @@ public class QueryExecutionResultHandlerUnitTests {
|
|||||||
|
|
||||||
// DATACMNS-938
|
// DATACMNS-938
|
||||||
Try<Option<Entity>> tryOfOption();
|
Try<Option<Entity>> tryOfOption();
|
||||||
|
|
||||||
|
List<Integer> listOfInteger();
|
||||||
}
|
}
|
||||||
|
|
||||||
static class Entity {}
|
static class Entity {}
|
||||||
|
|||||||
Reference in New Issue
Block a user