Revised reference example for linkable controller method signature

Issue: SPR-16710

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java

@@ -50,6 +50,7 @@ import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
 import org.springframework.web.servlet.DispatcherServlet;
+import org.springframework.web.servlet.ModelAndView;
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
 import org.springframework.web.util.UriComponents;
@@ -85,38 +86,38 @@ public class MvcUriComponentsBuilderTests {
 
 
 	@Test
-	public void testFromController() {
+	public void fromControllerPlain() {
 		UriComponents uriComponents = fromController(PersonControllerImpl.class).build();
 		assertThat(uriComponents.toUriString(), Matchers.endsWith("/people"));
 	}
 
 	@Test
-	public void testFromControllerUriTemplate() {
+	public void fromControllerUriTemplate() {
 		UriComponents uriComponents = fromController(PersonsAddressesController.class).buildAndExpand(15);
 		assertThat(uriComponents.toUriString(), endsWith("/people/15/addresses"));
 	}
 
 	@Test
-	public void testFromControllerSubResource() {
+	public void fromControllerSubResource() {
 		UriComponents uriComponents = fromController(PersonControllerImpl.class).pathSegment("something").build();
 
 		assertThat(uriComponents.toUriString(), endsWith("/people/something"));
 	}
 
 	@Test
-	public void testFromControllerTwoTypeLevelMappings() {
+	public void fromControllerTwoTypeLevelMappings() {
 		UriComponents uriComponents = fromController(InvalidController.class).build();
 		assertThat(uriComponents.toUriString(), is("http://localhost/persons"));
 	}
 
 	@Test
-	public void testFromControllerNotMapped() {
+	public void fromControllerNotMapped() {
 		UriComponents uriComponents = fromController(UnmappedController.class).build();
 		assertThat(uriComponents.toUriString(), is("http://localhost/"));
 	}
 
 	@Test
-	public void testFromControllerWithCustomBaseUrlViaStaticCall() {
+	public void fromControllerWithCustomBaseUrlViaStaticCall() {
 		UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base");
 		UriComponents uriComponents = fromController(builder, PersonControllerImpl.class).build();
 
@@ -125,9 +126,9 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromControllerWithCustomBaseUrlViaInstance() {
+	public void fromControllerWithCustomBaseUrlViaInstance() {
 		UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base");
-		MvcUriComponentsBuilder mvcBuilder = MvcUriComponentsBuilder.relativeTo(builder);
+		MvcUriComponentsBuilder mvcBuilder = relativeTo(builder);
 		UriComponents uriComponents = mvcBuilder.withController(PersonControllerImpl.class).build();
 
 		assertEquals("http://example.org:9090/base/people", uriComponents.toString());
@@ -135,7 +136,31 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodNamePathVariable() {
+	public void usesForwardedHostAsHostIfHeaderIsSet() {
+		this.request.addHeader("X-Forwarded-Host", "somethingDifferent");
+		UriComponents uriComponents = fromController(PersonControllerImpl.class).build();
+
+		assertThat(uriComponents.toUriString(), startsWith("http://somethingDifferent"));
+	}
+
+	@Test
+	public void usesForwardedHostAndPortFromHeader() {
+		request.addHeader("X-Forwarded-Host", "foobar:8088");
+		UriComponents uriComponents = fromController(PersonControllerImpl.class).build();
+
+		assertThat(uriComponents.toUriString(), startsWith("http://foobar:8088"));
+	}
+
+	@Test
+	public void usesFirstHostOfXForwardedHost() {
+		request.addHeader("X-Forwarded-Host", "barfoo:8888, localhost:8088");
+		UriComponents uriComponents = fromController(PersonControllerImpl.class).build();
+
+		assertThat(uriComponents.toUriString(), startsWith("http://barfoo:8888"));
+	}
+
+	@Test
+	public void fromMethodNamePathVariable() {
 		UriComponents uriComponents = fromMethodName(ControllerWithMethods.class,
 				"methodWithPathVariable", "1").build();
 
@@ -143,7 +168,7 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodNameTypeLevelPathVariable() {
+	public void fromMethodNameTypeLevelPathVariable() {
 		this.request.setContextPath("/myapp");
 		UriComponents uriComponents = fromMethodName(
 				PersonsAddressesController.class, "getAddressesForCountry", "DE").buildAndExpand("1");
@@ -152,7 +177,7 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodNameTwoPathVariables() {
+	public void fromMethodNameTwoPathVariables() {
 		DateTime now = DateTime.now();
 		UriComponents uriComponents = fromMethodName(
 				ControllerWithMethods.class, "methodWithTwoPathVariables", 1, now).build();
@@ -161,7 +186,7 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodNameWithPathVarAndRequestParam() {
+	public void fromMethodNameWithPathVarAndRequestParam() {
 		UriComponents uriComponents = fromMethodName(
 				ControllerWithMethods.class, "methodForNextPage", "1", 10, 5).build();
 
@@ -179,21 +204,21 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test  // SPR-11391
-	public void testFromMethodNameTypeLevelPathVariableWithoutArgumentValue() {
+	public void fromMethodNameTypeLevelPathVariableWithoutArgumentValue() {
 		UriComponents uriComponents = fromMethodName(UserContactController.class, "showCreate", 123).build();
 
 		assertThat(uriComponents.getPath(), is("/user/123/contacts/create"));
 	}
 
 	@Test
-	public void testFromMethodNameNotMapped() {
+	public void fromMethodNameNotMapped() {
 		UriComponents uriComponents = fromMethodName(UnmappedController.class, "unmappedMethod").build();
 
 		assertThat(uriComponents.toUriString(), is("http://localhost/"));
 	}
 
 	@Test
-	public void testFromMethodNameWithCustomBaseUrlViaStaticCall() {
+	public void fromMethodNameWithCustomBaseUrlViaStaticCall() {
 		UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base");
 		UriComponents uriComponents = fromMethodName(builder, ControllerWithMethods.class,
 				"methodWithPathVariable", "1").build();
@@ -203,9 +228,9 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodNameWithCustomBaseUrlViaInstance() {
+	public void fromMethodNameWithCustomBaseUrlViaInstance() {
 		UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base");
-		MvcUriComponentsBuilder mvcBuilder = MvcUriComponentsBuilder.relativeTo(builder);
+		MvcUriComponentsBuilder mvcBuilder = relativeTo(builder);
 		UriComponents uriComponents = mvcBuilder.withMethodName(ControllerWithMethods.class,
 				"methodWithPathVariable", "1").build();
 
@@ -213,14 +238,8 @@ public class MvcUriComponentsBuilderTests {
 		assertEquals("http://example.org:9090/base", builder.toUriString());
 	}
 
-	@Test
-	public void testFromMethodNameWithMetaAnnotation() {
-		UriComponents uriComponents = fromMethodName(MetaAnnotationController.class, "handleInput").build();
-		assertThat(uriComponents.toUriString(), is("http://localhost/input"));
-	}
-
 	@Test  // SPR-14405
-	public void testFromMappingNameWithOptionalParam() {
+	public void fromMethodNameWithOptionalParam() {
 		UriComponents uriComponents = fromMethodName(ControllerWithMethods.class,
 				"methodWithOptionalParam", new Object[] {null}).build();
 
@@ -228,7 +247,14 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodCall() {
+	public void fromMethodNameWithMetaAnnotation() {
+		UriComponents uriComponents = fromMethodName(MetaAnnotationController.class, "handleInput").build();
+
+		assertThat(uriComponents.toUriString(), is("http://localhost/input"));
+	}
+
+	@Test
+	public void fromMethodCallPlain() {
 		UriComponents uriComponents = fromMethodCall(on(ControllerWithMethods.class).myMethod(null)).build();
 
 		assertThat(uriComponents.toUriString(), startsWith("http://localhost"));
@@ -236,7 +262,7 @@ public class MvcUriComponentsBuilderTests {
 	}
 
  	@Test
-	public void testFromMethodCallOnSubclass() {
+	public void fromMethodCallOnSubclass() {
 		UriComponents uriComponents = fromMethodCall(on(ExtendedController.class).myMethod(null)).build();
 
 		assertThat(uriComponents.toUriString(), startsWith("http://localhost"));
@@ -244,16 +270,15 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodCallWithTypeLevelUriVars() {
+	public void fromMethodCallWithTypeLevelUriVars() {
 		UriComponents uriComponents = fromMethodCall(
 				on(PersonsAddressesController.class).getAddressesForCountry("DE")).buildAndExpand(15);
 
 		assertThat(uriComponents.toUriString(), endsWith("/people/15/addresses/DE"));
 	}
 
-
 	@Test
-	public void testFromMethodCallWithPathVar() {
+	public void fromMethodCallWithPathVariable() {
 		UriComponents uriComponents = fromMethodCall(
 				on(ControllerWithMethods.class).methodWithPathVariable("1")).build();
 
@@ -262,7 +287,7 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodCallWithPathVarAndRequestParams() {
+	public void fromMethodCallWithPathVariableAndRequestParams() {
 		UriComponents uriComponents = fromMethodCall(
 				on(ControllerWithMethods.class).methodForNextPage("1", 10, 5)).build();
 
@@ -274,7 +299,7 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodCallWithPathVarAndMultiValueRequestParams() {
+	public void fromMethodCallWithPathVariableAndMultiValueRequestParams() {
 		UriComponents uriComponents = fromMethodCall(
 				on(ControllerWithMethods.class).methodWithMultiValueRequestParams("1", Arrays.asList(3, 7), 5)).build();
 
@@ -286,7 +311,7 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodCallWithCustomBaseUrlViaStaticCall() {
+	public void fromMethodCallWithCustomBaseUrlViaStaticCall() {
 		UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base");
 		UriComponents uriComponents = fromMethodCall(builder, on(ControllerWithMethods.class).myMethod(null)).build();
 
@@ -295,17 +320,49 @@ public class MvcUriComponentsBuilderTests {
 	}
 
 	@Test
-	public void testFromMethodCallWithCustomBaseUrlViaInstance() {
+	public void fromMethodCallWithCustomBaseUrlViaInstance() {
 		UriComponentsBuilder builder = UriComponentsBuilder.fromUriString("http://example.org:9090/base");
-		MvcUriComponentsBuilder mvcBuilder = MvcUriComponentsBuilder.relativeTo(builder);
+		MvcUriComponentsBuilder mvcBuilder = relativeTo(builder);
 		UriComponents result = mvcBuilder.withMethodCall(on(ControllerWithMethods.class).myMethod(null)).build();
 
 		assertEquals("http://example.org:9090/base/something/else", result.toString());
 		assertEquals("http://example.org:9090/base", builder.toUriString());
 	}
 
+	@Test  // SPR-16710
+	public void fromMethodCallWithModelAndViewReturnType() {
+		UriComponents uriComponents = fromMethodCall(
+				on(BookingControllerWithModelAndView.class).getBooking(21L)).buildAndExpand(42);
+
+		assertEquals("http://localhost/hotels/42/bookings/21", uriComponents.encode().toUri().toString());
+	}
+
+	@Test  // SPR-16710
+	public void fromMethodCallWithObjectReturnType() {
+		UriComponents uriComponents = fromMethodCall(
+				on(BookingControllerWithObject.class).getBooking(21L)).buildAndExpand(42);
+
+		assertEquals("http://localhost/hotels/42/bookings/21", uriComponents.encode().toUri().toString());
+	}
+
+	@Test(expected = IllegalStateException.class)  // SPR-16710
+	public void fromMethodCallWithStringReturnType() {
+		UriComponents uriComponents = fromMethodCall(
+				on(BookingControllerWithString.class).getBooking(21L)).buildAndExpand(42);
+
+		assertEquals("http://localhost/hotels/42/bookings/21", uriComponents.encode().toUri().toString());
+	}
+
+	@Test  // SPR-16710
+	public void fromMethodNameWithStringReturnType() {
+		UriComponents uriComponents = fromMethodName(
+				BookingControllerWithString.class, "getBooking", 21L).buildAndExpand(42);
+
+		assertEquals("http://localhost/hotels/42/bookings/21", uriComponents.encode().toUri().toString());
+	}
+
 	@Test
-	public void testFromMappingName() {
+	public void fromMappingNamePlain() {
 		AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
 		context.setServletContext(new MockServletContext());
 		context.register(WebConfig.class);
@@ -317,12 +374,12 @@ public class MvcUriComponentsBuilderTests {
 		this.request.setContextPath("/base");
 
 		String mappingName = "PAC#getAddressesForCountry";
-		String url = MvcUriComponentsBuilder.fromMappingName(mappingName).arg(0, "DE").buildAndExpand(123);
+		String url = fromMappingName(mappingName).arg(0, "DE").buildAndExpand(123);
 		assertEquals("/base/people/123/addresses/DE", url);
 	}
 
 	@Test
-	public void testFromMappingNameWithCustomBaseUrl() {
+	public void fromMappingNameWithCustomBaseUrl() {
 		AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
 		context.setServletContext(new MockServletContext());
 		context.register(WebConfig.class);
@@ -331,42 +388,11 @@ public class MvcUriComponentsBuilderTests {
 		this.request.setAttribute(DispatcherServlet.WEB_APPLICATION_CONTEXT_ATTRIBUTE, context);
 
 		UriComponentsBuilder baseUrl = UriComponentsBuilder.fromUriString("http://example.org:9999/base");
-		MvcUriComponentsBuilder mvcBuilder = MvcUriComponentsBuilder.relativeTo(baseUrl);
+		MvcUriComponentsBuilder mvcBuilder = relativeTo(baseUrl);
 		String url = mvcBuilder.withMappingName("PAC#getAddressesForCountry").arg(0, "DE").buildAndExpand(123);
 		assertEquals("http://example.org:9999/base/people/123/addresses/DE", url);
 	}
 
-	@Test
-	public void usesForwardedHostAsHostIfHeaderIsSet() {
-		this.request.addHeader("X-Forwarded-Host", "somethingDifferent");
-		UriComponents uriComponents = fromController(PersonControllerImpl.class).build();
-
-		assertThat(uriComponents.toUriString(), startsWith("http://somethingDifferent"));
-	}
-
-	@Test
-	public void usesForwardedHostAndPortFromHeader() {
-		request.addHeader("X-Forwarded-Host", "foobar:8088");
-		UriComponents uriComponents = fromController(PersonControllerImpl.class).build();
-
-		assertThat(uriComponents.toUriString(), startsWith("http://foobar:8088"));
-	}
-
-	@Test
-	public void usesFirstHostOfXForwardedHost() {
-		request.addHeader("X-Forwarded-Host", "barfoo:8888, localhost:8088");
-		UriComponents uriComponents = fromController(PersonControllerImpl.class).build();
-
-		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,7 +542,18 @@ public class MvcUriComponentsBuilderTests {
 
 	@Controller
 	@RequestMapping("/hotels/{hotel}")
-	public class BookingController {
+	static class BookingControllerWithModelAndView {
+
+		@GetMapping("/bookings/{booking}")
+		public ModelAndView getBooking(@PathVariable Long booking) {
+			return new ModelAndView("url");
+		}
+	}
+
+
+	@Controller
+	@RequestMapping("/hotels/{hotel}")
+	static class BookingControllerWithObject {
 
 		@GetMapping("/bookings/{booking}")
 		public Object getBooking(@PathVariable Long booking) {
@@ -524,4 +561,15 @@ public class MvcUriComponentsBuilderTests {
 		}
 	}
 
+
+	@Controller
+	@RequestMapping("/hotels/{hotel}")
+	static class BookingControllerWithString {
+
+		@GetMapping("/bookings/{booking}")
+		public String getBooking(@PathVariable Long booking) {
+			return "url";
+		}
+	}
+
 }

src/docs/asciidoc/web/webmvc.adoc

@@ -3098,7 +3098,8 @@ per request, and also provides an option to remove and ignore such headers.
 [[mvc-links-to-controllers]]
 === Links to controllers
 
-Spring MVC provides a mechanism to prepare links to controller methods. For example:
+Spring MVC provides a mechanism to prepare links to controller methods. For example,
+the following MVC controller easily allows for link creation:
 
 [source,java,indent=0]
 [subs="verbatim,quotes"]
@@ -3108,7 +3109,7 @@ Spring MVC provides a mechanism to prepare links to controller methods. For exam
 	public class BookingController {
 
 		@GetMapping("/bookings/{booking}")
-		public String getBooking(@PathVariable Long booking) {
+		public ModelAndView getBooking(@PathVariable Long booking) {
 			// ...
 		}
 	}
@@ -3145,6 +3146,16 @@ akin to mock testing through proxies to avoid referring to the controller method
 	URI uri = uriComponents.encode().toUri();
 ----
 
+[NOTE]
+====
+Controller method signatures are limited in their design when supposed to be usable for
+link creation with `fromMethodCall`. Aside from needing a proper parameter signature,
+there is a technical limitation on the return type: namely generating a runtime proxy
+for link builder invocations, so the return type must not be `final`. In particular,
+the common `String` return type for view names does not work here; use `ModelAndView`
+or even plain `Object` (with a `String` return value) instead.
+====
+
 The above examples use static methods in `MvcUriComponentsBuilder`. Internally they rely
 on `ServletUriComponentsBuilder` to prepare a base URL from the scheme, host, port,
 context path and servlet path of the current request. This works well in most cases,