Fix missing URI template from client observations in ResTemplateAdapter
Prior to this commit, the `RestTemplateAdapter` would manually expand templated URIs. This means that the `RestTemplate` instance itself would never see the templated URIs and could not record it in the client observations at runtime. This commit ensures that when URI templates are available, the adapter uses the correct `exchange` method variant. Fixes gh-31144
This commit is contained in:
parent
e8fd29091c
commit
5fb567a37e
|
|
@ -16,10 +16,8 @@
|
||||||
|
|
||||||
package org.springframework.web.client.support;
|
package org.springframework.web.client.support;
|
||||||
|
|
||||||
import java.net.URI;
|
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
|
||||||
|
|
||||||
import org.springframework.core.ParameterizedTypeReference;
|
import org.springframework.core.ParameterizedTypeReference;
|
||||||
import org.springframework.http.HttpCookie;
|
import org.springframework.http.HttpCookie;
|
||||||
|
|
@ -41,6 +39,7 @@ import org.springframework.web.service.invoker.HttpServiceProxyFactory;
|
||||||
* {@link HttpServiceProxyFactory} configured with the given {@link RestTemplate}.
|
* {@link HttpServiceProxyFactory} configured with the given {@link RestTemplate}.
|
||||||
*
|
*
|
||||||
* @author Olga Maciaszek-Sharma
|
* @author Olga Maciaszek-Sharma
|
||||||
|
* @author Brian Clozel
|
||||||
* @since 6.1
|
* @since 6.1
|
||||||
*/
|
*/
|
||||||
public final class RestTemplateAdapter implements HttpExchangeAdapter {
|
public final class RestTemplateAdapter implements HttpExchangeAdapter {
|
||||||
|
|
@ -84,23 +83,21 @@ public final class RestTemplateAdapter implements HttpExchangeAdapter {
|
||||||
}
|
}
|
||||||
|
|
||||||
private RequestEntity<?> newRequest(HttpRequestValues values) {
|
private RequestEntity<?> newRequest(HttpRequestValues values) {
|
||||||
URI uri;
|
HttpMethod httpMethod = values.getHttpMethod();
|
||||||
|
Assert.notNull(httpMethod, "HttpMethod is required");
|
||||||
|
|
||||||
|
RequestEntity.BodyBuilder builder;
|
||||||
|
|
||||||
if (values.getUri() != null) {
|
if (values.getUri() != null) {
|
||||||
uri = values.getUri();
|
builder = RequestEntity.method(httpMethod, values.getUri());
|
||||||
}
|
}
|
||||||
else if (values.getUriTemplate() != null) {
|
else if (values.getUriTemplate() != null) {
|
||||||
String uriTemplate = values.getUriTemplate();
|
builder = RequestEntity.method(httpMethod, values.getUriTemplate(), values.getUriVariables());
|
||||||
Map<String, String> variables = values.getUriVariables();
|
|
||||||
uri = this.restTemplate.getUriTemplateHandler().expand(uriTemplate, variables);
|
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
throw new IllegalStateException("Neither full URL nor URI template");
|
throw new IllegalStateException("Neither full URL nor URI template");
|
||||||
}
|
}
|
||||||
|
|
||||||
HttpMethod httpMethod = values.getHttpMethod();
|
|
||||||
Assert.notNull(httpMethod, "HttpMethod is required");
|
|
||||||
|
|
||||||
RequestEntity.BodyBuilder builder = RequestEntity.method(httpMethod, uri);
|
|
||||||
builder.headers(values.getHeaders());
|
builder.headers(values.getHeaders());
|
||||||
|
|
||||||
if (!values.getCookies().isEmpty()) {
|
if (!values.getCookies().isEmpty()) {
|
||||||
|
|
|
||||||
|
|
@ -22,15 +22,17 @@ import java.lang.annotation.RetentionPolicy;
|
||||||
import java.lang.annotation.Target;
|
import java.lang.annotation.Target;
|
||||||
import java.net.URI;
|
import java.net.URI;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
|
import java.util.function.BiFunction;
|
||||||
import java.util.stream.Stream;
|
import java.util.stream.Stream;
|
||||||
|
|
||||||
|
import io.micrometer.observation.tck.TestObservationRegistry;
|
||||||
|
import io.micrometer.observation.tck.TestObservationRegistryAssert;
|
||||||
import okhttp3.mockwebserver.MockResponse;
|
import okhttp3.mockwebserver.MockResponse;
|
||||||
import okhttp3.mockwebserver.MockWebServer;
|
import okhttp3.mockwebserver.MockWebServer;
|
||||||
import okhttp3.mockwebserver.RecordedRequest;
|
import okhttp3.mockwebserver.RecordedRequest;
|
||||||
import org.junit.jupiter.params.ParameterizedTest;
|
import org.junit.jupiter.params.ParameterizedTest;
|
||||||
import org.junit.jupiter.params.provider.MethodSource;
|
import org.junit.jupiter.params.provider.MethodSource;
|
||||||
|
|
||||||
import org.springframework.cglib.core.internal.Function;
|
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.http.MediaType;
|
import org.springframework.http.MediaType;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
|
|
@ -62,6 +64,7 @@ import static org.assertj.core.api.Assertions.assertThat;
|
||||||
*
|
*
|
||||||
* @author Olga Maciaszek-Sharma
|
* @author Olga Maciaszek-Sharma
|
||||||
* @author Rossen Stoyanchev
|
* @author Rossen Stoyanchev
|
||||||
|
* @author Brian Clozel
|
||||||
*/
|
*/
|
||||||
@SuppressWarnings("JUnitMalformedDeclaration")
|
@SuppressWarnings("JUnitMalformedDeclaration")
|
||||||
class RestClientAdapterTests {
|
class RestClientAdapterTests {
|
||||||
|
|
@ -75,44 +78,49 @@ class RestClientAdapterTests {
|
||||||
|
|
||||||
public static Stream<Object[]> arguments() {
|
public static Stream<Object[]> arguments() {
|
||||||
return Stream.of(
|
return Stream.of(
|
||||||
args(url -> {
|
args((url, observationRegistry) -> {
|
||||||
RestClient restClient = RestClient.builder().baseUrl(url).build();
|
RestClient restClient = RestClient.builder().baseUrl(url).observationRegistry(observationRegistry).build();
|
||||||
return RestClientAdapter.create(restClient);
|
return RestClientAdapter.create(restClient);
|
||||||
}),
|
}),
|
||||||
args(url -> {
|
args((url, observationRegistry) -> {
|
||||||
RestTemplate restTemplate = new RestTemplate();
|
RestTemplate restTemplate = new RestTemplate();
|
||||||
|
restTemplate.setObservationRegistry(observationRegistry);
|
||||||
restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(url));
|
restTemplate.setUriTemplateHandler(new DefaultUriBuilderFactory(url));
|
||||||
return RestTemplateAdapter.create(restTemplate);
|
return RestTemplateAdapter.create(restTemplate);
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
@SuppressWarnings("resource")
|
@SuppressWarnings("resource")
|
||||||
private static Object[] args(Function<String, HttpExchangeAdapter> adapterFactory) {
|
private static Object[] args(BiFunction<String, TestObservationRegistry, HttpExchangeAdapter> adapterFactory) {
|
||||||
MockWebServer server = new MockWebServer();
|
MockWebServer server = new MockWebServer();
|
||||||
|
|
||||||
MockResponse response = new MockResponse();
|
MockResponse response = new MockResponse();
|
||||||
response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!");
|
response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!");
|
||||||
server.enqueue(response);
|
server.enqueue(response);
|
||||||
|
|
||||||
HttpExchangeAdapter adapter = adapterFactory.apply(server.url("/").toString());
|
TestObservationRegistry observationRegistry = TestObservationRegistry.create();
|
||||||
|
|
||||||
|
HttpExchangeAdapter adapter = adapterFactory.apply(server.url("/").toString(), observationRegistry);
|
||||||
Service service = HttpServiceProxyFactory.builderFor(adapter).build().createClient(Service.class);
|
Service service = HttpServiceProxyFactory.builderFor(adapter).build().createClient(Service.class);
|
||||||
|
|
||||||
return new Object[] { server, service };
|
return new Object[] { server, service, observationRegistry };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@ParameterizedAdapterTest
|
@ParameterizedAdapterTest
|
||||||
void greeting(MockWebServer server, Service service) throws Exception {
|
void greeting(MockWebServer server, Service service, TestObservationRegistry observationRegistry) throws Exception {
|
||||||
String response = service.getGreeting();
|
String response = service.getGreeting();
|
||||||
|
|
||||||
RecordedRequest request = server.takeRequest();
|
RecordedRequest request = server.takeRequest();
|
||||||
assertThat(response).isEqualTo("Hello Spring!");
|
assertThat(response).isEqualTo("Hello Spring!");
|
||||||
assertThat(request.getMethod()).isEqualTo("GET");
|
assertThat(request.getMethod()).isEqualTo("GET");
|
||||||
assertThat(request.getPath()).isEqualTo("/greeting");
|
assertThat(request.getPath()).isEqualTo("/greeting");
|
||||||
|
TestObservationRegistryAssert.assertThat(observationRegistry).hasObservationWithNameEqualTo("http.client.requests")
|
||||||
|
.that().hasLowCardinalityKeyValue("uri", "/greeting");
|
||||||
}
|
}
|
||||||
|
|
||||||
@ParameterizedAdapterTest
|
@ParameterizedAdapterTest
|
||||||
void greetingById(MockWebServer server, Service service) throws Exception {
|
void greetingById(MockWebServer server, Service service, TestObservationRegistry observationRegistry) throws Exception {
|
||||||
ResponseEntity<String> response = service.getGreetingById("456");
|
ResponseEntity<String> response = service.getGreetingById("456");
|
||||||
|
|
||||||
RecordedRequest request = server.takeRequest();
|
RecordedRequest request = server.takeRequest();
|
||||||
|
|
@ -120,10 +128,12 @@ class RestClientAdapterTests {
|
||||||
assertThat(response.getBody()).isEqualTo("Hello Spring!");
|
assertThat(response.getBody()).isEqualTo("Hello Spring!");
|
||||||
assertThat(request.getMethod()).isEqualTo("GET");
|
assertThat(request.getMethod()).isEqualTo("GET");
|
||||||
assertThat(request.getPath()).isEqualTo("/greeting/456");
|
assertThat(request.getPath()).isEqualTo("/greeting/456");
|
||||||
|
TestObservationRegistryAssert.assertThat(observationRegistry).hasObservationWithNameEqualTo("http.client.requests")
|
||||||
|
.that().hasLowCardinalityKeyValue("uri", "/greeting/{id}");
|
||||||
}
|
}
|
||||||
|
|
||||||
@ParameterizedAdapterTest
|
@ParameterizedAdapterTest
|
||||||
void greetingWithDynamicUri(MockWebServer server, Service service) throws Exception {
|
void greetingWithDynamicUri(MockWebServer server, Service service, TestObservationRegistry observationRegistry) throws Exception {
|
||||||
URI dynamicUri = server.url("/greeting/123").uri();
|
URI dynamicUri = server.url("/greeting/123").uri();
|
||||||
Optional<String> response = service.getGreetingWithDynamicUri(dynamicUri, "456");
|
Optional<String> response = service.getGreetingWithDynamicUri(dynamicUri, "456");
|
||||||
|
|
||||||
|
|
@ -131,6 +141,8 @@ class RestClientAdapterTests {
|
||||||
assertThat(response.orElse("empty")).isEqualTo("Hello Spring!");
|
assertThat(response.orElse("empty")).isEqualTo("Hello Spring!");
|
||||||
assertThat(request.getMethod()).isEqualTo("GET");
|
assertThat(request.getMethod()).isEqualTo("GET");
|
||||||
assertThat(request.getRequestUrl().uri()).isEqualTo(dynamicUri);
|
assertThat(request.getRequestUrl().uri()).isEqualTo(dynamicUri);
|
||||||
|
TestObservationRegistryAssert.assertThat(observationRegistry).hasObservationWithNameEqualTo("http.client.requests")
|
||||||
|
.that().hasLowCardinalityKeyValue("uri", "none");
|
||||||
}
|
}
|
||||||
|
|
||||||
@ParameterizedAdapterTest
|
@ParameterizedAdapterTest
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue