From bcbd9f00fe5ae4b20cbbdcf6b3763ede568cfc7c Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Fri, 12 Jan 2024 12:37:17 +0100 Subject: [PATCH 1/4] Use SmartInitializingSingleton instead of BeanPostProcessor to set up @LoadBalanced WebClient.Builder. --- .../LoadBalancerAutoConfiguration.java | 38 +++++++++---- ...cerRestClientBuilderBeanPostProcessor.java | 55 ------------------- .../RestClientBuilderCustomizer.java | 33 +++++++++++ .../LoadBalancerEnvironmentPropertyUtils.java | 2 +- src/checkstyle/checkstyle-suppressions.xml | 2 + 5 files changed, 63 insertions(+), 67 deletions(-) delete mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RestClientBuilderCustomizer.java diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java index 615228124..7c2064fc3 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.AutoConfiguration; import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -32,12 +33,12 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; -import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.retry.support.RetryTemplate; +import org.springframework.web.client.RestClient; import org.springframework.web.client.RestTemplate; /** @@ -49,7 +50,7 @@ * @author Gang Li * @author Olga Maciaszek-Sharma */ -@Configuration(proxyBeanMethods = false) +@AutoConfiguration @Conditional(BlockingRestClassesPresentCondition.class) @ConditionalOnBean(LoadBalancerClient.class) @EnableConfigurationProperties(LoadBalancerClientsProperties.class) @@ -59,6 +60,10 @@ public class LoadBalancerAutoConfiguration { @Autowired(required = false) private List restTemplates = Collections.emptyList(); + @LoadBalanced + @Autowired(required = false) + private List restClientBuilders = Collections.emptyList(); + @Autowired(required = false) private List transformers = Collections.emptyList(); @@ -74,6 +79,19 @@ public SmartInitializingSingleton loadBalancedRestTemplateInitializerDeprecated( }); } + @Bean + public SmartInitializingSingleton loadBalancedRestClientBuilderInitializer( + final ObjectProvider> restClientBuilderCustomizers) { + return () -> restClientBuilderCustomizers.ifAvailable(customizers -> { + for (RestClient.Builder restClientBuilder : LoadBalancerAutoConfiguration.this.restClientBuilders) { + for (RestClientBuilderCustomizer customizer : customizers) { + customizer.customize(restClientBuilder); + } + + } + }); + } + @Bean @ConditionalOnMissingBean public LoadBalancerRequestFactory loadBalancerRequestFactory(LoadBalancerClient loadBalancerClient) { @@ -101,11 +119,10 @@ public RestTemplateCustomizer restTemplateCustomizer(final LoadBalancerIntercept } @Bean - @ConditionalOnBean(LoadBalancerInterceptor.class) @ConditionalOnMissingBean - LoadBalancerRestClientBuilderBeanPostProcessor lbRestClientPostProcessor( - final LoadBalancerInterceptor loadBalancerInterceptor, ApplicationContext context) { - return new LoadBalancerRestClientBuilderBeanPostProcessor(loadBalancerInterceptor, context); + public RestClientBuilderCustomizer restClientBuilderCustomizer( + final LoadBalancerInterceptor loadBalancerInterceptor) { + return restClientBuilder -> restClientBuilder.requestInterceptor(loadBalancerInterceptor); } } @@ -174,11 +191,10 @@ public RestTemplateCustomizer restTemplateCustomizer( } @Bean - @ConditionalOnBean(RetryLoadBalancerInterceptor.class) @ConditionalOnMissingBean - LoadBalancerRestClientBuilderBeanPostProcessor lbRestClientPostProcessor( - final RetryLoadBalancerInterceptor loadBalancerInterceptor, ApplicationContext context) { - return new LoadBalancerRestClientBuilderBeanPostProcessor(loadBalancerInterceptor, context); + public RestClientBuilderCustomizer restClientBuilderCustomizer( + final RetryLoadBalancerInterceptor loadBalancerInterceptor) { + return restClientBuilder -> restClientBuilder.requestInterceptor(loadBalancerInterceptor); } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java deleted file mode 100644 index 6bd63cd72..000000000 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright 2012-2023 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.client.loadbalancer; - -import org.springframework.beans.BeansException; -import org.springframework.beans.factory.config.BeanPostProcessor; -import org.springframework.context.ApplicationContext; -import org.springframework.http.client.ClientHttpRequestInterceptor; -import org.springframework.web.client.RestClient; - -/** - * A {@link BeanPostProcessor} that adds the provided {@link ClientHttpRequestInterceptor} - * to all {@link RestClient.Builder} instances annotated with {@link LoadBalanced}. - * - * @author Olga Maciaszek-Sharma - * @since 4.1.0 - */ -public class LoadBalancerRestClientBuilderBeanPostProcessor implements BeanPostProcessor { - - private final ClientHttpRequestInterceptor loadBalancerInterceptor; - - private final ApplicationContext context; - - public LoadBalancerRestClientBuilderBeanPostProcessor(ClientHttpRequestInterceptor loadBalancerInterceptor, - ApplicationContext context) { - this.loadBalancerInterceptor = loadBalancerInterceptor; - this.context = context; - } - - @Override - public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { - if (bean instanceof RestClient.Builder) { - if (context.findAnnotationOnBean(beanName, LoadBalanced.class) == null) { - return bean; - } - ((RestClient.Builder) bean).requestInterceptor(loadBalancerInterceptor); - } - return bean; - } - -} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RestClientBuilderCustomizer.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RestClientBuilderCustomizer.java new file mode 100644 index 000000000..0273d1882 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RestClientBuilderCustomizer.java @@ -0,0 +1,33 @@ +/* + * Copyright 2012-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.web.client.RestClient; + +/** + * A customizer interface for {@link RestClient.Builder}. Used to set + * {@link LoadBalancerInterceptor} on the builder at the end of the singleton + * pre-instantiation phase. + * + * @author Olga Maciaszek-Sharma + * @since 4.1.1 + */ +public interface RestClientBuilderCustomizer { + + void customize(RestClient.Builder restClientBuilder); + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java index 3856dc674..60da875a0 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java @@ -24,7 +24,7 @@ public final class LoadBalancerEnvironmentPropertyUtils { private LoadBalancerEnvironmentPropertyUtils() { - throw new IllegalStateException("Should not instantiate a utility class"); + throw new UnsupportedOperationException("Cannot instantiate a utility class"); } public static boolean trueForClientOrDefault(Environment environment, String propertySuffix) { diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index 9d2f3dc55..24afedc95 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/src/checkstyle/checkstyle-suppressions.xml @@ -18,5 +18,7 @@ + + From c45e46599cd0036efe1bebbacc46799d0c2c58ef Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Fri, 12 Jan 2024 13:03:47 +0100 Subject: [PATCH 2/4] Add tests for @LoadBalanced clients defined in autoconfigurations. --- ...actLoadBalancerAutoConfigurationTests.java | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java index c81c40b4d..eb1e8f21b 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -101,6 +101,7 @@ void multipleRestTemplates() { void multipleRestClientBuilders() { applicationContextRunner.withUserConfiguration(TwoRestTemplatesAndTwoRestClientBuilders.class).run(context -> { final Map restClientBuilders = context.getBeansOfType(RestClient.Builder.class); + final Map restTemplates = context.getBeansOfType(RestTemplate.class); assertThat(restClientBuilders).isNotNull(); assertThat(restClientBuilders.values()).hasSize(2); @@ -114,9 +115,45 @@ void multipleRestClientBuilders() { assertThat(two.nonLoadBalancedRestClientBuilder).isNotNull(); two.nonLoadBalancedRestClientBuilder .requestInterceptors(interceptors -> assertThat(interceptors).isEmpty()); + + assertThat(restTemplates).isNotNull(); + Collection templates = restTemplates.values(); + assertThat(templates).hasSize(2); + + TwoRestTemplatesAndTwoRestClientBuilders.Two twoRestTemplate = context + .getBean(TwoRestTemplatesAndTwoRestClientBuilders.Two.class); + + assertThat(twoRestTemplate.loadBalanced).isNotNull(); + assertLoadBalanced(twoRestTemplate.loadBalanced); + + assertThat(twoRestTemplate.nonLoadBalanced).isNotNull(); + assertThat(twoRestTemplate.nonLoadBalanced.getInterceptors()).isEmpty(); }); } + @Test + void restTemplatesAndRestClientsFromUsersAutoConfiguration() { + applicationContextRunner + .withConfiguration(AutoConfigurations.of(TwoRestTemplatesAndTwoRestClientBuilders.class)) + .run(context -> { + final Map restClientBuilders = context + .getBeansOfType(RestClient.Builder.class); + + assertThat(restClientBuilders).isNotNull(); + assertThat(restClientBuilders.values()).hasSize(2); + + TwoRestTemplatesAndTwoRestClientBuilders.Two two = context + .getBean(TwoRestTemplatesAndTwoRestClientBuilders.Two.class); + + assertThat(two.loadBalancedRestClientBuilder).isNotNull(); + assertLoadBalanced(two.loadBalancedRestClientBuilder); + + assertThat(two.nonLoadBalancedRestClientBuilder).isNotNull(); + two.nonLoadBalancedRestClientBuilder + .requestInterceptors(interceptors -> assertThat(interceptors).isEmpty()); + }); + } + protected abstract void assertLoadBalanced(RestClient.Builder restClientBuilder); protected abstract void assertLoadBalanced(RestTemplate restTemplate); From 7fc64a07fc2862af7133adf46e2516512494ead4 Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Fri, 12 Jan 2024 13:06:23 +0100 Subject: [PATCH 3/4] Add tests for @LoadBalanced clients defined in autoconfigurations. --- ...actLoadBalancerAutoConfigurationTests.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java index eb1e8f21b..c92ba95db 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java @@ -101,7 +101,6 @@ void multipleRestTemplates() { void multipleRestClientBuilders() { applicationContextRunner.withUserConfiguration(TwoRestTemplatesAndTwoRestClientBuilders.class).run(context -> { final Map restClientBuilders = context.getBeansOfType(RestClient.Builder.class); - final Map restTemplates = context.getBeansOfType(RestTemplate.class); assertThat(restClientBuilders).isNotNull(); assertThat(restClientBuilders.values()).hasSize(2); @@ -115,19 +114,6 @@ void multipleRestClientBuilders() { assertThat(two.nonLoadBalancedRestClientBuilder).isNotNull(); two.nonLoadBalancedRestClientBuilder .requestInterceptors(interceptors -> assertThat(interceptors).isEmpty()); - - assertThat(restTemplates).isNotNull(); - Collection templates = restTemplates.values(); - assertThat(templates).hasSize(2); - - TwoRestTemplatesAndTwoRestClientBuilders.Two twoRestTemplate = context - .getBean(TwoRestTemplatesAndTwoRestClientBuilders.Two.class); - - assertThat(twoRestTemplate.loadBalanced).isNotNull(); - assertLoadBalanced(twoRestTemplate.loadBalanced); - - assertThat(twoRestTemplate.nonLoadBalanced).isNotNull(); - assertThat(twoRestTemplate.nonLoadBalanced.getInterceptors()).isEmpty(); }); } @@ -138,6 +124,7 @@ void restTemplatesAndRestClientsFromUsersAutoConfiguration() { .run(context -> { final Map restClientBuilders = context .getBeansOfType(RestClient.Builder.class); + final Map restTemplates = context.getBeansOfType(RestTemplate.class); assertThat(restClientBuilders).isNotNull(); assertThat(restClientBuilders.values()).hasSize(2); @@ -151,6 +138,19 @@ void restTemplatesAndRestClientsFromUsersAutoConfiguration() { assertThat(two.nonLoadBalancedRestClientBuilder).isNotNull(); two.nonLoadBalancedRestClientBuilder .requestInterceptors(interceptors -> assertThat(interceptors).isEmpty()); + + assertThat(restTemplates).isNotNull(); + Collection templates = restTemplates.values(); + assertThat(templates).hasSize(2); + + TwoRestTemplatesAndTwoRestClientBuilders.Two twoRestTemplate = context + .getBean(TwoRestTemplatesAndTwoRestClientBuilders.Two.class); + + assertThat(twoRestTemplate.loadBalanced).isNotNull(); + assertLoadBalanced(twoRestTemplate.loadBalanced); + + assertThat(twoRestTemplate.nonLoadBalanced).isNotNull(); + assertThat(twoRestTemplate.nonLoadBalanced.getInterceptors()).isEmpty(); }); } From 213f281523971e446b5e24f5e9ea4bc524708646 Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Tue, 16 Jan 2024 11:30:10 +0100 Subject: [PATCH 4/4] Add back LoadBalancerRestClientBuilderBeanPostProcessor. --- ...cerRestClientBuilderBeanPostProcessor.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java new file mode 100644 index 000000000..5d5bf4c97 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRestClientBuilderBeanPostProcessor.java @@ -0,0 +1,57 @@ +/* + * Copyright 2012-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.context.ApplicationContext; +import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.web.client.RestClient; + +/** + * A {@link BeanPostProcessor} that adds the provided {@link ClientHttpRequestInterceptor} + * to all {@link RestClient.Builder} instances annotated with {@link LoadBalanced}. + * + * @author Olga Maciaszek-Sharma + * @since 4.1.0 + * @deprecated to be removed in the next release. + */ +@Deprecated +public class LoadBalancerRestClientBuilderBeanPostProcessor implements BeanPostProcessor { + + private final ClientHttpRequestInterceptor loadBalancerInterceptor; + + private final ApplicationContext context; + + public LoadBalancerRestClientBuilderBeanPostProcessor(ClientHttpRequestInterceptor loadBalancerInterceptor, + ApplicationContext context) { + this.loadBalancerInterceptor = loadBalancerInterceptor; + this.context = context; + } + + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { + if (bean instanceof RestClient.Builder) { + if (context.findAnnotationOnBean(beanName, LoadBalanced.class) == null) { + return bean; + } + ((RestClient.Builder) bean).requestInterceptor(loadBalancerInterceptor); + } + return bean; + } + +}