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 1e63180d2..2c786492b 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 @@ -17,12 +17,17 @@ package org.springframework.cloud.context.properties; import java.beans.PropertyDescriptor; +import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.IdentityHashMap; +import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -32,6 +37,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.jspecify.annotations.Nullable; import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.AopProxyUtils; @@ -43,7 +49,14 @@ import org.springframework.beans.BeanWrapperImpl; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactoryUtils; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; +import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.beans.factory.support.AbstractBeanFactory; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.ConfigurationPropertiesBindingPostProcessor; import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration.RefreshProperties; import org.springframework.cloud.context.config.annotation.RefreshScope; import org.springframework.cloud.context.environment.EnvironmentChangeEvent; @@ -57,6 +70,7 @@ import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.stereotype.Component; import org.springframework.util.ObjectUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -72,8 +86,13 @@ * racing with an {@link EnvironmentChangeEvent}-triggered {@link #rebind()}) cannot * interleave their destroy/reset/re-initialize steps. That does not, however, make the * bean safe to read concurrently from other threads while a rebind is in progress: the - * bean is mutated in place, field by field, so a concurrent reader can observe transient - * intermediate state. Beans that need a consistent view across a refresh should use + * bean is mutated in place, field by field, so a concurrent reader can observe a mix of + * previous and new values. What it does not observe is unbound state: a fresh instance is + * bound first and its values are then copied into the live bean, so a scalar never passes + * through its class-level default, a nested object never through {@code null}, and a + * getter-only collection or map is brought to its new contents without being emptied + * first (the bean is destroyed, re-autowired and re-initialized afterwards, but not bound + * in place a second time). Beans that need a consistent view across a refresh should use * {@link RefreshScope} instead, which serializes reads against refreshes via a per-bean * {@link java.util.concurrent.locks.ReadWriteLock}. * @@ -110,6 +129,11 @@ public class ConfigurationPropertiesRebinder */ private static final int MAX_RESET_DEPTH = 25; + private static final Set INJECTION_ANNOTATIONS = Set.of( + "org.springframework.beans.factory.annotation.Autowired", + "org.springframework.beans.factory.annotation.Value", "jakarta.inject.Inject", "javax.inject.Inject", + "jakarta.annotation.Resource", "javax.annotation.Resource"); + public ConfigurationPropertiesRebinder(ConfigurationPropertiesBeans beans) { this(beans, Collections.emptySet()); } @@ -221,17 +245,11 @@ private boolean rebind(String name, ApplicationContext appContext) { appContext.getAutowireCapableBeanFactory().destroyBean(target); } else { - appContext.getAutowireCapableBeanFactory().destroyBean(target); - resetBeanToDefaults(target); - appContext.getAutowireCapableBeanFactory().autowireBean(target); - appContext.getAutowireCapableBeanFactory().initializeBean(target, name); + rebindInPlace(target, name, appContext); } } else { - appContext.getAutowireCapableBeanFactory().destroyBean(bean); - resetBeanToDefaults(bean); - appContext.getAutowireCapableBeanFactory().autowireBean(bean); - appContext.getAutowireCapableBeanFactory().initializeBean(bean, name); + rebindInPlace(bean, name, appContext); } return true; } @@ -266,10 +284,101 @@ private Advised innermostAdvised(Advised advised) throws Exception { } /** - * Reset bean properties to their class-level defaults so that removed properties do - * not retain stale values after rebinding. + * Rebind the live bean in place. Its new state is bound on a fresh instance first and + * then copied over, so that readers never observe unbound state; the bean is then + * destroyed, re-autowired and re-initialized, without being bound in place a second + * time. A bean that cannot be reset from a fresh instance is destroyed and + * re-initialized as before, which binds it in place. + */ + private void rebindInPlace(Object bean, String name, ApplicationContext appContext) throws Exception { + Object freshInstance = bindFreshInstance(bean, name, appContext); + AutowireCapableBeanFactory beanFactory = appContext.getAutowireCapableBeanFactory(); + beanFactory.destroyBean(bean); + if (freshInstance == null) { + beanFactory.autowireBean(bean); + beanFactory.initializeBean(bean, name); + return; + } + copyFreshInstance(bean, freshInstance); + beanFactory.autowireBean(bean); + initializeWithoutBinding(bean, name, beanFactory); + } + + /** + * Run the initialization callbacks of the live bean again, as + * {@link AutowireCapableBeanFactory#initializeBean} would, but without + * {@link ConfigurationPropertiesBindingPostProcessor}: the bean already holds the + * state bound on the fresh instance, and binding it in place would clear and refill + * its getter-only collections (Spring Boot's {@code CollectionBinder} merges into an + * existing collection that way), which is exactly the kind of transient state this + * class avoids. + */ + private void initializeWithoutBinding(Object bean, String name, AutowireCapableBeanFactory beanFactory) + throws Exception { + if (!(beanFactory instanceof AbstractBeanFactory abstractBeanFactory)) { + beanFactory.initializeBean(bean, name); + return; + } + for (BeanPostProcessor processor : abstractBeanFactory.getBeanPostProcessors()) { + if (!(processor instanceof ConfigurationPropertiesBindingPostProcessor) + && processor.postProcessBeforeInitialization(bean, name) == null) { + return; + } + } + if (bean instanceof InitializingBean initializingBean) { + initializingBean.afterPropertiesSet(); + } + invokeInitMethods(bean, name, abstractBeanFactory); + for (BeanPostProcessor processor : abstractBeanFactory.getBeanPostProcessors()) { + if (processor.postProcessAfterInitialization(bean, name) == null) { + return; + } + } + } + + private void invokeInitMethods(Object bean, String name, AbstractBeanFactory beanFactory) { + String[] initMethodNames; + try { + initMethodNames = (beanFactory.getMergedBeanDefinition(name) instanceof AbstractBeanDefinition definition) + ? definition.getInitMethodNames() : null; + } + catch (NoSuchBeanDefinitionException ex) { + return; + } + if (initMethodNames == null) { + return; + } + for (String initMethodName : initMethodNames) { + Method initMethod = ReflectionUtils.findMethod(AopUtils.getTargetClass(bean), initMethodName); + if (initMethod != null) { + ReflectionUtils.makeAccessible(initMethod); + ReflectionUtils.invokeMethod(initMethod, bean); + } + } + } + + /** + * Create a fresh instance of the bean's class and bind it from the + * {@link Environment}, so that its state can replace the live bean's state once the + * live bean has been destroyed. This happens before the live bean is touched: a + * binding failure surfaces as the rebind error and leaves the live bean as it was. + * The instance is not autowired; injected properties are left alone by the copy. + * Returns {@code null} when no trustworthy fresh instance can be created; the live + * bean is then re-bound in place and removed properties keep their stale values. */ - private void resetBeanToDefaults(Object bean) { + private @Nullable Object bindFreshInstance(Object bean, String name, ApplicationContext appContext) { + ConfigurationPropertiesBindingPostProcessor binder = appContext + .getBeanProvider(ConfigurationPropertiesBindingPostProcessor.class) + .getIfAvailable(); + if (binder == null) { + // Without the binder the fresh instance would only carry its class-level + // defaults, and copying those onto the live bean would discard its values. + if (logger.isDebugEnabled()) { + logger.debug("No ConfigurationPropertiesBindingPostProcessor available; skipping property reset " + + "before rebinding"); + } + return null; + } Class targetClass = AopUtils.getTargetClass(bean); if (!hasDefaultConstructor(targetClass)) { // Beans whose no-arg constructor (if any) is not their only constructor @@ -282,7 +391,7 @@ private void resetBeanToDefaults(Object bean) { logger.debug("No default constructor for " + targetClass.getName() + "; skipping property reset before rebinding"); } - return; + return null; } Object freshInstance; try { @@ -291,9 +400,21 @@ private void resetBeanToDefaults(Object bean) { catch (Exception ex) { logger.warn("Cannot create default instance of " + targetClass.getName() + " for reset; skipping property reset", ex); - return; + return null; + } + binder.postProcessBeforeInitialization(freshInstance, name); + return freshInstance; + } + + /** + * Replace the properties of the live bean with those of the freshly bound instance, + * so that removed properties revert to their defaults while concurrent readers never + * observe the bean's unbound class-level defaults (see gh-1727). + */ + private void copyFreshInstance(Object bean, @Nullable Object freshInstance) { + if (freshInstance != null) { + resetProperties(bean, freshInstance, Collections.newSetFromMap(new IdentityHashMap<>()), 0); } - resetProperties(bean, freshInstance, Collections.newSetFromMap(new IdentityHashMap<>()), 0); } /** @@ -318,7 +439,7 @@ private boolean hasDefaultConstructor(Class type) { return constructors.length == 1 && constructors[0].getParameterCount() == 0; } - private void resetProperties(Object bean, Object defaults, Set visited, int depth) { + private void resetProperties(Object bean, Object freshInstance, Set visited, int depth) { // Guard against cyclic object graphs and pathologically deep ones so that // recursion // always terminates; see MAX_RESET_DEPTH. @@ -326,7 +447,7 @@ private void resetProperties(Object bean, Object defaults, Set visited, return; } BeanWrapper target = new BeanWrapperImpl(bean); - BeanWrapper defaultsWrapper = new BeanWrapperImpl(defaults); + BeanWrapper freshWrapper = new BeanWrapperImpl(freshInstance); for (PropertyDescriptor pd : target.getPropertyDescriptors()) { String propertyName = pd.getName(); if ("class".equals(propertyName)) { @@ -335,60 +456,50 @@ private void resetProperties(Object bean, Object defaults, Set visited, try { // A property whose declared type the user excluded is left alone whatever // its accessors look like: a read-only property is not descended into and - // a writable one is not overwritten with the default instance's value. + // a writable one is not overwritten with the fresh instance's value. if (isNeverReset(pd.getPropertyType())) { continue; } - if (target.isWritableProperty(propertyName) && defaultsWrapper.isReadableProperty(propertyName)) { - if (target.isReadableProperty(propertyName) - && isNeverReset(target.getPropertyValue(propertyName))) { + if (target.isWritableProperty(propertyName) + && (freshWrapper.isReadableProperty(propertyName) || hasField(bean, propertyName))) { + // A setter that is injected (for example @Autowired or @Value) is not + // a + // bound property: the fresh instance is not autowired, so copying it + // would null the dependency before it is re-injected. + if (isInjected(bean, pd) || (target.isReadableProperty(propertyName) + && isNeverReset(target.getPropertyValue(propertyName)))) { continue; } - Object defaultValue = defaultsWrapper.getPropertyValue(propertyName); - Object currentValue = target.isReadableProperty(propertyName) - ? target.getPropertyValue(propertyName) : null; + // A write-only property (a setter without a getter, which the binder + // still binds) is read from its backing field on both instances. + Object freshValue = readProperty(freshWrapper, freshInstance, propertyName); + Object currentValue = readProperty(target, bean, propertyName); // Skip the setter call entirely when there's nothing to change: this // avoids needlessly invoking setters that don't tolerate being called // again with the same value, and setters that reject null are only // ever // invoked when the property actually needs to become null. - if (!ObjectUtils.nullSafeEquals(currentValue, defaultValue)) { + if (!ObjectUtils.nullSafeEquals(currentValue, freshValue)) { try { - target.setPropertyValue(propertyName, defaultValue); + target.setPropertyValue(propertyName, freshValue); } catch (Exception ex) { warnCannotReset(bean, propertyName, ex); } } } - else if (target.isReadableProperty(propertyName) && defaultsWrapper.isReadableProperty(propertyName)) { + else if (target.isReadableProperty(propertyName) && freshWrapper.isReadableProperty(propertyName)) { Object value = target.getPropertyValue(propertyName); - Object defaultValue = defaultsWrapper.getPropertyValue(propertyName); + Object freshValue = freshWrapper.getPropertyValue(propertyName); if (value instanceof Collection collection) { - try { - collection.clear(); - if (defaultValue instanceof Collection defaultCollection) { - collection.addAll(defaultCollection); - } - } - catch (UnsupportedOperationException ex) { - warnCannotReset(bean, propertyName, ex); - } + replaceContents(bean, propertyName, collection, freshValue); } else if (value instanceof Map map) { - try { - map.clear(); - if (defaultValue instanceof Map defaultMap) { - map.putAll(defaultMap); - } - } - catch (UnsupportedOperationException ex) { - warnCannotReset(bean, propertyName, ex); - } + replaceContents(bean, propertyName, map, freshValue); } - else if (value != null && defaultValue != null && isResettableNestedType(value.getClass()) + else if (value != null && freshValue != null && isResettableNestedType(value.getClass()) && isStableInstance(target, propertyName, value)) { - resetProperties(value, defaultValue, visited, depth + 1); + resetProperties(value, freshValue, visited, depth + 1); } } } @@ -417,11 +528,119 @@ private boolean isStableInstance(BeanWrapper target, String propertyName, Object } } + /** + * Bring a getter-only collection to the contents of the fresh instance without ever + * leaving it empty: nothing is touched when the contents are already equal; a list is + * updated element by element, so a reader iterating over it is not disturbed unless + * its size changes; any other collection drops only the removed elements before the + * new ones are added. + */ + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void replaceContents(Object bean, String propertyName, Collection collection, @Nullable Object freshValue) { + Collection fresh = (freshValue instanceof Collection freshCollection) ? freshCollection + : Collections.emptyList(); + if (collection.equals(fresh)) { + return; + } + try { + if (collection instanceof List list) { + replaceElements(list, fresh); + } + else { + collection.retainAll(fresh); + collection.addAll(fresh); + } + } + catch (UnsupportedOperationException ex) { + warnCannotReset(bean, propertyName, ex); + } + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private static void replaceElements(List list, Collection fresh) { + int index = 0; + Iterator iterator = fresh.iterator(); + while (iterator.hasNext() && index < list.size()) { + Object element = iterator.next(); + if (!ObjectUtils.nullSafeEquals(list.get(index), element)) { + list.set(index, element); + } + index++; + } + if (iterator.hasNext()) { + while (iterator.hasNext()) { + list.add(iterator.next()); + } + } + else if (index < list.size()) { + list.subList(index, list.size()).clear(); + } + } + + /** + * Bring a getter-only map to the contents of the fresh instance without ever leaving + * it empty: nothing is touched when the contents are already equal, and otherwise + * only the removed keys are dropped before the fresh entries are put. + */ + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void replaceContents(Object bean, String propertyName, Map map, @Nullable Object freshValue) { + Map fresh = (freshValue instanceof Map freshMap) ? freshMap : Collections.emptyMap(); + if (map.equals(fresh)) { + return; + } + try { + map.keySet().retainAll(fresh.keySet()); + map.putAll(fresh); + } + catch (UnsupportedOperationException ex) { + warnCannotReset(bean, propertyName, ex); + } + } + + private static boolean hasField(Object bean, String propertyName) { + return ReflectionUtils.findField(AopUtils.getTargetClass(bean), propertyName) != null; + } + + private static @Nullable Object readProperty(BeanWrapper wrapper, Object instance, String propertyName) { + if (wrapper.isReadableProperty(propertyName)) { + return wrapper.getPropertyValue(propertyName); + } + Field field = ReflectionUtils.findField(AopUtils.getTargetClass(instance), propertyName); + if (field == null) { + return null; + } + ReflectionUtils.makeAccessible(field); + return ReflectionUtils.getField(field, instance); + } + + /** + * Whether the given property is a dependency injection point (an annotated setter or + * backing field, for example {@code @Autowired} or {@code @Value}) rather than a + * bound property. + */ + private boolean isInjected(Object bean, PropertyDescriptor pd) { + Method writeMethod = pd.getWriteMethod(); + if (writeMethod != null && isInjected(writeMethod.getAnnotations())) { + return true; + } + Field field = ReflectionUtils.findField(AopUtils.getTargetClass(bean), pd.getName()); + return field != null && isInjected(field.getAnnotations()); + } + + private static boolean isInjected(Annotation[] annotations) { + for (Annotation annotation : annotations) { + if (INJECTION_ANNOTATIONS.contains(annotation.annotationType().getName())) { + return true; + } + } + return false; + } + /** * Log, at most once per bean type and property, that a property could not be reset to - * its default value and may therefore retain a stale value after this and future - * refreshes. The full exception is only logged at DEBUG, on every occurrence, to - * avoid spamming the log at the default level while still making the failure + * its freshly bound value and may therefore retain a stale value after this and + * future refreshes. The full exception is only logged at DEBUG, on every occurrence, + * to avoid spamming the log at the default level while still making the failure * discoverable. */ private void warnCannotReset(Object bean, String propertyName, Exception ex) { diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderTransientStateIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderTransientStateIntegrationTests.java new file mode 100644 index 000000000..14a0fdb60 --- /dev/null +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderTransientStateIntegrationTests.java @@ -0,0 +1,328 @@ +/* + * Copyright 2012-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.context.properties; + +import java.time.Clock; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.ConfigurationPropertiesBindException; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.autoconfigure.ConfigurationPropertiesRebinderAutoConfiguration; +import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; +import org.springframework.cloud.context.properties.ConfigurationPropertiesRebinderTransientStateIntegrationTests.TestConfiguration; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.PropertySource; +import org.springframework.test.annotation.DirtiesContext; + +import static org.assertj.core.api.Assertions.entry; +import static org.assertj.core.api.BDDAssertions.then; +import static org.assertj.core.api.BDDAssertions.thenThrownBy; + +/** + * A rebind must never pass the live bean through its unbound class-level defaults, which + * is what a concurrent reader would observe (see gh-1727): a nested object populated only + * by binding must never be set to {@code null}, a scalar must never be set to its + * class-level default on the way to its bound value, and a rebind whose binding fails + * must leave the bean as it was, and a getter-only collection or map must never be + * emptied on the way to its new contents. The tests check the values the bean's setters + * receive during a rebind and the operations its collections see. + */ +@SpringBootTest(classes = TestConfiguration.class, properties = { "test.timeout.seconds=30", "test.workers=4", + "test.tags[0]=a", "test.tags[1]=b", "test.labels.one=1", "test.labels.two=2" }) +public class ConfigurationPropertiesRebinderTransientStateIntegrationTests { + + @Autowired + private TestProperties properties; + + @Autowired + private ConfigurationPropertiesRebinder rebinder; + + @Autowired + private ConfigurableEnvironment environment; + + @Test + @DirtiesContext + public void rebindDoesNotNullNestedObjectPopulatedByBinding() { + then(this.properties.getTimeout().getSeconds()).isEqualTo(30); + + this.rebinder.rebind(); + + then(this.properties.timeoutAssignments).doesNotContainNull(); + then(this.properties.getTimeout().getSeconds()).isEqualTo(30); + } + + @Test + @DirtiesContext + public void rebindDoesNotSetScalarToClassLevelDefault() { + then(this.properties.getWorkers()).isEqualTo(4); + + this.rebinder.rebind(); + + then(this.properties.workerAssignments).containsOnly(4); + } + + @Test + @DirtiesContext + public void failedRebindLeavesTheBeanUntouched() { + then(this.properties.getWorkers()).isEqualTo(4); + findTestProperties().put("test.workers", "not-a-number"); + + thenThrownBy(() -> this.rebinder.rebind()).isInstanceOf(ConfigurationPropertiesBindException.class); + + then(this.properties.workerAssignments).containsOnly(4); + then(this.properties.getWorkers()).isEqualTo(4); + then(this.properties.getTimeout().getSeconds()).isEqualTo(30); + } + + @Test + @DirtiesContext + public void rebindBringsGetterOnlyMapToNewContentsWithoutEmptyingIt() { + then(this.properties.getLabels()).containsOnly(entry("one", "1"), entry("two", "2")); + findTestProperties().remove("test.labels.two"); + findTestProperties().put("test.labels.three", "3"); + this.rebinder.rebind(); + then(this.properties.getLabels()).containsOnly(entry("one", "1"), entry("three", "3")); + then(this.properties.getLabels().clears).isZero(); + } + + @Test + @DirtiesContext + public void rebindNeverClearsGetterOnlyList() { + TrackingList tags = this.properties.getTags(); + then(tags).containsExactly("a", "b"); + int clearsAtStartup = tags.clears; + this.rebinder.rebind(); + then(tags).containsExactly("a", "b"); + then(tags.clears).as("unchanged rebind").isEqualTo(clearsAtStartup); + findTestProperties().put("test.tags[0]", "c"); + this.rebinder.rebind(); + then(tags).containsExactly("c", "b"); + then(tags.clears).as("changed rebind").isEqualTo(clearsAtStartup); + findTestProperties().remove("test.tags[1]"); + this.rebinder.rebind(); + then(tags).containsExactly("c"); + then(tags.clears).as("shortened rebind").isEqualTo(clearsAtStartup); + findTestProperties().remove("test.tags[0]"); + this.rebinder.rebind(); + then(tags).isEmpty(); + then(tags.clears).as("removed rebind").isEqualTo(clearsAtStartup); + } + + @Test + @DirtiesContext + public void concurrentReaderNeverSeesGetterOnlyListEmptyDuringRebinds() throws Exception { + TrackingList tags = this.properties.getTags(); + then(tags).containsExactly("a", "b"); + AtomicBoolean stop = new AtomicBoolean(); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future reads = executor.submit(() -> { + int count = 0; + while (!stop.get()) { + int seen = 0; + for (String tag : tags) { + then(tag).isNotNull(); + seen++; + } + then(seen).as("elements seen by a reader").isEqualTo(2); + count++; + } + return count; + }); + for (int i = 0; i < 50; i++) { + findTestProperties().put("test.tags[0]", (i % 2 == 0) ? "c" : "a"); + this.rebinder.rebind(); + } + stop.set(true); + then(reads.get(10, TimeUnit.SECONDS)).isPositive(); + } + finally { + stop.set(true); + executor.shutdownNow(); + } + then(tags).containsExactly("a", "b"); + } + + @Test + @DirtiesContext + public void injectedSetterIsNeitherCopiedFromTheFreshInstanceNorNulled() { + Clock clock = this.properties.getClock(); + then(clock).isNotNull(); + this.rebinder.rebind(); + then(this.properties.getClock()).isSameAs(clock); + then(this.properties.clockAssignments).doesNotContainNull() + .allSatisfy(assigned -> then(assigned).isSameAs(clock)); + } + + private Map findTestProperties() { + for (PropertySource source : this.environment.getPropertySources()) { + if (source.getName().toLowerCase().contains("test")) { + @SuppressWarnings("unchecked") + Map map = (Map) source.getSource(); + return map; + } + } + throw new IllegalStateException("Could not find test property source"); + } + + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(RefreshAutoConfiguration.RefreshProperties.class) + @Import({ RefreshConfiguration.RebinderConfiguration.class, PropertyPlaceholderAutoConfiguration.class }) + protected static class TestConfiguration { + + @Bean + protected TestProperties testProperties() { + return new TestProperties(); + } + + @Bean + protected Clock testClock() { + return Clock.systemUTC(); + } + + } + + // Hack out a protected inner class for testing + protected static class RefreshConfiguration extends RefreshAutoConfiguration { + + @Configuration(proxyBeanMethods = false) + protected static class RebinderConfiguration extends ConfigurationPropertiesRebinderAutoConfiguration { + + public RebinderConfiguration(ApplicationContext context) { + super(context); + } + + } + + } + + @ConfigurationProperties("test") + protected static class TestProperties { + + final List timeoutAssignments = new ArrayList<>(); + + final List workerAssignments = new ArrayList<>(); + + final List clockAssignments = new ArrayList<>(); + + private final TrackingList tags = new TrackingList(); + + private Clock clock; + + private final TrackingMap labels = new TrackingMap(); + + private Timeout timeout; + + private int workers; + + public Timeout getTimeout() { + return this.timeout; + } + + public void setTimeout(Timeout timeout) { + this.timeoutAssignments.add(timeout); + this.timeout = timeout; + } + + public int getWorkers() { + return this.workers; + } + + public void setWorkers(int workers) { + this.workerAssignments.add(workers); + this.workers = workers; + } + + public TrackingList getTags() { + return this.tags; + } + + public Clock getClock() { + return this.clock; + } + + @Autowired + public void setClock(Clock clock) { + this.clockAssignments.add(clock); + this.clock = clock; + } + + public TrackingMap getLabels() { + return this.labels; + } + + } + + @SuppressWarnings("serial") + protected static class TrackingList extends ArrayList { + + int clears; + + @Override + public void clear() { + this.clears++; + super.clear(); + } + + } + + @SuppressWarnings("serial") + protected static class TrackingMap extends LinkedHashMap { + + int clears; + + @Override + public void clear() { + this.clears++; + super.clear(); + } + + } + + protected static class Timeout { + + private int seconds; + + public int getSeconds() { + return this.seconds; + } + + public void setSeconds(int seconds) { + this.seconds = seconds; + } + + } + +}