From 93a0365422cc361053c38ae06b1c90308e0eecac Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 7 Nov 2017 10:32:23 +0000 Subject: [PATCH] Prevent logging system from being pinged on context refresh A context refresh is actually only trying to recalculate environment properties. Spring Boot doesn't provide an explicit API for that so we create a "mini application" to mimic the startup. There is no need for the logging system to be touched at all, so we can try and filter out the listeners we know might do that. Fixes gh-260 --- .../BootstrapApplicationListener.java | 24 ++++++++ .../refresh/ContextRefresherTests.java | 58 +++++++++++++++++-- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java index b67846d8..f5ad1702 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java @@ -19,10 +19,13 @@ package org.springframework.cloud.bootstrap; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.boot.Banner.Mode; @@ -30,6 +33,7 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.builder.ParentContextApplicationContextInitializer; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; +import org.springframework.boot.logging.LoggingApplicationListener; import org.springframework.cloud.bootstrap.encrypt.EnvironmentDecryptApplicationInitializer; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ApplicationListener; @@ -159,6 +163,14 @@ public class BootstrapApplicationListener .environment(bootstrapEnvironment) .properties("spring.application.name:" + configName) .registerShutdownHook(false).logStartupInfo(false).web(false); + if (environment.getPropertySources().contains("refreshArgs")) { + // If we are doing a context refresh, really we only want to refresh the + // Environment, and there are some toxic listeners (like the + // LoggingApplicationListener) that affect global static state, so we need a + // way to switch those off. + builder.application() + .setListeners(filterListeners(builder.application().getListeners())); + } List> sources = new ArrayList<>(); for (String name : names) { Class cls = ClassUtils.resolveClassName(name, null); @@ -182,6 +194,18 @@ public class BootstrapApplicationListener return context; } + private Collection> filterListeners( + Set> listeners) { + Set> result = new LinkedHashSet<>(); + for (ApplicationListener listener : listeners) { + if (!(listener instanceof LoggingApplicationListener) + && !(listener instanceof LoggingSystemShutdownListener)) { + result.add(listener); + } + } + return result; + } + private void mergeDefaultProperties(MutablePropertySources environment, MutablePropertySources bootstrap) { String name = DEFAULT_PROPERTIES; diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java index 99211813..cba9d2f5 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java @@ -1,7 +1,5 @@ package org.springframework.cloud.context.refresh; -import static org.assertj.core.api.Assertions.assertThat; - import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -11,7 +9,9 @@ import java.util.Map; import org.junit.After; import org.junit.Test; import org.mockito.Mockito; + import org.springframework.boot.SpringApplication; +import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.test.util.EnvironmentTestUtils; import org.springframework.cloud.bootstrap.config.PropertySourceLocator; import org.springframework.cloud.context.scope.refresh.RefreshScope; @@ -22,6 +22,8 @@ import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; +import static org.assertj.core.api.Assertions.assertThat; + public class ContextRefresherTests { private RefreshScope scope = Mockito.mock(RefreshScope.class); @@ -32,6 +34,8 @@ public class ContextRefresherTests { if (context != null) { context.close(); } + System.clearProperty(LoggingSystem.SYSTEM_PROPERTY); + TestLoggingSystem.count = 0; } @Test @@ -82,11 +86,33 @@ public class ContextRefresherTests { EnvironmentTestUtils.addEnvironment(context, "spring.cloud.bootstrap.sources: org.springframework.cloud.context.refresh.ContextRefresherTests.PropertySourceConfiguration\n" + ""); - ConfigurableApplicationContext refresherContext = refresher.addConfigFilesToEnvironment(); - assertThat(refresherContext.getParent()).isNotNull().isInstanceOf(ConfigurableApplicationContext.class); - ConfigurableApplicationContext parent = (ConfigurableApplicationContext) refresherContext.getParent(); + ConfigurableApplicationContext refresherContext = refresher + .addConfigFilesToEnvironment(); + assertThat(refresherContext.getParent()).isNotNull() + .isInstanceOf(ConfigurableApplicationContext.class); + ConfigurableApplicationContext parent = (ConfigurableApplicationContext) refresherContext + .getParent(); assertThat(parent.isActive()).isFalse(); } + + @Test + public void loggingSystemNotInitialized() { + System.setProperty(LoggingSystem.SYSTEM_PROPERTY, + TestLoggingSystem.class.getName()); + TestLoggingSystem system = (TestLoggingSystem) LoggingSystem + .get(getClass().getClassLoader()); + assertThat(system.getCount()).isEqualTo(0); + try (ConfigurableApplicationContext context = SpringApplication.run(Empty.class, + "--spring.main.webEnvironment=false", "--debug=false", + "--spring.main.bannerMode=OFF", + "--spring.cloud.bootstrap.name=refresh")) { + assertThat(system.getCount()).isEqualTo(4); + ContextRefresher refresher = new ContextRefresher(context, scope); + refresher.refresh(); + assertThat(system.getCount()).isEqualTo(4); + } + } + private List names(MutablePropertySources propertySources) { List list = new ArrayList<>(); for (PropertySource p : propertySources) { @@ -109,4 +135,26 @@ public class ContextRefresherTests { } + @Configuration + protected static class Empty { + } + + public static class TestLoggingSystem extends LoggingSystem { + + private static int count; + + public TestLoggingSystem(ClassLoader loader) { + } + + public int getCount() { + return count; + } + + @Override + public void beforeInitialize() { + count++; + } + + } + }