Skip to content

Commit

Permalink
Fix URI override for HttpExchange
Browse files Browse the repository at this point in the history
Closes gh-29624
  • Loading branch information
rstoyanchev committed Dec 2, 2022
1 parent 0ccd2f8 commit a09f937
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 25 deletions.
Expand Up @@ -115,19 +115,20 @@ public HttpMethod getHttpMethod() {
}

/**
* Return the full URL to use, if set.
* <p>This is mutually exclusive with {@link #getUriTemplate() uriTemplate}.
* One of the two has a value but not both.
* Return the URL to use.
* <p>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() {
return this.uri;
}

/**
* Return the URL template for the request, if set.
* <p>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() {
Expand Down Expand Up @@ -248,38 +249,29 @@ public Builder setHttpMethod(HttpMethod httpMethod) {
}

/**
* Set the request URL as a full URL.
* <p>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.
* <p>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.
* <p>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;
}

Expand Down Expand Up @@ -399,7 +391,7 @@ public <T, P extends Publisher<T>> 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<String, String> uriVars = (this.uriVars != null ? new HashMap<>(this.uriVars) : Collections.emptyMap());

Object bodyValue = this.bodyValue;
Expand Down
Expand Up @@ -40,7 +40,6 @@ public boolean resolve(

if (argument != null) {
requestValues.setUri((URI) argument);
return true;
}

return true;
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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() {
Expand All @@ -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);
Expand Down
Expand Up @@ -18,6 +18,7 @@


import java.io.IOException;
import java.net.URI;
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -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;
Expand Down Expand Up @@ -77,7 +80,7 @@ void greeting() throws Exception {
}

@Test
void greetingWithRequestAttribute() throws Exception {
void greetingWithRequestAttribute() {

Map<String, Object> attributes = new HashMap<>();

Expand All @@ -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);
}
Expand All @@ -127,7 +142,9 @@ private interface TestHttpService {
@GetExchange("/greeting")
Mono<String> getGreetingWithAttribute(@RequestAttribute String myAttribute);

}
@GetExchange("/greetings/{id}")
String getGreetingById(@Nullable URI uri, @PathVariable String id);

}

}

0 comments on commit a09f937

Please sign in to comment.