From a09f93768a7d2ee2ce675d019bf6e8a660a18628 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 2 Dec 2022 17:01:54 +0000 Subject: [PATCH] Fix URI override for HttpExchange Closes gh-29624 --- .../service/invoker/HttpRequestValues.java | 30 +++++++------------ .../service/invoker/UrlArgumentResolver.java | 1 - .../invoker/UrlArgumentResolverTests.java | 7 +++-- .../WebClientHttpServiceProxyTests.java | 23 ++++++++++++-- 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java index 96ef73a9d6b0..e108f8418da2 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java +++ b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java @@ -115,9 +115,11 @@ public HttpMethod getHttpMethod() { } /** - * Return the full URL to use, if set. - *

This is mutually exclusive with {@link #getUriTemplate() uriTemplate}. - * One of the two has a value but not both. + * Return the URL to use. + *

Typically, this comes from a {@link URI} method argument, which provides + * the caller with the option to override the {@link #getUriTemplate() + * uriTemplate} from class and method {@code HttpExchange} annotations. + * annotation. */ @Nullable public URI getUri() { @@ -125,9 +127,8 @@ public URI getUri() { } /** - * Return the URL template for the request, if set. - *

This is mutually exclusive with a {@linkplain #getUri() full URL}. - * One of the two has a value but not both. + * Return the URL template for the request. This comes from the values in + * class and method {@code HttpExchange} annotations. */ @Nullable public String getUriTemplate() { @@ -248,38 +249,29 @@ public Builder setHttpMethod(HttpMethod httpMethod) { } /** - * Set the request URL as a full URL. - *

This is mutually exclusive with, and resets any previously set - * {@linkplain #setUriTemplate(String) URI template} or - * {@linkplain #setUriVariable(String, String) URI variables}. + * Set the URL to use. When set, this overrides the + * {@linkplain #setUriTemplate(String) URI template} from the + * {@code HttpExchange} annotation. */ public Builder setUri(URI uri) { this.uri = uri; - this.uriTemplate = null; - this.uriVars = null; return this; } /** * Set the request URL as a String template. - *

This is mutually exclusive with, and resets any previously set - * {@linkplain #setUri(URI) full URI}. */ public Builder setUriTemplate(String uriTemplate) { this.uriTemplate = uriTemplate; - this.uri = null; return this; } /** * Add a URI variable name-value pair. - *

This is mutually exclusive with, and resets any previously set - * {@linkplain #setUri(URI) full URI}. */ public Builder setUriVariable(String name, String value) { this.uriVars = (this.uriVars != null ? this.uriVars : new LinkedHashMap<>()); this.uriVars.put(name, value); - this.uri = null; return this; } @@ -399,7 +391,7 @@ public > void setBody(P body, ParameterizedTypeReferen public HttpRequestValues build() { URI uri = this.uri; - String uriTemplate = (this.uriTemplate != null || uri != null ? this.uriTemplate : ""); + String uriTemplate = (this.uriTemplate != null ? this.uriTemplate : ""); Map uriVars = (this.uriVars != null ? new HashMap<>(this.uriVars) : Collections.emptyMap()); Object bodyValue = this.bodyValue; diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.java index bdeee06724aa..3ca43c85051e 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.java @@ -40,7 +40,6 @@ public boolean resolve( if (argument != null) { requestValues.setUri((URI) argument); - return true; } return true; diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java index dc1a55f7e2d0..6e6ccfa240ac 100644 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java @@ -21,6 +21,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.lang.Nullable; import org.springframework.web.service.annotation.GetExchange; import static org.assertj.core.api.Assertions.assertThat; @@ -51,7 +52,7 @@ void url() { this.service.execute(dynamicUrl); assertThat(getRequestValues().getUri()).isEqualTo(dynamicUrl); - assertThat(getRequestValues().getUriTemplate()).isNull(); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); } @Test @@ -67,7 +68,9 @@ void notUrl() { @Test void ignoreNull() { this.service.execute(null); + assertThat(getRequestValues().getUri()).isNull(); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); } private HttpRequestValues getRequestValues() { @@ -78,7 +81,7 @@ private HttpRequestValues getRequestValues() { private interface Service { @GetExchange("/path") - void execute(URI uri); + void execute(@Nullable URI uri); @GetExchange void executeNotUri(String other); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientHttpServiceProxyTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientHttpServiceProxyTests.java index 9934cc7d0ebc..3b1e90747b4d 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientHttpServiceProxyTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientHttpServiceProxyTests.java @@ -18,6 +18,7 @@ import java.io.IOException; +import java.net.URI; import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -31,6 +32,8 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import org.springframework.lang.Nullable; +import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestAttribute; import org.springframework.web.reactive.function.client.WebClient; import org.springframework.web.service.annotation.GetExchange; @@ -77,7 +80,7 @@ void greeting() throws Exception { } @Test - void greetingWithRequestAttribute() throws Exception { + void greetingWithRequestAttribute() { Map attributes = new HashMap<>(); @@ -100,7 +103,19 @@ void greetingWithRequestAttribute() throws Exception { assertThat(attributes).containsEntry("myAttribute", "myAttributeValue"); } - private TestHttpService initHttpService() throws Exception { + @Test // gh-29624 + void uri() throws Exception { + String expectedBody = "hello"; + prepareResponse(response -> response.setResponseCode(200).setBody(expectedBody)); + + URI dynamicUri = this.server.url("/greeting/123").uri(); + String actualBody = initHttpService().getGreetingById(dynamicUri, "456"); + + assertThat(actualBody).isEqualTo(expectedBody); + assertThat(this.server.takeRequest().getRequestUrl().uri()).isEqualTo(dynamicUri); + } + + private TestHttpService initHttpService() { WebClient webClient = WebClient.builder().baseUrl(this.server.url("/").toString()).build(); return initHttpService(webClient); } @@ -127,7 +142,9 @@ private interface TestHttpService { @GetExchange("/greeting") Mono getGreetingWithAttribute(@RequestAttribute String myAttribute); - } + @GetExchange("/greetings/{id}") + String getGreetingById(@Nullable URI uri, @PathVariable String id); + } }