Polishing contribution

Closes gh-25951
spring-projects · Oct 23, 2020 · 5644a7a · 5644a7a
1 parent 7af7264
commit 5644a7a
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 58 deletions.
diff --git a/spring-web/src/main/java/org/springframework/web/util/DefaultUriBuilderFactory.java b/spring-web/src/main/java/org/springframework/web/util/DefaultUriBuilderFactory.java
@@ -336,32 +336,32 @@ public DefaultUriBuilder queryParam(String name, Object... values) {
 		}
 
 		@Override
-		public DefaultUriBuilder queryParamIfPresent(String name, Optional<?> optionalValue) {
-			this.uriComponentsBuilder.queryParamIfPresent(name, optionalValue);
+		public DefaultUriBuilder queryParam(String name, @Nullable Collection<?> values) {
+			this.uriComponentsBuilder.queryParam(name, values);
 			return this;
 		}
 
 		@Override
-		public DefaultUriBuilder queryParam(String name, @Nullable Collection<?> values) {
-			this.uriComponentsBuilder.queryParam(name, values);
+		public DefaultUriBuilder queryParamIfPresent(String name, Optional<?> value) {
+			this.uriComponentsBuilder.queryParamIfPresent(name, value);
 			return this;
 		}
 
 		@Override
-		public DefaultUriBuilder replaceQueryParam(String name, Object... values) {
-			this.uriComponentsBuilder.replaceQueryParam(name, values);
+		public DefaultUriBuilder queryParams(MultiValueMap<String, String> params) {
+			this.uriComponentsBuilder.queryParams(params);
 			return this;
 		}
 
 		@Override
-		public DefaultUriBuilder replaceQueryParam(String name, @Nullable Collection<?> values) {
+		public DefaultUriBuilder replaceQueryParam(String name, Object... values) {
 			this.uriComponentsBuilder.replaceQueryParam(name, values);
 			return this;
 		}
 
 		@Override
-		public DefaultUriBuilder queryParams(MultiValueMap<String, String> params) {
-			this.uriComponentsBuilder.queryParams(params);
+		public DefaultUriBuilder replaceQueryParam(String name, @Nullable Collection<?> values) {
+			this.uriComponentsBuilder.replaceQueryParam(name, values);
 			return this;
 		}
 

diff --git a/spring-web/src/main/java/org/springframework/web/util/UriBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriBuilder.java
@@ -186,15 +186,6 @@ public interface UriBuilder {
 	 */
 	UriBuilder queryParam(String name, Object... values);
 
-	/**
-	 * Delegates to {@link #queryParam(String, Object...)} or {@link #queryParam(String, Object...)} if and only if optionalValue has a value.
-	 * No action will be taken, and the query parameter name will not be added, if optionalValue is empty.
-	 * @param name the query parameter name
-	 * @param optionalValue an Optional, either empty or holding the query parameter value.
-	 * @return
-	 */
-	UriBuilder queryParamIfPresent(String name, Optional<?> optionalValue);
-
 	/**
 	 * Variant of {@link #queryParam(String, Object...)} with a Collection.
 	 * <p><strong>Note: </strong> please, review the Javadoc of
@@ -207,6 +198,16 @@ public interface UriBuilder {
 	 */
 	UriBuilder queryParam(String name, @Nullable Collection<?> values);
 
+	/**
+	 * Delegates to either {@link #queryParam(String, Object...)} or
+	 * {@link #queryParam(String, Collection)} if the given {@link Optional} has
+	 * a value, or else if it is empty, no query parameter is added at all.
+	 * @param name the query parameter name
+	 * @param value an Optional, either empty or holding the query parameter value.
+	 * @since 5.3
+	 */
+	UriBuilder queryParamIfPresent(String name, Optional<?> value);
+
 	/**
 	 * Add multiple query parameters and values.
 	 * <p><strong>Note: </strong> please, review the Javadoc of

diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java
@@ -710,24 +710,23 @@ public UriComponentsBuilder queryParam(String name, Object... values) {
 	}
 
 	@Override
-	public UriComponentsBuilder queryParamIfPresent(String name, Optional<?> optionalValue) {
-		if (optionalValue.isPresent()) {
-			Object value = optionalValue.get();
-			if (value instanceof Collection) {
-				queryParam(name, (Collection) value);
+	public UriComponentsBuilder queryParam(String name, @Nullable Collection<?> values) {
+		return queryParam(name, (CollectionUtils.isEmpty(values) ? EMPTY_VALUES : values.toArray()));
+	}
+
+	@Override
+	public UriComponentsBuilder queryParamIfPresent(String name, Optional<?> value) {
+		value.ifPresent(o -> {
+			if (o instanceof Collection) {
+				queryParam(name, (Collection<?>) o);
 			}
 			else {
-				queryParam(name, value);
+				queryParam(name, o);
 			}
-		}
+		});
 		return this;
 	}
 
-	@Override
-	public UriComponentsBuilder queryParam(String name, @Nullable Collection<?> values) {
-		return queryParam(name, (CollectionUtils.isEmpty(values) ? EMPTY_VALUES : values.toArray()));
-	}
-
 	/**
 	 * {@inheritDoc}
 	 * @since 4.0

diff --git a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java
@@ -18,11 +18,11 @@
 
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
@@ -206,33 +206,6 @@ void fromUriStringQueryParamEncodedAndContainingPlus() {
 		assertThat(uri.toString()).isEqualTo(httpUrl);
 	}
 
-
-	@Test
-	void queryParamIfPresent() {
-		UriComponentsBuilder builder = UriComponentsBuilder.newInstance();
-		UriComponents result = builder.queryParamIfPresent("baz", Optional.of("qux")).queryParamIfPresent("foo", Optional.empty()).build();
-
-		assertThat(result.getQuery()).isEqualTo("baz=qux");
-		MultiValueMap<String, String> expectedQueryParams = new LinkedMultiValueMap<>(1);
-		expectedQueryParams.add("baz", "qux");
-		assertThat(result.getQueryParams()).isEqualTo(expectedQueryParams);
-	}
-
-	@Test
-	void queryParamIfPresentCollection() {
-		Collection<String> c = new ArrayList<>();
-		c.add("foo");
-		c.add("bar");
-		UriComponentsBuilder builder = UriComponentsBuilder.newInstance();
-		UriComponents result = builder.queryParamIfPresent("baz", Optional.of(c)).build();
-
-		assertThat(result.getQuery()).isEqualTo("baz=foo&baz=bar");
-		MultiValueMap<String, String> expectedQueryParams = new LinkedMultiValueMap<>(1);
-		expectedQueryParams.add("baz", "foo");
-		expectedQueryParams.add("baz", "bar");
-		assertThat(result.getQueryParams()).isEqualTo(expectedQueryParams);
-	}
-
 	@Test  // SPR-10539
 	void fromUriStringIPv6Host() {
 		UriComponents result = UriComponentsBuilder
@@ -786,6 +759,30 @@ void queryParamWithList() {
 		assertThat(result.getQueryParams()).isEqualTo(expectedQueryParams);
 	}
 
+	@Test
+	void queryParamIfPresent() {
+		UriComponents result = UriComponentsBuilder.newInstance()
+				.queryParamIfPresent("baz", Optional.of("qux"))
+				.queryParamIfPresent("foo", Optional.empty())
+				.build();
+
+		assertThat(result.getQuery()).isEqualTo("baz=qux");
+		assertThat(result.getQueryParams())
+				.containsOnlyKeys("baz")
+				.containsEntry("baz", Collections.singletonList("qux"));
+	}
+
+	@Test
+	void queryParamIfPresentCollection() {
+		List<String> values = Arrays.asList("foo", "bar");
+		UriComponents result = UriComponentsBuilder.newInstance()
+				.queryParamIfPresent("baz", Optional.of(values))
+				.build();
+
+		assertThat(result.getQuery()).isEqualTo("baz=foo&baz=bar");
+		assertThat(result.getQueryParams()).containsOnlyKeys("baz").containsEntry("baz", values);
+	}
+
 	@Test
 	void emptyQueryParam() {
 		UriComponentsBuilder builder = UriComponentsBuilder.newInstance();