From 501f0c84932a2a92788b7f101bc34455e1152421 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 22 May 2015 09:24:23 +0200 Subject: [PATCH] DATACMNS-700 - ReflectionRepositoryInvoker now captures context for failed parameter conversions. Introduced QueryMethodParameterConversionException to capture the context of a failed ConversionException to allow better error reporting. Cleaned up test cases to not use deprecated API anymore. --- ...eryMethodParameterConversionException.java | 70 +++++++++++++++++++ .../support/ReflectionRepositoryInvoker.java | 13 +++- .../ReflectionRepositoryInvokerUnitTests.java | 45 ++++++++++-- 3 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 src/main/java/org/springframework/data/repository/support/QueryMethodParameterConversionException.java diff --git a/src/main/java/org/springframework/data/repository/support/QueryMethodParameterConversionException.java b/src/main/java/org/springframework/data/repository/support/QueryMethodParameterConversionException.java new file mode 100644 index 000000000..88405081d --- /dev/null +++ b/src/main/java/org/springframework/data/repository/support/QueryMethodParameterConversionException.java @@ -0,0 +1,70 @@ +/* + * Copyright 2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.repository.support; + +import org.springframework.core.MethodParameter; +import org.springframework.core.convert.ConversionException; +import org.springframework.util.Assert; + +/** + * Exception to represent a failed attempt to convert a source value into a query method parameter. + * + * @author Oliver Gierke + * @soundtrack Dave Matthews Band - The Dreaming Tree (DMB 2009 Europe) + * @since 1.11 + */ +public class QueryMethodParameterConversionException extends RuntimeException { + + private static final long serialVersionUID = -5818002272039533066L; + + private final Object source; + private final MethodParameter parameter; + + /** + * @param source can be {@literal null}. + * @param parameter the {@link MethodParameter} the value should've been converted for, must not be {@literal null}.. + * @param cause the original {@link ConversionException}, must not be {@literal null}. + */ + public QueryMethodParameterConversionException(Object source, MethodParameter parameter, ConversionException cause) { + + super("message", cause); + + Assert.notNull(parameter, "Method parameter must not be null!"); + Assert.notNull(cause, "ConversionException must not be null!"); + + this.parameter = parameter; + this.source = source; + } + + /** + * Returns the source value that we failed converting. + * + * @return the source can be {@literal null}. + */ + public Object getSource() { + return source; + } + + /** + * Returns the {@link MethodParameter} we tried to convert the source value for. + * + * @return the parameter will never be {@literal null}. + * @see #getSource() + */ + public MethodParameter getParameter() { + return parameter; + } +} diff --git a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java index efd1890ad..3643380cd 100644 --- a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Map.Entry; import org.springframework.core.MethodParameter; +import org.springframework.core.convert.ConversionException; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; import org.springframework.data.domain.Pageable; @@ -234,14 +235,22 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { Object value = unwrapSingleElement(rawParameters.get(parameterName)); - result[i] = targetType.isInstance(value) ? value : conversionService.convert(value, - TypeDescriptor.forObject(value), new TypeDescriptor(param)); + result[i] = targetType.isInstance(value) ? value : convert(value, param); } } return result; } + private Object convert(Object value, MethodParameter parameter) { + + try { + return conversionService.convert(value, TypeDescriptor.forObject(value), new TypeDescriptor(parameter)); + } catch (ConversionException o_O) { + throw new QueryMethodParameterConversionException(value, parameter, o_O); + } + } + /** * Invokes the given method with the given arguments on the backing repository. * diff --git a/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java b/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java index d6d5998b7..9c099b243 100644 --- a/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java @@ -25,14 +25,13 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.HashMap; import java.util.List; -import java.util.Map; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.domain.Page; @@ -44,10 +43,13 @@ import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; +import org.springframework.data.repository.query.Param; import org.springframework.data.repository.support.CrudRepositoryInvokerUnitTests.Person; import org.springframework.data.repository.support.CrudRepositoryInvokerUnitTests.PersonRepository; import org.springframework.data.repository.support.RepositoryInvocationTestUtils.VerifyingMethodInterceptor; import org.springframework.format.support.DefaultFormattingConversionService; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; /** * Integration tests for {@link ReflectionRepositoryInvoker}. @@ -155,8 +157,8 @@ public class ReflectionRepositoryInvokerUnitTests { @Test public void invokesQueryMethod() throws Exception { - HashMap parameters = new HashMap(); - parameters.put("firstName", new String[] { "John" }); + MultiValueMap parameters = new LinkedMultiValueMap(); + parameters.add("firstName", "John"); Method method = PersonRepository.class.getMethod("findByFirstName", String.class, Pageable.class); PersonRepository repository = mock(PersonRepository.class); @@ -170,8 +172,8 @@ public class ReflectionRepositoryInvokerUnitTests { @Test public void considersFormattingAnnotationsOnQueryMethodParameters() throws Exception { - Map parameters = Collections.singletonMap("date", - new String[] { "2013-07-18T10:49:00.000+02:00" }); + MultiValueMap parameters = new LinkedMultiValueMap(); + parameters.add("date", "2013-07-18T10:49:00.000+02:00"); Method method = PersonRepository.class.getMethod("findByCreatedUsingISO8601Date", Date.class, Pageable.class); PersonRepository repository = mock(PersonRepository.class); @@ -247,7 +249,8 @@ public class ReflectionRepositoryInvokerUnitTests { for (String[] ids : Arrays.asList(new String[] { "1,2" }, new String[] { "1", "2" })) { - Map parameters = Collections.singletonMap("ids", ids); + MultiValueMap parameters = new LinkedMultiValueMap(); + parameters.put("ids", Arrays.asList(ids)); Method method = PersonRepository.class.getMethod("findByIdIn", Collection.class); PersonRepository repository = mock(PersonRepository.class); @@ -256,6 +259,29 @@ public class ReflectionRepositoryInvokerUnitTests { } } + /** + * @see DATACMNS-700 + */ + @Test + public void failedParameterConversionCapturesContext() throws Exception { + + RepositoryInvoker invoker = getInvokerFor(mock(SimpleRepository.class)); + + MultiValueMap parameters = new LinkedMultiValueMap(); + parameters.add("value", "value"); + + Method method = SimpleRepository.class.getMethod("findByClass", int.class); + + try { + invoker.invokeQueryMethod(method, parameters, null, null); + } catch (QueryMethodParameterConversionException o_O) { + + assertThat(o_O.getParameter(), is(new MethodParameters(method).getParameters().get(0))); + assertThat(o_O.getSource(), is((Object) "value")); + assertThat(o_O.getCause(), is(instanceOf(ConversionFailedException.class))); + } + } + private static RepositoryInvoker getInvokerFor(Object repository) { RepositoryMetadata metadata = new DefaultRepositoryMetadata(repository.getClass().getInterfaces()[0]); @@ -310,4 +336,9 @@ public class ReflectionRepositoryInvokerUnitTests { void delete(Domain entity); } + + interface SimpleRepository extends Repository { + + Domain findByClass(@Param("value") int value); + } }