From 642b6736764068ffeee38379629db5fc15cf927d Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 31 Aug 2016 17:00:18 +0200 Subject: [PATCH] #486 - 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 | 6 +- .../hateoas/mvc/ResourceProcessorInvoker.java | 32 ++++++- ...dlerMethodReturnValueHandlerUnitTests.java | 92 +++++++++++++++---- 3 files changed, 105 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorHandlerMethodReturnValueHandler.java b/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorHandlerMethodReturnValueHandler.java index 39b80f77..28c5bb40 100644 --- a/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorHandlerMethodReturnValueHandler.java +++ b/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorHandlerMethodReturnValueHandler.java @@ -44,9 +44,9 @@ import org.springframework.web.method.support.ModelAndViewContainer; @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/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorInvoker.java b/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorInvoker.java index 020d93af..8b2e778b 100644 --- a/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorInvoker.java +++ b/src/main/java/org/springframework/hateoas/mvc/ResourceProcessorInvoker.java @@ -367,9 +367,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; @@ -391,6 +391,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/src/test/java/org/springframework/hateoas/mvc/ResourceProcessorHandlerMethodReturnValueHandlerUnitTests.java b/src/test/java/org/springframework/hateoas/mvc/ResourceProcessorHandlerMethodReturnValueHandlerUnitTests.java index 566a0b99..b8fff7b2 100644 --- a/src/test/java/org/springframework/hateoas/mvc/ResourceProcessorHandlerMethodReturnValueHandlerUnitTests.java +++ b/src/test/java/org/springframework/hateoas/mvc/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.hateoas.mvc.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; @@ -107,106 +105,145 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { resourceProcessors = new ArrayList>(); } + /** + * @see #362 + */ @Test public void supportsIfDelegateSupports() { assertSupport(true); } + /** + * @see #362 + */ @Test public void doesNotSupportIfDelegateDoesNot() { assertSupport(false); } + /** + * @see #362 + */ @Test public void postProcessesStringResource() throws Exception { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("stringResourceEntity", is(BAR), FOO); + invokeReturnValueHandler("stringResourceEntity", FOO, BAR); } + /** + * @see #362 + */ @Test public void postProcessesStringResourceInResponseEntity() throws Exception { 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); } + /** + * @see #362 + */ @Test public void postProcessesStringResourceInWildcardResponseEntity() throws Exception { 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); } + /** + * @see #362 + */ @Test public void postProcessesStringResources() throws Exception { resourceProcessors.add(StringResourcesProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("resources", is(BARS), FOOS); + invokeReturnValueHandler("resources", FOOS, BARS); } + /** + * @see #362 + */ @Test public void postProcessesSpecializedStringResource() throws Exception { 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); } + /** + * @see #362 + */ @Test public void postProcessesSpecializedStringUsingStringResourceProcessor() throws Exception { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("specializedStringResourceEntity", httpEntity(BAR_ENTITY), FOO_RES_ENTITY); + invokeReturnValueHandler("specializedStringResourceEntity", FOO_RES_ENTITY, BAR_ENTITY); } + /** + * @see #362 + */ @Test public void postProcessesLongResource() throws Exception { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("longResource", is(LONG_20), LONG_10); + invokeReturnValueHandler("longResource", LONG_10, LONG_20); } + /** + * @see #362 + */ @Test public void postProcessesSpecializedLongResource() throws Exception { 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); } + /** + * @see #362 + */ @Test public void doesNotPostProcesseLongResourceWithSpecializedLongResourceProcessor() throws Exception { 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); } + /** + * @see #362 + */ @Test public void postProcessesSpecializedLongResourceUsingLongResourceProcessor() throws Exception { resourceProcessors.add(StringResourceProcessor.INSTANCE); resourceProcessors.add(LongResourceProcessor.INSTANCE); - invokeReturnValueHandler("resourceEntity", is(LONG_20), LONG_10_RES); + invokeReturnValueHandler("resourceEntity", LONG_10_RES, LONG_20); } + /** + * @see #362 + */ @Test public void usesHeaderLinksResponseEntityIfConfigured() throws Exception { @@ -223,7 +260,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { } /** - * @see DATAREST-331 + * @see #362 */ @Test public void resourcesProcessorMatchesValueSubTypes() { @@ -234,14 +271,14 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { } /** - * @see DATAREST-479 + * @see #362 */ @Test public void doesNotInvokeAProcessorForASpecializedType() throws Exception { 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")); @@ -250,7 +287,7 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { } /** - * @see DATAREST-702 + * @see #362 */ @Test public void registersProcessorForProxyType() { @@ -263,9 +300,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 #486 + */ + @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!"); @@ -274,6 +322,8 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { HandlerMethodReturnValueHandler handler = new ResourceProcessorHandlerMethodReturnValueHandler(delegate, new ResourceProcessorInvoker(resourceProcessors)); handler.handleReturnValue(returnValue, methodParam, null, null); + + verify(delegate, times(1)).handleReturnValue(expected, methodParam, null, null); } private void assertSupport(boolean value) { @@ -363,6 +413,8 @@ public class ResourceProcessorHandlerMethodReturnValueHandlerUnitTests { ResponseEntity> resourcesResponseEntity(); Resources resourcesOfObject(); + + Resources wildcardedResources(); } static class StringResource extends Resource {