diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java index a82943ca9..4b7befaaa 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoader.java @@ -55,6 +55,7 @@ import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.client.UnknownContentTypeException; import static org.springframework.cloud.config.client.ConfigClientProperties.DEFAULT_PROFILE; import static org.springframework.cloud.config.client.ConfigClientProperties.STATE_HEADER; @@ -368,6 +369,15 @@ protected Environment getRemoteEnvironment(ConfigDataLoaderContext context, Conf continue; } } + catch (UnknownContentTypeException e) { + if (i < noOfUrls - 1 && properties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) { + logger.info( + "Failed to read the response of a config server. Will try the next url if available. Error : " + + e.getMessage()); + continue; + } + throw e; + } if (response == null) { if (i < noOfUrls - 1 && properties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) { diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java index b38ecbdd2..4618a7feb 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java @@ -57,6 +57,7 @@ import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.client.UnknownContentTypeException; import static org.springframework.cloud.config.client.ConfigClientProperties.STATE_HEADER; import static org.springframework.cloud.config.client.ConfigClientProperties.TOKEN_HEADER; @@ -310,6 +311,15 @@ private Environment getRemoteEnvironment(ConfigClientRequestTemplateFactory requ continue; } } + catch (UnknownContentTypeException e) { + if (i < noOfUrls - 1 && defaultProperties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) { + logger.info( + "Failed to read the response of a config server. Will try the next url if available. Error : " + + e.getMessage()); + continue; + } + throw e; + } if (response == null) { if (i < noOfUrls - 1 && defaultProperties.getMultipleUriStrategy() == MultipleUriStrategy.ALWAYS) { diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoaderTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoaderTests.java index 6d05d80d2..549418175 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoaderTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServerConfigDataLoaderTests.java @@ -62,6 +62,7 @@ import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.client.UnknownContentTypeException; import static java.lang.String.format; import static org.assertj.core.api.Assertions.assertThat; @@ -301,6 +302,22 @@ public void shouldThrowExceptionWhenNegativeConnectTimeoutSet() { .withMessageContaining("Invalid Value for Connect Timeout set."); } + @Test + public void shouldNotUseNextUriFor_UnreadableResponse_And_CONNECTION_TIMEOUT_ONLY_Strategy() throws Exception { + mockUnreadableResponseFromFirstUri(ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY); + + assertThatExceptionOfType(ConfigClientFailFastException.class) + .isThrownBy(() -> this.loader.load(context, resource)) + .withCauseInstanceOf(UnknownContentTypeException.class); + } + + @Test + public void shouldUseNextUriFor_UnreadableResponse_And_ALWAYS_Strategy() throws Exception { + mockUnreadableResponseFromFirstUri(ConfigClientProperties.MultipleUriStrategy.ALWAYS); + + assertThat(this.loader.load(context, resource)).isNotNull(); + } + @Test public void shouldNotUseNextUriFor_400_And_CONNECTION_TIMEOUT_ONLY_Strategy() throws Exception { assertNextUriIsNotTriedForClientError(ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY, @@ -779,8 +796,27 @@ private void assertNextUriIsTriedOnTimeout(ConfigClientProperties.MultipleUriStr assertThat(this.loader.load(context, resource)).isNotNull(); } + private void mockUnreadableResponseFromFirstUri(ConfigClientProperties.MultipleUriStrategy multipleUriStrategy) + throws Exception { + String badURI = "http://baduri"; + String goodURI = "http://localhost:8888"; + properties.setUri(new String[] { badURI, goodURI }); + properties.setFailFast(true); + properties.setMultipleUriStrategy(multipleUriStrategy); + this.loader = new ConfigServerConfigDataLoader(destination -> logger); + ClientHttpRequestFactory requestFactory = mock(ClientHttpRequestFactory.class); + mockRequestResponse(requestFactory, badURI, HttpStatus.OK, MediaType.TEXT_HTML); + mockRequestResponse(requestFactory, goodURI, HttpStatus.OK); + when(bootstrapContext.get(RestTemplate.class)).thenReturn(new RestTemplate(requestFactory)); + } + private void mockRequestResponse(ClientHttpRequestFactory requestFactory, String baseURI, HttpStatus status) throws Exception { + mockRequestResponse(requestFactory, baseURI, status, MediaType.APPLICATION_JSON); + } + + private void mockRequestResponse(ClientHttpRequestFactory requestFactory, String baseURI, HttpStatus status, + MediaType contentType) throws Exception { ClientHttpRequest request = mock(ClientHttpRequest.class); ClientHttpResponse response = mock(ClientHttpResponse.class); @@ -797,7 +833,7 @@ private void mockRequestResponse(ClientHttpRequestFactory requestFactory, String when(request.execute()).thenReturn(response); HttpHeaders headers = new HttpHeaders(); - headers.setContentType(MediaType.APPLICATION_JSON); + headers.setContentType(contentType); when(response.getHeaders()).thenReturn(headers); when(response.getStatusCode()).thenReturn(status); when(response.getBody()).thenReturn(new ByteArrayInputStream("{}".getBytes())); diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java index 66793fc9a..2def94348 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java @@ -57,6 +57,7 @@ import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.client.UnknownContentTypeException; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -300,6 +301,23 @@ public void shouldThrowExceptionWhenNegativeConnectTimeoutSet() { }).isInstanceOf(IllegalStateException.class).hasMessageContaining("Invalid Value for Connect Timeout set."); } + @Test + public void shouldNotUseNextUriFor_UnreadableResponse_And_CONNECTION_TIMEOUT_ONLY_Strategy() throws Exception { + mockUnreadableResponseFromFirstUri(ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY); + + Assertions.assertThatThrownBy(() -> this.locator.locateCollection(this.environment)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("fail fast property is set") + .hasCauseInstanceOf(UnknownContentTypeException.class); + } + + @Test + public void shouldUseNextUriFor_UnreadableResponse_And_ALWAYS_Strategy() throws Exception { + mockUnreadableResponseFromFirstUri(ConfigClientProperties.MultipleUriStrategy.ALWAYS); + + assertThat(this.locator.locateCollection(this.environment)).isNotNull(); + } + @Test public void shouldNotUseNextUriFor_400_And_CONNECTION_TIMEOUT_ONLY_Strategy() throws Exception { assertNextUriIsNotTriedForClientError(ConfigClientProperties.MultipleUriStrategy.CONNECTION_TIMEOUT_ONLY, @@ -621,8 +639,28 @@ private Map originValue(String value, String origin) { return map; } + private void mockUnreadableResponseFromFirstUri(ConfigClientProperties.MultipleUriStrategy multipleUriStrategy) + throws Exception { + ConfigClientProperties clientProperties = new ConfigClientProperties(this.environment); + String badURI = "http://baduri"; + String goodURI = "http://localhost:8888"; + clientProperties.setUri(new String[] { badURI, goodURI }); + clientProperties.setFailFast(true); + clientProperties.setMultipleUriStrategy(multipleUriStrategy); + this.locator = new ConfigServicePropertySourceLocator(clientProperties); + ClientHttpRequestFactory requestFactory = Mockito.mock(ClientHttpRequestFactory.class); + mockRequestResponse(requestFactory, badURI, HttpStatus.OK, MediaType.TEXT_HTML); + mockRequestResponse(requestFactory, goodURI, HttpStatus.OK); + this.locator.setRestTemplate(new RestTemplate(requestFactory)); + } + private void mockRequestResponse(ClientHttpRequestFactory requestFactory, String baseURI, HttpStatus status) throws Exception { + mockRequestResponse(requestFactory, baseURI, status, MediaType.APPLICATION_JSON); + } + + private void mockRequestResponse(ClientHttpRequestFactory requestFactory, String baseURI, HttpStatus status, + MediaType contentType) throws Exception { ClientHttpRequest request = Mockito.mock(ClientHttpRequest.class); ClientHttpResponse response = Mockito.mock(ClientHttpResponse.class); @@ -641,7 +679,7 @@ private void mockRequestResponse(ClientHttpRequestFactory requestFactory, String Mockito.when(request.execute()).thenReturn(response); HttpHeaders headers = new HttpHeaders(); - headers.setContentType(MediaType.APPLICATION_JSON); + headers.setContentType(contentType); Mockito.when(response.getHeaders()).thenReturn(headers); Mockito.when(response.getStatusCode()).thenReturn(status); Mockito.when(response.getBody()).thenReturn(new ByteArrayInputStream("{}".getBytes()));