From ad65119a2c6b3b1bd60904fddc80320cb89dc752 Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@pivotal.io>
Date: Thu, 4 Dec 2014 10:30:14 -0500
Subject: [PATCH] Polish ServletUriComponentsBuilder

---
 .../support/ServletUriComponentsBuilder.java  | 55 ++++++-----
 .../ServletUriComponentsBuilderTests.java     | 92 +++++++++----------
 2 files changed, 76 insertions(+), 71 deletions(-)

diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java
index 516a50249df..057d827d1b0 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/support/ServletUriComponentsBuilder.java
@@ -52,22 +52,18 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder {
 	}
 
 	/**
-	 * Prepare a builder from the host, port, scheme, and context path of
-	 * an HttpServletRequest.
+	 * Prepare a builder from the host, port, scheme, and context path of the
+	 * given HttpServletRequest.
 	 */
 	public static ServletUriComponentsBuilder fromContextPath(HttpServletRequest request) {
-		String path = request.getContextPath();
-		path = prependForwardedPrefix(request, path);
-		ServletUriComponentsBuilder builder = fromRequest(request);
-		builder.replacePath(path);
-		builder.replaceQuery(null);
+		ServletUriComponentsBuilder builder = initFromRequest(request);
+		builder.replacePath(prependForwardedPrefix(request, request.getContextPath()));
 		return builder;
 	}
 
 	/**
 	 * Prepare a builder from the host, port, scheme, context path, and
-	 * servlet mapping of an HttpServletRequest. The results may vary depending
-	 * on the type of servlet mapping used.
+	 * servlet mapping of the given HttpServletRequest.
 	 *
 	 * <p>If the servlet is mapped by name, e.g. {@code "/main/*"}, the path
 	 * will end with "/main". If the servlet is mapped otherwise, e.g.
@@ -83,12 +79,12 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder {
 	}
 
 	/**
-	 * Prepare a builder from the host, port, scheme, and path of
-	 * an HttpServletRequest.
+	 * Prepare a builder from the host, port, scheme, and path (but not the query)
+	 * of the HttpServletRequest.
 	 */
 	public static ServletUriComponentsBuilder fromRequestUri(HttpServletRequest request) {
-		ServletUriComponentsBuilder builder = fromRequest(request);
-		builder.replaceQuery(null);
+		ServletUriComponentsBuilder builder = initFromRequest(request);
+		builder.initPath(prependForwardedPrefix(request, request.getRequestURI()));
 		return builder;
 	}
 
@@ -97,6 +93,16 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder {
 	 * query string of an HttpServletRequest.
 	 */
 	public static ServletUriComponentsBuilder fromRequest(HttpServletRequest request) {
+		ServletUriComponentsBuilder builder = initFromRequest(request);
+		builder.initPath(prependForwardedPrefix(request, request.getRequestURI()));
+		builder.query(request.getQueryString());
+		return builder;
+	}
+
+	/**
+	 * Initialize a builder with a scheme, host,and port (but not path and query).
+	 */
+	private static ServletUriComponentsBuilder initFromRequest(HttpServletRequest request) {
 		String scheme = request.getScheme();
 		String host = request.getServerName();
 		int port = request.getServerPort();
@@ -126,20 +132,20 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder {
 			scheme = protocolHeader;
 		}
 
-		String path = request.getRequestURI();
-		path = prependForwardedPrefix(request, path);
-
 		ServletUriComponentsBuilder builder = new ServletUriComponentsBuilder();
 		builder.scheme(scheme);
 		builder.host(host);
 		if (scheme.equals("http") && port != 80 || scheme.equals("https") && port != 443) {
 			builder.port(port);
 		}
-		builder.initPath(path);
-		builder.query(request.getQueryString());
 		return builder;
 	}
 
+	private void initPath(String path) {
+		this.originalPath = path;
+		replacePath(path);
+	}
+
 	private static String prependForwardedPrefix(HttpServletRequest request, String path) {
 		String prefix = request.getHeader("X-Forwarded-Prefix");
 		if (StringUtils.hasText(prefix)) {
@@ -148,6 +154,9 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder {
 		return path;
 	}
 
+
+	// Alternative methods relying on RequestContextHolder to find the request
+
 	/**
 	 * Same as {@link #fromContextPath(HttpServletRequest)} except the
 	 * request is obtained through {@link RequestContextHolder}.
@@ -180,8 +189,9 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder {
 		return fromRequest(getCurrentRequest());
 	}
 
+
 	/**
-	 * Obtain the request through {@link RequestContextHolder}.
+	 * Get the request through {@link RequestContextHolder}.
 	 */
 	protected static HttpServletRequest getCurrentRequest() {
 		RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
@@ -192,13 +202,8 @@ public class ServletUriComponentsBuilder extends UriComponentsBuilder {
 		return servletRequest;
 	}
 
-	private void initPath(String path) {
-		this.originalPath = path;
-		replacePath(path);
-	}
-
 	/**
-	 * Removes any path extension from the {@link HttpServletRequest#getRequestURI()
+	 * Remove any path extension from the {@link HttpServletRequest#getRequestURI()
 	 * requestURI}. This method must be invoked before any calls to {@link #path(String)}
 	 * or {@link #pathSegment(String...)}.
 	 * <pre>
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/ServletUriComponentsBuilderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/ServletUriComponentsBuilderTests.java
index 0d4cf49e1ba..a3879dc22f3 100644
--- a/spring-webmvc/src/test/java/org/springframework/web/servlet/support/ServletUriComponentsBuilderTests.java
+++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/support/ServletUriComponentsBuilderTests.java
@@ -16,16 +16,16 @@
 
 package org.springframework.web.servlet.support;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
 import org.junit.Before;
 import org.junit.Test;
-
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.util.UriComponents;
 
-import static org.junit.Assert.*;
-
 /**
  * Unit tests for
  * {@link org.springframework.web.servlet.support.ServletUriComponentsBuilder}.
@@ -47,48 +47,48 @@ public class ServletUriComponentsBuilderTests {
 
 	@Test
 	public void fromRequest() {
-		request.setRequestURI("/mvc-showcase/data/param");
-		request.setQueryString("foo=123");
-		String result = ServletUriComponentsBuilder.fromRequest(request).build().toUriString();
+		this.request.setRequestURI("/mvc-showcase/data/param");
+		this.request.setQueryString("foo=123");
+		String result = ServletUriComponentsBuilder.fromRequest(this.request).build().toUriString();
 		assertEquals("http://localhost/mvc-showcase/data/param?foo=123", result);
 	}
 
 	@Test
 	public void fromRequestEncodedPath() {
-		request.setRequestURI("/mvc-showcase/data/foo%20bar");
-		String result = ServletUriComponentsBuilder.fromRequest(request).build().toUriString();
+		this.request.setRequestURI("/mvc-showcase/data/foo%20bar");
+		String result = ServletUriComponentsBuilder.fromRequest(this.request).build().toUriString();
 		assertEquals("http://localhost/mvc-showcase/data/foo%20bar", result);
 	}
 
 	@Test
 	public void fromRequestAtypicalHttpPort() {
-		request.setServerPort(8080);
-		String result = ServletUriComponentsBuilder.fromRequest(request).build().toUriString();
+		this.request.setServerPort(8080);
+		String result = ServletUriComponentsBuilder.fromRequest(this.request).build().toUriString();
 		assertEquals("http://localhost:8080", result);
 	}
 
 	@Test
 	public void fromRequestAtypicalHttpsPort() {
-		request.setScheme("https");
-		request.setServerPort(9043);
-		String result = ServletUriComponentsBuilder.fromRequest(request).build().toUriString();
+		this.request.setScheme("https");
+		this.request.setServerPort(9043);
+		String result = ServletUriComponentsBuilder.fromRequest(this.request).build().toUriString();
 		assertEquals("https://localhost:9043", result);
 	}
 
 	@Test
 	public void fromRequestUri() {
-		request.setRequestURI("/mvc-showcase/data/param");
-		request.setQueryString("foo=123");
-		String result = ServletUriComponentsBuilder.fromRequestUri(request).build().toUriString();
+		this.request.setRequestURI("/mvc-showcase/data/param");
+		this.request.setQueryString("foo=123");
+		String result = ServletUriComponentsBuilder.fromRequestUri(this.request).build().toUriString();
 		assertEquals("http://localhost/mvc-showcase/data/param", result);
 	}
 
 	@Test
 	public void fromRequestWithForwardedHost() {
-		request.addHeader("X-Forwarded-Host", "anotherHost");
-		request.setRequestURI("/mvc-showcase/data/param");
-		request.setQueryString("foo=123");
-		String result = ServletUriComponentsBuilder.fromRequest(request).build().toUriString();
+		this.request.addHeader("X-Forwarded-Host", "anotherHost");
+		this.request.setRequestURI("/mvc-showcase/data/param");
+		this.request.setQueryString("foo=123");
+		String result = ServletUriComponentsBuilder.fromRequest(this.request).build().toUriString();
 		assertEquals("http://anotherHost/mvc-showcase/data/param?foo=123", result);
 	}
 
@@ -96,10 +96,10 @@ public class ServletUriComponentsBuilderTests {
 
 	@Test
 	public void fromRequestWithForwardedHostIncludingPort() {
-		request.addHeader("X-Forwarded-Host", "webtest.foo.bar.com:443");
-		request.setRequestURI("/mvc-showcase/data/param");
-		request.setQueryString("foo=123");
-		UriComponents result = ServletUriComponentsBuilder.fromRequest(request).build();
+		this.request.addHeader("X-Forwarded-Host", "webtest.foo.bar.com:443");
+		this.request.setRequestURI("/mvc-showcase/data/param");
+		this.request.setQueryString("foo=123");
+		UriComponents result = ServletUriComponentsBuilder.fromRequest(this.request).build();
 
 		assertEquals("webtest.foo.bar.com", result.getHost());
 		assertEquals(443, result.getPort());
@@ -132,7 +132,7 @@ public class ServletUriComponentsBuilderTests {
 	public void fromRequestWithForwardedHostWithDefaultPort() {
 		this.request.setServerPort(10080);
 		this.request.addHeader("X-Forwarded-Host", "example.org");
-		UriComponents result = ServletUriComponentsBuilder.fromRequest(request).build();
+		UriComponents result = ServletUriComponentsBuilder.fromRequest(this.request).build();
 
 		assertEquals("example.org", result.getHost());
 		assertEquals("should have used the default port of the forwarded request", -1, result.getPort());
@@ -143,7 +143,7 @@ public class ServletUriComponentsBuilderTests {
 		this.request.setServerPort(10080);
 		this.request.addHeader("X-Forwarded-Proto", "https");
 		this.request.addHeader("X-Forwarded-Host", "example.org");
-		UriComponents result = ServletUriComponentsBuilder.fromRequest(request).build();
+		UriComponents result = ServletUriComponentsBuilder.fromRequest(this.request).build();
 
 		assertEquals("example.org", result.getHost());
 		assertEquals("should have derived scheme from header", "https", result.getScheme());
@@ -154,7 +154,7 @@ public class ServletUriComponentsBuilderTests {
 	public void fromRequestWithForwardedPrefix() {
 		this.request.setRequestURI("/bar");
 		this.request.addHeader("X-Forwarded-Prefix", "/foo");
-		UriComponents result = ServletUriComponentsBuilder.fromRequest(request).build();
+		UriComponents result = ServletUriComponentsBuilder.fromRequest(this.request).build();
 
 		assertEquals("http://localhost/foo/bar", result.toUriString());
 	}
@@ -163,51 +163,51 @@ public class ServletUriComponentsBuilderTests {
 	public void fromRequestWithForwardedPrefixTrailingSlash() {
 		this.request.setRequestURI("/bar");
 		this.request.addHeader("X-Forwarded-Prefix", "/foo/");
-		UriComponents result = ServletUriComponentsBuilder.fromRequest(request).build();
+		UriComponents result = ServletUriComponentsBuilder.fromRequest(this.request).build();
 
 		assertEquals("http://localhost/foo/bar", result.toUriString());
 	}
 
 	@Test
 	public void fromContextPath() {
-		request.setRequestURI("/mvc-showcase/data/param");
-		request.setQueryString("foo=123");
-		String result = ServletUriComponentsBuilder.fromContextPath(request).build().toUriString();
+		this.request.setRequestURI("/mvc-showcase/data/param");
+		this.request.setQueryString("foo=123");
+		String result = ServletUriComponentsBuilder.fromContextPath(this.request).build().toUriString();
 		assertEquals("http://localhost/mvc-showcase", result);
 	}
 
 	@Test
 	public void fromContextPathWithForwardedPrefix() {
-		request.addHeader("X-Forwarded-Prefix", "/prefix");
-		request.setContextPath("/mvc-showcase");
-		request.setRequestURI("/mvc-showcase/simple");
-		String result = ServletUriComponentsBuilder.fromContextPath(request).build().toUriString();
+		this.request.addHeader("X-Forwarded-Prefix", "/prefix");
+		this.request.setContextPath("/mvc-showcase");
+		this.request.setRequestURI("/mvc-showcase/simple");
+		String result = ServletUriComponentsBuilder.fromContextPath(this.request).build().toUriString();
 		assertEquals("http://localhost/prefix/mvc-showcase", result);
 	}
 
 	@Test
 	public void fromServletMapping() {
-		request.setRequestURI("/mvc-showcase/app/simple");
-		request.setServletPath("/app");
-		request.setQueryString("foo=123");
-		String result = ServletUriComponentsBuilder.fromServletMapping(request).build().toUriString();
+		this.request.setRequestURI("/mvc-showcase/app/simple");
+		this.request.setServletPath("/app");
+		this.request.setQueryString("foo=123");
+		String result = ServletUriComponentsBuilder.fromServletMapping(this.request).build().toUriString();
 		assertEquals("http://localhost/mvc-showcase/app", result);
 	}
 
 	@Test
 	public void fromServletMappingWithForwardedPrefix() {
-		request.addHeader("X-Forwarded-Prefix", "/prefix");
-		request.setContextPath("/mvc-showcase");
-		request.setServletPath("/app");
-		request.setRequestURI("/mvc-showcase/app/simple");
-		String result = ServletUriComponentsBuilder.fromServletMapping(request).build().toUriString();
+		this.request.addHeader("X-Forwarded-Prefix", "/prefix");
+		this.request.setContextPath("/mvc-showcase");
+		this.request.setServletPath("/app");
+		this.request.setRequestURI("/mvc-showcase/app/simple");
+		String result = ServletUriComponentsBuilder.fromServletMapping(this.request).build().toUriString();
 		assertEquals("http://localhost/prefix/mvc-showcase/app", result);
 	}
 
 	@Test
 	public void fromCurrentRequest() {
-		request.setRequestURI("/mvc-showcase/data/param");
-		request.setQueryString("foo=123");
+		this.request.setRequestURI("/mvc-showcase/data/param");
+		this.request.setQueryString("foo=123");
 		RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(this.request));
 		try {
 			String result = ServletUriComponentsBuilder.fromCurrentRequest().build().toUriString();