From 30f34cf3bcc0f9efe6026fe6f0b05581299fd3ae Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 18 Nov 2016 11:53:34 -0500 Subject: [PATCH 01/14] Make sure we copy variables to new InfoEndpoint. Fixes #143. --- .../RefreshEndpointAutoConfiguration.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java index bac20fea..7c613ae4 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java @@ -58,11 +58,14 @@ import org.springframework.integration.monitor.IntegrationMBeanExporter; @AutoConfigureAfter(EndpointAutoConfiguration.class) public class RefreshEndpointAutoConfiguration { - @ConditionalOnBean(EndpointAutoConfiguration.class) @ConditionalOnMissingClass("org.springframework.boot.actuate.info.InfoContributor") - @Bean - InfoEndpointRebinderConfiguration infoEndpointRebinderConfiguration() { - return new InfoEndpointRebinderConfiguration(); + protected static class InfoEndpointAutoConfiguration { + + @ConditionalOnBean(EndpointAutoConfiguration.class) + @Bean + InfoEndpointRebinderConfiguration infoEndpointRebinderConfiguration() { + return new InfoEndpointRebinderConfiguration(); + } } @ConditionalOnMissingBean @@ -168,7 +171,7 @@ public class RefreshEndpointAutoConfiguration { } private InfoEndpoint infoEndpoint(InfoEndpoint endpoint) { - return new InfoEndpoint(endpoint.invoke()) { + InfoEndpoint newEndpoint = new InfoEndpoint(endpoint.invoke()) { @Override public Map invoke() { Map info = new LinkedHashMap( @@ -177,6 +180,10 @@ public class RefreshEndpointAutoConfiguration { return info; } }; + newEndpoint.setId(endpoint.getId()); + newEndpoint.setEnabled(endpoint.isEnabled()); + newEndpoint.setSensitive(endpoint.isSensitive()); + return newEndpoint; } } From 2266cc9c1a027b5baaadade8e4e49db305581449 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 21 Nov 2016 11:01:02 -0500 Subject: [PATCH 02/14] Added TODO to remove InfoEndpointRebinderConfiguration --- .../cloud/autoconfigure/RefreshEndpointAutoConfiguration.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java index 7c613ae4..a09858d1 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java @@ -58,6 +58,8 @@ import org.springframework.integration.monitor.IntegrationMBeanExporter; @AutoConfigureAfter(EndpointAutoConfiguration.class) public class RefreshEndpointAutoConfiguration { + //TODO Remove this class and InfoEndpointRebinderConfiguration once we no longer + //need to support Boot 1.3.x @ConditionalOnMissingClass("org.springframework.boot.actuate.info.InfoContributor") protected static class InfoEndpointAutoConfiguration { From 0a6466d9c0e111205da8e11ac19fc20b4f7b3304 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 21 Nov 2016 12:01:21 -0500 Subject: [PATCH 03/14] Revert "Added TODO to remove InfoEndpointRebinderConfiguration" This reverts commit 2266cc9c1a027b5baaadade8e4e49db305581449. --- .../cloud/autoconfigure/RefreshEndpointAutoConfiguration.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java index a09858d1..7c613ae4 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java @@ -58,8 +58,6 @@ import org.springframework.integration.monitor.IntegrationMBeanExporter; @AutoConfigureAfter(EndpointAutoConfiguration.class) public class RefreshEndpointAutoConfiguration { - //TODO Remove this class and InfoEndpointRebinderConfiguration once we no longer - //need to support Boot 1.3.x @ConditionalOnMissingClass("org.springframework.boot.actuate.info.InfoContributor") protected static class InfoEndpointAutoConfiguration { From dde952982ee238414a6f1b66ef0cd5f470c85a03 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 21 Nov 2016 12:01:31 -0500 Subject: [PATCH 04/14] Revert "Make sure we copy variables to new InfoEndpoint. Fixes #143." This reverts commit 30f34cf3bcc0f9efe6026fe6f0b05581299fd3ae. --- .../RefreshEndpointAutoConfiguration.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java index 7c613ae4..bac20fea 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java @@ -58,14 +58,11 @@ import org.springframework.integration.monitor.IntegrationMBeanExporter; @AutoConfigureAfter(EndpointAutoConfiguration.class) public class RefreshEndpointAutoConfiguration { + @ConditionalOnBean(EndpointAutoConfiguration.class) @ConditionalOnMissingClass("org.springframework.boot.actuate.info.InfoContributor") - protected static class InfoEndpointAutoConfiguration { - - @ConditionalOnBean(EndpointAutoConfiguration.class) - @Bean - InfoEndpointRebinderConfiguration infoEndpointRebinderConfiguration() { - return new InfoEndpointRebinderConfiguration(); - } + @Bean + InfoEndpointRebinderConfiguration infoEndpointRebinderConfiguration() { + return new InfoEndpointRebinderConfiguration(); } @ConditionalOnMissingBean @@ -171,7 +168,7 @@ public class RefreshEndpointAutoConfiguration { } private InfoEndpoint infoEndpoint(InfoEndpoint endpoint) { - InfoEndpoint newEndpoint = new InfoEndpoint(endpoint.invoke()) { + return new InfoEndpoint(endpoint.invoke()) { @Override public Map invoke() { Map info = new LinkedHashMap( @@ -180,10 +177,6 @@ public class RefreshEndpointAutoConfiguration { return info; } }; - newEndpoint.setId(endpoint.getId()); - newEndpoint.setEnabled(endpoint.isEnabled()); - newEndpoint.setSensitive(endpoint.isSensitive()); - return newEndpoint; } } From 8d3d07bde696c1f0b02c9abcc8d4965afda70001 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 21 Nov 2016 12:07:01 -0500 Subject: [PATCH 05/14] We no longer need InfoEndpointRebinderConfiguration since we will not be supporting Boot 1.3.x in Dalston. See #145. --- .../RefreshEndpointAutoConfiguration.java | 55 ------------------- 1 file changed, 55 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java index bac20fea..32c58d29 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java @@ -58,13 +58,6 @@ import org.springframework.integration.monitor.IntegrationMBeanExporter; @AutoConfigureAfter(EndpointAutoConfiguration.class) public class RefreshEndpointAutoConfiguration { - @ConditionalOnBean(EndpointAutoConfiguration.class) - @ConditionalOnMissingClass("org.springframework.boot.actuate.info.InfoContributor") - @Bean - InfoEndpointRebinderConfiguration infoEndpointRebinderConfiguration() { - return new InfoEndpointRebinderConfiguration(); - } - @ConditionalOnMissingBean @ConditionalOnEnabledHealthIndicator("refresh") @Bean @@ -133,52 +126,4 @@ public class RefreshEndpointAutoConfiguration { } } - - private static class InfoEndpointRebinderConfiguration - implements ApplicationListener, BeanPostProcessor { - - @Autowired - private ConfigurableEnvironment environment; - - private Map map = new LinkedHashMap(); - - @Override - public void onApplicationEvent(EnvironmentChangeEvent event) { - for (String key : event.getKeys()) { - if (key.startsWith("info.")) { - this.map.put(key.substring("info.".length()), - this.environment.getProperty(key)); - } - } - } - - @Override - public Object postProcessAfterInitialization(Object bean, String beanName) - throws BeansException { - if (bean instanceof InfoEndpoint) { - return infoEndpoint((InfoEndpoint) bean); - } - return bean; - } - - @Override - public Object postProcessBeforeInitialization(Object bean, String beanName) - throws BeansException { - return bean; - } - - private InfoEndpoint infoEndpoint(InfoEndpoint endpoint) { - return new InfoEndpoint(endpoint.invoke()) { - @Override - public Map invoke() { - Map info = new LinkedHashMap( - super.invoke()); - info.putAll(InfoEndpointRebinderConfiguration.this.map); - return info; - } - }; - } - - } - } \ No newline at end of file From 1e447eb9d1ef9ef42595dfda526d38d12cf40bd6 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 8 Dec 2016 09:49:43 -0700 Subject: [PATCH 06/14] More deprecation in Discovery. These functions will be taken over by ServiceRegistry --- .../cloud/client/discovery/DiscoveryClient.java | 10 ++++++---- .../cloud/client/discovery/DiscoveryLifecycle.java | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryClient.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryClient.java index 285876cf..8555c72d 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryClient.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryClient.java @@ -31,23 +31,25 @@ public interface DiscoveryClient { * A human readable description of the implementation, used in HealthIndicator * @return the description */ - public String description(); + String description(); /** + * @deprecated use the {@link org.springframework.cloud.client.serviceregistry.Registration} bean instead + * * @return ServiceInstance with information used to register the local service */ - public ServiceInstance getLocalServiceInstance(); + ServiceInstance getLocalServiceInstance(); /** * Get all ServiceInstances associated with a particular serviceId * @param serviceId the serviceId to query * @return a List of ServiceInstance */ - public List getInstances(String serviceId); + List getInstances(String serviceId); /** * @return all known service ids */ - public List getServices(); + List getServices(); } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryLifecycle.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryLifecycle.java index d7a8f4e3..cd7252b1 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryLifecycle.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryLifecycle.java @@ -21,6 +21,8 @@ import org.springframework.core.Ordered; /** * @author Spencer Gibb + * @deprecated use {@link org.springframework.cloud.client.serviceregistry.AutoServiceRegistration} instead. This class will be removed in the next release train. */ +@Deprecated public interface DiscoveryLifecycle extends SmartLifecycle, Ordered { } From 35b5d53841c43bfd165082b1b3b2a8ce0a5a71e6 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 8 Dec 2016 09:50:28 -0700 Subject: [PATCH 07/14] Move endpoint config to nested class --- .../ServiceRegistryAutoConfiguration.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java index 0454733a..11fa0c62 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/ServiceRegistryAutoConfiguration.java @@ -11,18 +11,20 @@ import org.springframework.context.annotation.Configuration; /** * @author Spencer Gibb */ -@ConditionalOnBean(ServiceRegistry.class) -@ConditionalOnClass(Endpoint.class) @Configuration public class ServiceRegistryAutoConfiguration { - @Autowired(required = false) - private Registration registration; + @ConditionalOnBean(ServiceRegistry.class) + @ConditionalOnClass(Endpoint.class) + protected class ServiceRegistryEndpointConfiguration { + @Autowired(required = false) + private Registration registration; - @Bean - public ServiceRegistryEndpoint serviceRegistryEndpoint(ServiceRegistry serviceRegistry) { - ServiceRegistryEndpoint endpoint = new ServiceRegistryEndpoint(serviceRegistry); - endpoint.setRegistration(registration); - return endpoint; + @Bean + public ServiceRegistryEndpoint serviceRegistryEndpoint(ServiceRegistry serviceRegistry) { + ServiceRegistryEndpoint endpoint = new ServiceRegistryEndpoint(serviceRegistry); + endpoint.setRegistration(registration); + return endpoint; + } } } From f7e59a4bfec365af2260e72e45d425a59c450e9e Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 8 Dec 2016 09:52:33 -0700 Subject: [PATCH 08/14] Change auto-registration intitialization. The import-selected AutoServiceRegistrationConfiguration now creates the configuration properties bean, which is used as a marker bean for AutoServiceRegistrationAutoConfiguration. This gives auto registration implementations a chance to create an AutoServiceRegistration impl bean. --- ...oServiceRegistrationAutoConfiguration.java | 28 +++++++++++++++++++ .../AutoServiceRegistrationConfiguration.java | 16 ----------- .../AutoServiceRegistrationProperties.java | 3 ++ ...ceRegistrationAutoConfigurationTests.java} | 3 +- 4 files changed, 33 insertions(+), 17 deletions(-) create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationAutoConfiguration.java rename spring-cloud-commons/src/test/java/org/springframework/cloud/client/serviceregistry/{AutoServiceRegistrationConfigurationTests.java => AutoServiceRegistrationAutoConfigurationTests.java} (95%) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationAutoConfiguration.java new file mode 100644 index 00000000..8b4b7d05 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationAutoConfiguration.java @@ -0,0 +1,28 @@ +package org.springframework.cloud.client.serviceregistry; + +import javax.annotation.PostConstruct; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.context.annotation.Configuration; + +/** + * @author Spencer Gibb + */ +@Configuration +@ConditionalOnBean(AutoServiceRegistrationProperties.class) +public class AutoServiceRegistrationAutoConfiguration { + + @Autowired(required = false) + private AutoServiceRegistration autoServiceRegistration; + + @Autowired + private AutoServiceRegistrationProperties properties; + + @PostConstruct + protected void init() { + if (autoServiceRegistration == null && this.properties.isFailFast()) { + throw new IllegalStateException("Auto Service Registration has been requested, but there is no AutoServiceRegistration bean"); + } + } +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationConfiguration.java index 45a95d54..5cc7d689 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationConfiguration.java @@ -1,28 +1,12 @@ package org.springframework.cloud.client.serviceregistry; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Configuration; -import javax.annotation.PostConstruct; - /** * @author Spencer Gibb */ @Configuration @EnableConfigurationProperties(AutoServiceRegistrationProperties.class) public class AutoServiceRegistrationConfiguration { - - @Autowired(required = false) - private AutoServiceRegistration autoServiceRegistration; - - @Autowired - private AutoServiceRegistrationProperties properties; - - @PostConstruct - protected void init() { - if (autoServiceRegistration == null && this.properties.isFailFast()) { - throw new IllegalStateException("Auto Service Registration has been requested, but there is no AutoServiceRegistration bean"); - } - } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationProperties.java index 5a8b22d5..d489afe8 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationProperties.java @@ -8,6 +8,9 @@ import org.springframework.boot.context.properties.ConfigurationProperties; @ConfigurationProperties("spring.cloud.service-registry.auto-registration") public class AutoServiceRegistrationProperties { + /** If Auto-Service Registration is enabled, default to true. */ + private boolean enabled = true; + /** Should startup fail if there is no AutoServiceRegistration, default to false. */ private boolean failFast = false; diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationAutoConfigurationTests.java similarity index 95% rename from spring-cloud-commons/src/test/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationConfigurationTests.java rename to spring-cloud-commons/src/test/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationAutoConfigurationTests.java index 31476e79..39e82067 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/serviceregistry/AutoServiceRegistrationAutoConfigurationTests.java @@ -20,7 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Spencer Gibb */ -public class AutoServiceRegistrationConfigurationTests { +public class AutoServiceRegistrationAutoConfigurationTests { @Rule public ExpectedException exception = ExpectedException.none(); @@ -69,6 +69,7 @@ public class AutoServiceRegistrationConfigurationTests { private AnnotationConfigApplicationContext setup(String property, Class... classes) { ArrayList list = new ArrayList<>(); list.add(AutoServiceRegistrationConfiguration.class); + list.add(AutoServiceRegistrationAutoConfiguration.class); list.addAll(Arrays.asList(classes)); AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); context.register(list.toArray(new Class[0])); From c5d73294c405d80cf89565cce26c5f9fc5c4dd54 Mon Sep 17 00:00:00 2001 From: Vitalii Date: Tue, 20 Dec 2016 19:12:08 +0100 Subject: [PATCH 09/14] Fixed typo in event class name (#152) --- docs/src/main/asciidoc/spring-cloud-commons.adoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index fd45c41d..ccdadf02 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -217,10 +217,10 @@ application that includes that jar on its classpath. === Environment Changes -The application will listen for an `EnvironmentChangedEvent` and react +The application will listen for an `EnvironmentChangeEvent` and react to the change in a couple of standard ways (additional `ApplicationListeners` can be added as `@Beans` by the user in the -normal way). When an `EnvironmentChangedEvent` is observed it will +normal way). When an `EnvironmentChangeEvent` is observed it will have a list of key values that have changed, and the application will use those to: @@ -231,12 +231,12 @@ Note that the Config Client does not by default poll for changes in the `Environment`, and generally we would not recommend that approach for detecting changes (although you could set it up with a `@Scheduled` annotation). If you have a scaled-out client application -then it is better to broadcast the `EnvironmentChangedEvent` to all +then it is better to broadcast the `EnvironmentChangeEvent` to all the instances instead of having them polling for changes (e.g. using the https://github.com/spring-cloud/spring-cloud-bus[Spring Cloud Bus]). -The `EnvironmentChangedEvent` covers a large class of refresh use +The `EnvironmentChangeEvent` covers a large class of refresh use cases, as long as you can actually make a change to the `Environment` and publish the event (those APIs are public and part of core Spring). You can verify the changes are bound to From 82175d1cac2a1eddce1ef2e9a761bfc70b486d3e Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Thu, 29 Dec 2016 19:43:24 -0500 Subject: [PATCH 10/14] Fixed retry policy see spring-cloud/spring-cloud-netflx#1577 --- docs/src/main/asciidoc/spring-cloud-commons.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index ccdadf02..bcdc27e0 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -363,7 +363,7 @@ for details of how the `RestTemplate` is set up. A load balanced `RestTemplate` can be configured to retry failed requests. By default this logic is disabled, you can enable it by setting -`spring.cloud.loadbalancer.retry=true`. The load balanced `RestTemplate` will +`spring.cloud.loadbalancer.retry.enable=true`. The load balanced `RestTemplate` will honor some of the Ribbon configuration values related to retrying failed requests. The properties you can use are `client.ribbon.MaxAutoRetries`, `client.ribbon.MaxAutoRetriesNextServer`, and `client.ribbon.OkToRetryOnAllOperations`. From 1762a6a59fecd64fbe1a9adea9ad5294d464e205 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Thu, 29 Dec 2016 19:44:31 -0500 Subject: [PATCH 11/14] Fixed typo in 'enabled' --- docs/src/main/asciidoc/spring-cloud-commons.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index bcdc27e0..b8e4c6f9 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -363,7 +363,7 @@ for details of how the `RestTemplate` is set up. A load balanced `RestTemplate` can be configured to retry failed requests. By default this logic is disabled, you can enable it by setting -`spring.cloud.loadbalancer.retry.enable=true`. The load balanced `RestTemplate` will +`spring.cloud.loadbalancer.retry.enabled=true`. The load balanced `RestTemplate` will honor some of the Ribbon configuration values related to retrying failed requests. The properties you can use are `client.ribbon.MaxAutoRetries`, `client.ribbon.MaxAutoRetriesNextServer`, and `client.ribbon.OkToRetryOnAllOperations`. From 69192f6f67d575e321414409dad5dd7f6f89426c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20B=C3=A5tstrand?= Date: Wed, 4 Jan 2017 10:50:41 +0100 Subject: [PATCH 12/14] Exposing context names, fixes https://github.com/spring-cloud/spring-cloud-netflix/issues/1585. --- .../cloud/context/named/NamedContextFactory.java | 6 ++++++ .../cloud/context/named/NamedContextFactoryTests.java | 3 +++ 2 files changed, 9 insertions(+) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/NamedContextFactory.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/NamedContextFactory.java index 44a76aa0..2053dd88 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/NamedContextFactory.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/NamedContextFactory.java @@ -2,8 +2,10 @@ package org.springframework.cloud.context.named; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.springframework.beans.BeansException; @@ -61,6 +63,10 @@ public abstract class NamedContextFactory getContextNames() { + return new HashSet<>(contexts.keySet()); + } + @Override public void destroy() { Collection values = this.contexts.values(); diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/named/NamedContextFactoryTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/named/NamedContextFactoryTests.java index a02ee26b..7ed010b6 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/named/NamedContextFactoryTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/named/NamedContextFactoryTests.java @@ -12,6 +12,7 @@ import lombok.Data; import lombok.NoArgsConstructor; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; @@ -37,6 +38,8 @@ public class NamedContextFactoryTests { Bar bar = factory.getInstance("bar", Bar.class); assertThat("bar was null", bar, is(notNullValue())); + assertThat("context names not exposed", factory.getContextNames(), hasItems("foo", "bar")); + Bar foobar = factory.getInstance("foo", Bar.class); assertThat("bar was not null", foobar, is(nullValue())); From c87b8b33fd3902823d595e30644714af783aa967 Mon Sep 17 00:00:00 2001 From: Will Tran Date: Wed, 4 Jan 2017 10:16:15 -0500 Subject: [PATCH 13/14] Customize load balanced requests according to the chosen ServiceInstance Applications can define their own LoadBalancerRequestTransformer beans which can modify the HttpRequest to be executed. These beans can be @Ordered in case of multiple transformers. Fixes #162 --- .../LoadBalancerAutoConfiguration.java | 26 ++- .../loadbalancer/LoadBalancerInterceptor.java | 26 ++- .../LoadBalancerRequestFactory.java | 69 +++++++ .../LoadBalancerRequestTransformer.java | 33 ++++ .../RetryLoadBalancerInterceptor.java | 43 +++-- ...ancerRequestFactoryConfigurationTests.java | 172 ++++++++++++++++++ .../LoadBalancerRequestFactoryTests.java | 123 +++++++++++++ .../RetryLoadBalancerInterceptorTest.java | 17 +- 8 files changed, 471 insertions(+), 38 deletions(-) create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactory.java create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestTransformer.java create mode 100644 spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java create mode 100644 spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryTests.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 a0d51d93..3a892aaf 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 2013-2015 the original author or authors. + * Copyright 2013-2017 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. @@ -26,7 +26,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -39,6 +38,7 @@ import org.springframework.web.client.RestTemplate; * * @author Spencer Gibb * @author Dave Syer + * @author Will Tran */ @Configuration @ConditionalOnClass(RestTemplate.class) @@ -65,12 +65,24 @@ public class LoadBalancerAutoConfiguration { }; } + @Autowired(required = false) + private List transformers = Collections.emptyList(); + + @Bean + @ConditionalOnMissingBean + public LoadBalancerRequestFactory loadBalancerRequestFactory( + LoadBalancerClient loadBalancerClient) { + return new LoadBalancerRequestFactory(loadBalancerClient, transformers); + } + @Configuration @ConditionalOnMissingClass("org.springframework.retry.support.RetryTemplate") static class LoadBalancerInterceptorConfig { @Bean - public LoadBalancerInterceptor ribbonInterceptor(LoadBalancerClient loadBalancerClient) { - return new LoadBalancerInterceptor(loadBalancerClient); + public LoadBalancerInterceptor ribbonInterceptor( + LoadBalancerClient loadBalancerClient, + LoadBalancerRequestFactory requestFactory) { + return new LoadBalancerInterceptor(loadBalancerClient, requestFactory); } @Bean @@ -108,8 +120,10 @@ public class LoadBalancerAutoConfiguration { @Bean public RetryLoadBalancerInterceptor ribbonInterceptor( LoadBalancerClient loadBalancerClient, LoadBalancerRetryProperties properties, - LoadBalancedRetryPolicyFactory lbRetryPolicyFactory) { - return new RetryLoadBalancerInterceptor(loadBalancerClient, retryTemplate(), properties, lbRetryPolicyFactory); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, + LoadBalancerRequestFactory requestFactory) { + return new RetryLoadBalancerInterceptor(loadBalancerClient, retryTemplate(), properties, + lbRetryPolicyFactory, requestFactory); } @Bean diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java index 0d989ae0..0db5379d 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2017 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. @@ -18,7 +18,7 @@ package org.springframework.cloud.client.loadbalancer; import java.io.IOException; import java.net.URI; -import org.springframework.cloud.client.ServiceInstance; + import org.springframework.http.HttpRequest; import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpRequestInterceptor; @@ -28,13 +28,21 @@ import org.springframework.http.client.ClientHttpResponse; * @author Spencer Gibb * @author Dave Syer * @author Ryan Baxter + * @author William Tran */ public class LoadBalancerInterceptor implements ClientHttpRequestInterceptor { private LoadBalancerClient loadBalancer; + private LoadBalancerRequestFactory requestFactory; + + public LoadBalancerInterceptor(LoadBalancerClient loadBalancer, LoadBalancerRequestFactory requestFactory) { + this.loadBalancer = loadBalancer; + this.requestFactory = requestFactory; + } public LoadBalancerInterceptor(LoadBalancerClient loadBalancer) { - this.loadBalancer = loadBalancer; + // for backwards compatibility + this(loadBalancer, new LoadBalancerRequestFactory(loadBalancer)); } @Override @@ -42,16 +50,6 @@ public class LoadBalancerInterceptor implements ClientHttpRequestInterceptor { final ClientHttpRequestExecution execution) throws IOException { final URI originalUri = request.getURI(); String serviceName = originalUri.getHost(); - return this.loadBalancer.execute(serviceName, - new LoadBalancerRequest() { - @Override - public ClientHttpResponse apply(final ServiceInstance instance) - throws Exception { - HttpRequest serviceRequest = new ServiceRequestWrapper(request, - instance, loadBalancer); - return execution.execute(serviceRequest, body); - } - - }); + return this.loadBalancer.execute(serviceName, requestFactory.createRequest(request, body, execution)); } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactory.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactory.java new file mode 100644 index 00000000..1a88c986 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactory.java @@ -0,0 +1,69 @@ +/* + * Copyright 2017 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 + * + * http://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 java.util.List; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.http.HttpRequest; +import org.springframework.http.client.ClientHttpRequestExecution; +import org.springframework.http.client.ClientHttpResponse; + +/** + * Creates {@link LoadBalancerRequest}s for {@link LoadBalancerInterceptor} and + * {@link RetryLoadBalancerInterceptor}. Applies + * {@link LoadBalancerRequestTransformer}s to the intercepted + * {@link HttpRequest}. + * + * @author William Tran + * + */ +public class LoadBalancerRequestFactory { + + private LoadBalancerClient loadBalancer; + private List transformers; + + public LoadBalancerRequestFactory(LoadBalancerClient loadBalancer, + List transformers) { + this.loadBalancer = loadBalancer; + this.transformers = transformers; + } + + public LoadBalancerRequestFactory(LoadBalancerClient loadBalancer) { + this.loadBalancer = loadBalancer; + } + + public LoadBalancerRequest createRequest(final HttpRequest request, + final byte[] body, final ClientHttpRequestExecution execution) { + return new LoadBalancerRequest() { + + @Override + public ClientHttpResponse apply(final ServiceInstance instance) + throws Exception { + HttpRequest serviceRequest = new ServiceRequestWrapper(request, instance, loadBalancer); + if (transformers != null) { + for (LoadBalancerRequestTransformer transformer : transformers) { + serviceRequest = transformer.transformRequest(serviceRequest, instance); + } + } + return execution.execute(serviceRequest, body); + } + + }; + } + +} diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestTransformer.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestTransformer.java new file mode 100644 index 00000000..bf25e419 --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestTransformer.java @@ -0,0 +1,33 @@ +/* + * Copyright 2017 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 + * + * http://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.cloud.client.ServiceInstance; +import org.springframework.core.annotation.Order; +import org.springframework.http.HttpRequest; + +/** + * Allows applications to transform the load balanced {@link HttpRequest} given + * the chosen {@link ServiceInstance} + * + * @author Will Tran + */ +@Order(LoadBalancerRequestTransformer.DEFAULT_ORDER) +public interface LoadBalancerRequestTransformer { + public static final int DEFAULT_ORDER = 0; + + HttpRequest transformRequest(HttpRequest request, ServiceInstance instance); +} \ No newline at end of file diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index a5cae45f..ab3a289e 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -1,3 +1,19 @@ +/* + * Copyright 2016-2017 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 + * + * http://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 java.io.IOException; @@ -15,6 +31,7 @@ import org.springframework.retry.support.RetryTemplate; /** * @author Ryan Baxter + * @author Will Tran */ public class RetryLoadBalancerInterceptor implements ClientHttpRequestInterceptor { @@ -22,15 +39,26 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto private RetryTemplate retryTemplate; private LoadBalancerClient loadBalancer; private LoadBalancerRetryProperties lbProperties; + private LoadBalancerRequestFactory requestFactory; public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, LoadBalancerRetryProperties lbProperties, - LoadBalancedRetryPolicyFactory lbRetryPolicyFactory) { + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, + LoadBalancerRequestFactory requestFactory) { this.loadBalancer = loadBalancer; this.lbRetryPolicyFactory = lbRetryPolicyFactory; this.retryTemplate = retryTemplate; this.lbProperties = lbProperties; + this.requestFactory = requestFactory; + } + + public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, RetryTemplate retryTemplate, + LoadBalancerRetryProperties lbProperties, + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory) { + // for backwards compatibility + this(loadBalancer, retryTemplate, lbProperties, lbRetryPolicyFactory, + new LoadBalancerRequestFactory(loadBalancer)); } @Override @@ -59,18 +87,7 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto } return RetryLoadBalancerInterceptor.this.loadBalancer.execute( serviceName, serviceInstance, - new LoadBalancerRequest() { - - @Override - public ClientHttpResponse apply( - final ServiceInstance instance) - throws Exception { - HttpRequest serviceRequest = new ServiceRequestWrapper( - request, instance, loadBalancer); - return execution.execute(serviceRequest, body); - } - - }); + requestFactory.createRequest(request, body, execution)); } }); } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java new file mode 100644 index 00000000..4cdbc0a5 --- /dev/null +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryConfigurationTests.java @@ -0,0 +1,172 @@ +/* + * Copyright 2017 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 + * + * http://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 static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.annotation.Order; +import org.springframework.http.HttpRequest; +import org.springframework.http.client.ClientHttpRequestExecution; + +@RunWith(MockitoJUnitRunner.class) +public class LoadBalancerRequestFactoryConfigurationTests { + + @Mock + private HttpRequest request; + @Mock + private HttpRequest transformedRequest; + @Mock + private HttpRequest transformedRequest2; + @Mock + private ClientHttpRequestExecution execution; + @Mock + private ServiceInstance instance; + + private byte[] body = new byte[] {}; + private ArgumentCaptor httpRequestCaptor; + private LoadBalancerRequestFactory lbReqFactory; + private LoadBalancerRequest lbRequest; + + @Before + public void setup() { + httpRequestCaptor = ArgumentCaptor.forClass(HttpRequest.class); + } + + protected ConfigurableApplicationContext init(Class config) { + ConfigurableApplicationContext context = new SpringApplicationBuilder().web(false) + .properties("spring.aop.proxyTargetClass=true") + .sources(config, LoadBalancerAutoConfiguration.class).run(); + + lbReqFactory = context.getBean(LoadBalancerRequestFactory.class); + lbRequest = lbReqFactory.createRequest(request, body, execution); + return context; + } + + @Test + public void transformer() throws Exception { + ConfigurableApplicationContext context = init(Transformer.class); + + LoadBalancerRequestTransformer transformer = context.getBean("transformer", + LoadBalancerRequestTransformer.class); + when(transformer.transformRequest(any(ServiceRequestWrapper.class), eq(instance))) + .thenReturn(transformedRequest); + + lbRequest.apply(instance); + + verify(execution).execute(httpRequestCaptor.capture(), eq(body)); + assertEquals( + "transformer should have transformed the ServiceRequestWrapper into transformedRequest", + transformedRequest, + httpRequestCaptor.getValue()); + } + + @Test + public void noTransformer() throws Exception { + init(NoTransformer.class); + + lbRequest.apply(instance); + + verify(execution).execute(httpRequestCaptor.capture(), eq(body)); + assertEquals( + "ServiceRequestWrapper should be executed", + ServiceRequestWrapper.class, + httpRequestCaptor.getValue().getClass()); + } + + @Test + public void transformersAreOrdered() throws Exception { + ConfigurableApplicationContext context = init(TransformersAreOrdered.class); + + LoadBalancerRequestTransformer transformer = context.getBean("transformer", + LoadBalancerRequestTransformer.class); + when(transformer.transformRequest(any(ServiceRequestWrapper.class), eq(instance))) + .thenReturn(transformedRequest); + LoadBalancerRequestTransformer transformer2 = context.getBean("transformer2", + LoadBalancerRequestTransformer.class); + when(transformer2.transformRequest(transformedRequest, instance)).thenReturn(transformedRequest2); + + lbRequest.apply(instance); + + verify(execution).execute(httpRequestCaptor.capture(), eq(body)); + assertEquals( + "transformer2 should run after transformer", + transformedRequest2, + httpRequestCaptor.getValue()); + } + + @Configuration + static class Transformer { + + @Bean + public LoadBalancerClient loadBalancerClient() { + return mock(LoadBalancerClient.class); + } + + @Bean + public LoadBalancerRequestTransformer transformer() { + return mock(LoadBalancerRequestTransformer.class); + } + + } + + @Configuration + static class TransformersAreOrdered { + + @Bean + public LoadBalancerClient loadBalancerClient() { + return mock(LoadBalancerClient.class); + } + + @Bean + public LoadBalancerRequestTransformer transformer() { + return mock(LoadBalancerRequestTransformer.class); + } + + @Bean + @Order(LoadBalancerRequestTransformer.DEFAULT_ORDER + 1) + public LoadBalancerRequestTransformer transformer2() { + return mock(LoadBalancerRequestTransformer.class); + } + + } + + @Configuration + static class NoTransformer { + + @Bean + public LoadBalancerClient loadBalancerClient() { + return mock(LoadBalancerClient.class); + } + + } + +} diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryTests.java new file mode 100644 index 00000000..b2640b1d --- /dev/null +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRequestFactoryTests.java @@ -0,0 +1,123 @@ +/* + * Copyright 2017 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 + * + * http://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 static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.http.HttpRequest; +import org.springframework.http.client.ClientHttpRequestExecution; +import org.springframework.http.client.ClientHttpResponse; + +@RunWith(MockitoJUnitRunner.class) +public class LoadBalancerRequestFactoryTests { + + @Mock + private LoadBalancerClient loadBalancer; + @Mock + private HttpRequest request; + @Mock + private HttpRequest transformedRequest1; + @Mock + private HttpRequest transformedRequest2; + + private byte[] body = new byte[] {}; + + @Mock + private ClientHttpRequestExecution execution; + @Mock + private ServiceInstance instance; + @Mock + private LoadBalancerRequestTransformer transformer1; + @Mock + private LoadBalancerRequestTransformer transformer2; + + private ArgumentCaptor httpRequestCaptor; + + @Before + public void setup() { + httpRequestCaptor = ArgumentCaptor.forClass(HttpRequest.class); + } + + @Test + public void testNullTransformers() throws Exception { + executeLbRequest(null); + + verify(execution).execute(httpRequestCaptor.capture(), eq(body)); + Assert.assertEquals("request should be of type ServiceRequestWrapper", ServiceRequestWrapper.class, + httpRequestCaptor.getValue().getClass()); + } + + @Test + public void testEmptyTransformers() throws Exception { + List transformers = Collections.emptyList(); + + executeLbRequest(transformers); + + verify(execution).execute(httpRequestCaptor.capture(), eq(body)); + Assert.assertEquals("request should be of type ServiceRequestWrapper", ServiceRequestWrapper.class, + httpRequestCaptor.getValue().getClass()); + } + + @Test + public void testOneTransformer() throws Exception { + List transformers = Arrays.asList(transformer1); + when(transformer1.transformRequest(any(ServiceRequestWrapper.class), eq(instance))).thenReturn(transformedRequest1); + + executeLbRequest(transformers); + + verify(execution).execute(httpRequestCaptor.capture(), eq(body)); + assertEquals("transformer1 should have transformed request into transformedRequest1", transformedRequest1, + httpRequestCaptor.getValue()); + } + + @Test + public void testTwoTransformers() throws Exception { + List transformers = Arrays.asList(transformer1, transformer2); + when(transformer1.transformRequest(any(ServiceRequestWrapper.class), eq(instance))).thenReturn(transformedRequest1); + when(transformer2.transformRequest(transformedRequest1, instance)) + .thenReturn(transformedRequest2); + + executeLbRequest(transformers); + + verify(execution).execute(httpRequestCaptor.capture(), eq(body)); + assertEquals("transformer2 should have transformed transformedRequest1 into transformedRequest2", + transformedRequest2, + httpRequestCaptor.getValue()); + } + + private void executeLbRequest(List transformers) throws Exception { + LoadBalancerRequestFactory lbReqFactory = new LoadBalancerRequestFactory(loadBalancer, transformers); + LoadBalancerRequest lbRequest = lbReqFactory.createRequest(request, body, execution); + lbRequest.apply(instance); + } + +} diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java index 1b4ba551..42bf0a5f 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java @@ -35,12 +35,14 @@ public class RetryLoadBalancerInterceptorTest { private LoadBalancerClient client; private RetryTemplate retryTemplate; private LoadBalancerRetryProperties lbProperties; + private LoadBalancerRequestFactory lbRequestFactory; @Before public void setUp() throws Exception { client = mock(LoadBalancerClient.class); retryTemplate = spy(new RetryTemplate()); lbProperties = new LoadBalancerRetryProperties(); + lbRequestFactory = mock(LoadBalancerRequestFactory.class); } @@ -62,11 +64,12 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()); lbProperties.setEnabled(false); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); + verify(lbRequestFactory).createRequest(request, body, execution); } @Test @@ -80,11 +83,12 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); + verify(lbRequestFactory).createRequest(request, body, execution); } @Test @@ -100,12 +104,13 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); assertThat(rsp, is(clientHttpResponse)); verify(retryTemplate, times(1)).setRetryPolicy(eq(interceptorRetryPolicy)); + verify(lbRequestFactory).createRequest(request, body, execution); } @Test @@ -121,13 +126,14 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); verify(client, times(2)).execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class)); assertThat(rsp, is(clientHttpResponse)); verify(retryTemplate, times(1)).setRetryPolicy(any(InterceptorRetryPolicy.class)); + verify(lbRequestFactory, times(2)).createRequest(request, body, execution); } @Test(expected = IOException.class) @@ -144,9 +150,10 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory, lbRequestFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); + verify(lbRequestFactory).createRequest(request, body, execution); } } \ No newline at end of file From e1460f9534549d009279be71fa442e1efaaa3c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E7=8C=BFDD?= Date: Tue, 10 Jan 2017 00:29:52 +0800 Subject: [PATCH 14/14] fixed : Retrying Failed Requests' property is wrong (#155)