diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java index 1285abe7..67530c4b 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java @@ -105,6 +105,7 @@ public class LoadBalancerAutoConfiguration { @ConditionalOnClass(RetryTemplate.class) public static class RetryAutoConfiguration { @Bean + @ConditionalOnMissingBean public RetryTemplate retryTemplate() { RetryTemplate template = new RetryTemplate(); template.setThrowLastExceptionOnExhausted(true); diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index f33ab29f..d59f1103 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -16,16 +16,23 @@ package org.springframework.cloud.client.loadbalancer; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.net.URI; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpRequest; +import org.springframework.http.HttpStatus; import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.retry.RecoveryCallback; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryException; import org.springframework.retry.backoff.BackOffPolicy; import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; @@ -89,27 +96,90 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto serviceName)); return template .execute(new RetryCallback() { - @Override - public ClientHttpResponse doWithRetry(RetryContext context) - throws IOException { - ServiceInstance serviceInstance = null; - if (context instanceof LoadBalancedRetryContext) { - LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext) context; - serviceInstance = lbContext.getServiceInstance(); - } - if (serviceInstance == null) { - serviceInstance = loadBalancer.choose(serviceName); - } - ClientHttpResponse response = RetryLoadBalancerInterceptor.this.loadBalancer.execute( - serviceName, serviceInstance, - requestFactory.createRequest(request, body, execution)); - int statusCode = response.getRawStatusCode(); - if(retryPolicy != null && retryPolicy.retryableStatusCode(statusCode)) { - response.close(); - throw new RetryableStatusCodeException(serviceName, statusCode); - } - return response; - } - }); + @Override + public ClientHttpResponse doWithRetry(RetryContext context) + throws IOException { + ServiceInstance serviceInstance = null; + if (context instanceof LoadBalancedRetryContext) { + LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext) context; + serviceInstance = lbContext.getServiceInstance(); + } + if (serviceInstance == null) { + serviceInstance = loadBalancer.choose(serviceName); + } + ClientHttpResponse response = RetryLoadBalancerInterceptor.this.loadBalancer.execute( + serviceName, serviceInstance, + requestFactory.createRequest(request, body, execution)); + int statusCode = response.getRawStatusCode(); + if (retryPolicy != null && retryPolicy.retryableStatusCode(statusCode)) { + ClientHttpResponseWrapper wrapper = new ClientHttpResponseWrapper(response); + wrapper.init(); + throw new RetryableStatusCodeException(serviceName, statusCode, wrapper, null); + } + return response; + } + }, new RecoveryCallback() { + @Override + public ClientHttpResponse recover(RetryContext retryContext) throws Exception { + Throwable lastThrowable = retryContext.getLastThrowable(); + if (lastThrowable != null && lastThrowable instanceof RetryableStatusCodeException) { + RetryableStatusCodeException ex = (RetryableStatusCodeException) lastThrowable; + return (ClientHttpResponse) ex.getResponse(); + } + throw new RetryException("Could not recover", lastThrowable); + } + }); } + + public static class ClientHttpResponseWrapper implements ClientHttpResponse { + + private ClientHttpResponse response; + private InputStream content; + + public ClientHttpResponseWrapper(ClientHttpResponse response) { + this.response = response; + } + + public void init() throws IOException { + InputStream body = response.getBody(); + ByteArrayOutputStream temp = new ByteArrayOutputStream(); + byte[] buffer = new byte[4096]; + int length = 0; + while ((length = body.read(buffer)) != -1) { + temp.write(buffer, 0, length); + } + content = new ByteArrayInputStream(temp.toByteArray()); + response.close(); + } + + @Override + public HttpStatus getStatusCode() throws IOException { + return response.getStatusCode(); + } + + @Override + public int getRawStatusCode() throws IOException { + return response.getRawStatusCode(); + } + + @Override + public String getStatusText() throws IOException { + return response.getStatusText(); + } + + @Override + public void close() { + response.close(); + } + + @Override + public InputStream getBody() throws IOException { + return content; + } + + @Override + public HttpHeaders getHeaders() { + return response.getHeaders(); + } + } } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java index d5d4b2db..ddce92d8 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryableStatusCodeException.java @@ -1,6 +1,7 @@ package org.springframework.cloud.client.loadbalancer; import java.io.IOException; +import java.net.URI; /** * Exception to be thrown when the status code is deemed to be retryable. @@ -10,7 +11,25 @@ public class RetryableStatusCodeException extends IOException { private static final String MESSAGE = "Service %s returned a status code of %d"; + private Object response; + + private URI uri; + public RetryableStatusCodeException(String serviceId, int statusCode) { super(String.format(MESSAGE, serviceId, statusCode)); } + + public RetryableStatusCodeException(String serviceId, int statusCode, Object response, URI uri) { + super(String.format(MESSAGE, serviceId, statusCode)); + this.response = response; + this.uri = uri; + } + + public Object getResponse() { + return response; + } + + public URI getUri() { + return uri; + } } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java index f3e47315..5a45d7f6 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java @@ -1,5 +1,6 @@ package org.springframework.cloud.client.loadbalancer; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.net.URI; @@ -15,6 +16,7 @@ import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpResponse; import org.springframework.mock.http.client.MockClientHttpResponse; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryException; import org.springframework.retry.backoff.BackOffContext; import org.springframework.retry.backoff.BackOffInterruptedException; import org.springframework.retry.backoff.BackOffPolicy; @@ -58,7 +60,7 @@ public class RetryLoadBalancerInterceptorTest { lbProperties = null; } - @Test(expected = IOException.class) + @Test(expected = RetryException.class) public void interceptDisableRetry() throws Throwable { HttpRequest request = mock(HttpRequest.class); when(request.getURI()).thenReturn(new URI("http://foo")); @@ -140,6 +142,7 @@ public class RetryLoadBalancerInterceptorTest { HttpRequest request = mock(HttpRequest.class); when(request.getURI()).thenReturn(new URI("http://foo")); InputStream notFoundStream = mock(InputStream.class); + when(notFoundStream.read(any(byte[].class))).thenReturn(-1); ClientHttpResponse clientHttpResponseNotFound = new MockClientHttpResponse(notFoundStream, HttpStatus.NOT_FOUND); ClientHttpResponse clientHttpResponseOk = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); @@ -164,6 +167,36 @@ public class RetryLoadBalancerInterceptorTest { verify(lbRequestFactory, times(2)).createRequest(request, body, execution); } + @Test + public void interceptRetryFailOnStatusCode() throws Throwable { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://foo")); + InputStream notFoundStream = new ByteArrayInputStream("foo".getBytes()); + ClientHttpResponse clientHttpResponseNotFound = new MockClientHttpResponse(notFoundStream, HttpStatus.NOT_FOUND); + LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); + when(policy.retryableStatusCode(eq(HttpStatus.NOT_FOUND.value()))).thenReturn(true); + when(policy.canRetryNextServer(any(LoadBalancedRetryContext.class))).thenReturn(false); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); + when(lbRetryPolicyFactory.create(eq("foo"), any(ServiceInstanceChooser.class))).thenReturn(policy); + ServiceInstance serviceInstance = mock(ServiceInstance.class); + when(client.choose(eq("foo"))).thenReturn(serviceInstance); + when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))). + thenReturn(clientHttpResponseNotFound); + lbProperties.setEnabled(true); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + byte[] body = new byte[]{}; + ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); + ClientHttpResponse rsp = interceptor.intercept(request, body, execution); + verify(client, times(1)).execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class)); + verify(lbRequestFactory, times(1)).createRequest(request, body, execution); + verify(policy, times(2)).canRetryNextServer(any(LoadBalancedRetryContext.class)); + //call twice in a retry attempt + byte[] content = new byte[1024]; + int length = rsp.getBody().read(content); + assertThat(length, is("foo".getBytes().length)); + assertThat(new String(content, 0, length), is("foo")); + } + @Test public void interceptRetry() throws Throwable { HttpRequest request = mock(HttpRequest.class); @@ -192,7 +225,7 @@ public class RetryLoadBalancerInterceptorTest { assertThat(backOffPolicy.getBackoffAttempts(), is(1)); } - @Test(expected = IOException.class) + @Test(expected = RetryException.class) public void interceptFailedRetry() throws Exception { HttpRequest request = mock(HttpRequest.class); when(request.getURI()).thenReturn(new URI("http://foo")); diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/commons/httpclient/DefaultApacheHttpClientConnectionManagerFactoryTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/commons/httpclient/DefaultApacheHttpClientConnectionManagerFactoryTests.java index cfc9e25b..b748bd9d 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/commons/httpclient/DefaultApacheHttpClientConnectionManagerFactoryTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/commons/httpclient/DefaultApacheHttpClientConnectionManagerFactoryTests.java @@ -1,11 +1,5 @@ package org.springframework.cloud.commons.httpclient; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; - import java.lang.reflect.Field; import java.util.concurrent.TimeUnit; @@ -19,8 +13,15 @@ import org.apache.http.conn.socket.ConnectionSocketFactory; import org.apache.http.impl.conn.DefaultHttpClientConnectionOperator; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.junit.Test; + import org.springframework.util.ReflectionUtils; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + /** * @author Ryan Baxter * @author Michael Wirth @@ -59,35 +60,40 @@ public class DefaultApacheHttpClientConnectionManagerFactoryTests { @Test public void newConnectionManagerWithSSL() throws Exception { HttpClientConnectionManager connectionManager = new DefaultApacheHttpClientConnectionManagerFactory() - .newConnectionManager(false, 2, 6); + .newConnectionManager(false, 2, 6); Lookup socketFactoryRegistry = getConnectionSocketFactoryLookup( - connectionManager); + connectionManager); assertThat(socketFactoryRegistry.lookup("https"), is(notNullValue())); - assertThat(getX509TrustManager(socketFactoryRegistry).getAcceptedIssuers(), is(notNullValue())); + assertThat(getX509TrustManager(socketFactoryRegistry).getAcceptedIssuers(), + is(notNullValue())); } @Test public void newConnectionManagerWithDisabledSSLValidation() throws Exception { HttpClientConnectionManager connectionManager = new DefaultApacheHttpClientConnectionManagerFactory() - .newConnectionManager(true, 2, 6); + .newConnectionManager(true, 2, 6); Lookup socketFactoryRegistry = getConnectionSocketFactoryLookup( - connectionManager); + connectionManager); assertThat(socketFactoryRegistry.lookup("https"), is(notNullValue())); - assertThat(getX509TrustManager(socketFactoryRegistry).getAcceptedIssuers(), is(nullValue())); + assertThat(getX509TrustManager(socketFactoryRegistry).getAcceptedIssuers(), + is(nullValue())); } private Lookup getConnectionSocketFactoryLookup( - HttpClientConnectionManager connectionManager) { - DefaultHttpClientConnectionOperator connectionOperator = getField(connectionManager, "connectionOperator"); + HttpClientConnectionManager connectionManager) { + DefaultHttpClientConnectionOperator connectionOperator = getField( + connectionManager, "connectionOperator"); return getField(connectionOperator, "socketFactoryRegistry"); } private X509TrustManager getX509TrustManager( - Lookup socketFactoryRegistry) { - ConnectionSocketFactory connectionSocketFactory = socketFactoryRegistry.lookup("https"); - SSLSocketFactory sslSocketFactory = getField(connectionSocketFactory, "socketfactory"); + Lookup socketFactoryRegistry) { + ConnectionSocketFactory connectionSocketFactory = socketFactoryRegistry + .lookup("https"); + SSLSocketFactory sslSocketFactory = getField(connectionSocketFactory, + "socketfactory"); SSLContextSpi sslContext = getField(sslSocketFactory, "context"); return getField(sslContext, "trustManager"); } @@ -99,4 +105,4 @@ public class DefaultApacheHttpClientConnectionManagerFactoryTests { Object value = ReflectionUtils.getField(field, target); return (T) value; } -} \ No newline at end of file +} diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/ConfigurationPropertiesRebinderAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/ConfigurationPropertiesRebinderAutoConfiguration.java index 19281bd2..65c09423 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/ConfigurationPropertiesRebinderAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/ConfigurationPropertiesRebinderAutoConfiguration.java @@ -16,8 +16,6 @@ package org.springframework.cloud.autoconfigure; -import java.util.Collections; - import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -25,7 +23,6 @@ import org.springframework.boot.autoconfigure.condition.SearchStrategy; import org.springframework.boot.context.properties.ConfigurationBeanFactoryMetaData; import org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor; import org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessorRegistrar; -import org.springframework.cloud.context.environment.EnvironmentChangeEvent; import org.springframework.cloud.context.properties.ConfigurationPropertiesBeans; import org.springframework.cloud.context.properties.ConfigurationPropertiesRebinder; import org.springframework.context.ApplicationContext; @@ -65,16 +62,26 @@ public class ConfigurationPropertiesRebinderAutoConfiguration @ConditionalOnMissingBean(search = SearchStrategy.CURRENT) public ConfigurationPropertiesRebinder configurationPropertiesRebinder( ConfigurationPropertiesBeans beans) { - ConfigurationPropertiesRebinder rebinder = new ConfigurationPropertiesRebinder(beans); + ConfigurationPropertiesRebinder rebinder = new ConfigurationPropertiesRebinder( + beans); return rebinder; } @Override public void afterSingletonsInstantiated() { - // After all beans are initialized send a pre-emptive EnvironmentChangeEvent - // so that anything that needs to rebind gets a chance now (especially for - // beans in the parent context) - this.context.publishEvent( - new EnvironmentChangeEvent(Collections. emptySet())); + // After all beans are initialized explicitly rebind beans from the parent + // so that changes during the initialization of the current context are + // reflected. In particular this can be important when low level services like + // decryption are bootstrapped in the parent, but need to change their + // configuration before the child context is processed. + if (this.context.getParent() != null) { + // TODO: make this optional? (E.g. when creating child contexts that prefer to + // be isolated.) + ConfigurationPropertiesRebinder rebinder = context + .getBean(ConfigurationPropertiesRebinder.class); + for (String name : context.getParent().getBeanDefinitionNames()) { + rebinder.rebind(name); + } + } } } \ No newline at end of file diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java index c0a7d61a..94465b91 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java @@ -108,7 +108,7 @@ public class PropertySourceBootstrapConfiguration implements } insertPropertySources(propertySources, composite); reinitializeLoggingSystem(environment, logConfig, logFile); - setLogLevels(environment); + setLogLevels(applicationContext, environment); handleIncludedProfiles(environment); } } @@ -140,13 +140,14 @@ public class PropertySourceBootstrapConfiguration implements } } - private void setLogLevels(ConfigurableEnvironment environment) { + private void setLogLevels(ConfigurableApplicationContext applicationContext, + ConfigurableEnvironment environment) { LoggingRebinder rebinder = new LoggingRebinder(); rebinder.setEnvironment(environment); // We can't fire the event in the ApplicationContext here (too early), but we can // create our own listener and poke it (it doesn't need the key changes) - rebinder.onApplicationEvent( - new EnvironmentChangeEvent(Collections. emptySet())); + rebinder.onApplicationEvent(new EnvironmentChangeEvent(applicationContext, + Collections.emptySet())); } private void insertPropertySources(MutablePropertySources propertySources, diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java index 471718d1..af49fdc1 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java @@ -116,7 +116,7 @@ public class EnvironmentDecryptApplicationInitializer implements // The parent is actually the bootstrap context, and it is fully // initialized, so we can fire an EnvironmentChangeEvent there to rebind // @ConfigurationProperties, in case they were encrypted. - parent.publishEvent(new EnvironmentChangeEvent(found)); + parent.publishEvent(new EnvironmentChangeEvent(parent, found)); } } diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentChangeEvent.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentChangeEvent.java index d0fb5e50..d58e24d8 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentChangeEvent.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentChangeEvent.java @@ -32,10 +32,15 @@ public class EnvironmentChangeEvent extends ApplicationEvent { private Set keys; public EnvironmentChangeEvent(Set keys) { - super(keys); + // Backwards compatible constructor with less utility (practically no use at all) + this(keys, keys); + } + + public EnvironmentChangeEvent(Object context, Set keys) { + super(context); this.keys = keys; } - + /** * @return the keys */ diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentManager.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentManager.java index d1addd47..12a89dd1 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentManager.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/environment/EnvironmentManager.java @@ -68,7 +68,7 @@ public class EnvironmentManager implements ApplicationEventPublisherAware { Map result = new LinkedHashMap(map); if (!map.isEmpty()) { map.clear(); - publish(new EnvironmentChangeEvent(result.keySet())); + publish(new EnvironmentChangeEvent(publisher, result.keySet())); } return result; } @@ -88,7 +88,7 @@ public class EnvironmentManager implements ApplicationEventPublisherAware { if (!value.equals(environment.getProperty(name))) { map.put(name, value); - publish(new EnvironmentChangeEvent(Collections.singleton(name))); + publish(new EnvironmentChangeEvent(publisher, Collections.singleton(name))); } } diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinder.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinder.java index 16d68f81..e7d6693c 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinder.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinder.java @@ -129,7 +129,11 @@ public class ConfigurationPropertiesRebinder @Override public void onApplicationEvent(EnvironmentChangeEvent event) { - rebind(); + if (this.applicationContext.equals(event.getSource()) + // Backwards compatible + || event.getKeys().equals(event.getSource())) { + rebind(); + } } } diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java index 8b6a9cc2..1a476a44 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java @@ -35,9 +35,15 @@ public class ContextRefresher { private static final String REFRESH_ARGS_PROPERTY_SOURCE = "refreshArgs"; - private static final String[] DEFAULT_PROPERTY_SOURCES = new String[] { //order matters, cli args aren't first, things get messy + private static final String[] DEFAULT_PROPERTY_SOURCES = new String[] { // order + // matters, + // cli args + // aren't + // first, + // things get + // messy CommandLinePropertySource.COMMAND_LINE_PROPERTY_SOURCE_NAME, - "defaultProperties"}; + "defaultProperties" }; private Set standardSources = new HashSet<>( Arrays.asList(StandardEnvironment.SYSTEM_PROPERTIES_PROPERTY_SOURCE_NAME, @@ -61,7 +67,7 @@ public class ContextRefresher { addConfigFilesToEnvironment(); Set keys = changes(before, extract(this.context.getEnvironment().getPropertySources())).keySet(); - this.context.publishEvent(new EnvironmentChangeEvent(keys)); + this.context.publishEvent(new EnvironmentChangeEvent(context, keys)); this.scope.refreshAll(); return keys; } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java index a4587ac0..0598c86c 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java @@ -15,13 +15,13 @@ */ package org.springframework.cloud.context.properties; -import static org.junit.Assert.assertEquals; - import javax.annotation.PostConstruct; import org.junit.Test; import org.junit.runner.RunWith; + import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -35,15 +35,22 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit4.SpringRunner; +import static org.junit.Assert.assertEquals; + @RunWith(SpringRunner.class) @SpringBootTest(classes = TestConfiguration.class) +@ActiveProfiles("config") public class ConfigurationPropertiesRebinderIntegrationTests { @Autowired private TestProperties properties; + @Autowired + private ConfigProperties config; + @Autowired private ConfigurationPropertiesRebinder rebinder; @@ -54,44 +61,55 @@ public class ConfigurationPropertiesRebinderIntegrationTests { @DirtiesContext public void testSimpleProperties() throws Exception { assertEquals("Hello scope!", this.properties.getMessage()); - assertEquals(2, this.properties.getCount()); + assertEquals(1, this.properties.getCount()); // Change the dynamic property source... TestPropertyValues.of("message:Foo").applyTo(this.environment); // ...but don't refresh, so the bean stays the same: assertEquals("Hello scope!", this.properties.getMessage()); - assertEquals(2, this.properties.getCount()); + assertEquals(1, this.properties.getCount()); + } + + @Test + @DirtiesContext + public void testRefreshInParent() throws Exception { + assertEquals("main", this.config.getName()); + // Change the dynamic property source... + TestPropertyValues.of("config.name=foo").applyTo(this.environment); + // ...and then refresh, so the bean is re-initialized: + this.rebinder.rebind(); + assertEquals("foo", this.config.getName()); } @Test @DirtiesContext public void testRefresh() throws Exception { - assertEquals(2, this.properties.getCount()); + assertEquals(1, this.properties.getCount()); assertEquals("Hello scope!", this.properties.getMessage()); // Change the dynamic property source... TestPropertyValues.of("message:Foo").applyTo(this.environment); // ...and then refresh, so the bean is re-initialized: this.rebinder.rebind(); assertEquals("Foo", this.properties.getMessage()); - assertEquals(3, this.properties.getCount()); + assertEquals(2, this.properties.getCount()); } @Test @DirtiesContext public void testRefreshByName() throws Exception { - assertEquals(2, this.properties.getCount()); + assertEquals(1, this.properties.getCount()); assertEquals("Hello scope!", this.properties.getMessage()); // Change the dynamic property source... TestPropertyValues.of("message:Foo").applyTo(this.environment); // ...and then refresh, so the bean is re-initialized: this.rebinder.rebind("properties"); assertEquals("Foo", this.properties.getMessage()); - assertEquals(3, this.properties.getCount()); + assertEquals(2, this.properties.getCount()); } @Configuration @EnableConfigurationProperties @Import({ RefreshConfiguration.RebinderConfiguration.class, - PropertyPlaceholderAutoConfiguration.class }) + PropertyPlaceholderAutoConfiguration.class }) protected static class TestConfiguration { @Bean @@ -104,8 +122,8 @@ public class ConfigurationPropertiesRebinderIntegrationTests { // Hack out a protected inner class for testing protected static class RefreshConfiguration extends RefreshAutoConfiguration { @Configuration - protected static class RebinderConfiguration extends - ConfigurationPropertiesRebinderAutoConfiguration { + protected static class RebinderConfiguration + extends ConfigurationPropertiesRebinderAutoConfiguration { } } @@ -142,4 +160,17 @@ public class ConfigurationPropertiesRebinderIntegrationTests { } } + @ConfigurationProperties("config") + @ConditionalOnMissingBean(ConfigProperties.class) + public static class ConfigProperties { + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderLifecycleIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderLifecycleIntegrationTests.java index d8b1c9f7..a6c41c6f 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderLifecycleIntegrationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderLifecycleIntegrationTests.java @@ -55,14 +55,14 @@ public class ConfigurationPropertiesRebinderLifecycleIntegrationTests { @Test @DirtiesContext public void testRefresh() throws Exception { - assertEquals(1, this.properties.getCount()); + assertEquals(0, this.properties.getCount()); assertEquals("Hello scope!", this.properties.getMessage()); // Change the dynamic property source... TestPropertyValues.of("message:Foo").applyTo(this.environment); // ...and then refresh, so the bean is re-initialized: this.rebinder.rebind(); assertEquals("Foo", this.properties.getMessage()); - assertEquals(2, this.properties.getCount()); + assertEquals(1, this.properties.getCount()); } @Configuration diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/logging/LoggingRebinderTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/logging/LoggingRebinderTests.java index 35f5764c..b752360c 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/logging/LoggingRebinderTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/logging/LoggingRebinderTests.java @@ -24,6 +24,7 @@ import org.junit.After; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import org.springframework.boot.logging.LogLevel; import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.test.util.TestPropertyValues; @@ -52,7 +53,7 @@ public class LoggingRebinderTests { TestPropertyValues.of("logging.level.org.springframework.web=TRACE") .applyTo(environment); this.rebinder.setEnvironment(environment); - this.rebinder.onApplicationEvent(new EnvironmentChangeEvent( + this.rebinder.onApplicationEvent(new EnvironmentChangeEvent(environment, Collections.singleton("logging.level.org.springframework.web"))); assertTrue(this.logger.isTraceEnabled()); } @@ -64,7 +65,7 @@ public class LoggingRebinderTests { TestPropertyValues.of("logging.level.org.springframework.web=trace") .applyTo(environment); this.rebinder.setEnvironment(environment); - this.rebinder.onApplicationEvent(new EnvironmentChangeEvent( + this.rebinder.onApplicationEvent(new EnvironmentChangeEvent(environment, Collections.singleton("logging.level.org.springframework.web"))); assertTrue(this.logger.isTraceEnabled()); } diff --git a/spring-cloud-context/src/test/resources/application-config.properties b/spring-cloud-context/src/test/resources/application-config.properties new file mode 100644 index 00000000..d4408d3f --- /dev/null +++ b/spring-cloud-context/src/test/resources/application-config.properties @@ -0,0 +1 @@ +config.name: main diff --git a/spring-cloud-context/src/test/resources/bootstrap-config.properties b/spring-cloud-context/src/test/resources/bootstrap-config.properties new file mode 100644 index 00000000..d61d540a --- /dev/null +++ b/spring-cloud-context/src/test/resources/bootstrap-config.properties @@ -0,0 +1,2 @@ +spring.main.sources: org.springframework.cloud.context.properties.ConfigurationPropertiesRebinderIntegrationTests.ConfigProperties +config.name: parent