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 a6d0f93c..28322b2f 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 @@ -19,13 +19,15 @@ package org.springframework.cloud.client.discovery; import java.util.List; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.core.Ordered; /** * DiscoveryClient represents read operations commonly available to Discovery service such as * Netflix Eureka or consul.io * @author Spencer Gibb + * @author Olga Maciaszek-Sharma */ -public interface DiscoveryClient { +public interface DiscoveryClient extends Ordered { /** * A human readable description of the implementation, used in HealthIndicator @@ -45,4 +47,8 @@ public interface DiscoveryClient { */ List getServices(); + @Override + default int getOrder() { + return Ordered.LOWEST_PRECEDENCE; + } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClient.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClient.java index 21a328de..b653890d 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClient.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClient.java @@ -7,18 +7,21 @@ import java.util.List; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; /** - * A {@link DiscoveryClient} composed of other Discovery Client's and will delegate the + * A {@link DiscoveryClient} composed of other Discovery Clients that will delegate the * calls to each of them in order * * @author Biju Kunjummen + * @author Olga Maciaszek-Sharma */ public class CompositeDiscoveryClient implements DiscoveryClient { private final List discoveryClients; public CompositeDiscoveryClient(List discoveryClients) { + AnnotationAwareOrderComparator.sort(discoveryClients); this.discoveryClients = discoveryClients; } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClient.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClient.java index f9b3d4fb..c81b7317 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClient.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClient.java @@ -12,6 +12,7 @@ import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperti * properties file as a source of service instances * * @author Biju Kunjummen + * @author Olga Maciaszek-Sharma */ public class SimpleDiscoveryClient implements DiscoveryClient { @@ -42,4 +43,9 @@ public class SimpleDiscoveryClient implements DiscoveryClient { public List getServices() { return new ArrayList<>(this.simpleDiscoveryProperties.getInstances().keySet()); } + + @Override + public int getOrder() { + return simpleDiscoveryProperties.getOrder(); + } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryProperties.java index e9273b52..0abe74ea 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryProperties.java @@ -10,13 +10,18 @@ import javax.annotation.PostConstruct; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.core.Ordered; /** * Properties to hold the details of a * {@link org.springframework.cloud.client.discovery.DiscoveryClient} service instances - * for a given service + * for a given service. + * It also holds the user-configurable order that will be used to establish the + * precedence of this client in the list of clients + * used by {@link org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClient}. * * @author Biju Kunjummen + * @author Olga Maciaszek-Sharma */ @ConfigurationProperties(prefix = "spring.cloud.discovery.client.simple") @@ -30,8 +35,10 @@ public class SimpleDiscoveryProperties { */ private SimpleServiceInstance local = new SimpleServiceInstance(); + private int order = Ordered.LOWEST_PRECEDENCE; + public Map> getInstances() { - return this.instances; + return instances; } public void setInstances(Map> instances) { @@ -39,7 +46,15 @@ public class SimpleDiscoveryProperties { } public SimpleServiceInstance getLocal() { - return this.local; + return local; + } + + public int getOrder() { + return order; + } + + public void setOrder(int order) { + this.order = order; } @PostConstruct diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientOrderTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientOrderTest.java new file mode 100644 index 00000000..d30fcd33 --- /dev/null +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientOrderTest.java @@ -0,0 +1,57 @@ +package org.springframework.cloud.client.discovery.composite; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.A_CUSTOM_DISCOVERY_CLIENT; +import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.CUSTOM_SERVICE_ID; +import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.THIRD_DISCOVERY_CLIENT; + +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.test.context.junit4.SpringRunner; + +/** + * Tests for the support of ordered {@link DiscoveryClient} instances in {@link CompositeDiscoveryClient} + * + * @author Olga Maciaszek-Sharma + */ +@RunWith(SpringRunner.class) +@SpringBootTest(properties = "spring.cloud.discovery.client.simple.order:2", classes = { + CompositeDiscoveryClientTestsConfig.class }) +public class CompositeDiscoveryClientOrderTest { + + @Autowired + DiscoveryClient discoveryClient; + + @Test + public void shouldGetOrderedDiscoveryClients() { + // when: + List discoveryClients = ((CompositeDiscoveryClient) discoveryClient) + .getDiscoveryClients(); + + // then: + assertThat(discoveryClients.get(0).description()) + .isEqualTo(A_CUSTOM_DISCOVERY_CLIENT); + assertThat(discoveryClients.get(1).description()) + .isEqualTo("Simple Discovery Client"); + assertThat(discoveryClients.get(2).description()) + .isEqualTo(THIRD_DISCOVERY_CLIENT); + } + + @Test + public void shouldOnlyReturnServiceInstancesForTheHighestPrecedenceDiscoveryClient() { + // when: + List serviceInstances = discoveryClient + .getInstances(CUSTOM_SERVICE_ID); + + // then: + assertThat(serviceInstances).hasSize(1); + assertThat(serviceInstances.get(0).getPort()).isEqualTo(123); + } +} diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTests.java index 841ab3cd..8eae5829 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTests.java @@ -1,25 +1,19 @@ package org.springframework.cloud.client.discovery.composite; +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.CUSTOM_SERVICE_ID; + import java.net.URI; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; + import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.core.annotation.Order; import org.springframework.test.context.junit4.SpringRunner; -import static org.assertj.core.api.Assertions.assertThat; - /** * Tests for behavior of Composite Discovery Client * @@ -32,7 +26,8 @@ import static org.assertj.core.api.Assertions.assertThat; "spring.cloud.discovery.client.simple.instances.service1[0].uri=http://s1-1:8080", "spring.cloud.discovery.client.simple.instances.service1[1].uri=https://s1-2:8443", "spring.cloud.discovery.client.simple.instances.service2[0].uri=https://s2-1:8080", - "spring.cloud.discovery.client.simple.instances.service2[1].uri=https://s2-2:443" }) + "spring.cloud.discovery.client.simple.instances.service2[1].uri=https://s2-2:443", }, classes = { + CompositeDiscoveryClientTestsConfig.class }) public class CompositeDiscoveryClientTests { @Autowired @@ -40,11 +35,11 @@ public class CompositeDiscoveryClientTests { @Test public void getInstancesByServiceIdShouldDelegateCall() { - assertThat(this.discoveryClient).isInstanceOf(CompositeDiscoveryClient.class); + assertThat(discoveryClient).isInstanceOf(CompositeDiscoveryClient.class); - assertThat(this.discoveryClient.getInstances("service1")).hasSize(2); + assertThat(discoveryClient.getInstances("service1")).hasSize(2); - ServiceInstance s1 = this.discoveryClient.getInstances("service1").get(0); + ServiceInstance s1 = discoveryClient.getInstances("service1").get(0); assertThat(s1.getHost()).isEqualTo("s1-1"); assertThat(s1.getPort()).isEqualTo(8080); assertThat(s1.getUri()).isEqualTo(URI.create("http://s1-1:8080")); @@ -53,53 +48,23 @@ public class CompositeDiscoveryClientTests { @Test public void getServicesShouldAggregateAllServiceNames() { - assertThat(this.discoveryClient.getServices()).containsOnlyOnce("service1", "service2", "custom"); + assertThat(discoveryClient.getServices()).containsOnlyOnce("service1", "service2", + CUSTOM_SERVICE_ID); } @Test public void getDescriptionShouldBeComposite() { - assertThat(this.discoveryClient.description()).isEqualTo("Composite Discovery Client"); + assertThat(discoveryClient.description()).isEqualTo("Composite Discovery Client"); } @Test public void getInstancesShouldRespectOrder() { - assertThat(this.discoveryClient.getInstances("custom")).hasSize(1); - assertThat(this.discoveryClient.getInstances("custom")).hasSize(1); + assertThat(discoveryClient.getInstances(CUSTOM_SERVICE_ID)).hasSize(1); + assertThat(discoveryClient.getInstances(CUSTOM_SERVICE_ID)).hasSize(1); } @Test public void getInstancesByUnknownServiceIdShouldReturnAnEmptyList() { - assertThat(this.discoveryClient.getInstances("unknown")).hasSize(0); - } - - @EnableAutoConfiguration - @Configuration - public static class Config { - - @Bean - @Order(1) - public DiscoveryClient customDiscoveryClient() { - return new DiscoveryClient() { - @Override - public String description() { - return "A custom discovery client"; - } - - @Override - public List getInstances(String serviceId) { - if (serviceId.equals("custom")) { - ServiceInstance s1 = new DefaultServiceInstance("custom", "host", - 123, false); - return Arrays.asList(s1); - } - return Collections.emptyList(); - } - - @Override - public List getServices() { - return Arrays.asList("custom"); - } - }; - } + assertThat(discoveryClient.getInstances("unknown")).hasSize(0); } } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTestsConfig.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTestsConfig.java new file mode 100644 index 00000000..b332602e --- /dev/null +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTestsConfig.java @@ -0,0 +1,66 @@ +package org.springframework.cloud.client.discovery.composite; + +import static java.util.Collections.singletonList; + +import java.util.Collections; +import java.util.List; + +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * Test configuration for {@link CompositeDiscoveryClient} tests. + * + * @author Olga Maciaszek-Sharma + */ +@Configuration +@EnableAutoConfiguration +public class CompositeDiscoveryClientTestsConfig { + + static final String A_CUSTOM_DISCOVERY_CLIENT = "A custom discovery client"; + static final String THIRD_DISCOVERY_CLIENT = "Third discovery client"; + static final String CUSTOM_SERVICE_ID = "custom"; + + @Bean + public DiscoveryClient customDiscoveryClient() { + return aDiscoveryClient(1, A_CUSTOM_DISCOVERY_CLIENT); + } + + @Bean + public DiscoveryClient thirdOrderCustomDiscoveryClient() { + return aDiscoveryClient(3, THIRD_DISCOVERY_CLIENT); + } + + private DiscoveryClient aDiscoveryClient(int order, String description) { + return new DiscoveryClient() { + @Override + public String description() { + return description; + } + + @Override + public List getInstances(String serviceId) { + if (serviceId.equals(CUSTOM_SERVICE_ID)) { + ServiceInstance s1 = new DefaultServiceInstance(CUSTOM_SERVICE_ID, + "host", 123, false); + return singletonList(s1); + } + return Collections.emptyList(); + } + + @Override + public List getServices() { + return singletonList(CUSTOM_SERVICE_ID); + } + + @Override + public int getOrder() { + return order; + } + }; + } +}