From b47cdaa259a783d135fdbe73e69f9a0e30e819b1 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 22 May 2024 14:24:56 +0200 Subject: [PATCH] Lazily register DynamicValuesPropertySource in the TestContext framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit, as a result of commit 6cdb34410b, the DynamicValuesPropertySource was eagerly registered in the Environment even if the DynamicPropertyRegistry was never used to register dynamic properties. This commit ensures that the DynamicValuesPropertySource is only registered if we know that the DynamicPropertyRegistry is actually used -- either by a @⁠DynamicPropertySource method in a test class or via a bean in the ApplicationContext that invokes add() on the DynamicPropertyRegistry bean. See gh-32271 Closes gh-32871 --- .../DefaultDynamicPropertyRegistry.java | 79 +++++++++++++++++++ .../DynamicPropertiesContextCustomizer.java | 53 +++++-------- .../support/DynamicValuesPropertySource.java | 14 ---- .../context/DynamicPropertiesTestSuite.java | 55 +++++++++++++ ...ltTestPropertySourcesIntegrationTests.java | 63 +++++++++++++++ 5 files changed, 218 insertions(+), 46 deletions(-) create mode 100644 spring-test/src/main/java/org/springframework/test/context/support/DefaultDynamicPropertyRegistry.java create mode 100644 spring-test/src/test/java/org/springframework/test/context/DynamicPropertiesTestSuite.java create mode 100644 spring-test/src/test/java/org/springframework/test/context/support/DefaultTestPropertySourcesIntegrationTests.java diff --git a/spring-test/src/main/java/org/springframework/test/context/support/DefaultDynamicPropertyRegistry.java b/spring-test/src/main/java/org/springframework/test/context/support/DefaultDynamicPropertyRegistry.java new file mode 100644 index 00000000000..e079c379541 --- /dev/null +++ b/spring-test/src/main/java/org/springframework/test/context/support/DefaultDynamicPropertyRegistry.java @@ -0,0 +1,79 @@ +/* + * Copyright 2002-2024 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.test.context.support; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Supplier; + +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.util.Assert; + +/** + * Default {@link DynamicPropertyRegistry} implementation. + * + * @author Sam Brannen + * @since 6.2 + */ +final class DefaultDynamicPropertyRegistry implements DynamicPropertyRegistry { + + final Map> valueSuppliers = Collections.synchronizedMap(new LinkedHashMap<>()); + + private final ConfigurableEnvironment environment; + + private final boolean lazilyRegisterPropertySource; + + private final Lock propertySourcesLock = new ReentrantLock(); + + + DefaultDynamicPropertyRegistry(ConfigurableEnvironment environment, boolean lazilyRegisterPropertySource) { + this.environment = environment; + this.lazilyRegisterPropertySource = lazilyRegisterPropertySource; + } + + + @Override + public void add(String name, Supplier valueSupplier) { + Assert.hasText(name, "'name' must not be null or blank"); + Assert.notNull(valueSupplier, "'valueSupplier' must not be null"); + if (this.lazilyRegisterPropertySource) { + ensurePropertySourceIsRegistered(); + } + this.valueSuppliers.put(name, valueSupplier); + } + + private void ensurePropertySourceIsRegistered() { + MutablePropertySources propertySources = this.environment.getPropertySources(); + this.propertySourcesLock.lock(); + try { + PropertySource ps = propertySources.get(DynamicValuesPropertySource.PROPERTY_SOURCE_NAME); + if (ps == null) { + propertySources.addFirst(new DynamicValuesPropertySource(this.valueSuppliers)); + } + } + finally { + this.propertySourcesLock.unlock(); + } + } + +} diff --git a/spring-test/src/main/java/org/springframework/test/context/support/DynamicPropertiesContextCustomizer.java b/spring-test/src/main/java/org/springframework/test/context/support/DynamicPropertiesContextCustomizer.java index dce421cb467..ed6df381b1f 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/DynamicPropertiesContextCustomizer.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/DynamicPropertiesContextCustomizer.java @@ -26,7 +26,7 @@ import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.core.env.ConfigurableEnvironment; -import org.springframework.core.env.PropertySource; +import org.springframework.core.env.MutablePropertySources; import org.springframework.lang.Nullable; import org.springframework.test.context.ContextCustomizer; import org.springframework.test.context.DynamicPropertyRegistry; @@ -75,33 +75,35 @@ class DynamicPropertiesContextCustomizer implements ContextCustomizer { @Override public void customizeContext(ConfigurableApplicationContext context, MergedContextConfiguration mergedConfig) { - DynamicValuesPropertySource propertySource = getOrAdd(context.getEnvironment()); - - if (!context.containsBean(DYNAMIC_PROPERTY_REGISTRY_BEAN_NAME)) { - ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); - beanFactory.registerSingleton(DYNAMIC_PROPERTY_REGISTRY_BEAN_NAME, propertySource.dynamicPropertyRegistry); + ConfigurableEnvironment environment = context.getEnvironment(); + ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); + if (!(beanFactory instanceof BeanDefinitionRegistry beanDefinitionRegistry)) { + throw new IllegalStateException("BeanFactory must be a BeanDefinitionRegistry"); } - if (!context.containsBean(DYNAMIC_PROPERTY_SOURCE_BEAN_INITIALIZER_BEAN_NAME)) { - if (!(context.getBeanFactory() instanceof BeanDefinitionRegistry registry)) { - throw new IllegalStateException("BeanFactory must be a BeanDefinitionRegistry"); - } - BeanDefinition beanDefinition = new RootBeanDefinition(DynamicPropertySourceBeanInitializer.class); - beanDefinition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); - registry.registerBeanDefinition(DYNAMIC_PROPERTY_SOURCE_BEAN_INITIALIZER_BEAN_NAME, beanDefinition); + DefaultDynamicPropertyRegistry dynamicPropertyRegistry = + new DefaultDynamicPropertyRegistry(environment, this.methods.isEmpty()); + beanFactory.registerSingleton(DYNAMIC_PROPERTY_REGISTRY_BEAN_NAME, dynamicPropertyRegistry); + + BeanDefinition beanDefinition = new RootBeanDefinition(DynamicPropertySourceBeanInitializer.class); + beanDefinition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); + beanDefinitionRegistry.registerBeanDefinition( + DYNAMIC_PROPERTY_SOURCE_BEAN_INITIALIZER_BEAN_NAME, beanDefinition); + + if (!this.methods.isEmpty()) { + MutablePropertySources propertySources = environment.getPropertySources(); + propertySources.addFirst(new DynamicValuesPropertySource(dynamicPropertyRegistry.valueSuppliers)); + this.methods.forEach(method -> { + ReflectionUtils.makeAccessible(method); + ReflectionUtils.invokeMethod(method, null, dynamicPropertyRegistry); + }); } - - this.methods.forEach(method -> { - ReflectionUtils.makeAccessible(method); - ReflectionUtils.invokeMethod(method, null, propertySource.dynamicPropertyRegistry); - }); } Set getMethods() { return this.methods; } - @Override public boolean equals(@Nullable Object other) { return (this == other || (other instanceof DynamicPropertiesContextCustomizer that && @@ -113,17 +115,4 @@ class DynamicPropertiesContextCustomizer implements ContextCustomizer { return this.methods.hashCode(); } - - private static DynamicValuesPropertySource getOrAdd(ConfigurableEnvironment environment) { - PropertySource propertySource = environment.getPropertySources() - .get(DynamicValuesPropertySource.PROPERTY_SOURCE_NAME); - if (propertySource == null) { - environment.getPropertySources().addFirst(new DynamicValuesPropertySource()); - return getOrAdd(environment); - } - Assert.state(propertySource instanceof DynamicValuesPropertySource, - "Incorrect DynamicValuesPropertySource type registered"); - return (DynamicValuesPropertySource) propertySource; - } - } diff --git a/spring-test/src/main/java/org/springframework/test/context/support/DynamicValuesPropertySource.java b/spring-test/src/main/java/org/springframework/test/context/support/DynamicValuesPropertySource.java index 8205a6c5568..8d60699e04b 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/DynamicValuesPropertySource.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/DynamicValuesPropertySource.java @@ -17,14 +17,11 @@ package org.springframework.test.context.support; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.Map; import java.util.function.Supplier; import org.springframework.core.env.MapPropertySource; import org.springframework.lang.Nullable; -import org.springframework.test.context.DynamicPropertyRegistry; -import org.springframework.util.Assert; import org.springframework.util.function.SupplierUtils; /** @@ -39,20 +36,9 @@ class DynamicValuesPropertySource extends MapPropertySource { static final String PROPERTY_SOURCE_NAME = "Dynamic Test Properties"; - final DynamicPropertyRegistry dynamicPropertyRegistry; - - - DynamicValuesPropertySource() { - this(Collections.synchronizedMap(new LinkedHashMap<>())); - } DynamicValuesPropertySource(Map> valueSuppliers) { super(PROPERTY_SOURCE_NAME, Collections.unmodifiableMap(valueSuppliers)); - this.dynamicPropertyRegistry = (name, valueSupplier) -> { - Assert.hasText(name, "'name' must not be null or blank"); - Assert.notNull(valueSupplier, "'valueSupplier' must not be null"); - valueSuppliers.put(name, valueSupplier); - }; } @Override diff --git a/spring-test/src/test/java/org/springframework/test/context/DynamicPropertiesTestSuite.java b/spring-test/src/test/java/org/springframework/test/context/DynamicPropertiesTestSuite.java new file mode 100644 index 00000000000..e2842bd53e5 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/DynamicPropertiesTestSuite.java @@ -0,0 +1,55 @@ +/* + * Copyright 2002-2024 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.test.context; + +import org.junit.platform.suite.api.SelectClasses; +import org.junit.platform.suite.api.Suite; + +/** + * JUnit Platform based test suite for tests that involve the Spring TestContext + * Framework and dynamic properties. + * + *

This suite is only intended to be used manually within an IDE. + * + *

Logging Configuration

+ * + *

In order for our log4j2 configuration to be used in an IDE, you must + * set the following system property before running any tests — for + * example, in Run Configurations in Eclipse. + * + * 

+ * -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager
+ *
+ * + * @author Sam Brannen + * @since 6.2 + */ +@Suite +@SelectClasses( + value = { + DynamicPropertyRegistryIntegrationTests.class, + DynamicPropertySourceIntegrationTests.class + }, + names = { + "org.springframework.test.context.junit.jupiter.nested.DynamicPropertySourceNestedTests", + "org.springframework.test.context.support.DefaultTestPropertySourcesIntegrationTests", + "org.springframework.test.context.support.DynamicPropertiesContextCustomizerFactoryTests", + "org.springframework.test.context.support.DynamicValuesPropertySourceTests" + } +) +class DynamicPropertiesTestSuite { +} diff --git a/spring-test/src/test/java/org/springframework/test/context/support/DefaultTestPropertySourcesIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/support/DefaultTestPropertySourcesIntegrationTests.java new file mode 100644 index 00000000000..ccda9d35577 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/support/DefaultTestPropertySourcesIntegrationTests.java @@ -0,0 +1,63 @@ +/* + * Copyright 2002-2024 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.test.context.support; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests which ensure that test-related property sources are not + * registered by default. + * + * @author Sam Brannen + * @since 6.2 + */ +@SpringJUnitConfig +@DirtiesContext +class DefaultTestPropertySourcesIntegrationTests { + + @Autowired + ConfigurableEnvironment env; + + + @Test + void ensureTestRelatedPropertySourcesAreNotRegisteredByDefault() { + assertPropertySourceIsNotRegistered(TestPropertySourceUtils.INLINED_PROPERTIES_PROPERTY_SOURCE_NAME); + assertPropertySourceIsNotRegistered(DynamicValuesPropertySource.PROPERTY_SOURCE_NAME); + } + + private void assertPropertySourceIsNotRegistered(String name) { + MutablePropertySources propertySources = this.env.getPropertySources(); + assertThat(propertySources.contains(name)) + .as("PropertySource \"%s\" should not be registered by default", name) + .isFalse(); + } + + + @Configuration + static class Config { + } + +}