From bfbadec0f0b30a0f5d1af03f47f8b5a8248ae286 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 17 Dec 2019 16:31:11 +0000 Subject: [PATCH] Prevent listeners being added multiple times on restart (and at other times potentially - e.g. if 2 contexts share a parent). Plus some general tidy up on compiler warnings. Fixes gh-613 --- .../RefreshEndpointAutoConfiguration.java | 3 +- .../BootstrapApplicationListener.java | 37 ++++++++++++++----- .../PropertySourceBootstrapConfiguration.java | 31 ++++++++-------- .../config/PropertySourceLocator.java | 8 ++-- .../named/ClientFactoryObjectProvider.java | 5 +-- .../context/restart/RestartEndpoint.java | 28 ++++++++++++++ .../context/scope/refresh/RefreshScope.java | 9 +---- .../endpoint/event/RefreshEventListener.java | 6 +-- .../restart/RestartIntegrationTests.java | 2 +- 9 files changed, 85 insertions(+), 44 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java index ae895fdc..39ad4a7e 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshEndpointAutoConfiguration.java @@ -19,6 +19,7 @@ package org.springframework.cloud.autoconfigure; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.autoconfigure.endpoint.EndpointAutoConfiguration; +import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnAvailableEndpoint; import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnEnabledEndpoint; import org.springframework.boot.actuate.autoconfigure.health.ConditionalOnEnabledHealthIndicator; import org.springframework.boot.actuate.health.Health; @@ -68,7 +69,7 @@ public class RefreshEndpointAutoConfiguration { @Bean @ConditionalOnBean(ContextRefresher.class) - @ConditionalOnEnabledEndpoint + @ConditionalOnAvailableEndpoint @ConditionalOnMissingBean public RefreshEndpoint refreshEndpoint(ContextRefresher contextRefresher) { return new RefreshEndpoint(contextRefresher); 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 d6263260..4b172283 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 @@ -302,28 +302,43 @@ public class BootstrapApplicationListener } + @SuppressWarnings("unchecked") private void apply(ConfigurableApplicationContext context, SpringApplication application, ConfigurableEnvironment environment) { + if (application.getAllSources().contains(BootstrapMarkerConfiguration.class)) { + return; + } + application.addPrimarySources(Arrays.asList(BootstrapMarkerConfiguration.class)); @SuppressWarnings("rawtypes") - List initializers = getOrderedBeansOfType(context, - ApplicationContextInitializer.class); - application.addInitializers(initializers - .toArray(new ApplicationContextInitializer[initializers.size()])); + Set target = new LinkedHashSet<>(application.getInitializers()); + target.addAll( + getOrderedBeansOfType(context, ApplicationContextInitializer.class)); + application.setInitializers(target); addBootstrapDecryptInitializer(application); } + @SuppressWarnings("unchecked") private void addBootstrapDecryptInitializer(SpringApplication application) { DelegatingEnvironmentDecryptApplicationInitializer decrypter = null; + Set> initializers = new LinkedHashSet<>(); for (ApplicationContextInitializer ini : application.getInitializers()) { if (ini instanceof EnvironmentDecryptApplicationInitializer) { - @SuppressWarnings("unchecked") - ApplicationContextInitializer del = (ApplicationContextInitializer) ini; + @SuppressWarnings("rawtypes") + ApplicationContextInitializer del = ini; decrypter = new DelegatingEnvironmentDecryptApplicationInitializer(del); + initializers.add(ini); + initializers.add(decrypter); + } + else if (ini instanceof DelegatingEnvironmentDecryptApplicationInitializer) { + // do nothing + } + else { + initializers.add(ini); } } - if (decrypter != null) { - application.addInitializers(decrypter); - } + ArrayList> target = new ArrayList>( + initializers); + application.setInitializers(target); } private List getOrderedBeansOfType(ListableBeanFactory context, @@ -345,6 +360,10 @@ public class BootstrapApplicationListener this.order = order; } + private static class BootstrapMarkerConfiguration { + + } + private static class AncestorInitializer implements ApplicationContextInitializer, Ordered { 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 88a5ebed..27f7cad6 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 @@ -102,9 +102,9 @@ public class PropertySourceBootstrapConfiguration implements if (source == null || source.size() == 0) { continue; } - List sourceList = new ArrayList<>(); - for (PropertySource p : source) { - sourceList.add(new BootstrapPropertySource(p)); + List> sourceList = new ArrayList<>(); + for (PropertySource p : source) { + sourceList.add(new BootstrapPropertySource<>(p)); } logger.info("Located property source: " + sourceList); composite.addAll(sourceList); @@ -114,7 +114,7 @@ public class PropertySourceBootstrapConfiguration implements MutablePropertySources propertySources = environment.getPropertySources(); String logConfig = environment.resolvePlaceholders("${logging.config:}"); LogFile logFile = LogFile.get(environment); - for (PropertySource p : environment.getPropertySources()) { + for (PropertySource p : environment.getPropertySources()) { if (p.getName().startsWith(BOOTSTRAP_PROPERTY_SOURCE_NAME)) { propertySources.remove(p.getName()); } @@ -166,13 +166,13 @@ public class PropertySourceBootstrapConfiguration implements private void insertPropertySources(MutablePropertySources propertySources, List> composite) { MutablePropertySources incoming = new MutablePropertySources(); - List> reversedComposite = new ArrayList(composite); + List> reversedComposite = new ArrayList<>(composite); // Reverse the list so that when we call addFirst below we are maintaining the - // same order of PropertySournces + // same order of PropertySources // Wherever we call addLast we can use the order in the List since the first item // will end up before the rest Collections.reverse(reversedComposite); - for (PropertySource p : reversedComposite) { + for (PropertySource p : reversedComposite) { incoming.addFirst(p); } PropertySourceBootstrapProperties remoteProperties = new PropertySourceBootstrapProperties(); @@ -180,31 +180,31 @@ public class PropertySourceBootstrapConfiguration implements Bindable.ofInstance(remoteProperties)); if (!remoteProperties.isAllowOverride() || (!remoteProperties.isOverrideNone() && remoteProperties.isOverrideSystemProperties())) { - for (PropertySource p : reversedComposite) { + for (PropertySource p : reversedComposite) { propertySources.addFirst(p); } return; } if (remoteProperties.isOverrideNone()) { - for (PropertySource p : composite) { + for (PropertySource p : composite) { propertySources.addLast(p); } return; } if (propertySources.contains(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) { if (!remoteProperties.isOverrideSystemProperties()) { - for (PropertySource p : composite) { + for (PropertySource p : composite) { propertySources.addAfter(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, p); } } else { - for (PropertySource p : reversedComposite) { + for (PropertySource p : reversedComposite) { propertySources.addBefore(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, p); } } } else { - for (PropertySource p : composite) { + for (PropertySource p : composite) { propertySources.addLast(p); } } @@ -276,7 +276,7 @@ class BootstrapPropertySource extends EnumerablePropertySource { @Override public Object getProperty(String name) { - return p.getProperty(name); + return this.p.getProperty(name); } @Override @@ -285,9 +285,10 @@ class BootstrapPropertySource extends EnumerablePropertySource { if (!(this.p instanceof EnumerablePropertySource)) { throw new IllegalStateException( "Failed to enumerate property names due to non-enumerable property source: " - + p); + + this.p); } - names.addAll(Arrays.asList(((EnumerablePropertySource) p).getPropertyNames())); + names.addAll( + Arrays.asList(((EnumerablePropertySource) this.p).getPropertyNames())); return StringUtils.toStringArray(names); } diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java index d6dc46cf..57f6320e 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceLocator.java @@ -44,15 +44,15 @@ public interface PropertySourceLocator { PropertySource locate(Environment environment); default Collection> locateCollection(Environment environment) { - PropertySource propertySource = locate(environment); + PropertySource propertySource = locate(environment); if (propertySource == null) { - return Collections.EMPTY_LIST; + return Collections.emptyList(); } if (CompositePropertySource.class.isInstance(propertySource)) { Collection> sources = ((CompositePropertySource) propertySource) .getPropertySources(); List> filteredSources = new ArrayList<>(); - for (PropertySource p : sources) { + for (PropertySource p : sources) { if (p != null) { filteredSources.add(p); } @@ -60,7 +60,7 @@ public interface PropertySourceLocator { return filteredSources; } else { - return (List) Arrays.asList(propertySource); + return Arrays.asList(propertySource); } } diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/ClientFactoryObjectProvider.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/ClientFactoryObjectProvider.java index b0a67ea5..8e5aa8aa 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/ClientFactoryObjectProvider.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/named/ClientFactoryObjectProvider.java @@ -34,7 +34,7 @@ import org.springframework.lang.Nullable; */ class ClientFactoryObjectProvider implements ObjectProvider { - private final NamedContextFactory clientFactory; + private final NamedContextFactory clientFactory; private final String name; @@ -42,7 +42,7 @@ class ClientFactoryObjectProvider implements ObjectProvider { private ObjectProvider provider; - ClientFactoryObjectProvider(NamedContextFactory clientFactory, String name, + ClientFactoryObjectProvider(NamedContextFactory clientFactory, String name, Class type) { this.clientFactory = clientFactory; this.name = name; @@ -111,7 +111,6 @@ class ClientFactoryObjectProvider implements ObjectProvider { return delegate().spliterator(); } - @SuppressWarnings("unchecked") private ObjectProvider delegate() { if (this.provider == null) { this.provider = this.clientFactory.getProvider(this.name, this.type); diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/restart/RestartEndpoint.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/restart/RestartEndpoint.java index 182aa580..2b86a9a6 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/restart/RestartEndpoint.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/restart/RestartEndpoint.java @@ -23,15 +23,19 @@ import java.util.Collections; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.boot.SpringApplication; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.endpoint.annotation.WriteOperation; import org.springframework.boot.context.event.ApplicationPreparedEvent; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.support.GenericApplicationContext; import org.springframework.integration.monitor.IntegrationMBeanExporter; import org.springframework.util.ClassUtils; @@ -83,6 +87,7 @@ public class RestartEndpoint implements ApplicationListener { + + @Override + public void initialize(GenericApplicationContext context) { + context.registerBean(PostProcessor.class, () -> new PostProcessor()); + } + + } + + class PostProcessor implements BeanPostProcessor { + + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) + throws BeansException { + if (bean instanceof RestartEndpoint) { + return RestartEndpoint.this; + } + return bean; + } + + } + /** * Pause endpoint configuration. */ diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java index 0f5e0bde..ee6d9ceb 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/refresh/RefreshScope.java @@ -45,14 +45,7 @@ import org.springframework.jmx.export.annotation.ManagedResource; * *

* Note that all beans in this scope are only initialized when first accessed, so - * the scope forces lazy initialization semantics. The implementation involves creating a - * proxy for every bean in the scope, so there is a flag - * {@link #setProxyTargetClass(boolean) proxyTargetClass} which controls the proxy - * creation, defaulting to JDK dynamic proxies and therefore only exposing the interfaces - * implemented by a bean. If callers need access to other methods, then the flag needs to - * be set (and CGLib must be present on the classpath). Because this scope automatically - * proxies all its beans, there is no need to add <aop:auto-proxy/> to - * any bean definitions. + * the scope forces lazy initialization semantics. *

* *

diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/endpoint/event/RefreshEventListener.java b/spring-cloud-context/src/main/java/org/springframework/cloud/endpoint/event/RefreshEventListener.java index 0f1171fd..54bc045c 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/endpoint/event/RefreshEventListener.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/endpoint/event/RefreshEventListener.java @@ -28,9 +28,9 @@ import org.springframework.context.ApplicationEvent; import org.springframework.context.event.SmartApplicationListener; /** - * Calls {@link RefreshEventListener#refresh} when a {@link RefreshEvent} is received. - * Only responds to {@link RefreshEvent} after receiving an {@link ApplicationReadyEvent}, - * as the RefreshEvents might come too early in the application lifecycle. + * Calls {@link ContextRefresher#refresh} when a {@link RefreshEvent} is received. Only + * responds to {@link RefreshEvent} after receiving an {@link ApplicationReadyEvent}, as + * the RefreshEvents might come too early in the application lifecycle. * * @author Spencer Gibb */ diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java index 13004f72..97856464 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/restart/RestartIntegrationTests.java @@ -59,7 +59,7 @@ public class RestartIntegrationTests { then(this.context.getParent().getParent()).isNull(); RestartEndpoint next = this.context.getBean(RestartEndpoint.class); - then(next).isNotSameAs(endpoint); + then(next).isSameAs(endpoint); this.context = next.doRestart(); then(this.context).isNotNull();