From 82c95d5c4eb46f4563244eb99a708ce91bb1d3dd Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 1 Sep 2016 16:32:54 +0200 Subject: [PATCH] DATAREST-881 - Fixed assignment checks of ResolvableTypes for wildcarded method return types. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ResourceProcessorInvoker suffered from Spring's ResolvableType.fromClass(…) using strong assignability checks between raw types and wildcarded right hand side types (i.e. Collection VS. Collection). This has been reported [0] and fixed already but we have to switch to ResolvableType.fromRawClass(…) to get the desired assignment check behavior. Tightened unit tests to verify the calls to the delegate handler, which had been disabled by accident and thus prevented us from detecting the issue with ResolvableType beforehand. [0] https://jira.spring.io/browse/SPR-14648 --- ...cessorHandlerMethodReturnValueHandler.java | 10 ++-- .../rest/webmvc/ResourceProcessorInvoker.java | 36 ++++++++++++-- ...sourceProcessorInvokingHandlerAdapter.java | 4 +- ...dlerMethodReturnValueHandlerUnitTests.java | 48 ++++++++++++------- 4 files changed, 72 insertions(+), 26 deletions(-) diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandler.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandler.java index 3e5e172f1..2476586ce 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandler.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2016 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. @@ -39,13 +39,15 @@ import org.springframework.web.method.support.ModelAndViewContainer; * configured {@link ResourceProcessor}s. * * @author Oliver Gierke + * @deprecated in favor of Spring HATEOAS' version of this class */ +@Deprecated @RequiredArgsConstructor public class ResourceProcessorHandlerMethodReturnValueHandler implements HandlerMethodReturnValueHandler { - static final ResolvableType RESOURCE_TYPE = ResolvableType.forClass(Resource.class); - static final ResolvableType RESOURCES_TYPE = ResolvableType.forClass(Resources.class); - private static final ResolvableType HTTP_ENTITY_TYPE = ResolvableType.forClass(HttpEntity.class); + static final ResolvableType RESOURCE_TYPE = ResolvableType.forRawClass(Resource.class); + static final ResolvableType RESOURCES_TYPE = ResolvableType.forRawClass(Resources.class); + private static final ResolvableType HTTP_ENTITY_TYPE = ResolvableType.forRawClass(HttpEntity.class); static final Field CONTENT_FIELD = ReflectionUtils.findField(Resources.class, "content"); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvoker.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvoker.java index 0dceb8a02..b799715cd 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvoker.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvoker.java @@ -38,7 +38,9 @@ import org.springframework.util.ReflectionUtils; * * @author Oliver Gierke * @since 2.5 + * @deprecated in favor of Spring HATEOAS' variant of this type. */ +@Deprecated public class ResourceProcessorInvoker { private final List processors; @@ -319,7 +321,7 @@ public class ResourceProcessorInvoker { * * @author Oliver Gierke */ - static class ResourcesProcessorWrapper extends ResourceProcessorInvoker.DefaultProcessorWrapper { + public static class ResourcesProcessorWrapper extends ResourceProcessorInvoker.DefaultProcessorWrapper { /** * Creates a new {@link ResourcesProcessorWrapper} for the given {@link ResourceProcessor}. @@ -366,9 +368,9 @@ public class ResourceProcessorInvoker { ResolvableType superType = null; - for (Class resourcesType : Arrays.> asList(resources.getClass(), Resources.class)) { + for (Class resourcesType : Arrays.>asList(resources.getClass(), Resources.class)) { - superType = ResolvableType.forClass(resourcesType, getRawType(target)); + superType = getSuperType(target, resourcesType); if (superType != null) { break; @@ -390,6 +392,34 @@ public class ResourceProcessorInvoker { return false; } + + /** + * Returns the {@link ResolvableType} for the given raw super class. + * + * @param source must not be {@literal null}. + * @param superType must not be {@literal null}. + * @return + */ + private static ResolvableType getSuperType(ResolvableType source, Class superType) { + + if (source.getRawClass().equals(superType)) { + return source; + } + + ResolvableType candidate = source.getSuperType(); + + if (superType.isAssignableFrom(candidate.getRawClass())) { + return candidate; + } + + for (ResolvableType interfaces : source.getInterfaces()) { + if (superType.isAssignableFrom(interfaces.getRawClass())) { + return interfaces; + } + } + + return ResolvableType.forClass(superType); + } } /** diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvokingHandlerAdapter.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvokingHandlerAdapter.java index 92575a388..e58150046 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvokingHandlerAdapter.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceProcessorInvokingHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2016 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. @@ -38,7 +38,9 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandl * * @author Oliver Gierke * @author Phil Webb + * @deprecated in favor of Spring HATEOAS' variant of this class. */ +@Deprecated public class ResourceProcessorInvokingHandlerAdapter extends RequestMappingHandlerAdapter { private static final Method RETURN_VALUE_HANDLER_METHOD = ReflectionUtils diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandlerUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandlerUnitTests.java index b08b0a78b..2c0f8cbd8 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandlerUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceProcessorHandlerMethodReturnValueHandlerUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 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. @@ -19,7 +19,6 @@ import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; -import static org.springframework.data.rest.webmvc.HttpEntityMatcher.*; import static org.springframework.util.ReflectionUtils.*; import java.lang.reflect.Method; @@ -29,7 +28,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.hamcrest.Matcher; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -64,6 +62,7 @@ import org.springframework.web.method.support.ModelAndViewContainer; * @author Oliver Gierke * @author Jon Brisbin */ +@Deprecated @RunWith(MockitoJUnitRunner.class) public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { @@ -126,7 +125,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("stringResourceEntity", is(BAR), FOO); + invokeReturnValueHandler("stringResourceEntity", FOO, BAR); } @Test @@ -135,7 +134,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("stringResourceEntity", httpEntity(BAR_RESP_ENTITY), FOO_RESP_ENTITY); + invokeReturnValueHandler("stringResourceEntity", FOO_RESP_ENTITY, BAR_RESP_ENTITY); } @Test @@ -144,7 +143,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("resourceEntity", httpEntity(BAR_RESP_ENTITY), FOO_RESP_ENTITY); + invokeReturnValueHandler("resourceEntity", FOO_RESP_ENTITY, BAR_RESP_ENTITY); } @Test @@ -153,7 +152,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourcesProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("resources", is(BARS), FOOS); + invokeReturnValueHandler("resources", FOOS, BARS); } @Test @@ -162,7 +161,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(SpecializedStringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("stringResourceEntity", httpEntity(BAR_RES_ENTITY), FOO_RES_ENTITY); + invokeReturnValueHandler("stringResourceEntity", FOO_RES_ENTITY, BAR_RES_ENTITY); } @Test @@ -171,7 +170,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("specializedStringResourceEntity", httpEntity(BAR_ENTITY), FOO_RES_ENTITY); + invokeReturnValueHandler("specializedStringResourceEntity", FOO_RES_ENTITY, BAR_ENTITY); } @Test @@ -180,7 +179,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("longResource", is(LONG_20), LONG_10); + invokeReturnValueHandler("longResource", LONG_10, LONG_20); } @Test @@ -189,7 +188,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(SpecializedLongResourceProcessor.INSTANCE); - invokeReturnValueHandler("specializedLongResourceEntity", httpEntity(LONG_20_RES_ENTITY), LONG_10_RES_ENTITY); + invokeReturnValueHandler("specializedLongResourceEntity", LONG_10_RES_ENTITY, LONG_20_RES_ENTITY); } @Test @@ -198,7 +197,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(SpecializedLongResourceProcessor.INSTANCE); - invokeReturnValueHandler("numberResourceEntity", httpEntity(LONG_10_ENTITY), LONG_10_ENTITY); + invokeReturnValueHandler("numberResourceEntity", LONG_10_ENTITY, LONG_10_ENTITY); } @Test @@ -207,7 +206,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("resourceEntity", is(LONG_20), LONG_10_RES); + invokeReturnValueHandler("resourceEntity", LONG_10_RES, LONG_20); } @Test @@ -249,7 +248,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { SampleProjection projection = factory.createProjection(SampleProjection.class, new Sample()); Resource resource = new Resource(projection); - invokeReturnValueHandler("object", is(resource), resource); + invokeReturnValueHandler("object", resource, resource); assertThat(projectionProcessor.invoked, is(true)); } @@ -276,7 +275,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { EmbeddedWrappers wrappers = new EmbeddedWrappers(false); Resources value = new Resources( - Collections. singleton(wrappers.emptyCollectionOf(Object.class))); + Collections.singleton(wrappers.emptyCollectionOf(Object.class))); ResourcesProcessorWrapper wrapper = new ResourcesProcessorWrapper(new SpecialResourcesProcessor()); ResolvableType type = ResolvableType.forMethodReturnType(Controller.class.getMethod("resourcesOfObject")); @@ -298,9 +297,20 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { new ResourceProcessorHandlerMethodReturnValueHandler(delegate, new ResourceProcessorInvoker(resourceProcessors)); } - // Helpers ---------------------------------------------------------// - private void invokeReturnValueHandler(String method, final Matcher matcher, Object returnValue) throws Exception { - final MethodParameter methodParam = METHOD_PARAMS.get(method); + /** + * @see DATAREST-881 + */ + @Test + public void processesElementsForWildcardedResources() throws Exception { + + resourceProcessors.add(StringResourceProcessor.INSTANCE); + + invokeReturnValueHandler("wildcardedResources", FOOS, BARS); + } + + private void invokeReturnValueHandler(String method, Object returnValue, Object expected) throws Exception { + + MethodParameter methodParam = METHOD_PARAMS.get(method); if (methodParam == null) { throw new IllegalArgumentException("Invalid method!"); @@ -398,6 +408,8 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { ResponseEntity> resourcesResponseEntity(); Resources resourcesOfObject(); + + Resources wildcardedResources(); } static class StringResource extends Resource {