From f28a5d0cf72a02023c439e838bf0ebefe83303b6 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 11 Apr 2018 16:05:54 +0200 Subject: [PATCH] Proper exception for controller method return types that do not work with MvcUriComponentsBuilder (e.g. final classes) Includes direct use of ControllerMethodInvocationInterceptor for return type Object, avoiding the attempt to generate an Object subclass. Issue: SPR-16710 --- .../annotation/MvcUriComponentsBuilder.java | 57 +++++++---- .../MvcUriComponentsBuilderTests.java | 96 ++++++++++--------- 2 files changed, 93 insertions(+), 60 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java index f217e911b8..fbf4bbcb1d 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java @@ -253,10 +253,8 @@ public class MvcUriComponentsBuilder { * controller.getAddressesForCountry("US") * builder = MvcUriComponentsBuilder.fromMethodCall(controller); * - * *

Note: This method extracts values from "Forwarded" * and "X-Forwarded-*" headers if found. See class-level docs. - * * @param info either the value returned from a "mock" controller * invocation or the "mock" controller itself after an invocation * @return a UriComponents instance @@ -610,7 +608,11 @@ public class MvcUriComponentsBuilder { @SuppressWarnings("unchecked") private static T initProxy(Class type, ControllerMethodInvocationInterceptor interceptor) { - if (type.isInterface()) { + if (type == Object.class) { + return (T) interceptor; + } + + else if (type.isInterface()) { ProxyFactory factory = new ProxyFactory(EmptyTargetSource.INSTANCE); factory.addInterface(type); factory.addInterface(MethodInvocationInfo.class); @@ -709,8 +711,18 @@ public class MvcUriComponentsBuilder { } + public interface MethodInvocationInfo { + + Class getControllerType(); + + Method getControllerMethod(); + + Object[] getArgumentValues(); + } + + private static class ControllerMethodInvocationInterceptor - implements org.springframework.cglib.proxy.MethodInterceptor, MethodInterceptor { + implements org.springframework.cglib.proxy.MethodInterceptor, MethodInterceptor, MethodInvocationInfo { private final Class controllerType; @@ -727,15 +739,15 @@ public class MvcUriComponentsBuilder { @Override @Nullable public Object intercept(Object obj, Method method, Object[] args, @Nullable MethodProxy proxy) { - if (method.getName().equals("getControllerMethod")) { + if (method.getName().equals("getControllerType")) { + return this.controllerType; + } + else if (method.getName().equals("getControllerMethod")) { return this.controllerMethod; } else if (method.getName().equals("getArgumentValues")) { return this.argumentValues; } - else if (method.getName().equals("getControllerType")) { - return this.controllerType; - } else if (ReflectionUtils.isObjectMethod(method)) { return ReflectionUtils.invokeMethod(method, obj, args); } @@ -743,7 +755,13 @@ public class MvcUriComponentsBuilder { this.controllerMethod = method; this.argumentValues = args; Class returnType = method.getReturnType(); - return (void.class == returnType ? null : returnType.cast(initProxy(returnType, this))); + try { + return (returnType == void.class ? null : returnType.cast(initProxy(returnType, this))); + } + catch (Throwable ex) { + throw new IllegalStateException( + "Failed to create proxy for controller method return type: " + method, ex); + } } } @@ -752,16 +770,23 @@ public class MvcUriComponentsBuilder { public Object invoke(org.aopalliance.intercept.MethodInvocation inv) throws Throwable { return intercept(inv.getThis(), inv.getMethod(), inv.getArguments(), null); } - } + @Override + public Class getControllerType() { + return this.controllerType; + } - public interface MethodInvocationInfo { - - Method getControllerMethod(); - - Object[] getArgumentValues(); + @Override + public Method getControllerMethod() { + Assert.state(this.controllerMethod != null, "Not initialized yet"); + return this.controllerMethod; + } - Class getControllerType(); + @Override + public Object[] getArgumentValues() { + Assert.state(this.argumentValues != null, "Not initialized yet"); + return this.argumentValues; + } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java index 5327686c31..c8987f241f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -40,6 +40,7 @@ import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockServletContext; import org.springframework.stereotype.Controller; import org.springframework.util.MultiValueMap; +import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -54,17 +55,9 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.endsWith; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; -import static org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder.fromController; -import static org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder.fromMethodCall; -import static org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder.fromMethodName; -import static org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder.on; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; +import static org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder.*; /** * Unit tests for {@link org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBuilder}. @@ -142,15 +135,15 @@ public class MvcUriComponentsBuilderTests { } @Test - public void testFromMethodNamePathVariable() throws Exception { - UriComponents uriComponents = fromMethodName( - ControllerWithMethods.class, "methodWithPathVariable", new Object[]{"1"}).build(); + public void testFromMethodNamePathVariable() { + UriComponents uriComponents = fromMethodName(ControllerWithMethods.class, + "methodWithPathVariable", "1").build(); assertThat(uriComponents.toUriString(), is("http://localhost/something/1/foo")); } @Test - public void testFromMethodNameTypeLevelPathVariable() throws Exception { + public void testFromMethodNameTypeLevelPathVariable() { this.request.setContextPath("/myapp"); UriComponents uriComponents = fromMethodName( PersonsAddressesController.class, "getAddressesForCountry", "DE").buildAndExpand("1"); @@ -159,7 +152,7 @@ public class MvcUriComponentsBuilderTests { } @Test - public void testFromMethodNameTwoPathVariables() throws Exception { + public void testFromMethodNameTwoPathVariables() { DateTime now = DateTime.now(); UriComponents uriComponents = fromMethodName( ControllerWithMethods.class, "methodWithTwoPathVariables", 1, now).build(); @@ -168,7 +161,7 @@ public class MvcUriComponentsBuilderTests { } @Test - public void testFromMethodNameWithPathVarAndRequestParam() throws Exception { + public void testFromMethodNameWithPathVarAndRequestParam() { UriComponents uriComponents = fromMethodName( ControllerWithMethods.class, "methodForNextPage", "1", 10, 5).build(); @@ -178,59 +171,56 @@ public class MvcUriComponentsBuilderTests { assertThat(queryParams.get("offset"), contains("10")); } - // SPR-12977 - - @Test - public void fromMethodNameWithBridgedMethod() throws Exception { + @Test // SPR-12977 + public void fromMethodNameWithBridgedMethod() { UriComponents uriComponents = fromMethodName(PersonCrudController.class, "get", (long) 42).build(); + assertThat(uriComponents.toUriString(), is("http://localhost/42")); } - // SPR-11391 - - @Test - public void testFromMethodNameTypeLevelPathVariableWithoutArgumentValue() throws Exception { + @Test // SPR-11391 + public void testFromMethodNameTypeLevelPathVariableWithoutArgumentValue() { UriComponents uriComponents = fromMethodName(UserContactController.class, "showCreate", 123).build(); assertThat(uriComponents.getPath(), is("/user/123/contacts/create")); } @Test - public void testFromMethodNameNotMapped() throws Exception { + public void testFromMethodNameNotMapped() { UriComponents uriComponents = fromMethodName(UnmappedController.class, "unmappedMethod").build(); assertThat(uriComponents.toUriString(), is("http://localhost/")); } @Test - public void testFromMethodNameWithCustomBaseUrlViaStaticCall() throws Exception { + public void testFromMethodNameWithCustomBaseUrlViaStaticCall() { UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base"); UriComponents uriComponents = fromMethodName(builder, ControllerWithMethods.class, - "methodWithPathVariable", new Object[] {"1"}).build(); + "methodWithPathVariable", "1").build(); assertEquals("http://example.org:9090/base/something/1/foo", uriComponents.toString()); assertEquals("http://example.org:9090/base", builder.toUriString()); } @Test - public void testFromMethodNameWithCustomBaseUrlViaInstance() throws Exception { + public void testFromMethodNameWithCustomBaseUrlViaInstance() { UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base"); MvcUriComponentsBuilder mvcBuilder = MvcUriComponentsBuilder.relativeTo(builder); UriComponents uriComponents = mvcBuilder.withMethodName(ControllerWithMethods.class, - "methodWithPathVariable", new Object[] {"1"}).build(); + "methodWithPathVariable", "1").build(); assertEquals("http://example.org:9090/base/something/1/foo", uriComponents.toString()); assertEquals("http://example.org:9090/base", builder.toUriString()); } @Test - public void testFromMethodNameWithMetaAnnotation() throws Exception { + public void testFromMethodNameWithMetaAnnotation() { UriComponents uriComponents = fromMethodName(MetaAnnotationController.class, "handleInput").build(); assertThat(uriComponents.toUriString(), is("http://localhost/input")); } - @Test // SPR-14405 - public void testFromMappingNameWithOptionalParam() throws Exception { + @Test // SPR-14405 + public void testFromMappingNameWithOptionalParam() { UriComponents uriComponents = fromMethodName(ControllerWithMethods.class, "methodWithOptionalParam", new Object[] {null}).build(); @@ -255,8 +245,8 @@ public class MvcUriComponentsBuilderTests { @Test public void testFromMethodCallWithTypeLevelUriVars() { - UriComponents uriComponents = fromMethodCall(on( - PersonsAddressesController.class).getAddressesForCountry("DE")).buildAndExpand(15); + UriComponents uriComponents = fromMethodCall( + on(PersonsAddressesController.class).getAddressesForCountry("DE")).buildAndExpand(15); assertThat(uriComponents.toUriString(), endsWith("/people/15/addresses/DE")); } @@ -264,8 +254,8 @@ public class MvcUriComponentsBuilderTests { @Test public void testFromMethodCallWithPathVar() { - UriComponents uriComponents = fromMethodCall(on( - ControllerWithMethods.class).methodWithPathVariable("1")).build(); + UriComponents uriComponents = fromMethodCall( + on(ControllerWithMethods.class).methodWithPathVariable("1")).build(); assertThat(uriComponents.toUriString(), startsWith("http://localhost")); assertThat(uriComponents.toUriString(), endsWith("/something/1/foo")); @@ -273,8 +263,8 @@ public class MvcUriComponentsBuilderTests { @Test public void testFromMethodCallWithPathVarAndRequestParams() { - UriComponents uriComponents = fromMethodCall(on( - ControllerWithMethods.class).methodForNextPage("1", 10, 5)).build(); + UriComponents uriComponents = fromMethodCall( + on(ControllerWithMethods.class).methodForNextPage("1", 10, 5)).build(); assertThat(uriComponents.getPath(), is("/something/1/foo")); @@ -285,8 +275,8 @@ public class MvcUriComponentsBuilderTests { @Test public void testFromMethodCallWithPathVarAndMultiValueRequestParams() { - UriComponents uriComponents = fromMethodCall(on( - ControllerWithMethods.class).methodWithMultiValueRequestParams("1", Arrays.asList(3, 7), 5)).build(); + UriComponents uriComponents = fromMethodCall( + on(ControllerWithMethods.class).methodWithMultiValueRequestParams("1", Arrays.asList(3, 7), 5)).build(); assertThat(uriComponents.getPath(), is("/something/1/foo")); @@ -315,7 +305,7 @@ public class MvcUriComponentsBuilderTests { } @Test - public void testFromMappingName() throws Exception { + public void testFromMappingName() { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); context.setServletContext(new MockServletContext()); context.register(WebConfig.class); @@ -332,7 +322,7 @@ public class MvcUriComponentsBuilderTests { } @Test - public void testFromMappingNameWithCustomBaseUrl() throws Exception { + public void testFromMappingNameWithCustomBaseUrl() { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); context.setServletContext(new MockServletContext()); context.register(WebConfig.class); @@ -370,6 +360,13 @@ public class MvcUriComponentsBuilderTests { assertThat(uriComponents.toUriString(), startsWith("http://barfoo:8888")); } + @Test // SPR-16710 + public void withStringReturnType() { + UriComponents uriComponents = MvcUriComponentsBuilder.fromMethodCall( + on(BookingController.class).getBooking(21L)).buildAndExpand(42); + assertEquals("http://localhost/hotels/42/bookings/21", uriComponents.encode().toUri().toString()); + } + static class Person { @@ -516,4 +513,15 @@ public class MvcUriComponentsBuilderTests { } } + + @Controller + @RequestMapping("/hotels/{hotel}") + public class BookingController { + + @GetMapping("/bookings/{booking}") + public Object getBooking(@PathVariable Long booking) { + return "url"; + } + } + }