#486 - Fixed assignment checks of ResolvableTypes for wildcarded method return types.

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
This commit is contained in:
Oliver Gierke
2016-08-31 17:00:18 +02:00
parent e5c6485985
commit 642b673676
3 changed files with 105 additions and 25 deletions

View File

@@ -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");

View File

@@ -367,9 +367,9 @@ public class ResourceProcessorInvoker {
ResolvableType superType = null;
for (Class<?> resourcesType : Arrays.<Class<?>> asList(resources.getClass(), Resources.class)) {
for (Class<?> resourcesType : Arrays.<Class<?>>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);
}
}
/**

View File

@@ -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<ResourceProcessor<?>>();
}
/**
* @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<Object> value = new Resources<Object>(
Collections.<Object> singleton(wrappers.emptyCollectionOf(Object.class)));
Collections.<Object>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<Resources<?>> resourcesResponseEntity();
Resources<Object> resourcesOfObject();
Resources<?> wildcardedResources();
}
static class StringResource extends Resource<String> {