From 5644a7aebbb7cea8fdd16e873f6b2a3229ac96e9 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 23 Oct 2020 17:43:17 +0100 Subject: [PATCH] Polishing contribution Closes gh-25951 --- .../web/util/DefaultUriBuilderFactory.java | 18 +++---- .../springframework/web/util/UriBuilder.java | 19 +++---- .../web/util/UriComponentsBuilder.java | 23 ++++---- .../web/util/UriComponentsBuilderTests.java | 53 +++++++++---------- 4 files changed, 55 insertions(+), 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 index 0438faaf7c9..7d06cb61a02 100644 --- 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 class DefaultUriBuilderFactory implements UriBuilderFactory { } @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 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 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 index c5b69c55bd5..8f65ab0972f 100644 --- 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. *

Note: 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. *

Note: 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 index 672989dd3bf..b76798e6f1c 100644 --- 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 class UriComponentsBuilder implements UriBuilder, Cloneable { } @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 index 2e2e37c90bf..93f77aa3e93 100644 --- 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 @@ package org.springframework.web.util; 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 @@ class UriComponentsBuilderTests { 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 expectedQueryParams = new LinkedMultiValueMap<>(1); - expectedQueryParams.add("baz", "qux"); - assertThat(result.getQueryParams()).isEqualTo(expectedQueryParams); - } - - @Test - void queryParamIfPresentCollection() { - Collection 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 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 @@ class UriComponentsBuilderTests { 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 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();