From ad08ca55f415d305f326e6feb563e75bd209f8ca Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 7 Mar 2016 17:44:41 +0000 Subject: [PATCH] Change in contract for @LoadBalanced Users now need to create their own bean and qualify it, and Spring Cloud will customize it (instead of providing the @Bean itself). This is much better for users, since they remain in control of the bean declarations, and can choose which one (if any) is @Primary. It's a breaking change for some apps (if they rely on a @LoadBalanced RestTemplate being automatically injected). --- .../main/asciidoc/spring-cloud-commons.adoc | 10 +- .../LoadBalancerAutoConfiguration.java | 27 ++- .../LoadBalancerAutoConfigurationTests.java | 184 ++++++++++-------- 3 files changed, 127 insertions(+), 94 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index 63c265c0..4265b7ee 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -318,15 +318,21 @@ for details of how the `RestTemplate` is set up. If you want a `RestTemplate` that is not load balanced, create a `RestTemplate` bean and inject it as normal. To access the load balanced `RestTemplate use -the provided `@LoadBalanced` `Qualifier`. +the `@LoadBalanced` qualifier when you create your `@Bean`. -IMPORTANT: Notice the `@Primary` annotation on the plain `RestTemplate` declaration. +IMPORTANT: Notice the `@Primary` annotation on the plain `RestTemplate` declaration in the example below, to disambiguate the unqualified `@Autowired` injection. [source,java,indent=0] ---- @Configuration public class MyConfiguration { + @LoadBalanced + @Bean + RestTemplate loadBalanced() { + return new RestTemplate(); + } + @Primary @Bean RestTemplate restTemplate() { 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 19c61f56..88dc2f7d 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 @@ -17,8 +17,11 @@ package org.springframework.cloud.client.loadbalancer; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import org.springframework.beans.factory.SmartInitializingSingleton; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -38,15 +41,23 @@ import org.springframework.web.client.RestTemplate; @ConditionalOnBean(LoadBalancerClient.class) public class LoadBalancerAutoConfiguration { - @Bean @LoadBalanced - public RestTemplate loadBalancedRestTemplate( - List customizers) { - RestTemplate restTemplate = new RestTemplate(); - for (RestTemplateCustomizer customizer : customizers) { - customizer.customize(restTemplate); - } - return restTemplate; + @Autowired(required = false) + private List restTemplates = Collections.emptyList(); + + @Bean + public SmartInitializingSingleton loadBalancedRestTemplateInitializer( + final List customizers) { + return new SmartInitializingSingleton() { + @Override + public void afterSingletonsInstantiated() { + for (RestTemplate restTemplate : LoadBalancerAutoConfiguration.this.restTemplates) { + for (RestTemplateCustomizer customizer : customizers) { + customizer.customize(restTemplate); + } + } + } + }; } @Bean diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java index f672d9bc..3606fa0d 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfigurationTests.java @@ -1,20 +1,11 @@ package org.springframework.cloud.client.loadbalancer; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.junit.Assert.assertThat; - import java.net.URI; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Random; -import lombok.SneakyThrows; - import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.builder.SpringApplicationBuilder; @@ -27,107 +18,132 @@ import org.springframework.context.annotation.Primary; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.web.client.RestTemplate; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertThat; + +import lombok.SneakyThrows; + /** * @author Spencer Gibb */ public class LoadBalancerAutoConfigurationTests { - @Test - public void restTemplateGetsLoadBalancerInterceptor() { - ConfigurableApplicationContext context = init(OneRestTemplate.class); - final Map restTemplates = context.getBeansOfType(RestTemplate.class); + @Test + public void restTemplateGetsLoadBalancerInterceptor() { + ConfigurableApplicationContext context = init(OneRestTemplate.class); + final Map restTemplates = context + .getBeansOfType(RestTemplate.class); - assertThat(restTemplates, is(notNullValue())); - assertThat(restTemplates.values(), hasSize(1)); - RestTemplate restTemplate = restTemplates.values().iterator().next(); - assertThat(restTemplate, is(notNullValue())); + assertThat(restTemplates, is(notNullValue())); + assertThat(restTemplates.values(), hasSize(1)); + RestTemplate restTemplate = restTemplates.values().iterator().next(); + assertThat(restTemplate, is(notNullValue())); - assertLoadBalanced(restTemplate); - } + assertLoadBalanced(restTemplate); + } - protected void assertLoadBalanced(RestTemplate restTemplate) { - List interceptors = restTemplate.getInterceptors(); - assertThat(interceptors, hasSize(1)); - ClientHttpRequestInterceptor interceptor = interceptors.get(0); - assertThat(interceptor, is(instanceOf(LoadBalancerInterceptor.class))); - } + protected void assertLoadBalanced(RestTemplate restTemplate) { + List interceptors = restTemplate.getInterceptors(); + assertThat(interceptors, hasSize(1)); + ClientHttpRequestInterceptor interceptor = interceptors.get(0); + assertThat(interceptor, is(instanceOf(LoadBalancerInterceptor.class))); + } - @Test - public void multipleRestTemplates() { - ConfigurableApplicationContext context = init(TwoRestTemplates.class); - final Map restTemplates = context.getBeansOfType(RestTemplate.class); + @Test + public void multipleRestTemplates() { + ConfigurableApplicationContext context = init(TwoRestTemplates.class); + final Map restTemplates = context + .getBeansOfType(RestTemplate.class); - assertThat(restTemplates, is(notNullValue())); - Collection templates = restTemplates.values(); - assertThat(templates, hasSize(2)); + assertThat(restTemplates, is(notNullValue())); + Collection templates = restTemplates.values(); + assertThat(templates, hasSize(2)); - TwoRestTemplates.Two two = context.getBean(TwoRestTemplates.Two.class); + TwoRestTemplates.Two two = context.getBean(TwoRestTemplates.Two.class); - assertThat(two.loadBalanced, is(notNullValue())); - assertLoadBalanced(two.loadBalanced); + assertThat(two.loadBalanced, is(notNullValue())); + assertLoadBalanced(two.loadBalanced); - assertThat(two.nonLoadBalanced, is(notNullValue())); - assertThat(two.nonLoadBalanced.getInterceptors(), is(empty())); - } + assertThat(two.nonLoadBalanced, is(notNullValue())); + assertThat(two.nonLoadBalanced.getInterceptors(), is(empty())); + } + protected ConfigurableApplicationContext init(Class config) { + return new SpringApplicationBuilder().web(false) + .properties("spring.aop.proxyTargetClass=true") + .sources(config, LoadBalancerAutoConfiguration.class).run(); + } - protected ConfigurableApplicationContext init(Class config) { - return new SpringApplicationBuilder().web(false).sources(config, LoadBalancerAutoConfiguration.class).run(); - } + @Configuration + protected static class OneRestTemplate { - @Configuration - protected static class OneRestTemplate { + @LoadBalanced + @Bean + RestTemplate loadBalancedRestTemplate() { + return new RestTemplate(); + } - @Bean - LoadBalancerClient loadBalancerClient() { - return new NoopLoadBalancerClient(); - } + @Bean + LoadBalancerClient loadBalancerClient() { + return new NoopLoadBalancerClient(); + } - } + } - @Configuration - protected static class TwoRestTemplates { + @Configuration + protected static class TwoRestTemplates { - @Primary - @Bean - RestTemplate restTemplate() { - return new RestTemplate(); - } + @Primary + @Bean + RestTemplate restTemplate() { + return new RestTemplate(); + } - @Bean - LoadBalancerClient loadBalancerClient() { - return new NoopLoadBalancerClient(); - } + @LoadBalanced + @Bean + RestTemplate loadBalancedRestTemplate() { + return new RestTemplate(); + } - @Configuration - protected static class Two { - @Autowired - RestTemplate nonLoadBalanced; + @Bean + LoadBalancerClient loadBalancerClient() { + return new NoopLoadBalancerClient(); + } - @Autowired - @LoadBalanced - RestTemplate loadBalanced; - } + @Configuration + protected static class Two { + @Autowired + RestTemplate nonLoadBalanced; - } + @Autowired + @LoadBalanced + RestTemplate loadBalanced; + } - private static class NoopLoadBalancerClient implements LoadBalancerClient { - private final Random random = new Random(); + } - @Override - public ServiceInstance choose(String serviceId) { - return new DefaultServiceInstance(serviceId, serviceId, random.nextInt(40000), false); - } + private static class NoopLoadBalancerClient implements LoadBalancerClient { + private final Random random = new Random(); - @Override - @SneakyThrows - public T execute(String serviceId, LoadBalancerRequest request) { - return request.apply(choose(serviceId)); - } + @Override + public ServiceInstance choose(String serviceId) { + return new DefaultServiceInstance(serviceId, serviceId, + this.random.nextInt(40000), false); + } - @Override - public URI reconstructURI(ServiceInstance instance, URI original) { - return DefaultServiceInstance.getUri(instance); - } - } + @Override + @SneakyThrows + public T execute(String serviceId, LoadBalancerRequest request) { + return request.apply(choose(serviceId)); + } + + @Override + public URI reconstructURI(ServiceInstance instance, URI original) { + return DefaultServiceInstance.getUri(instance); + } + } }