Skip to content

Commit 8504e7e

Browse files
author
OlgaMaciaszek
committed
Make CompositeDiscoveryClient use ordered DiscoveryClient instances.
Fixes spring-projectsgh-304.
1 parent 0e259ea commit 8504e7e

File tree

7 files changed

+173
-55
lines changed

7 files changed

+173
-55
lines changed

Diff for: ‎spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/DiscoveryClient.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919
import java.util.List;
2020

2121
import org.springframework.cloud.client.ServiceInstance;
22+
import org.springframework.core.Ordered;
2223

2324
/**
2425
* DiscoveryClient represents read operations commonly available to Discovery service such as
2526
* Netflix Eureka or consul.io
2627
* @author Spencer Gibb
28+
* @author Olga Maciaszek-Sharma
2729
*/
28-
public interface DiscoveryClient {
30+
public interface DiscoveryClient extends Ordered {
2931

3032
/**
3133
* A human readable description of the implementation, used in HealthIndicator
@@ -45,4 +47,8 @@ public interface DiscoveryClient {
4547
*/
4648
List<String> getServices();
4749

50+
@Override
51+
default int getOrder() {
52+
return Ordered.LOWEST_PRECEDENCE;
53+
}
4854
}

Diff for: ‎spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClient.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,21 @@
77

88
import org.springframework.cloud.client.ServiceInstance;
99
import org.springframework.cloud.client.discovery.DiscoveryClient;
10+
import org.springframework.core.annotation.AnnotationAwareOrderComparator;
1011

1112
/**
12-
* A {@link DiscoveryClient} composed of other Discovery Client's and will delegate the
13+
* A {@link DiscoveryClient} composed of other Discovery Clients that will delegate the
1314
* calls to each of them in order
1415
*
1516
* @author Biju Kunjummen
17+
* @author Olga Maciaszek-Sharma
1618
*/
1719
public class CompositeDiscoveryClient implements DiscoveryClient {
1820

1921
private final List<DiscoveryClient> discoveryClients;
2022

2123
public CompositeDiscoveryClient(List<DiscoveryClient> discoveryClients) {
24+
AnnotationAwareOrderComparator.sort(discoveryClients);
2225
this.discoveryClients = discoveryClients;
2326
}
2427

Diff for: ‎spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClient.java

+6
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* properties file as a source of service instances
1313
*
1414
* @author Biju Kunjummen
15+
* @author Olga Maciaszek-Sharma
1516
*/
1617
public class SimpleDiscoveryClient implements DiscoveryClient {
1718

@@ -42,4 +43,9 @@ public List<ServiceInstance> getInstances(String serviceId) {
4243
public List<String> getServices() {
4344
return new ArrayList<>(this.simpleDiscoveryProperties.getInstances().keySet());
4445
}
46+
47+
@Override
48+
public int getOrder() {
49+
return simpleDiscoveryProperties.getOrder();
50+
}
4551
}

Diff for: ‎spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryProperties.java

+18-3
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,18 @@
1010

1111
import org.springframework.boot.context.properties.ConfigurationProperties;
1212
import org.springframework.cloud.client.ServiceInstance;
13+
import org.springframework.core.Ordered;
1314

1415
/**
1516
* Properties to hold the details of a
1617
* {@link org.springframework.cloud.client.discovery.DiscoveryClient} service instances
17-
* for a given service
18+
* for a given service.
19+
* It also holds the user-configurable order that will be used to establish the
20+
* precedence of this client in the list of clients
21+
* used by {@link org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClient}.
1822
*
1923
* @author Biju Kunjummen
24+
* @author Olga Maciaszek-Sharma
2025
*/
2126

2227
@ConfigurationProperties(prefix = "spring.cloud.discovery.client.simple")
@@ -30,16 +35,26 @@ public class SimpleDiscoveryProperties {
3035
*/
3136
private SimpleServiceInstance local = new SimpleServiceInstance();
3237

38+
private int order = Ordered.LOWEST_PRECEDENCE;
39+
3340
public Map<String, List<SimpleServiceInstance>> getInstances() {
34-
return this.instances;
41+
return instances;
3542
}
3643

3744
public void setInstances(Map<String, List<SimpleServiceInstance>> instances) {
3845
this.instances = instances;
3946
}
4047

4148
public SimpleServiceInstance getLocal() {
42-
return this.local;
49+
return local;
50+
}
51+
52+
public int getOrder() {
53+
return order;
54+
}
55+
56+
public void setOrder(int order) {
57+
this.order = order;
4358
}
4459

4560
@PostConstruct

Diff for: ‎spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientOrderTest.java

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package org.springframework.cloud.client.discovery.composite;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.A_CUSTOM_DISCOVERY_CLIENT;
5+
import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.CUSTOM_SERVICE_ID;
6+
import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.THIRD_DISCOVERY_CLIENT;
7+
8+
import java.util.List;
9+
10+
import org.junit.Test;
11+
import org.junit.runner.RunWith;
12+
13+
import org.springframework.beans.factory.annotation.Autowired;
14+
import org.springframework.boot.test.context.SpringBootTest;
15+
import org.springframework.cloud.client.ServiceInstance;
16+
import org.springframework.cloud.client.discovery.DiscoveryClient;
17+
import org.springframework.test.context.junit4.SpringRunner;
18+
19+
/**
20+
* Tests for the support of ordered {@link DiscoveryClient} instances in {@link CompositeDiscoveryClient}
21+
*
22+
* @author Olga Maciaszek-Sharma
23+
*/
24+
@RunWith(SpringRunner.class)
25+
@SpringBootTest(properties = "spring.cloud.discovery.client.simple.order:2", classes = {
26+
CompositeDiscoveryClientTestsConfig.class })
27+
public class CompositeDiscoveryClientOrderTest {
28+
29+
@Autowired
30+
DiscoveryClient discoveryClient;
31+
32+
@Test
33+
public void shouldGetOrderedDiscoveryClients() {
34+
// when:
35+
List<DiscoveryClient> discoveryClients = ((CompositeDiscoveryClient) discoveryClient)
36+
.getDiscoveryClients();
37+
38+
// then:
39+
assertThat(discoveryClients.get(0).description())
40+
.isEqualTo(A_CUSTOM_DISCOVERY_CLIENT);
41+
assertThat(discoveryClients.get(1).description())
42+
.isEqualTo("Simple Discovery Client");
43+
assertThat(discoveryClients.get(2).description())
44+
.isEqualTo(THIRD_DISCOVERY_CLIENT);
45+
}
46+
47+
@Test
48+
public void shouldOnlyReturnServiceInstancesForTheHighestPrecedenceDiscoveryClient() {
49+
// when:
50+
List<ServiceInstance> serviceInstances = discoveryClient
51+
.getInstances(CUSTOM_SERVICE_ID);
52+
53+
// then:
54+
assertThat(serviceInstances).hasSize(1);
55+
assertThat(serviceInstances.get(0).getPort()).isEqualTo(123);
56+
}
57+
}

Diff for: ‎spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTests.java

+15-50
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,19 @@
11
package org.springframework.cloud.client.discovery.composite;
22

3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClientTestsConfig.CUSTOM_SERVICE_ID;
5+
36
import java.net.URI;
4-
import java.util.Arrays;
5-
import java.util.Collections;
6-
import java.util.List;
77

88
import org.junit.Test;
99
import org.junit.runner.RunWith;
10+
1011
import org.springframework.beans.factory.annotation.Autowired;
11-
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
1212
import org.springframework.boot.test.context.SpringBootTest;
13-
import org.springframework.cloud.client.DefaultServiceInstance;
1413
import org.springframework.cloud.client.ServiceInstance;
1514
import org.springframework.cloud.client.discovery.DiscoveryClient;
16-
import org.springframework.context.annotation.Bean;
17-
import org.springframework.context.annotation.Configuration;
18-
import org.springframework.core.annotation.Order;
1915
import org.springframework.test.context.junit4.SpringRunner;
2016

21-
import static org.assertj.core.api.Assertions.assertThat;
22-
2317
/**
2418
* Tests for behavior of Composite Discovery Client
2519
*
@@ -32,19 +26,20 @@
3226
"spring.cloud.discovery.client.simple.instances.service1[0].uri=http://s1-1:8080",
3327
"spring.cloud.discovery.client.simple.instances.service1[1].uri=https://s1-2:8443",
3428
"spring.cloud.discovery.client.simple.instances.service2[0].uri=https://s2-1:8080",
35-
"spring.cloud.discovery.client.simple.instances.service2[1].uri=https://s2-2:443" })
29+
"spring.cloud.discovery.client.simple.instances.service2[1].uri=https://s2-2:443", }, classes = {
30+
CompositeDiscoveryClientTestsConfig.class })
3631
public class CompositeDiscoveryClientTests {
3732

3833
@Autowired
3934
private DiscoveryClient discoveryClient;
4035

4136
@Test
4237
public void getInstancesByServiceIdShouldDelegateCall() {
43-
assertThat(this.discoveryClient).isInstanceOf(CompositeDiscoveryClient.class);
38+
assertThat(discoveryClient).isInstanceOf(CompositeDiscoveryClient.class);
4439

45-
assertThat(this.discoveryClient.getInstances("service1")).hasSize(2);
40+
assertThat(discoveryClient.getInstances("service1")).hasSize(2);
4641

47-
ServiceInstance s1 = this.discoveryClient.getInstances("service1").get(0);
42+
ServiceInstance s1 = discoveryClient.getInstances("service1").get(0);
4843
assertThat(s1.getHost()).isEqualTo("s1-1");
4944
assertThat(s1.getPort()).isEqualTo(8080);
5045
assertThat(s1.getUri()).isEqualTo(URI.create("http://s1-1:8080"));
@@ -53,53 +48,23 @@ public void getInstancesByServiceIdShouldDelegateCall() {
5348

5449
@Test
5550
public void getServicesShouldAggregateAllServiceNames() {
56-
assertThat(this.discoveryClient.getServices()).containsOnlyOnce("service1", "service2", "custom");
51+
assertThat(discoveryClient.getServices()).containsOnlyOnce("service1", "service2",
52+
CUSTOM_SERVICE_ID);
5753
}
5854

5955
@Test
6056
public void getDescriptionShouldBeComposite() {
61-
assertThat(this.discoveryClient.description()).isEqualTo("Composite Discovery Client");
57+
assertThat(discoveryClient.description()).isEqualTo("Composite Discovery Client");
6258
}
6359

6460
@Test
6561
public void getInstancesShouldRespectOrder() {
66-
assertThat(this.discoveryClient.getInstances("custom")).hasSize(1);
67-
assertThat(this.discoveryClient.getInstances("custom")).hasSize(1);
62+
assertThat(discoveryClient.getInstances(CUSTOM_SERVICE_ID)).hasSize(1);
63+
assertThat(discoveryClient.getInstances(CUSTOM_SERVICE_ID)).hasSize(1);
6864
}
6965

7066
@Test
7167
public void getInstancesByUnknownServiceIdShouldReturnAnEmptyList() {
72-
assertThat(this.discoveryClient.getInstances("unknown")).hasSize(0);
73-
}
74-
75-
@EnableAutoConfiguration
76-
@Configuration
77-
public static class Config {
78-
79-
@Bean
80-
@Order(1)
81-
public DiscoveryClient customDiscoveryClient() {
82-
return new DiscoveryClient() {
83-
@Override
84-
public String description() {
85-
return "A custom discovery client";
86-
}
87-
88-
@Override
89-
public List<ServiceInstance> getInstances(String serviceId) {
90-
if (serviceId.equals("custom")) {
91-
ServiceInstance s1 = new DefaultServiceInstance("custom", "host",
92-
123, false);
93-
return Arrays.asList(s1);
94-
}
95-
return Collections.emptyList();
96-
}
97-
98-
@Override
99-
public List<String> getServices() {
100-
return Arrays.asList("custom");
101-
}
102-
};
103-
}
68+
assertThat(discoveryClient.getInstances("unknown")).hasSize(0);
10469
}
10570
}

Diff for: ‎spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/composite/CompositeDiscoveryClientTestsConfig.java

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package org.springframework.cloud.client.discovery.composite;
2+
3+
import static java.util.Collections.singletonList;
4+
5+
import java.util.Collections;
6+
import java.util.List;
7+
8+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
9+
import org.springframework.cloud.client.DefaultServiceInstance;
10+
import org.springframework.cloud.client.ServiceInstance;
11+
import org.springframework.cloud.client.discovery.DiscoveryClient;
12+
import org.springframework.context.annotation.Bean;
13+
import org.springframework.context.annotation.Configuration;
14+
15+
/**
16+
* Test configuration for {@link CompositeDiscoveryClient} tests.
17+
*
18+
* @author Olga Maciaszek-Sharma
19+
*/
20+
@Configuration
21+
@EnableAutoConfiguration
22+
public class CompositeDiscoveryClientTestsConfig {
23+
24+
static final String A_CUSTOM_DISCOVERY_CLIENT = "A custom discovery client";
25+
static final String THIRD_DISCOVERY_CLIENT = "Third discovery client";
26+
static final String CUSTOM_SERVICE_ID = "custom";
27+
28+
@Bean
29+
public DiscoveryClient customDiscoveryClient() {
30+
return aDiscoveryClient(1, A_CUSTOM_DISCOVERY_CLIENT);
31+
}
32+
33+
@Bean
34+
public DiscoveryClient thirdOrderCustomDiscoveryClient() {
35+
return aDiscoveryClient(3, THIRD_DISCOVERY_CLIENT);
36+
}
37+
38+
private DiscoveryClient aDiscoveryClient(int order, String description) {
39+
return new DiscoveryClient() {
40+
@Override
41+
public String description() {
42+
return description;
43+
}
44+
45+
@Override
46+
public List<ServiceInstance> getInstances(String serviceId) {
47+
if (serviceId.equals(CUSTOM_SERVICE_ID)) {
48+
ServiceInstance s1 = new DefaultServiceInstance(CUSTOM_SERVICE_ID,
49+
"host", 123, false);
50+
return singletonList(s1);
51+
}
52+
return Collections.emptyList();
53+
}
54+
55+
@Override
56+
public List<String> getServices() {
57+
return singletonList(CUSTOM_SERVICE_ID);
58+
}
59+
60+
@Override
61+
public int getOrder() {
62+
return order;
63+
}
64+
};
65+
}
66+
}

0 commit comments

Comments
 (0)