From 615c25d4fa049f97226aa2062cd5cd053ab56b1c Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 12 Jan 2026 09:51:17 +0000 Subject: [PATCH] Polishing contribution Closes gh-36126 --- .../HttpServiceProxyRegistryFactoryBean.java | 11 +- ...pServiceProxyRegistryFactoryBeanTests.java | 109 +++++------------- 2 files changed, 34 insertions(+), 86 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java index af46d9829af..660e5c5564a 100644 --- a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java +++ b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java @@ -177,14 +177,14 @@ public final class HttpServiceProxyRegistryFactoryBean private @Nullable Object clientBuilder; - private final @Nullable StringValueResolver embeddedValueResolver; - private final HttpServiceProxyFactory.Builder proxyFactoryBuilder = HttpServiceProxyFactory.builder(); - ConfigurableGroup(HttpServiceGroup group, @Nullable StringValueResolver embeddedValueResolver) { + ConfigurableGroup(HttpServiceGroup group, @Nullable StringValueResolver valueResolver) { this.group = group; this.groupAdapter = getGroupAdapter(group.clientType()); - this.embeddedValueResolver = embeddedValueResolver; + if (valueResolver != null) { + this.proxyFactoryBuilder.embeddedValueResolver(valueResolver); + } } private static HttpServiceGroupAdapter getGroupAdapter(HttpServiceGroup.ClientType clientType) { @@ -229,9 +229,6 @@ public final class HttpServiceProxyRegistryFactoryBean public Map, Object> createProxies() { Map, Object> map = new LinkedHashMap<>(this.group.httpServiceTypes().size()); HttpExchangeAdapter adapter = this.groupAdapter.createExchangeAdapter(getClientBuilder()); - if (this.embeddedValueResolver != null) { - this.proxyFactoryBuilder.embeddedValueResolver(this.embeddedValueResolver); - } HttpServiceProxyFactory factory = this.proxyFactoryBuilder.exchangeAdapter(adapter).build(); this.group.httpServiceTypes().forEach(type -> map.put(type, factory.createClient(type))); return map; diff --git a/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBeanTests.java b/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBeanTests.java index 4e6d371934e..18ee7b3dae5 100644 --- a/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBeanTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBeanTests.java @@ -20,6 +20,7 @@ import java.net.URI; import java.util.List; import java.util.function.Predicate; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; @@ -42,7 +43,6 @@ import org.springframework.web.testfixture.http.client.MockClientHttpRequest; import org.springframework.web.testfixture.http.client.MockClientHttpResponse; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.atLeastOnce; @@ -59,19 +59,20 @@ public class HttpServiceProxyRegistryFactoryBeanTests { void twoGroups() { String echoName = "echo"; String greetingName = "greeting"; - GroupsMetadata groupsMetadata = new GroupsMetadata(); + GroupsMetadata metadata = new GroupsMetadata(); List echoServices = List.of(EchoA.class.getName(), EchoB.class.getName()); - groupsMetadata.getOrCreateGroup(echoName, REST_CLIENT).httpServiceTypeNames().addAll(echoServices); + metadata.getOrCreateGroup(echoName, REST_CLIENT).httpServiceTypeNames().addAll(echoServices); List greetingServices = List.of(GreetingA.class.getName(), GreetingB.class.getName()); - groupsMetadata.getOrCreateGroup(greetingName, REST_CLIENT).httpServiceTypeNames().addAll(greetingServices); + metadata.getOrCreateGroup(greetingName, REST_CLIENT).httpServiceTypeNames().addAll(greetingServices); Predicate echoFilter = group -> group.name().equals(echoName); Predicate greetingFilter = group -> group.name().equals(greetingName); TestConfigurer groupConfigurer = new TestConfigurer(List.of(echoFilter, greetingFilter)); - HttpServiceProxyRegistry registry = initProxyRegistry(groupConfigurer, groupsMetadata); + HttpServiceProxyRegistry registry = initProxyRegistry(groupConfigurer, metadata, null); + assertThat(registry.getGroupNames()).containsExactlyInAnyOrder(echoName, greetingName); assertThat(registry.getClientTypesInGroup(echoName)).containsExactlyInAnyOrder(EchoA.class, EchoB.class); assertThat(registry.getClientTypesInGroup(greetingName)).containsExactlyInAnyOrder(GreetingA.class, GreetingB.class); @@ -84,9 +85,6 @@ public class HttpServiceProxyRegistryFactoryBeanTests { @Test void initializeClientBuilder() throws Exception { - GroupsMetadata groupsMetadata = new GroupsMetadata(); - groupsMetadata.getOrCreateGroup("echo", REST_CLIENT).httpServiceTypeNames().add(EchoA.class.getName()); - ClientHttpRequestFactory requestFactory = Mockito.mock(ClientHttpRequestFactory.class); MockClientHttpRequest request = new MockClientHttpRequest(); request.setResponse(new MockClientHttpResponse()); @@ -95,95 +93,47 @@ public class HttpServiceProxyRegistryFactoryBeanTests { RestClient.Builder clientBuilder = RestClient.builder().baseUrl("/").requestFactory(requestFactory); RestClientHttpServiceGroupConfigurer groupConfigurer = groups -> groups.forEachClient(group -> clientBuilder); - HttpServiceProxyRegistry registry = initProxyRegistry(groupConfigurer, groupsMetadata); + GroupsMetadata metadata = new GroupsMetadata(); + metadata.getOrCreateGroup("echo", REST_CLIENT).httpServiceTypeNames().add(EchoA.class.getName()); + + HttpServiceProxyRegistry registry = initProxyRegistry(groupConfigurer, metadata, null); registry.getClient(EchoA.class).handle("foo"); verify(requestFactory, atLeastOnce()).createRequest(any(), any()); } @Test - void propertyPlaceholderInHttpExchangeUrlIsResolved() throws Exception { - GroupsMetadata groupsMetadata = new GroupsMetadata(); - groupsMetadata.getOrCreateGroup("testGroup", REST_CLIENT) - .httpServiceTypeNames() - .add(PlaceholderService.class.getName()); + void propertyPlaceholder() throws Exception { + GroupsMetadata metadata = new GroupsMetadata(); + metadata.getOrCreateGroup("group", REST_CLIENT).httpServiceTypeNames().add(PlaceholderService.class.getName()); ClientHttpRequestFactory requestFactory = Mockito.mock(ClientHttpRequestFactory.class); - MockClientHttpRequest mockRequest = new MockClientHttpRequest(); - mockRequest.setResponse(new MockClientHttpResponse()); + MockClientHttpRequest request = new MockClientHttpRequest(); + request.setResponse(new MockClientHttpResponse()); ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(URI.class); - given(requestFactory.createRequest(uriCaptor.capture(), any())).willReturn(mockRequest); - - StringValueResolver resolver = value -> { - if (value.contains("${test.base.url}")) { - return value.replace("${test.base.url}", "https://api.example.com"); - } - return value; - }; + given(requestFactory.createRequest(uriCaptor.capture(), any())).willReturn(request); RestClient.Builder clientBuilder = RestClient.builder().requestFactory(requestFactory); - RestClientHttpServiceGroupConfigurer configurer = groups -> groups.forEachClient(group -> clientBuilder); - AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); - context.registerBean(RestClientHttpServiceGroupConfigurer.class, () -> configurer); - context.refresh(); - - HttpServiceProxyRegistryFactoryBean factoryBean = new HttpServiceProxyRegistryFactoryBean(groupsMetadata); - - factoryBean.setApplicationContext(context); - factoryBean.setBeanClassLoader(getClass().getClassLoader()); - factoryBean.setEmbeddedValueResolver(resolver); - factoryBean.afterPropertiesSet(); - - HttpServiceProxyRegistry registry = factoryBean.getObject(); - PlaceholderService service = registry.getClient(PlaceholderService.class); - service.callEndpoint(); - - URI requestedUri = uriCaptor.getValue(); - - assertThat(requestedUri.toString()) - .startsWith("https://api.example.com") - .doesNotContain("${") - .contains("/endpoint"); - } - - @Test - void withoutResolverPlaceholderRemainsUnresolved() throws Exception { - GroupsMetadata groupsMetadata = new GroupsMetadata(); - groupsMetadata.getOrCreateGroup("testGroup", REST_CLIENT) - .httpServiceTypeNames() - .add(PlaceholderService.class.getName()); - - ClientHttpRequestFactory requestFactory = Mockito.mock(ClientHttpRequestFactory.class); - MockClientHttpRequest capturedRequest = new MockClientHttpRequest(); - capturedRequest.setResponse(new MockClientHttpResponse()); - given(requestFactory.createRequest(any(), any())).willReturn(capturedRequest); - - RestClient.Builder clientBuilder = RestClient.builder().requestFactory(requestFactory); - RestClientHttpServiceGroupConfigurer configurer = groups -> - groups.forEachClient(group -> clientBuilder); - - AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); - context.registerBean(RestClientHttpServiceGroupConfigurer.class, () -> configurer); - context.refresh(); - - HttpServiceProxyRegistryFactoryBean factoryBean = new HttpServiceProxyRegistryFactoryBean(groupsMetadata); - factoryBean.setApplicationContext(context); - factoryBean.setBeanClassLoader(getClass().getClassLoader()); - factoryBean.afterPropertiesSet(); + String baseUrl = "https://api.example.com"; + HttpServiceProxyRegistry registry = initProxyRegistry(configurer, metadata, value -> { + if (value.contains("${test.base.url}")) { + return value.replace("${test.base.url}", baseUrl); + } + return value; + }); - HttpServiceProxyRegistry registry = factoryBean.getObject(); PlaceholderService service = registry.getClient(PlaceholderService.class); + service.invoke(); - assertThatThrownBy(service::callEndpoint) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("test.base.url"); + assertThat(uriCaptor.getValue().toString()).isEqualTo(baseUrl); } private HttpServiceProxyRegistry initProxyRegistry( - RestClientHttpServiceGroupConfigurer groupConfigurer, GroupsMetadata groupsMetadata) { + RestClientHttpServiceGroupConfigurer groupConfigurer, GroupsMetadata groupsMetadata, + @Nullable StringValueResolver valueResolver) { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); context.registerBean(RestClientHttpServiceGroupConfigurer.class, () -> groupConfigurer); @@ -192,6 +142,7 @@ public class HttpServiceProxyRegistryFactoryBeanTests { HttpServiceProxyRegistryFactoryBean factoryBean = new HttpServiceProxyRegistryFactoryBean(groupsMetadata); factoryBean.setApplicationContext(context); factoryBean.setBeanClassLoader(getClass().getClassLoader()); + factoryBean.setEmbeddedValueResolver(valueResolver); factoryBean.afterPropertiesSet(); return factoryBean.getObject(); @@ -226,7 +177,7 @@ public class HttpServiceProxyRegistryFactoryBeanTests { @HttpExchange(url = "${test.base.url}") interface PlaceholderService { - @GetExchange("/endpoint") - String callEndpoint(); + @GetExchange + String invoke(); } }