Skip to content

Commit d7237ce

Browse files
CarsonCookCarsonCook
authored
fix: Handle multiple discovery service instances in static refresh (#1582)
* Parse DS list and try until successful response for refresh Signed-off-by: Carson Cook <carson.cook@ibm.com> * Add unit tests Signed-off-by: Carson Cook <carson.cook@ibm.com> * Switch discovery url locations to array Signed-off-by: Carson Cook <carson.cook@ibm.com> * Improve default error response Signed-off-by: Carson Cook <carson.cook@ibm.com>
1 parent 1dd4a89 commit d7237ce

File tree

5 files changed

+116
-31
lines changed

5 files changed

+116
-31
lines changed

‎api-catalog-services/src/main/java/org/zowe/apiml/apicatalog/discovery/DiscoveryConfigProperties.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
public class DiscoveryConfigProperties {
2424

2525
@Value("${apiml.service.discoveryServiceUrls}")
26-
private String locations;
26+
private String[] locations;
2727

2828
@Value("${apiml.service.eurekaUserName:eureka}")
2929
private String eurekaUserName;

‎api-catalog-services/src/main/java/org/zowe/apiml/apicatalog/instance/InstanceRetrievalService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ private InstanceInfo extractSingleInstanceFromApplication(String serviceId, Stri
194194
* @return request information
195195
*/
196196
private List<Pair<String, Pair<String, String>>> constructServiceInfoQueryRequest(String serviceId, boolean getDelta) {
197-
String[] discoveryServiceUrls = discoveryConfigProperties.getLocations().split(",");
197+
String[] discoveryServiceUrls = discoveryConfigProperties.getLocations();
198198
List<Pair<String, Pair<String, String>>> discoveryPairs = new ArrayList<>(discoveryServiceUrls.length);
199199
for (String discoveryUrl : discoveryServiceUrls) {
200200
String discoveryServiceLocatorUrl = discoveryUrl + APPS_ENDPOINT;

‎api-catalog-services/src/main/java/org/zowe/apiml/apicatalog/staticapi/StaticAPIService.java

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.zowe.apiml.apicatalog.staticapi;
1111

1212
import lombok.RequiredArgsConstructor;
13+
import lombok.extern.slf4j.Slf4j;
1314
import org.springframework.beans.factory.annotation.Qualifier;
1415
import org.springframework.beans.factory.annotation.Value;
1516
import org.springframework.http.HttpEntity;
@@ -20,10 +21,13 @@
2021
import org.springframework.web.client.RestTemplate;
2122
import org.zowe.apiml.apicatalog.discovery.DiscoveryConfigProperties;
2223

24+
import java.util.ArrayList;
2325
import java.util.Base64;
26+
import java.util.List;
2427

2528
@Service
2629
@RequiredArgsConstructor
30+
@Slf4j
2731
public class StaticAPIService {
2832

2933
private static final String REFRESH_ENDPOINT = "discovery/api/v1/staticApi";
@@ -43,11 +47,26 @@ public class StaticAPIService {
4347
private final DiscoveryConfigProperties discoveryConfigProperties;
4448

4549
public StaticAPIResponse refresh() {
46-
String discoveryServiceUrl = getDiscoveryServiceUrl();
47-
HttpEntity<?> entity = getHttpEntity(discoveryServiceUrl);
48-
ResponseEntity<String> restResponse = restTemplate.exchange(discoveryServiceUrl,
49-
HttpMethod.POST, entity, String.class);
50-
return new StaticAPIResponse(restResponse.getStatusCode().value(), restResponse.getBody());
50+
List<String> discoveryServiceUrls = getDiscoveryServiceUrls();
51+
for (int i = 0; i < discoveryServiceUrls.size(); i++) {
52+
53+
String discoveryServiceUrl = discoveryServiceUrls.get(i);
54+
HttpEntity<?> entity = getHttpEntity(discoveryServiceUrl);
55+
56+
try {
57+
ResponseEntity<String> response = restTemplate.exchange(discoveryServiceUrl, HttpMethod.POST, entity, String.class);
58+
59+
// Return response if successful response or if none have been successful and this is the last URL to try
60+
if (response.getStatusCode().is2xxSuccessful() || i == discoveryServiceUrls.size() - 1) {
61+
return new StaticAPIResponse(response.getStatusCode().value(), response.getBody());
62+
}
63+
64+
} catch (Exception e) {
65+
log.debug("Error refreshing static APIs from {}, error message: {}", discoveryServiceUrl, e.getMessage());
66+
}
67+
}
68+
69+
return new StaticAPIResponse(500, "Error making static API refresh request to the Discovery Service");
5170
}
5271

5372
private HttpEntity<?> getHttpEntity(String discoveryServiceUrl) {
@@ -62,7 +81,13 @@ private HttpEntity<?> getHttpEntity(String discoveryServiceUrl) {
6281
return new HttpEntity<>(null, httpHeaders);
6382
}
6483

65-
private String getDiscoveryServiceUrl() {
66-
return discoveryConfigProperties.getLocations().replace("/eureka", "") + REFRESH_ENDPOINT;
84+
private List<String> getDiscoveryServiceUrls() {
85+
String[] discoveryServiceLocations = discoveryConfigProperties.getLocations();
86+
87+
List<String> discoveryServiceUrls = new ArrayList<>();
88+
for (String location : discoveryServiceLocations) {
89+
discoveryServiceUrls.add(location.replace("/eureka", "") + REFRESH_ENDPOINT);
90+
}
91+
return discoveryServiceUrls;
6792
}
6893
}

‎api-catalog-services/src/test/java/org/zowe/apiml/apicatalog/instance/InstanceRetrievalServiceTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class InstanceRetrievalServiceTest {
6767
void setup() {
6868

6969
instanceRetrievalService = new InstanceRetrievalService(discoveryConfigProperties, restTemplate);
70-
discoveryServiceList = discoveryConfigProperties.getLocations().split(",");
70+
discoveryServiceList = discoveryConfigProperties.getLocations();
7171
discoveryServiceAllAppsUrl = discoveryServiceList[0] + APPS_ENDPOINT;
7272
}
7373

‎api-catalog-services/src/test/java/org/zowe/apiml/apicatalog/staticapi/StaticAPIServiceTest.java

Lines changed: 81 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010
package org.zowe.apiml.apicatalog.staticapi;
1111

12+
import org.junit.jupiter.api.Nested;
1213
import org.junit.jupiter.api.Test;
1314
import org.junit.jupiter.api.extension.ExtendWith;
1415
import org.mockito.InjectMocks;
@@ -28,6 +29,14 @@ class StaticAPIServiceTest {
2829

2930
private static final String REFRESH_ENDPOINT = "discovery/api/v1/staticApi";
3031

32+
private static final String DISCOVERY_LOCATION = "https://localhost:60004/eureka/";
33+
private static final String DISCOVERY_LOCATION_2 = "https://localhost:60005/eureka/";
34+
private static final String DISCOVERY_URL = "https://localhost:60004/";
35+
private static final String DISCOVERY_URL_2 = "https://localhost:60005/";
36+
37+
private static final String DISCOVERY_LOCATION_HTTP = "http://localhost:60004/eureka/";
38+
private static final String DISCOVERY_URL_HTTP = "http://localhost:60004/";
39+
3140
@InjectMocks
3241
private StaticAPIService staticAPIService;
3342

@@ -36,43 +45,94 @@ class StaticAPIServiceTest {
3645
@Mock
3746
private DiscoveryConfigProperties discoveryConfigProperties;
3847

39-
@Test
40-
void givenRefreshAPIWithSecureDiscoveryService_whenRefreshEndpointPresentResponse_thenReturnApiResponseCodeWithBody() {
41-
String discoveryUrl = "https://localhost:60004/";
42-
when(discoveryConfigProperties.getLocations()).thenReturn("https://localhost:60004/eureka/");
48+
@Nested
49+
class WhenRefreshEndpointPresentsResponse {
4350

44-
when(restTemplate.exchange(
45-
discoveryUrl + REFRESH_ENDPOINT,
46-
HttpMethod.POST, getHttpEntity(discoveryUrl), String.class))
47-
.thenReturn(new ResponseEntity<>("This is body", HttpStatus.OK));
51+
@Test
52+
void givenRefreshAPIWithSecureDiscoveryService_thenReturnApiResponseCodeWithBody() {
53+
when(discoveryConfigProperties.getLocations()).thenReturn(new String[]{DISCOVERY_LOCATION});
4854

55+
mockRestTemplateExchange(DISCOVERY_URL, new ResponseEntity<>("This is body", HttpStatus.OK));
4956

50-
StaticAPIResponse actualResponse = staticAPIService.refresh();
51-
StaticAPIResponse expectedResponse = new StaticAPIResponse(200, "This is body");
52-
assertEquals(expectedResponse, actualResponse);
57+
StaticAPIResponse actualResponse = staticAPIService.refresh();
58+
StaticAPIResponse expectedResponse = new StaticAPIResponse(200, "This is body");
59+
assertEquals(expectedResponse, actualResponse);
60+
}
61+
62+
@Test
63+
void givenRefreshAPIWithUnSecureDiscoveryService_thenReturnApiResponseCodeWithBody() {
64+
when(discoveryConfigProperties.getLocations()).thenReturn(new String[]{DISCOVERY_LOCATION_HTTP});
65+
66+
mockRestTemplateExchange(DISCOVERY_URL_HTTP, new ResponseEntity<>("This is body", HttpStatus.OK));
67+
68+
StaticAPIResponse actualResponse = staticAPIService.refresh();
69+
StaticAPIResponse expectedResponse = new StaticAPIResponse(200, "This is body");
70+
assertEquals(expectedResponse, actualResponse);
71+
}
72+
73+
@Nested
74+
class GivenTwoDiscoveryUrls {
75+
private final String[] discoveryLocations = {DISCOVERY_LOCATION, DISCOVERY_LOCATION_2};
76+
77+
@Test
78+
void whenFirstFails_thenReturnResponseFromSecond() {
79+
when(discoveryConfigProperties.getLocations()).thenReturn(discoveryLocations);
80+
81+
mockRestTemplateExchange(DISCOVERY_URL, new ResponseEntity<>(HttpStatus.NOT_FOUND));
82+
mockRestTemplateExchange(DISCOVERY_URL_2, new ResponseEntity<>("body", HttpStatus.OK));
83+
84+
StaticAPIResponse actualResponse = staticAPIService.refresh();
85+
StaticAPIResponse expectedResponse = new StaticAPIResponse(200, "body");
86+
assertEquals(expectedResponse, actualResponse);
87+
}
88+
89+
@Test
90+
void whenFirstSucceeds_thenReturnResponseFromFirst() {
91+
when(discoveryConfigProperties.getLocations()).thenReturn(discoveryLocations);
92+
93+
mockRestTemplateExchange(DISCOVERY_URL, new ResponseEntity<>("body", HttpStatus.OK));
94+
95+
StaticAPIResponse actualResponse = staticAPIService.refresh();
96+
StaticAPIResponse expectedResponse = new StaticAPIResponse(200, "body");
97+
assertEquals(expectedResponse, actualResponse);
98+
}
99+
100+
@Test
101+
void whenBothFail_thenReturnResponseFromSecond() {
102+
when(discoveryConfigProperties.getLocations()).thenReturn(discoveryLocations);
103+
104+
mockRestTemplateExchange(DISCOVERY_URL, new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR));
105+
mockRestTemplateExchange(DISCOVERY_URL_2, new ResponseEntity<>("body", HttpStatus.NOT_FOUND));
106+
107+
StaticAPIResponse actualResponse = staticAPIService.refresh();
108+
StaticAPIResponse expectedResponse = new StaticAPIResponse(404, "body");
109+
assertEquals(expectedResponse, actualResponse);
110+
}
111+
}
53112
}
54113

55114
@Test
56-
void givenRefreshAPIWithUnSecureDiscoveryService_whenRefreshEndpointPresentResponse_thenReturnApiResponseCodeWithBody() {
57-
String discoveryUrl = "http://localhost:60004/";
58-
when(discoveryConfigProperties.getLocations()).thenReturn("http://localhost:60004/eureka/");
59-
60-
when(restTemplate.exchange(
61-
discoveryUrl + REFRESH_ENDPOINT,
62-
HttpMethod.POST, getHttpEntity(discoveryUrl), String.class))
63-
.thenReturn(new ResponseEntity<>("This is body", HttpStatus.OK));
115+
void givenNoDiscoveryLocations_whenAttemptRefresh_thenReturn500() {
116+
when(discoveryConfigProperties.getLocations()).thenReturn(new String[]{});
64117

65118
StaticAPIResponse actualResponse = staticAPIService.refresh();
66-
StaticAPIResponse expectedResponse = new StaticAPIResponse(200, "This is body");
119+
StaticAPIResponse expectedResponse = new StaticAPIResponse(500, "Error making static API refresh request to the Discovery Service");
67120
assertEquals(expectedResponse, actualResponse);
68121
}
69122

123+
private void mockRestTemplateExchange(String discoveryUrl, ResponseEntity expectedResponse) {
124+
when(restTemplate.exchange(
125+
discoveryUrl + REFRESH_ENDPOINT,
126+
HttpMethod.POST, getHttpEntity(discoveryUrl), String.class
127+
)).thenReturn(expectedResponse);
128+
}
129+
70130
private HttpEntity<?> getHttpEntity(String discoveryServiceUrl) {
71131
boolean isHttp = discoveryServiceUrl.startsWith("http://");
72132
HttpHeaders httpHeaders = new HttpHeaders();
73133
httpHeaders.add("Accept", "application/json");
74134
if (isHttp) {
75-
String basicToken = "Basic " + Base64.getEncoder().encodeToString( "null:null".getBytes());
135+
String basicToken = "Basic " + Base64.getEncoder().encodeToString("null:null".getBytes());
76136
httpHeaders.add("Authorization", basicToken);
77137
}
78138

0 commit comments

Comments
 (0)