Minor fixes: UriComponentsBuilder, UriComponents, docs

After the latest changes, two small fixes in the clone method to copy
the encode flag, and in the encodeUriTemplate method to account for
possible null query params.

Improvements in the URI encoding section.

Issue: SPR-17039, SPR-17027

spring-web/src/main/java/org/springframework/web/util/HierarchicalUriComponents.java

@@ -100,6 +100,9 @@ final class HierarchicalUriComponents extends UriComponents {
 		}
 	};
 
+	private static final MultiValueMap<String, String> EMPTY_QUERY_PARAMS =
+			CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<>(0));
+
 
 	@Nullable
 	private final String userInfo;
@@ -127,22 +130,21 @@ final class HierarchicalUriComponents extends UriComponents {
 	 * @param host the host
 	 * @param port the port
 	 * @param path the path
-	 * @param queryParams the query parameters
+	 * @param query the query parameters
 	 * @param fragment the fragment
 	 * @param encoded whether the components are already encoded
 	 */
 	HierarchicalUriComponents(@Nullable String scheme, @Nullable String fragment, @Nullable String userInfo,
 			@Nullable String host, @Nullable String port, @Nullable PathComponent path,
-			@Nullable MultiValueMap<String, String> queryParams, boolean encoded) {
+			@Nullable MultiValueMap<String, String> query, boolean encoded) {
 
 		super(scheme, fragment);
 
 		this.userInfo = userInfo;
 		this.host = host;
 		this.port = port;
-		this.path = (path != null ? path : NULL_PATH_COMPONENT);
-		this.queryParams = CollectionUtils.unmodifiableMultiValueMap(
-				queryParams != null ? queryParams : new LinkedMultiValueMap<>(0));
+		this.path = path != null ? path : NULL_PATH_COMPONENT;
+		this.queryParams = query != null ? CollectionUtils.unmodifiableMultiValueMap(query) : EMPTY_QUERY_PARAMS;
 		this.encodeState = encoded ? EncodeState.FULLY_ENCODED : EncodeState.RAW;
 
 		// Check for illegal characters..
@@ -151,19 +153,18 @@ final class HierarchicalUriComponents extends UriComponents {
 		}
 	}
 
-	private HierarchicalUriComponents(@Nullable String scheme, @Nullable String fragment, @Nullable String userInfo,
-			@Nullable String host, @Nullable String port, @Nullable PathComponent path,
-			@Nullable MultiValueMap<String, String> queryParams, EncodeState encodeState,
-			@Nullable UnaryOperator<String> variableEncoder) {
+	private HierarchicalUriComponents(@Nullable String scheme, @Nullable String fragment,
+			@Nullable String userInfo, @Nullable String host, @Nullable String port,
+			PathComponent path, MultiValueMap<String, String> queryParams,
+			EncodeState encodeState, @Nullable UnaryOperator<String> variableEncoder) {
 
 		super(scheme, fragment);
 
 		this.userInfo = userInfo;
 		this.host = host;
 		this.port = port;
-		this.path = (path != null ? path : NULL_PATH_COMPONENT);
-		this.queryParams = CollectionUtils.unmodifiableMultiValueMap(
-				queryParams != null ? queryParams : new LinkedMultiValueMap<>(0));
+		this.path = path;
+		this.queryParams = queryParams;
 		this.encodeState = encodeState;
 		this.variableEncoder = variableEncoder;
 	}
@@ -254,6 +255,11 @@ final class HierarchicalUriComponents extends UriComponents {
 
 	// Encoding
 
+	/**
+	 * Identical to {@link #encode()} but skipping over URI variable placeholders.
+	 * Also {@link #variableEncoder} is initialized with the given charset for
+	 * use later when URI variables are expanded.
+	 */
 	HierarchicalUriComponents encodeTemplate(Charset charset) {
 		if (this.encodeState.isEncoded()) {
 			return this;
@@ -268,10 +274,10 @@ final class HierarchicalUriComponents extends UriComponents {
 		String userInfoTo = (getUserInfo() != null ? encoder.apply(getUserInfo(), Type.USER_INFO) : null);
 		String hostTo = (getHost() != null ? encoder.apply(getHost(), getHostType()) : null);
 		PathComponent pathTo = this.path.encode(encoder);
-		MultiValueMap<String, String> paramsTo = encodeQueryParams(encoder);
+		MultiValueMap<String, String> queryParamsTo = encodeQueryParams(encoder);
 
 		return new HierarchicalUriComponents(schemeTo, fragmentTo, userInfoTo,
-				hostTo, this.port, pathTo, paramsTo, EncodeState.TEMPLATE_ENCODED, this.variableEncoder);
+				hostTo, this.port, pathTo, queryParamsTo, EncodeState.TEMPLATE_ENCODED, this.variableEncoder);
 	}
 
 	@Override
@@ -287,10 +293,10 @@ final class HierarchicalUriComponents extends UriComponents {
 		String hostTo = (this.host != null ? encodeUriComponent(this.host, charset, getHostType()) : null);
 		BiFunction<String, Type, String> encoder = (s, type) -> encodeUriComponent(s, charset, type);
 		PathComponent pathTo = this.path.encode(encoder);
-		MultiValueMap<String, String> paramsTo = encodeQueryParams(encoder);
+		MultiValueMap<String, String> queryParamsTo = encodeQueryParams(encoder);
 
 		return new HierarchicalUriComponents(schemeTo, fragmentTo, userInfoTo,
-				hostTo, this.port, pathTo, paramsTo, EncodeState.FULLY_ENCODED, null);
+				hostTo, this.port, pathTo, queryParamsTo, EncodeState.FULLY_ENCODED, null);
 	}
 
 	private MultiValueMap<String, String> encodeQueryParams(BiFunction<String, Type, String> encoder) {
@@ -300,11 +306,11 @@ final class HierarchicalUriComponents extends UriComponents {
 			String name = encoder.apply(key, Type.QUERY_PARAM);
 			List<String> encodedValues = new ArrayList<>(values.size());
 			for (String value : values) {
-				encodedValues.add(encoder.apply(value, Type.QUERY_PARAM));
+				encodedValues.add(value != null ? encoder.apply(value, Type.QUERY_PARAM) : null);
 			}
 			result.put(name, encodedValues);
 		});
-		return result;
+		return CollectionUtils.unmodifiableMultiValueMap(result);
 	}
 
 	/**
@@ -428,10 +434,10 @@ final class HierarchicalUriComponents extends UriComponents {
 		String hostTo = expandUriComponent(this.host, uriVariables, this.variableEncoder);
 		String portTo = expandUriComponent(this.port, uriVariables, this.variableEncoder);
 		PathComponent pathTo = this.path.expand(uriVariables, this.variableEncoder);
-		MultiValueMap<String, String> paramsTo = expandQueryParams(uriVariables);
+		MultiValueMap<String, String> queryParamsTo = expandQueryParams(uriVariables);
 
 		return new HierarchicalUriComponents(schemeTo, fragmentTo, userInfoTo,
-				hostTo, portTo, pathTo, paramsTo, this.encodeState, this.variableEncoder);
+				hostTo, portTo, pathTo, queryParamsTo, this.encodeState, this.variableEncoder);
 	}
 
 	private MultiValueMap<String, String> expandQueryParams(UriTemplateVariables variables) {
@@ -446,7 +452,7 @@ final class HierarchicalUriComponents extends UriComponents {
 			}
 			result.put(name, expandedValues);
 		});
-		return result;
+		return CollectionUtils.unmodifiableMultiValueMap(result);
 	}
 
 	@Override

spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java

@@ -150,6 +150,8 @@ public class UriComponentsBuilder implements UriBuilder, Cloneable {
 		this.pathBuilder = other.pathBuilder.cloneBuilder();
 		this.queryParams.putAll(other.queryParams);
 		this.fragment = other.fragment;
+		this.encodeTemplate = other.encodeTemplate;
+		this.charset = other.charset;
 	}
 
 

spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java

@@ -753,10 +753,10 @@ public class UriComponentsBuilderTests {
 	@Test
 	public void testClone() {
 		UriComponentsBuilder builder1 = UriComponentsBuilder.newInstance();
-		builder1.scheme("http").host("e1.com").path("/p1").pathSegment("ps1").queryParam("q1").fragment("f1");
+		builder1.scheme("http").host("e1.com").path("/p1").pathSegment("ps1").queryParam("q1").fragment("f1").encode();
 
 		UriComponentsBuilder builder2 = (UriComponentsBuilder) builder1.clone();
-		builder2.scheme("https").host("e2.com").path("p2").pathSegment("ps2").queryParam("q2").fragment("f2");
+		builder2.scheme("https").host("e2.com").path("p2").pathSegment("{ps2}").queryParam("q2").fragment("f2");
 
 		UriComponents result1 = builder1.build();
 		assertEquals("http", result1.getScheme());
@@ -765,10 +765,10 @@ public class UriComponentsBuilderTests {
 		assertEquals("q1", result1.getQuery());
 		assertEquals("f1", result1.getFragment());
 
-		UriComponents result2 = builder2.build();
+		UriComponents result2 = builder2.buildAndExpand("ps2;a");
 		assertEquals("https", result2.getScheme());
 		assertEquals("e2.com", result2.getHost());
-		assertEquals("/p1/ps1/p2/ps2", result2.getPath());
+		assertEquals("/p1/ps1/p2/ps2%3Ba", result2.getPath());
 		assertEquals("q1&q2", result2.getQuery());
 		assertEquals("f2", result2.getFragment());
 	}

src/docs/asciidoc/web/web-uris.adoc

@@ -8,9 +8,8 @@
 [source,java,indent=0]
 [subs="verbatim,quotes"]
 ----
-	String uriTemplate = "http://example.com/hotels/{hotel}";
-
-	UriComponents uriComponents = UriComponentsBuilder.fromUriString(uriTemplate)  // <1>
+	UriComponents uriComponents = UriComponentsBuilder
+			.fromUriString("http://example.com/hotels/{hotel}")  // <1>
 			.queryParam("q", "{q}")  // <2>
 			.encode() // <3>
 			.build(); // <4>
@@ -23,18 +22,40 @@
 <4> Build a `UriComponents`.
 <5> Expand variables, and obtain the `URI`.
 
-The above can also be done in shorthand form:
+The above can be consolidated into one chain and shortened with `buildAndExpand`:
 
 [source,java,indent=0]
 [subs="verbatim,quotes"]
 ----
-	URI uri = UriComponentsBuilder.fromUriString(uriTemplate)
+	URI uri = UriComponentsBuilder
+			.fromUriString("http://example.com/hotels/{hotel}")
 			.queryParam("q", "{q}")
 			.encode()
 			.buildAndExpand("Westin", "123")
 			.toUri();
 ----
 
+It can be shortened further by going directly to URI (which implies encoding):
+
+[source,java,indent=0]
+[subs="verbatim,quotes"]
+----
+	URI uri = UriComponentsBuilder
+			.fromUriString("http://example.com/hotels/{hotel}")
+			.queryParam("q", "{q}")
+			.build("Westin", "123");
+----
+
+Or shorter further yet, with a full URI template:
+
+[source,java,indent=0]
+[subs="verbatim,quotes"]
+----
+	URI uri = UriComponentsBuilder
+			.fromUriString("http://example.com/hotels/{hotel}?q={q}")
+			.build("Westin", "123");
+----
+
 
 [[web-uribuilder]]
 = UriBuilder
@@ -125,15 +146,34 @@ Example usage using option 1:
 [source,java,indent=0]
 [subs="verbatim,quotes"]
 ----
-	UriComponentsBuilder.fromPath("/hotel list/{city}")
+URI uri = UriComponentsBuilder.fromPath("/hotel list/{city}")
 			.queryParam("q", "{q}")
 			.encode()
-			.buildAndexpand("New York", "foo+bar")
-			.toUriString();
+			.buildAndExpand("New York", "foo+bar")
+			.toUri();
 
 	// Result is "/hotel%20list/New%20York?foo%2Bbar"
 ----
 
+The above can be shortened by going directly to URI (which implies encoding):
+
+[source,java,indent=0]
+[subs="verbatim,quotes"]
+----
+URI uri = UriComponentsBuilder.fromPath("/hotel list/{city}")
+			.queryParam("q", "{q}")
+			.build("New York", "foo+bar")
+----
+
+Or shorter further yet, with a full URI template:
+
+[source,java,indent=0]
+[subs="verbatim,quotes"]
+----
+URI uri = UriComponentsBuilder.fromPath("/hotel list/{city}?q={q}")
+			.build("New York", "foo+bar")
+----
+
 The `WebClient` and the `RestTemplate` expand and encode URI templates internally through
 the `UriBuilderFactory` strategy. Both can be configured with a custom strategy: