Skip to content

Commit f96dec2

Browse files
ryanjbaxterryanjbaxter
committed
Merge branch '4.1.x' into 4.2.x
2 parents 1154402 + d959f4a commit f96dec2

File tree

4 files changed

+157
-4
lines changed

4 files changed

+157
-4
lines changed

‎spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,22 @@ protected Environment getRemoteEnvironment(ConfigDataLoaderContext context, Conf
369369
}
370370
}
371371

372-
if (response == null || response.getStatusCode() != HttpStatus.OK) {
372+
if (response == null) {
373+
if (i < noOfUrls - 1 && properties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) {
374+
logger.info("Failed to fetch configs from server at : " + uri
375+
+ ". The response was null. Will try the next url if available.");
376+
continue;
377+
}
378+
379+
return null;
380+
}
381+
else if (response.getStatusCode() != HttpStatus.OK) {
382+
if (i < noOfUrls - 1 && properties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) {
383+
logger.info("Failed to fetch configs from server at : " + uri
384+
+ ". Will try the next url if available. StatusCode : " + response.getStatusCode());
385+
continue;
386+
}
387+
373388
return null;
374389
}
375390

‎spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,22 @@ private Environment getRemoteEnvironment(ConfigClientRequestTemplateFactory requ
307307
}
308308
}
309309

310-
if (response == null || response.getStatusCode() != HttpStatus.OK) {
310+
if (response == null) {
311+
if (i < noOfUrls - 1 && defaultProperties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) {
312+
logger.info("Failed to fetch configs from server at : " + uri
313+
+ ". The response was null. Will try the next url if available.");
314+
continue;
315+
}
316+
317+
return null;
318+
}
319+
else if (response.getStatusCode() != HttpStatus.OK) {
320+
if (i < noOfUrls - 1 && defaultProperties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) {
321+
logger.info("Failed to fetch configs from server at : " + uri
322+
+ ". Will try the next url if available. StatusCode : " + response.getStatusCode());
323+
continue;
324+
}
325+
311326
return null;
312327
}
313328

‎spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoaderTests.java

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ public void init() {
133133
.thenReturn(mock(ConfigClientRequestTemplateFactory.class));
134134
when(bootstrapContext.get(RestTemplate.class)).thenReturn(restTemplate);
135135
when(resource.getProperties()).thenReturn(properties);
136+
when(resource.isOptional()).thenReturn(true);
136137

137138
when(resource.getProfiles()).thenReturn(PROFILES);
138139
}
@@ -336,6 +337,60 @@ public void shouldUseNextUriFor_500_And_ALWAYS_Strategy() throws Exception {
336337
assertNextUriIsTried(ConfigClientProperties.MultipleUriStrategy.ALWAYS, HttpStatus.INTERNAL_SERVER_ERROR);
337338
}
338339

340+
@Test
341+
public void shouldUseNextUriFor_NoExceptionNotOK_And_CONNECTION_TIMEOUT_ONLY_Strategy_FailFastIsFalse()
342+
throws Exception {
343+
// At the time of this writing, TEMPORARY_REDIRECT will not cause an exception to
344+
// be thrown back to
345+
// getRemoteEnvironment, but since status is not OK, the method returns null and
346+
// locate method will
347+
// simply return null since fail-fast=false. (Second URL is never tried, due to
348+
// the strategy.
349+
350+
// Set up with two URIs.
351+
String badURI = "http://baduri";
352+
String goodURI = "http://localhost:8888";
353+
String[] uris = new String[] { badURI, goodURI };
354+
properties.setUri(uris);
355+
properties.setFailFast(false);
356+
// Strategy is CONNECTION_TIMEOUT_ONLY, so it should not try the next URI for
357+
// TEMPORARY_REDIRECT
358+
properties.setMultipleUriStrategy(ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY);
359+
this.loader = new ConfigServerConfigDataLoader(destination -> logger);
360+
ClientHttpRequestFactory requestFactory = mock(ClientHttpRequestFactory.class);
361+
RestTemplate restTemplate = new RestTemplate(requestFactory);
362+
mockRequestResponse(requestFactory, badURI, HttpStatus.TEMPORARY_REDIRECT);
363+
mockRequestResponse(requestFactory, goodURI, HttpStatus.OK);
364+
when(bootstrapContext.get(RestTemplate.class)).thenReturn(restTemplate);
365+
assertThat(this.loader.load(context, resource)).isNull();
366+
}
367+
368+
@Test
369+
public void shouldUseNextUriFor_NoExceptionNotOK_And_CONNECTION_TIMEOUT_ONLY_Strategy_WithFailFastIsTrue()
370+
throws Exception {
371+
// At the time of this writing, TEMPORARY_REDIRECT will not cause an exception to
372+
// be thrown back to
373+
// getRemoteEnvironment, but since status is not OK, the method returns null and
374+
// locate method will
375+
// throw an IllegalStateException with no cause, since fail-fast=true. Second URL
376+
// is never tried, due to the strategy.
377+
assertNextUriIsNotTried(true, ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY,
378+
HttpStatus.TEMPORARY_REDIRECT, null // IllegalStateException has no cause,
379+
// because getRemoteEnvironment did
380+
// not throw an exception
381+
);
382+
}
383+
384+
@Test
385+
public void shouldUseNextUriFor_NoExceptionNotOK_And_ALWAYS_Strategy() throws Exception {
386+
// At the time of this writing, TEMPORARY_REDIRECT will not cause an exception to
387+
// be thrown back to
388+
// getRemoteEnvironment, but since status is not OK, the method will treat it the
389+
// same as an exception and
390+
// thus try the next URL.
391+
assertNextUriIsTried(ConfigClientProperties.MultipleUriStrategy.ALWAYS, HttpStatus.TEMPORARY_REDIRECT);
392+
}
393+
339394
@Test
340395
public void shouldUseNextUriFor_TimeOut_And_ALWAYS_Strategy() throws Exception {
341396
assertNextUriIsTriedOnTimeout(ConfigClientProperties.MultipleUriStrategy.ALWAYS);
@@ -631,12 +686,18 @@ private ConfigClientRequestTemplateFactory factory(ConfigClientProperties proper
631686

632687
private void assertNextUriIsNotTried(ConfigClientProperties.MultipleUriStrategy multipleUriStrategy,
633688
HttpStatus firstUriResponse, Class<? extends Exception> expectedCause) throws Exception {
689+
assertNextUriIsNotTried(true, multipleUriStrategy, firstUriResponse, expectedCause);
690+
}
691+
692+
private void assertNextUriIsNotTried(boolean failFast,
693+
ConfigClientProperties.MultipleUriStrategy multipleUriStrategy, HttpStatus firstUriResponse,
694+
Class<? extends Exception> expectedCause) throws Exception {
634695
// Set up with two URIs.
635696
String badURI = "http://baduri";
636697
String goodURI = "http://localhost:8888";
637698
String[] uris = new String[] { badURI, goodURI };
638699
properties.setUri(uris);
639-
properties.setFailFast(true);
700+
properties.setFailFast(failFast);
640701
// Strategy is CONNECTION_TIMEOUT_ONLY, so it should not try the next URI for
641702
// INTERNAL_SERVER_ERROR
642703
properties.setMultipleUriStrategy(multipleUriStrategy);

‎spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.io.IOException;
2121
import java.net.URI;
2222
import java.util.ArrayList;
23+
import java.util.Collections;
2324
import java.util.HashMap;
2425
import java.util.Iterator;
2526
import java.util.LinkedHashMap;
@@ -332,6 +333,61 @@ public void shouldUseNextUriFor_500_And_ALWAYS_Strategy() throws Exception {
332333
assertNextUriIsTried(ConfigClientProperties.MultipleUriStrategy.ALWAYS, HttpStatus.INTERNAL_SERVER_ERROR);
333334
}
334335

336+
@Test
337+
public void shouldUseNextUriFor_NoExceptionNotOK_And_CONNECTION_TIMEOUT_ONLY_Strategy_FailFastIsFalse()
338+
throws Exception {
339+
// At the time of this writing, TEMPORARY_REDIRECT will not cause an exception to
340+
// be thrown back to
341+
// getRemoteEnvironment, but since status is not OK, the method returns null and
342+
// locate method will
343+
// simply return null since fail-fast=false. (Second URL is never tried, due to
344+
// the strategy.
345+
346+
// Set up with two URIs.
347+
ConfigClientProperties clientProperties = new ConfigClientProperties(this.environment);
348+
String badURI = "http://baduri";
349+
String goodURI = "http://localhost:8888";
350+
String[] uris = new String[] { badURI, goodURI };
351+
clientProperties.setUri(uris);
352+
clientProperties.setFailFast(false);
353+
// Strategy is CONNECTION_TIMEOUT_ONLY, so it should not try the next URI for
354+
// TEMPORARY_REDIRECT
355+
clientProperties.setMultipleUriStrategy(ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY);
356+
this.locator = new ConfigServicePropertySourceLocator(clientProperties);
357+
ClientHttpRequestFactory requestFactory = Mockito.mock(ClientHttpRequestFactory.class);
358+
RestTemplate restTemplate1 = new RestTemplate(requestFactory);
359+
mockRequestResponse(requestFactory, badURI, HttpStatus.TEMPORARY_REDIRECT);
360+
mockRequestResponse(requestFactory, goodURI, HttpStatus.OK);
361+
this.locator.setRestTemplate(restTemplate1);
362+
assertThat(this.locator.locateCollection(this.environment)).isEqualTo(Collections.emptyList());
363+
}
364+
365+
@Test
366+
public void shouldUseNextUriFor_NoExceptionNotOK_And_CONNECTION_TIMEOUT_ONLY_Strategy_WithFailFastIsTrue()
367+
throws Exception {
368+
// At the time of this writing, TEMPORARY_REDIRECT will not cause an exception to
369+
// be thrown back to
370+
// getRemoteEnvironment, but since status is not OK, the method returns null and
371+
// locate method will
372+
// throw an IllegalStateException with no cause, since fail-fast=true. Second URL
373+
// is never tried, due to the strategy.
374+
assertNextUriIsNotTried(true, ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY,
375+
HttpStatus.TEMPORARY_REDIRECT, null // IllegalStateException has no cause,
376+
// because getRemoteEnvironment did
377+
// not throw an exception
378+
);
379+
}
380+
381+
@Test
382+
public void shouldUseNextUriFor_NoExceptionNotOK_And_ALWAYS_Strategy() throws Exception {
383+
// At the time of this writing, TEMPORARY_REDIRECT will not cause an exception to
384+
// be thrown back to
385+
// getRemoteEnvironment, but since status is not OK, the method will treat it the
386+
// same as an exception and
387+
// thus try the next URL.
388+
assertNextUriIsTried(ConfigClientProperties.MultipleUriStrategy.ALWAYS, HttpStatus.TEMPORARY_REDIRECT);
389+
}
390+
335391
@Test
336392
public void shouldUseNextUriFor_TimeOut_And_ALWAYS_Strategy() throws Exception {
337393
assertNextUriIsTriedOnTimeout(ConfigClientProperties.MultipleUriStrategy.ALWAYS);
@@ -459,14 +515,20 @@ public void shouldPreserveOrder() {
459515

460516
private void assertNextUriIsNotTried(ConfigClientProperties.MultipleUriStrategy multipleUriStrategy,
461517
HttpStatus firstUriResponse, Class<? extends Exception> expectedCause) {
518+
assertNextUriIsNotTried(true, multipleUriStrategy, firstUriResponse, expectedCause);
519+
}
520+
521+
private void assertNextUriIsNotTried(boolean failFast,
522+
ConfigClientProperties.MultipleUriStrategy multipleUriStrategy, HttpStatus firstUriResponse,
523+
Class<? extends Exception> expectedCause) {
462524
AbstractThrowableAssert throwableAssert = Assertions.assertThatThrownBy(() -> {
463525
// Set up with two URIs.
464526
ConfigClientProperties clientProperties = new ConfigClientProperties(this.environment);
465527
String badURI = "http://baduri";
466528
String goodURI = "http://localhost:8888";
467529
String[] uris = new String[] { badURI, goodURI };
468530
clientProperties.setUri(uris);
469-
clientProperties.setFailFast(true);
531+
clientProperties.setFailFast(failFast);
470532
// Strategy is CONNECTION_TIMEOUT_ONLY, so it should not try the next URI for
471533
// INTERNAL_SERVER_ERROR
472534
clientProperties.setMultipleUriStrategy(multipleUriStrategy);

0 commit comments

Comments
 (0)