From bda34f5c7d2087c3a6d29dd27b71c1608c7d00a4 Mon Sep 17 00:00:00 2001 From: Lucian Holland Date: Fri, 6 Nov 2020 16:10:18 +0100 Subject: [PATCH] Add SpringBootConfigStorage to allow mutable properties in Spring Boot apps QUES-36 Also move Spring boot tools into their own module to avoid pulling in unnecessary dependencies to non-spring-boot apps. --- pom.xml | 11 ++ spring-boot-infrastructure/pom.xml | 46 ++++++++ ...urationPropertiesScopingPostProcessor.java | 58 ++++++++++ .../config/SpringBootConfigStorage.java | 102 ++++++++++++++++++ .../pkb/common/testing/IntegrationTest.java | 2 +- spring-infrastructure/pom.xml | 13 +-- .../common/config/SpringConfigStorage.java | 26 ----- 7 files changed, 219 insertions(+), 39 deletions(-) create mode 100644 spring-boot-infrastructure/pom.xml create mode 100644 spring-boot-infrastructure/src/main/java/com/pkb/common/config/ConfigurationPropertiesScopingPostProcessor.java create mode 100644 spring-boot-infrastructure/src/main/java/com/pkb/common/config/SpringBootConfigStorage.java rename {spring-infrastructure => spring-boot-infrastructure}/src/main/java/com/pkb/common/testing/IntegrationTest.java (86%) delete mode 100644 spring-infrastructure/src/main/java/com/pkb/common/config/SpringConfigStorage.java diff --git a/pom.xml b/pom.xml index 6fe57f0a..645f6ca5 100644 --- a/pom.xml +++ b/pom.xml @@ -52,6 +52,9 @@ 12-a115018-86851 3.0.7-PKB + 5.2.3.RELEASE + 20-789c720-63866 + Hoxton.SR8 5.3.15.Final @@ -64,6 +67,7 @@ spring-infrastructure testlogging testsupport + spring-boot-infrastructure @@ -210,6 +214,13 @@ + + org.springframework.cloud + spring-cloud-dependencies + ${spring-cloud.version} + pom + import + org.hibernate hibernate-core diff --git a/spring-boot-infrastructure/pom.xml b/spring-boot-infrastructure/pom.xml new file mode 100644 index 00000000..bf8a7ee8 --- /dev/null +++ b/spring-boot-infrastructure/pom.xml @@ -0,0 +1,46 @@ + + + + pkb-common + com.pkb.pkbcommon + ${revision}${changelist} + + 4.0.0 + + spring-boot-infrastructure + + + + org.springframework + spring-test + ${spring.version} + compile + + + org.springframework + spring-beans + ${spring.version} + + + org.springframework.cloud + spring-cloud-context + + + org.junit.jupiter + junit-jupiter-api + compile + + + com.pkb.pkbcommon + config + + + org.springframework + spring-context + ${spring.version} + + + + \ No newline at end of file diff --git a/spring-boot-infrastructure/src/main/java/com/pkb/common/config/ConfigurationPropertiesScopingPostProcessor.java b/spring-boot-infrastructure/src/main/java/com/pkb/common/config/ConfigurationPropertiesScopingPostProcessor.java new file mode 100644 index 00000000..46a8c1af --- /dev/null +++ b/spring-boot-infrastructure/src/main/java/com/pkb/common/config/ConfigurationPropertiesScopingPostProcessor.java @@ -0,0 +1,58 @@ +package com.pkb.common.config; + +import org.springframework.aop.scope.ScopedProxyUtils; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.BeanDefinitionHolder; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.config.Scope; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; +import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.context.annotation.ScopedProxyMode; +import org.springframework.core.annotation.AnnotationAttributes; +import org.springframework.core.type.AnnotationMetadata; + +import javax.annotation.ParametersAreNonnullByDefault; +import java.util.Optional; + +/** + * This post-processor works around a bug/limitation of Spring Boot/Spring Cloud whereby + * beans discovered by the @EnableConfigurationProperties or @ConfigurationPropertiesScan annotations + * aren't ever put into custom scopes like the RefreshScope. This post-processor mimics what the + * standard Spring bean component scanning does by reading the scope attributes and then creating + * a scoped proxy if the requested scope requires it. It also ensure that the bean definition + * has the appropriate scope marked on it. + * + * To use this, define a static method on Configuration class that returns an instance of this as a Bean. + * You should probably give it an @Order attribute as well to ensure that this executes as early as possible + * so that other standard scope processing steps can operate on the corrected bean definition. + */ +@ParametersAreNonnullByDefault +public class ConfigurationPropertiesScopingPostProcessor implements BeanDefinitionRegistryPostProcessor { + + @Override + public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { + for (String name : registry.getBeanDefinitionNames()) { + BeanDefinition beanDefinition = registry.getBeanDefinition(name); + if (beanDefinition.getClass().getName().equals("org.springframework.boot.context.properties.ConfigurationPropertiesValueObjectBeanDefinition")) { + GenericBeanDefinition gbd = (GenericBeanDefinition) beanDefinition; + Optional annAttrs = Optional.ofNullable(AnnotationAttributes.fromMap(AnnotationMetadata.introspect(gbd.getBeanClass()).getAnnotationAttributes(Scope.class.getName(), false))); + ScopedProxyMode scopedProxyMode = annAttrs.map(attrs -> attrs.getEnum("proxyMode")).orElse(ScopedProxyMode.NO); + gbd.setScope(annAttrs.map(attrs -> attrs.getString("value")).orElse(gbd.getScope())); + if (scopedProxyMode == ScopedProxyMode.NO) { + continue; + } + boolean proxyTargetClass = scopedProxyMode == ScopedProxyMode.TARGET_CLASS; + registry.removeBeanDefinition(name); + BeanDefinitionHolder scopedProxy = ScopedProxyUtils.createScopedProxy(new BeanDefinitionHolder(gbd, name), registry, proxyTargetClass); + registry.registerBeanDefinition(scopedProxy.getBeanName(), scopedProxy.getBeanDefinition()); + } + } + } + + @Override + public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { + //No-op + } +} diff --git a/spring-boot-infrastructure/src/main/java/com/pkb/common/config/SpringBootConfigStorage.java b/spring-boot-infrastructure/src/main/java/com/pkb/common/config/SpringBootConfigStorage.java new file mode 100644 index 00000000..37a650c6 --- /dev/null +++ b/spring-boot-infrastructure/src/main/java/com/pkb/common/config/SpringBootConfigStorage.java @@ -0,0 +1,102 @@ +package com.pkb.common.config; + +import org.springframework.cloud.context.scope.refresh.RefreshScope; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; + +import javax.annotation.ParametersAreNonnullByDefault; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * This class provides an implementation of the PKB ConfigStorage interface that interacts + * helpfully with the Spring environment. This class can be passed to both the TestSupportAgent + * and any PKB config objects that require a ConfigStorage. It will read properties from the + * Spring environment, allowing Spring Boot applications to be be configured using all the standard + * Spring properties mechanisms such as application.properties and the OS environment - so you don't + * need to provide an additional config.properties file just to control shared PKB components that + * use PKB Common configuration classes + * + * Additionally, this class maintains an override map that is loaded into the Spring environment + * as a high-priority property source. If mutations to properties are requested (e.g. from the + * TestSupportAgent) these overrides will be visible in the spring environment. + * + * In a typical Spring Boot application, properties are not accessed directly from the environment but + * are bound to type-safe classes annotated with @ConfigurationProperties. Since this binding happens when + * the Spring context is loading, in order for later changes to the Spring environment to be visible + * to these classes and their clients, it is necessary to do a selective refresh of the spring context. + * ConfigurationProperties beans that contain mutable properties that may need to be updated at runtime + * should be annotated with @RefreshScope. Additionally, you should include the {@link ConfigurationPropertiesScopingPostProcessor} + * bean in your context to ensure that all ConfigurationProperties classes get included in the RefreshScope, + * irrespective of how they are constructed. Once you do this, this class will ensure that configuration + * beans are dynamically re-bound at runtime when properties change. + * + * It is recommended to use @ConstructorBinding for ConfigurationProperties classes; this can be used with + * kotlin data classes, java value objects or at least a POJO with final fields and no setters. This ensures + * that properties present an immutable interface to clients and can only be changed by the framework. + * + * + * NB This class and the above-described mechanism only ensure that the relevant properties classes will + * return the updated values on the next method call to one of the property getters. As yet there is no + * mechanism for notifying clients that an update has occured (although in principle this would not be difficult) + * nor does it make any attempt to ensure that any dependencies of these properties beans - including spring + * infrastructure that may have been wired together according to their values - are dynamically updated. + */ +@ParametersAreNonnullByDefault +public class SpringBootConfigStorage extends AbstractBaseConfigStorage implements ImmutableConfigStorage { + private final ConfigurableEnvironment environment; + private final RefreshScope refreshScope; + private final Map overrides = new LinkedHashMap<>(); + + public SpringBootConfigStorage(ConfigurableEnvironment environment, RefreshScope refreshScope) { + this.environment = environment; + this.refreshScope = refreshScope; + environment.getPropertySources().addFirst(new MapPropertySource("TestSupportOverrides", overrides)); + } + + + @Override + public String getString(String key) { + return environment.getProperty(key); + } + + @Override + public String getString(String key, String defaultValue) { + return environment.getProperty(key, defaultValue); + } + + @Override + public boolean isMutableConfigEnabled() { + return getBoolean("mutableConfig.enabled", false); + } + + @Override + public void setValue(String key, String value) { + if (isMutableConfigEnabled()) { + overrides.put(key, value); + refreshScope.refreshAll(); + } + } + + @Override + public OverrideRemovalResult removeOverrideAtKey(String key) { + if (!isMutableConfigEnabled()) { + return OverrideRemovalResult.NO_OP_AS_CONFIG_IS_IMMUTABLE; + } + if (overrides.remove(key) == null) { + return OverrideRemovalResult.KEY_NOT_FOUND; + } + refreshScope.refreshAll(); + return OverrideRemovalResult.REMOVED; + } + + @Override + public void reset() { + if (isMutableConfigEnabled()) { + overrides.clear(); + refreshScope.refreshAll(); + } + } + +} + diff --git a/spring-infrastructure/src/main/java/com/pkb/common/testing/IntegrationTest.java b/spring-boot-infrastructure/src/main/java/com/pkb/common/testing/IntegrationTest.java similarity index 86% rename from spring-infrastructure/src/main/java/com/pkb/common/testing/IntegrationTest.java rename to spring-boot-infrastructure/src/main/java/com/pkb/common/testing/IntegrationTest.java index 5d33ddae..642a314e 100644 --- a/spring-infrastructure/src/main/java/com/pkb/common/testing/IntegrationTest.java +++ b/spring-boot-infrastructure/src/main/java/com/pkb/common/testing/IntegrationTest.java @@ -5,7 +5,7 @@ import java.lang.annotation.*; -@Target({ElementType.TYPE, ElementType.METHOD}) +@Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) @Inherited @ActiveProfiles("integration-test") diff --git a/spring-infrastructure/pom.xml b/spring-infrastructure/pom.xml index 0014f30c..76c2deb3 100644 --- a/spring-infrastructure/pom.xml +++ b/spring-infrastructure/pom.xml @@ -13,8 +13,7 @@ pkb-common/spring-infrastructure - 5.2.3.RELEASE - 20-789c720-63866 + @@ -27,12 +26,6 @@ spring-context ${spring.version} - - org.springframework - spring-test - ${spring.version} - compile - com.pkb.pkbcommon infrastructure @@ -53,10 +46,6 @@ junit-jupiter-api compile - - org.junit.jupiter - junit-jupiter-params - com.pkb rest-assured diff --git a/spring-infrastructure/src/main/java/com/pkb/common/config/SpringConfigStorage.java b/spring-infrastructure/src/main/java/com/pkb/common/config/SpringConfigStorage.java deleted file mode 100644 index 84f82dd1..00000000 --- a/spring-infrastructure/src/main/java/com/pkb/common/config/SpringConfigStorage.java +++ /dev/null @@ -1,26 +0,0 @@ -package com.pkb.common.config; - -import org.jetbrains.annotations.NotNull; -import org.springframework.core.env.Environment; - -public class SpringConfigStorage extends AbstractBaseConfigStorage implements ImmutableConfigStorage { - - - private final Environment environment; - - public SpringConfigStorage(@NotNull Environment environment) { - this.environment = environment; - } - - @Override - public String getString(String key) { - return environment.getProperty(key); - } - - @Override - public String getString(String key, String defaultValue) { - return environment.getProperty(key, defaultValue); - } - - -}