From 11a19ca90528709c7508ac7d265be2f2efeaeb7a Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Thu, 14 Sep 2023 15:50:46 +0200 Subject: [PATCH 01/16] Enable structured types as config values This commit enables structured types as config values. That is, something like ``` @BaiganConfig public interface SomeComplexConfiguration { SomeConfigObject someConfig(); } ``` can now be deserialized, e.g. from a config file with content ``` [{ "alias": "some.complex.configuration.some.config", "defaultValue": {"config_key":"a value"}}] ``` How it works is that the repositories have to be constructed with a class `BaiganConfigClasses`, a container for all interfaces annotated with `@BaiganConfig`. The repository then deserializes first to `JsonNode`, then deserializes this to the correct class for each config key. The `BaiganConfigClasses` is provided as a bean. It is constructed by the `ContextAwareConfigurationMethodInvocationHandler` when creating the proxies for the `BaiganConfig` interfaces. Beyond this bean, users of the library do not have to add any extra configuration to enable structured config types. They only have to make sure that the `ObjectMapper` can deserialize the classes correctly. To this end, the `ObjectMapper` can now be injected into the repositories. The change is backwards compatible with respect to config files. All old config files can be processes as before. This is covered in an End2End test. It is not generally backwards compatible regarding the code as deprecated constructors for the `S3ConfigurationRepository` were removed and all constructors for repositories now require `BaiganConfigClasses` to be passed. --- .../baigan/proxy/BaiganConfigClasses.java | 24 +++++ .../ConfigurationBeanDefinitionRegistrar.java | 48 +++++++--- .../org/zalando/baigan/proxy/ProxyUtils.java | 12 ++- ...eConfigurationMethodInvocationHandler.java | 13 ++- .../AbstractConfigurationRepository.java | 53 ++++++++++- .../service/EtcdConfigurationRepository.java | 12 ++- .../FileSystemConfigurationRepository.java | 13 ++- .../aws/S3ConfigurationRepository.java | 70 ++------------- .../aws/S3ConfigurationRepositoryBuilder.java | 41 ++++++++- .../github/GitConfigurationRepository.java | 8 +- ...figurationBeanDefinitionRegistrarTest.java | 35 +++++--- .../baigan/context/SpringTestContext.java | 48 ---------- .../org/zalando/baigan/e2e/End2EndIT.java | 34 -------- .../org/zalando/baigan/e2e/TestContext.java | 22 +---- .../simples3repo/SimpleS3RepoEnd2EndIT.java | 87 +++++++++++++++++++ .../simples3repo/SomePlainConfiguration.java | 9 ++ .../SomeComplexConfiguration.java | 9 ++ .../structureds3repo/SomeConfigObject.java | 36 ++++++++ .../SomeOtherConfigObject.java | 36 ++++++++ .../StructuredS3RepoEnd2EndIT.java | 80 +++++++++++++++++ .../ConfigurationServiceBeanFactoryIT.java | 21 +++-- ...FileSystemConfigurationRepositoryTest.java | 10 ++- 22 files changed, 496 insertions(+), 225 deletions(-) create mode 100644 src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java delete mode 100644 src/test/java/org/zalando/baigan/context/SpringTestContext.java delete mode 100644 src/test/java/org/zalando/baigan/e2e/End2EndIT.java create mode 100644 src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java create mode 100644 src/test/java/org/zalando/baigan/e2e/simples3repo/SomePlainConfiguration.java create mode 100644 src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java create mode 100644 src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeConfigObject.java create mode 100644 src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java create mode 100644 src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java diff --git a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java new file mode 100644 index 0000000..422d616 --- /dev/null +++ b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java @@ -0,0 +1,24 @@ +package org.zalando.baigan.proxy; + +import org.springframework.stereotype.Component; + +import java.util.Map; + +@Component +public class BaiganConfigClasses { + private Map> configTypesByKey; + + public BaiganConfigClasses() {} + + public BaiganConfigClasses(Map> configTypesByKey) { + this.configTypesByKey = configTypesByKey; + } + + public void setConfigTypesByKey(Map> configTypesByKey) { + this.configTypesByKey = configTypesByKey; + } + + public Map> getConfigTypesByKey() { + return configTypesByKey; + } +} diff --git a/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java b/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java index bf05155..e1d579c 100644 --- a/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java +++ b/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java @@ -31,21 +31,25 @@ import org.zalando.baigan.annotation.BaiganConfig; import org.zalando.baigan.annotation.ConfigurationServiceScan; +import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import static java.util.stream.Collectors.toMap; +import static org.zalando.baigan.proxy.ProxyUtils.createKey; + /** * ImportBeanDefinitionRegistrar implementation that finds the * {@link ConfigurationServiceScan} annotations, delegates the scanning of * packages and proxy bean creation further down to the corresponding * implementations. * - * @see ConfigurationServiceBeanFactory - * * @author mchand - * + * @see ConfigurationServiceBeanFactory */ public class ConfigurationBeanDefinitionRegistrar implements ImportBeanDefinitionRegistrar { @@ -83,30 +87,41 @@ private void createAndRegisterBeanDefinitions(final Set packages, final ClassPathScanningCandidateComponentProvider scanner = new ClassPathScanningCandidateInterfaceProvider(); scanner.addIncludeFilter(new AnnotationTypeFilter(BaiganConfig.class)); + List> baiganConfigClasses = new ArrayList<>(); + for (final String singlePackage : packages) { final Set candidates = scanner.findCandidateComponents(singlePackage); for (BeanDefinition definition : candidates) { if (definition instanceof GenericBeanDefinition) { final GenericBeanDefinition genericDefinition = (GenericBeanDefinition) definition; - registerAsBean(registry, genericDefinition); + final Class baiganConfigClass = registerAsBean(registry, genericDefinition); + baiganConfigClasses.add(baiganConfigClass); } else { throw new IllegalStateException( - String.format( - "Unable to read required metadata of configuration candidate [%s]", - definition - ) + String.format( + "Unable to read required metadata of configuration candidate [%s]", + definition + ) ); } } } + Map> configTypesByKey = baiganConfigClasses.stream().flatMap(clazz -> + Arrays.stream(clazz.getMethods()).map(method -> new ConfigType(createKey(clazz, method), method.getReturnType())) + ).collect(toMap(c -> c.key, c -> c.type)); + GenericBeanDefinition beanDefinition = new GenericBeanDefinition(); + beanDefinition.setBeanClass(BaiganConfigClasses.class); + beanDefinition.getPropertyValues().add("configTypesByKey", configTypesByKey); + registry.registerBeanDefinition("baiganConfigClasses", beanDefinition); } - private void registerAsBean(final BeanDefinitionRegistry registry, final GenericBeanDefinition genericDefinition) { + private Class registerAsBean(final BeanDefinitionRegistry registry, final GenericBeanDefinition genericDefinition) { try { final Class interfaceToImplement = genericDefinition.resolveBeanClass( - registry.getClass().getClassLoader() + registry.getClass().getClassLoader() ); registerAsBean(registry, interfaceToImplement); + return interfaceToImplement; } catch (final ClassNotFoundException e) { throw new IllegalStateException("Unable to register annotated interface as configuration bean", e); } @@ -114,7 +129,7 @@ private void registerAsBean(final BeanDefinitionRegistry registry, final Generic private void registerAsBean(final BeanDefinitionRegistry registry, final Class interfaceToImplement) { final BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition( - ConfigurationServiceBeanFactory.class + ConfigurationServiceBeanFactory.class ); builder.addPropertyValue("candidateInterface", interfaceToImplement); @@ -123,7 +138,7 @@ private void registerAsBean(final BeanDefinitionRegistry registry, final Class type; + + public ConfigType(String key, Class type) { + this.key = key; + this.type = type; + } + } } diff --git a/src/main/java/org/zalando/baigan/proxy/ProxyUtils.java b/src/main/java/org/zalando/baigan/proxy/ProxyUtils.java index 197c2ee..5743e93 100644 --- a/src/main/java/org/zalando/baigan/proxy/ProxyUtils.java +++ b/src/main/java/org/zalando/baigan/proxy/ProxyUtils.java @@ -19,6 +19,8 @@ import com.google.common.base.CaseFormat; import com.google.common.base.Strings; +import java.lang.reflect.Method; + /** * The class to contain utility methods used in proxying configuration beans. * @@ -28,7 +30,15 @@ public class ProxyUtils { private static final String NAMESPACE_SEPARATOR = "."; - public static String dottify(final String text) { + public static String createKey(final Class clazz, Method method) { + final String methodName = method.getName(); + final String nameSpace = clazz.getSimpleName(); + + return ProxyUtils.dottify(nameSpace) + "." + + ProxyUtils.dottify(methodName); + } + + private static String dottify(final String text) { if (Strings.isNullOrEmpty(text)) { return NAMESPACE_SEPARATOR; diff --git a/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java b/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java index 2f6c671..68ab853 100644 --- a/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java +++ b/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java @@ -26,6 +26,7 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Suppliers.memoize; +import static org.zalando.baigan.proxy.ProxyUtils.createKey; /** * This class provides a concrete implementation for the Method invocation @@ -60,12 +61,8 @@ public void setBeanFactory(final BeanFactory beanFactory) throws BeansException @Override protected Object handleInvocation(Object proxy, Method method, - Object[] args) throws Throwable { - final String methodName = method.getName(); - final String nameSpace = getNamespace(proxy); - - final String key = ProxyUtils.dottify(nameSpace) + "." - + ProxyUtils.dottify(methodName); + Object[] args) throws Throwable { + final String key = createKey(getClass(proxy), method); final Object result = getConfig(key); if (result == null) { LOG.warn("Configuration not found for key: {}", key); @@ -107,10 +104,10 @@ protected Object handleInvocation(Object proxy, Method method, return null; } - private String getNamespace(final Object proxy) { + private Class getClass(final Object proxy) { final Class[] interfaces = proxy.getClass().getInterfaces(); checkState(interfaces.length == 1, "Expected exactly one interface on proxy object."); - return interfaces[0].getSimpleName(); + return interfaces[0]; } private Object getConfig(final String key) { diff --git a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java index 0941194..b0623b2 100644 --- a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java @@ -1,26 +1,71 @@ package org.zalando.baigan.service; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.guava.GuavaModule; +import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; import java.io.IOException; import java.io.UncheckedIOException; import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; public abstract class AbstractConfigurationRepository implements ConfigurationRepository { - ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); + final ObjectMapper objectMapper; + final BaiganConfigClasses baiganConfigClasses; + + protected AbstractConfigurationRepository(final BaiganConfigClasses baiganConfigClasses, final ObjectMapper objectMapper) { + this.baiganConfigClasses = requireNonNull(baiganConfigClasses, "baiganConfigClasses has to be not null. Get them from the bean of the same name."); + this.objectMapper = requireNonNull(objectMapper, "objectMapper has to be not null."); + } @Nonnull - protected List getConfigurations(final String text) { + protected List> getConfigurations(final String text) { try { - return objectMapper.readValue(text, new TypeReference>() { + List> rawConfigs = objectMapper.readValue(text, new TypeReference<>() { }); - } catch (IOException e) { + return rawConfigs.stream().map(config -> deserializeConfig(config, findClass(config.getAlias()))).collect(toList()); + } catch ( + IOException e) { throw new UncheckedIOException("Unable to deserialize the Configuration.", e); } } + + private Configuration deserializeConfig(Configuration config, Class targetClass) { + Set> typedConditions = Optional.ofNullable(config.getConditions()).orElse(Set.of()).stream().map(c -> { + try { + return new Condition<>(c.getParamName(), c.getConditionType(), objectMapper.treeToValue(c.getValue(), targetClass)); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + }).collect(toSet()); + try { + T typedDefaultValue = objectMapper.treeToValue(config.getDefaultValue(), targetClass); + return new Configuration<>(config.getAlias(), config.getDescription(), typedConditions, typedDefaultValue); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + private Class findClass(String alias) { + List> matchingClasses = baiganConfigClasses.getConfigTypesByKey().entrySet().stream().filter(entry -> alias.startsWith(entry.getKey())).map(Map.Entry::getValue).collect(toList()); + + if (matchingClasses.size() == 1) { + return matchingClasses.get(0); + } else { + throw new RuntimeException("Did not find exactly one matching BaiganConfig for alias " + alias + " in " + baiganConfigClasses.getConfigTypesByKey() + ": matching classes " + matchingClasses); + } + } } diff --git a/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java index a5f7c9d..f44a80e 100644 --- a/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java @@ -1,11 +1,14 @@ package org.zalando.baigan.service; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.zalando.baigan.etcd.service.EtcdClient; import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; import java.io.IOException; @@ -16,22 +19,25 @@ /** * @author mchand */ - +// TODO E2E test public class EtcdConfigurationRepository extends AbstractConfigurationRepository { + private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final String ETCD_URL_ENV_NAME = "ETCD_URL"; private static final String CONFIG_PATH_PREFIX = "/v2/keys/"; private Logger LOG = LoggerFactory.getLogger(EtcdConfigurationRepository.class); private EtcdClient etcdClient; @VisibleForTesting - public EtcdConfigurationRepository(final EtcdClient etcdClient) { + public EtcdConfigurationRepository(final EtcdClient etcdClient, final BaiganConfigClasses baiganConfigClasses) { + super(baiganConfigClasses, objectMapper); checkArgument(etcdClient != null); this.etcdClient = etcdClient; } - public EtcdConfigurationRepository() { + public EtcdConfigurationRepository(final BaiganConfigClasses baiganConfigClasses) { + super(baiganConfigClasses, objectMapper); etcdClient = new EtcdClient(getUrl()); } diff --git a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java index dd90d8d..a92cd9d 100644 --- a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java @@ -1,5 +1,8 @@ package org.zalando.baigan.service; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.guava.GuavaModule; +import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -8,6 +11,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; import java.io.IOException; @@ -27,15 +31,18 @@ * * @author mchand */ +// TODO E2E test public class FileSystemConfigurationRepository extends AbstractConfigurationRepository { + private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final Logger LOG = LoggerFactory.getLogger(FileSystemConfigurationRepository.class); private final LoadingCache> cachedConfigurations; private final String fileName; - - public FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds) { + @VisibleForTesting + FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds, final BaiganConfigClasses baiganConfigClasses) { + super(baiganConfigClasses, objectMapper); this.fileName = fileName; cachedConfigurations = CacheBuilder.newBuilder() @@ -80,7 +87,7 @@ public void put(@Nonnull String key, @Nonnull String value) { protected Map loadConfigurations(String filename) { final String configurationText = loadResource(filename); - final Collection configurations = getConfigurations( + final Collection> configurations = getConfigurations( configurationText); final ImmutableMap.Builder builder = ImmutableMap.builder(); diff --git a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java index 70940a6..ca05bdf 100644 --- a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java @@ -1,15 +1,14 @@ package org.zalando.baigan.service.aws; import com.amazonaws.services.kms.AWSKMS; -import com.amazonaws.services.kms.AWSKMSClientBuilder; import com.amazonaws.services.s3.AmazonS3; -import com.amazonaws.services.s3.AmazonS3ClientBuilder; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.proxy.BaiganConfigClasses; import org.zalando.baigan.service.AbstractConfigurationRepository; -import org.zalando.baigan.service.ConfigurationRepository; import javax.annotation.Nonnull; import java.util.List; @@ -25,67 +24,16 @@ public class S3ConfigurationRepository extends AbstractConfigurationRepository { private static final Logger LOG = LoggerFactory.getLogger(S3ConfigurationRepository.class); - /** - * The default refresh interval which is 60 seconds - */ - private static final long DEFAULT_REFRESH_INTERVAL = 60; - private final S3FileLoader s3Loader; private final long refreshInterval; private final ScheduledThreadPoolExecutor executor; - private volatile Map configurationsMap = ImmutableMap.of(); - - /** - * @param bucketName The name of the bucket - * @param key The object key, usually, the "full path" to the JSON file stored in the bucket - * - * @deprecated Use {@link S3ConfigurationRepositoryBuilder instead} - *

- * Provides a {@link ConfigurationRepository} that reads configurations from a JSON file stored in a S3 bucket. - * It refreshes configurations using the default refresh interval (60 seconds) - */ - @Deprecated - public S3ConfigurationRepository(@Nonnull final String bucketName, @Nonnull final String key) { - this(bucketName, key, DEFAULT_REFRESH_INTERVAL); - } - - /** - * @param bucketName The name of the bucket - * @param key The object key, usually, the "full path" to the JSON file stored in the bucket - * @param refreshInterval The interval, in seconds, to refresh the configurations. A value of 0 disables refreshing - *

- * @see #S3ConfigurationRepository(String, String) - * - * @deprecated Use {@link S3ConfigurationRepositoryBuilder instead} - *

- * Provides a {@link ConfigurationRepository} that reads configurations from a JSON file stored in a S3 bucket. - */ - @Deprecated - public S3ConfigurationRepository(@Nonnull final String bucketName, @Nonnull final String key, final long refreshInterval) { - this(bucketName, key, refreshInterval, new ScheduledThreadPoolExecutor(1)); - } - - /** - * @param bucketName The name of the bucket - * @param key The object key, usually, the "full path" to the JSON file stored in the bucket - * @param refreshInterval The interval, in seconds, to refresh the configurations. A value of 0 disables refreshing - * @param executor The executor to refresh the configurations in the specified interval - *

- * @see #S3ConfigurationRepository(String, String) - * - * @deprecated Use {@link S3ConfigurationRepositoryBuilder instead} - *

- * Provides a {@link ConfigurationRepository} that reads configurations from a JSON file stored in a S3 bucket. - */ - @Deprecated - public S3ConfigurationRepository(@Nonnull final String bucketName, @Nonnull final String key, - final long refreshInterval, final ScheduledThreadPoolExecutor executor) { - this(bucketName, key, refreshInterval, executor, AmazonS3ClientBuilder.defaultClient(), AWSKMSClientBuilder.defaultClient()); - } + private volatile Map> configurationsMap = ImmutableMap.of(); S3ConfigurationRepository(@Nonnull final String bucketName, @Nonnull final String key, final long refreshInterval, final ScheduledThreadPoolExecutor executor, - final AmazonS3 s3Client, final AWSKMS kmsClient) { + final AmazonS3 s3Client, final AWSKMS kmsClient, final BaiganConfigClasses baiganConfigClasses, + final ObjectMapper objectMapper) { + super(baiganConfigClasses, objectMapper); checkNotNull(bucketName, "bucketName is required"); checkNotNull(key, "key is required"); checkArgument(refreshInterval >= 0, "refreshInterval has to be >= 0"); @@ -105,9 +53,9 @@ public S3ConfigurationRepository(@Nonnull final String bucketName, @Nonnull fina protected void loadConfigurations() { final String configurationText = s3Loader.loadContent(); - final List configurations = getConfigurations(configurationText); - final ImmutableMap.Builder builder = ImmutableMap.builder(); - for (final Configuration configuration : configurations) { + final List> configurations = getConfigurations(configurationText); + final ImmutableMap.Builder> builder = ImmutableMap.builder(); + for (final Configuration configuration : configurations) { builder.put(configuration.getAlias(), configuration); } configurationsMap = builder.build(); diff --git a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java index 9f63d76..2419ffe 100644 --- a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java @@ -4,11 +4,24 @@ import com.amazonaws.services.kms.AWSKMSClientBuilder; import com.amazonaws.services.s3.AmazonS3; import com.amazonaws.services.s3.AmazonS3ClientBuilder; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.zalando.baigan.proxy.BaiganConfigClasses; +import javax.annotation.Nonnull; import java.util.concurrent.ScheduledThreadPoolExecutor; import static com.google.common.base.Preconditions.checkNotNull; +/** + * Builder class for an S3ConfigurationRepository. + *

+ * Must specify non-null values for + * - {@link S3ConfigurationRepositoryBuilder#bucketName} + * - {@link S3ConfigurationRepositoryBuilder#key} + * - {@link S3ConfigurationRepositoryBuilder#baiganConfigClasses} + *

+ * The latter is typically set as the Spring bean named "baiganConfigClasses" provided by the library. + */ public class S3ConfigurationRepositoryBuilder { private ScheduledThreadPoolExecutor executor; @@ -17,6 +30,8 @@ public class S3ConfigurationRepositoryBuilder { private long refreshIntervalInSeconds = 60; private String bucketName; private String key; + private BaiganConfigClasses baiganConfigClasses; + private ObjectMapper objectMapper; /** * @param s3Client The S3 client to be used to fetch the configuration file. @@ -41,7 +56,7 @@ public S3ConfigurationRepositoryBuilder kmsClient(final AWSKMS kmsClient) { /** * @param bucketName The name of the S3 bucket that holds the configuration file. */ - public S3ConfigurationRepositoryBuilder bucketName(final String bucketName) { + public S3ConfigurationRepositoryBuilder bucketName(@Nonnull final String bucketName) { this.bucketName = checkNotNull(bucketName, "bucketName must not be null"); return this; } @@ -49,7 +64,7 @@ public S3ConfigurationRepositoryBuilder bucketName(final String bucketName) { /** * @param key The S3 key pointing to the JSON configuration file in the specified bucket. */ - public S3ConfigurationRepositoryBuilder key(final String key) { + public S3ConfigurationRepositoryBuilder key(@Nonnull final String key) { this.key = checkNotNull(key, "key must not be null"); return this; } @@ -63,13 +78,28 @@ public S3ConfigurationRepositoryBuilder refreshIntervalInSeconds(final long refr } /** - * @param executor The {@link ScheduledThreadPoolExecutor} used to run the configuration refresh. + * @param executor The {@link ScheduledThreadPoolExecutor} used to run the configuration refresh. If this is not + * specified, a new {@link ScheduledThreadPoolExecutor} with a single thread is used. */ public S3ConfigurationRepositoryBuilder executor(ScheduledThreadPoolExecutor executor) { this.executor = executor; return this; } + /** + * @param baiganConfigClasses Contains the list of classes annotated with {@link org.zalando.baigan.annotation.BaiganConfig}. + * This is typically set as the Spring bean named "baiganConfigClasses" provided by the library. + */ + public S3ConfigurationRepositoryBuilder baiganConfigClasses(@Nonnull BaiganConfigClasses baiganConfigClasses) { + this.baiganConfigClasses = checkNotNull(baiganConfigClasses); + return this; + } + + public S3ConfigurationRepositoryBuilder objectMapper(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + return this; + } + public S3ConfigurationRepository build() { if (executor == null) { executor = new ScheduledThreadPoolExecutor(1); @@ -80,7 +110,10 @@ public S3ConfigurationRepository build() { if (kmsClient == null) { kmsClient = AWSKMSClientBuilder.defaultClient(); } + if (objectMapper == null) { + objectMapper = new ObjectMapper(); + } - return new S3ConfigurationRepository(bucketName, key, refreshIntervalInSeconds, executor, s3Client, kmsClient); + return new S3ConfigurationRepository(bucketName, key, refreshIntervalInSeconds, executor, s3Client, kmsClient, baiganConfigClasses, objectMapper); } } diff --git a/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java index 05a8578..171ef82 100644 --- a/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java @@ -1,10 +1,13 @@ package org.zalando.baigan.service.github; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.google.common.cache.CacheBuilder; import com.google.common.cache.LoadingCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.proxy.BaiganConfigClasses; import org.zalando.baigan.service.AbstractConfigurationRepository; import javax.annotation.Nonnull; @@ -21,14 +24,17 @@ * * @author mchand */ +// TODO remove or add E2E test @Deprecated public class GitConfigurationRepository extends AbstractConfigurationRepository { + private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final Logger LOG = LoggerFactory.getLogger(GitConfigurationRepository.class); private final LoadingCache> cachedConfigurations; private final GitConfig gitConfig; - public GitConfigurationRepository(long refreshIntervalInMinutes, GitConfig gitConfig) { + public GitConfigurationRepository(long refreshIntervalInMinutes, GitConfig gitConfig, BaiganConfigClasses baiganConfigClasses) { + super(baiganConfigClasses, objectMapper); this.gitConfig = gitConfig; cachedConfigurations = CacheBuilder.newBuilder() .refreshAfterWrite(refreshIntervalInMinutes, TimeUnit.MINUTES) diff --git a/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java b/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java index 0666a47..03e0c5b 100644 --- a/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java +++ b/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java @@ -1,12 +1,12 @@ /** * Copyright (C) 2015 Zalando SE (http://tech.zalando.com) - * + *

* 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 - * - * http://www.apache.org/licenses/LICENSE-2.0 - * + *

+ * http://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. @@ -28,14 +28,17 @@ import org.zalando.baigan.annotation.ConfigurationServiceScan; import org.zalando.baigan.proxy.ConfigurationBeanDefinitionRegistrar; -import static org.hamcrest.CoreMatchers.equalTo; +import java.util.Map; + import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; /** - * * @author mchand - * */ public class ConfigurationBeanDefinitionRegistrarTest { @@ -47,10 +50,10 @@ public void testRegistration() { final AnnotationMetadata metaData = Mockito .mock(AnnotationMetadata.class); Mockito.when(metaData.getAnnotationAttributes( - ConfigurationServiceScan.class.getName())) + ConfigurationServiceScan.class.getName())) .thenReturn(ImmutableMap.of("value", - new String[] { "org.zalando.baigan.context" }, - "basePackages", new String[] {})); + new String[]{"org.zalando.baigan.context"}, + "basePackages", new String[]{})); final BeanDefinitionRegistry registry = Mockito .mock(BeanDefinitionRegistry.class); @@ -61,13 +64,17 @@ public void testRegistration() { .forClass(String.class); registrar.registerBeanDefinitions(metaData, registry); - Mockito.verify(registry).registerBeanDefinition(beanName.capture(), + verify(registry, times(2)).registerBeanDefinition(beanName.capture(), beanDefinition.capture()); - assertThat(beanName.getValue(), equalTo( - "org.zalando.baigan.context.SuperSonicBaiganProxyConfigurationFactoryBean")); + assertThat(beanName.getAllValues(), contains( + "org.zalando.baigan.context.SuperSonicBaiganProxyConfigurationFactoryBean", + "baiganConfigClasses") + ); - assertThat(beanDefinition.getValue(), instanceOf(GenericBeanDefinition.class)); + assertThat(beanDefinition.getAllValues().get(0), instanceOf(GenericBeanDefinition.class)); + assertThat(beanDefinition.getAllValues().get(1).getPropertyValues().get("configTypesByKey"), + equalTo(Map.of("super.sonic.speed", String.class))); } diff --git a/src/test/java/org/zalando/baigan/context/SpringTestContext.java b/src/test/java/org/zalando/baigan/context/SpringTestContext.java deleted file mode 100644 index 0805846..0000000 --- a/src/test/java/org/zalando/baigan/context/SpringTestContext.java +++ /dev/null @@ -1,48 +0,0 @@ -package org.zalando.baigan.context; - -import com.google.common.collect.ImmutableSet; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.zalando.baigan.model.Condition; -import org.zalando.baigan.service.ConfigurationRepository; - -import javax.annotation.Nonnull; -import java.util.Optional; -import java.util.Set; - -/** - * @author mchand - */ -@Configuration -public class SpringTestContext { - - @Bean - public ConfigurationRepository configurationRepository() { - return new ConfigurationRepository() { - final static String KEY = "test.config.enable.xyz.feature"; - - public void put(@Nonnull String key, @Nonnull String value) { - } - - @Nonnull - public Optional get(@Nonnull String key) { - if (KEY.equalsIgnoreCase(key)) { - return Optional.of(mockConfiguration(key)); - } else { - return Optional.empty(); - } - - } - - private org.zalando.baigan.model.Configuration mockConfiguration( - final String key) { - - final Set> conditions = ImmutableSet.of(); - - return new org.zalando.baigan.model.Configuration<>( - key, "This is a test configuration object.", conditions, - Boolean.FALSE); - } - }; - } -} diff --git a/src/test/java/org/zalando/baigan/e2e/End2EndIT.java b/src/test/java/org/zalando/baigan/e2e/End2EndIT.java deleted file mode 100644 index 86f164f..0000000 --- a/src/test/java/org/zalando/baigan/e2e/End2EndIT.java +++ /dev/null @@ -1,34 +0,0 @@ -package org.zalando.baigan.e2e; - -import com.amazonaws.services.s3.AmazonS3; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit.jupiter.SpringExtension; -import org.zalando.baigan.fixture.SomeConfiguration; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.nullValue; -import static org.zalando.baigan.e2e.TestContext.S3_CONFIG_BUCKET; -import static org.zalando.baigan.e2e.TestContext.S3_CONFIG_KEY; - -@ExtendWith(SpringExtension.class) -@ContextConfiguration(classes = {TestContext.class}) -public class End2EndIT { - - @Autowired - private AmazonS3 s3; - - @Autowired - private SomeConfiguration someConfiguration; - - @Test - public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException { - assertThat(someConfiguration.someValue(), nullValue()); - s3.putObject(S3_CONFIG_BUCKET, S3_CONFIG_KEY, "[{ \"alias\": \"some.configuration.some.value\", \"defaultValue\": \"a value\"}]"); - Thread.sleep(1100); - assertThat(someConfiguration.someValue(), equalTo("a value")); - } -} diff --git a/src/test/java/org/zalando/baigan/e2e/TestContext.java b/src/test/java/org/zalando/baigan/e2e/TestContext.java index 61bafd1..a30b119 100644 --- a/src/test/java/org/zalando/baigan/e2e/TestContext.java +++ b/src/test/java/org/zalando/baigan/e2e/TestContext.java @@ -18,18 +18,13 @@ import org.testcontainers.junit.jupiter.Testcontainers; import org.testcontainers.utility.DockerImageName; import org.zalando.baigan.BaiganSpringContext; -import org.zalando.baigan.annotation.ConfigurationServiceScan; -import org.zalando.baigan.service.ConfigurationRepository; -import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; import static org.testcontainers.containers.localstack.LocalStackContainer.Service.KMS; import static org.testcontainers.containers.localstack.LocalStackContainer.Service.S3; -@Configuration -@ComponentScan(basePackageClasses = BaiganSpringContext.class) -@ConfigurationServiceScan(basePackages = "org.zalando.baigan.fixture") +@ComponentScan(basePackageClasses = {BaiganSpringContext.class}) @Testcontainers -class TestContext { +public class TestContext { public static final String S3_CONFIG_BUCKET = "some-bucket"; public static final String S3_CONFIG_KEY = "some-key"; @@ -39,19 +34,6 @@ class TestContext { DockerImageName.parse("localstack/localstack:2.1.0") ).withServices(S3, KMS).withEnv("DEFAULT_REGION", Regions.EU_CENTRAL_1.getName()); - @Bean - ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms) { - amazonS3.putObject(S3_CONFIG_BUCKET, S3_CONFIG_KEY, "[]"); - - return new S3ConfigurationRepositoryBuilder() - .bucketName(S3_CONFIG_BUCKET) - .key(S3_CONFIG_KEY) - .s3Client(amazonS3) - .kmsClient(kms) - .refreshIntervalInSeconds(1) - .build(); - } - @Bean AWSKMS kms() { localstack.start(); diff --git a/src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java new file mode 100644 index 0000000..cb3cc39 --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java @@ -0,0 +1,87 @@ +package org.zalando.baigan.e2e.simples3repo; + +import com.amazonaws.services.kms.AWSKMS; +import com.amazonaws.services.s3.AmazonS3; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.zalando.baigan.annotation.ConfigurationServiceScan; +import org.zalando.baigan.e2e.TestContext; +import org.zalando.baigan.proxy.BaiganConfigClasses; +import org.zalando.baigan.service.aws.S3ConfigurationRepository; +import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; + +import java.util.concurrent.ScheduledThreadPoolExecutor; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = {TestContext.class, SimpleS3RepoEnd2EndIT.RepoConfig.class}) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class SimpleS3RepoEnd2EndIT { + + @Autowired + private AmazonS3 s3; + + @Autowired + private SomePlainConfiguration someConfiguration; + + @Autowired + private ScheduledThreadPoolExecutor executor; + + @Test + public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException { + assertThat(someConfiguration.someValue(), nullValue()); + assertThat(someConfiguration.isThisTrue(), nullValue()); + s3.putObject(TestContext.S3_CONFIG_BUCKET, TestContext.S3_CONFIG_KEY, "[{ \"alias\": \"some.plain.configuration.some.value\", \"defaultValue\": \"a value\"}]"); + Thread.sleep(1100); + assertThat(someConfiguration.someValue(), equalTo("a value")); + assertThat(someConfiguration.isThisTrue(), nullValue()); + s3.putObject( + TestContext.S3_CONFIG_BUCKET, + TestContext.S3_CONFIG_KEY, + "[{ \"alias\": \"some.plain.configuration.is.this.true\", \"defaultValue\": true}, " + + "{ \"alias\": \"some.plain.configuration.some.value\", \"defaultValue\": \"a value\"}]" + ); + Thread.sleep(1100); + assertThat(someConfiguration.someValue(), equalTo("a value")); + assertThat(someConfiguration.isThisTrue(), equalTo(true)); + } + + @AfterAll + public void cleanup() { + executor.shutdownNow(); + } + + @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.simples3repo") + static class RepoConfig { + + @Bean(destroyMethod = "shutdownNow") + ScheduledThreadPoolExecutor baiganRefresherPoolExecutor(){ + return new ScheduledThreadPoolExecutor(1); + } + + @Bean + S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, BaiganConfigClasses baiganConfigClasses, ScheduledThreadPoolExecutor executor) { + amazonS3.putObject(TestContext.S3_CONFIG_BUCKET, TestContext.S3_CONFIG_KEY, "[]"); + + return new S3ConfigurationRepositoryBuilder() + .bucketName(TestContext.S3_CONFIG_BUCKET) + .key(TestContext.S3_CONFIG_KEY) + .s3Client(amazonS3) + .kmsClient(kms) + .refreshIntervalInSeconds(1) + .executor(executor) + .baiganConfigClasses(baiganConfigClasses) + .build(); + } + + } +} diff --git a/src/test/java/org/zalando/baigan/e2e/simples3repo/SomePlainConfiguration.java b/src/test/java/org/zalando/baigan/e2e/simples3repo/SomePlainConfiguration.java new file mode 100644 index 0000000..139e5ff --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/simples3repo/SomePlainConfiguration.java @@ -0,0 +1,9 @@ +package org.zalando.baigan.e2e.simples3repo; + +import org.zalando.baigan.annotation.BaiganConfig; + +@BaiganConfig +public interface SomePlainConfiguration { + String someValue(); + Boolean isThisTrue(); +} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java new file mode 100644 index 0000000..91adc16 --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java @@ -0,0 +1,9 @@ +package org.zalando.baigan.e2e.structureds3repo; + +import org.zalando.baigan.annotation.BaiganConfig; + +@BaiganConfig +public interface SomeComplexConfiguration { + SomeConfigObject someConfig(); + SomeOtherConfigObject someOtherConfig(); +} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeConfigObject.java b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeConfigObject.java new file mode 100644 index 0000000..78988dc --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeConfigObject.java @@ -0,0 +1,36 @@ +package org.zalando.baigan.e2e.structureds3repo; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +import java.util.Objects; +import java.util.StringJoiner; + +public class SomeConfigObject { + private final String configKey; + + @JsonCreator + public SomeConfigObject(@JsonProperty("config_key") String configKey) { + this.configKey = configKey; + } + + @Override + public String toString() { + return new StringJoiner(", ", SomeConfigObject.class.getSimpleName() + "[", "]") + .add("configKey='" + configKey + "'") + .toString(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SomeConfigObject config = (SomeConfigObject) o; + return Objects.equals(configKey, config.configKey); + } + + @Override + public int hashCode() { + return Objects.hash(configKey); + } +} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java new file mode 100644 index 0000000..2e8efcf --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java @@ -0,0 +1,36 @@ +package org.zalando.baigan.e2e.structureds3repo; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +import java.util.Objects; +import java.util.StringJoiner; + +public class SomeOtherConfigObject { + private final String anotherConfigKey; + + @JsonCreator + public SomeOtherConfigObject(@JsonProperty("other_config_key") String anotherConfigKey) { + this.anotherConfigKey = anotherConfigKey; + } + + @Override + public String toString() { + return new StringJoiner(", ", SomeOtherConfigObject.class.getSimpleName() + "[", "]") + .add("anotherConfigKey='" + anotherConfigKey + "'") + .toString(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SomeOtherConfigObject config = (SomeOtherConfigObject) o; + return Objects.equals(anotherConfigKey, config.anotherConfigKey); + } + + @Override + public int hashCode() { + return Objects.hash(anotherConfigKey); + } +} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java new file mode 100644 index 0000000..6ec4f5b --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java @@ -0,0 +1,80 @@ +package org.zalando.baigan.e2e.structureds3repo; + +import com.amazonaws.services.kms.AWSKMS; +import com.amazonaws.services.s3.AmazonS3; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.zalando.baigan.annotation.ConfigurationServiceScan; +import org.zalando.baigan.e2e.TestContext; +import org.zalando.baigan.proxy.BaiganConfigClasses; +import org.zalando.baigan.service.aws.S3ConfigurationRepository; +import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; + +import java.util.concurrent.ScheduledThreadPoolExecutor; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.zalando.baigan.e2e.TestContext.S3_CONFIG_BUCKET; +import static org.zalando.baigan.e2e.TestContext.S3_CONFIG_KEY; + +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = {TestContext.class, StructuredS3RepoEnd2EndIT.RepoConfig.class}) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class StructuredS3RepoEnd2EndIT { + + @Autowired + private AmazonS3 s3; + + @Autowired + private SomeComplexConfiguration someConfiguration; + + @Autowired + private ScheduledThreadPoolExecutor executor; + + @Test + public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException { + assertThat(someConfiguration.someConfig(), nullValue()); + s3.putObject( + S3_CONFIG_BUCKET, + S3_CONFIG_KEY, + "[{ \"alias\": \"some.complex.configuration.some.config\", \"defaultValue\": {\"config_key\":\"a value\"}}," + + "{ \"alias\": \"some.complex.configuration.some.other.config\", \"defaultValue\": {\"other_config_key\":\"other value\"}}]"); + Thread.sleep(1100); + assertThat(someConfiguration.someConfig(), equalTo(new SomeConfigObject("a value"))); + } + + @AfterAll + public void cleanup() { + executor.shutdownNow(); + } + + @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.structureds3repo") + static class RepoConfig { + + @Bean(destroyMethod = "shutdownNow") + ScheduledThreadPoolExecutor baiganRefresherPoolExecutor(){ + return new ScheduledThreadPoolExecutor(1); + } + + @Bean + S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, BaiganConfigClasses baiganConfigClasses, ScheduledThreadPoolExecutor executorService) { + amazonS3.putObject(TestContext.S3_CONFIG_BUCKET, TestContext.S3_CONFIG_KEY, "[]"); + return new S3ConfigurationRepositoryBuilder() + .bucketName(TestContext.S3_CONFIG_BUCKET) + .key(TestContext.S3_CONFIG_KEY) + .s3Client(amazonS3) + .kmsClient(kms) + .refreshIntervalInSeconds(1) + .executor(executorService) + .baiganConfigClasses(baiganConfigClasses) + .build(); + } + } +} diff --git a/src/test/java/org/zalando/baigan/proxy/ConfigurationServiceBeanFactoryIT.java b/src/test/java/org/zalando/baigan/proxy/ConfigurationServiceBeanFactoryIT.java index bdce511..ee60ef2 100644 --- a/src/test/java/org/zalando/baigan/proxy/ConfigurationServiceBeanFactoryIT.java +++ b/src/test/java/org/zalando/baigan/proxy/ConfigurationServiceBeanFactoryIT.java @@ -9,14 +9,11 @@ import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.FilterType; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.zalando.baigan.BaiganSpringContext; import org.zalando.baigan.annotation.BaiganConfig; import org.zalando.baigan.annotation.ConfigurationServiceScan; -import org.zalando.baigan.context.SpringTestContext; import org.zalando.baigan.service.ConfigurationRepository; import org.zalando.baigan.service.github.GitConfig; @@ -28,14 +25,14 @@ @ExtendWith(SpringExtension.class) @ContextConfiguration(classes = {ConfigurationServiceBeanFactoryIT.TestContext.class}) +/* + * The purpose of this test is to prove that BeanPostProcessor and BeanFactoryPostProcessor are actually executed. + * This is to ensure that Baigan does not silently break annotations like @Cachable or @Traced, + * which had happened before. + */ public class ConfigurationServiceBeanFactoryIT { - @Configuration - @ComponentScan( - basePackageClasses = {BaiganSpringContext.class}, - excludeFilters = @ComponentScan.Filter( - classes = SpringTestContext.class, - type = FilterType.ASSIGNABLE_TYPE)) + @ComponentScan(basePackageClasses = {BaiganSpringContext.class}) @ConfigurationServiceScan(basePackages = "org.zalando.baigan.proxy") static class TestContext { @@ -108,12 +105,14 @@ public interface TestFeature { private MyDependency myDependency; @Test - public void allowsPostProcessingOfBeans() throws Exception { + public void allowsPostProcessingOfBeans() { assertThat(gitConfig.getGitHost(), is("post-processed.com")); } @Test - public void allowsPostProcessingOfFactoryBeans() throws Exception { + public void allowsPostProcessingOfFactoryBeans() { + // A bean of type MyDependency exists only because it is registered in TestBeanFactoryPostProcessor. + // It's existence proves that the post processor is running. assertThat(myDependency, is(notNullValue())); } } diff --git a/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java b/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java index 6aa55ba..26e73e7 100644 --- a/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java +++ b/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java @@ -1,6 +1,9 @@ package org.zalando.baigan.service; import org.junit.jupiter.api.Test; +import org.zalando.baigan.proxy.BaiganConfigClasses; + +import java.util.Map; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; @@ -9,7 +12,12 @@ public class FileSystemConfigurationRepositoryTest { @Test public void testReadConfigurationsFromFile() { - final ConfigurationRepository repo = new FileSystemConfigurationRepository("src/test/resources/example.json", 180); + final Map> configTypesByKey = Map.of( + "express.feature.toggle", Boolean.class, + "express.feature.service.url", String.class, + "pession.sync.feature.toggle", Boolean.class + ); + final ConfigurationRepository repo = new FileSystemConfigurationRepository("src/test/resources/example.json", 180, new BaiganConfigClasses(configTypesByKey)); assertThat(repo.get("express.feature.toggle").get().getDefaultValue(), equalTo(false)); } } From 58f3dceee0e2df98b9b2e105abd3250f36c9f6d7 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Fri, 22 Sep 2023 09:53:29 +0200 Subject: [PATCH 02/16] Simplify ContextAwareConfigurationMethodInvocationHandler As the repositories now return the type checking and constructor logic can now simplified. Primitive types would in principle work. However, if no config can be found, then trying to return null for a primitive return type of a configuration would cause a NullPointerException. To make sure users do not declare primitives as return types, the BaiganConfigClasses throws and exception if there are primitive return types, causing the starting of the ApplicationContext to fail. --- README.md | 4 +- .../baigan/proxy/BaiganConfigClasses.java | 5 +++ ...eConfigurationMethodInvocationHandler.java | 45 +++---------------- .../AbstractConfigurationRepository.java | 5 ++- .../ChainedConfigurationRepository.java | 2 +- .../context/MethodInvocationHandlerTest.java | 12 ++--- 6 files changed, 24 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 47057cc..54c6264 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,8 @@ And the _@ConfigurationServiceScan_ annotation hints the Baigan registrar to loo } ``` +**Note**: Primitives are not supported as return types. + The above example code enables the application to inject _ExpressFeature_ spring bean into any other Spring bean: ```Java @@ -118,7 +120,7 @@ A configuration object should conform to the following JSON Schema: }, "conditionType": { "description": "Type of condition to evaluate. This can be custom defined, with custom defined properties.", - "type": "object", + "type": "object" } } } diff --git a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java index 422d616..0b956da 100644 --- a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java +++ b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java @@ -15,6 +15,11 @@ public BaiganConfigClasses(Map> configTypesByKey) { } public void setConfigTypesByKey(Map> configTypesByKey) { + configTypesByKey.forEach((key, value) -> { + if (value.isPrimitive()) { + throw new IllegalArgumentException("Config " + key + " has an illegal return type " + value + ". Primitives are not supported as return type."); + } + }); this.configTypesByKey = configTypesByKey; } diff --git a/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java b/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java index 68ab853..4b79f4c 100644 --- a/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java +++ b/src/main/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandler.java @@ -1,7 +1,6 @@ package org.zalando.baigan.proxy.handler; import com.google.common.base.Supplier; -import com.google.common.primitives.Primitives; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.BeansException; @@ -12,13 +11,10 @@ import org.zalando.baigan.context.ContextProviderRetriever; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.provider.ContextProvider; -import org.zalando.baigan.proxy.ProxyUtils; import org.zalando.baigan.service.ConditionsProcessor; import org.zalando.baigan.service.ConfigurationRepository; -import java.lang.reflect.Constructor; import java.lang.reflect.Method; -import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -60,48 +56,19 @@ public void setBeanFactory(final BeanFactory beanFactory) throws BeansException } @Override - protected Object handleInvocation(Object proxy, Method method, - Object[] args) throws Throwable { + protected Object handleInvocation(Object proxy, Method method, Object[] args) { final String key = createKey(getClass(proxy), method); final Object result = getConfig(key); if (result == null) { LOG.warn("Configuration not found for key: {}", key); return null; } - - final Class declaredReturnType = method.getReturnType(); - - try { - - Constructor constructor; - if (declaredReturnType.isInstance(result)) { - return result; - } else if (declaredReturnType.isPrimitive()) { - final Class resultClass = result.getClass(); - constructor = Primitives.wrap(declaredReturnType) - .getDeclaredConstructor(resultClass); - } else if (declaredReturnType.isEnum()) { - for (Object t : Arrays - .asList(declaredReturnType.getEnumConstants())) { - if (result.toString().equalsIgnoreCase(t.toString())) { - return t; - } - } - LOG.warn("Unable to map [{}] to enum type [{}].", result, declaredReturnType.getName()); - return null; - } else { - constructor = declaredReturnType - .getDeclaredConstructor(result.getClass()); - } - return constructor.newInstance(result); - } catch (Exception exception) { - LOG.warn( - "Wrong or Incompatible configuration. Cannot find a constructor to create object of type " - + declaredReturnType - + " for value of the configuration key " + key, - exception); + if (!method.getReturnType().isInstance(result)) { + LOG.error("Configuration repository returned object of wrong type. Expected: {}, actual: {}", method.getReturnType(), result.getClass()); + return null; } - return null; + + return result; } private Class getClass(final Object proxy) { diff --git a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java index b0623b2..7ea95f5 100644 --- a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java @@ -4,7 +4,6 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.datatype.guava.GuavaModule; import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.proxy.BaiganConfigClasses; @@ -60,7 +59,9 @@ private Configuration deserializeConfig(Configuration config, C } private Class findClass(String alias) { - List> matchingClasses = baiganConfigClasses.getConfigTypesByKey().entrySet().stream().filter(entry -> alias.startsWith(entry.getKey())).map(Map.Entry::getValue).collect(toList()); + List> matchingClasses = baiganConfigClasses.getConfigTypesByKey().entrySet().stream() + .filter(entry -> alias.equals(entry.getKey())) + .map(Map.Entry::getValue).collect(toList()); if (matchingClasses.size() == 1) { return matchingClasses.get(0); diff --git a/src/main/java/org/zalando/baigan/service/ChainedConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/ChainedConfigurationRepository.java index 912bcd9..a403c5a 100644 --- a/src/main/java/org/zalando/baigan/service/ChainedConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/ChainedConfigurationRepository.java @@ -51,6 +51,6 @@ public Optional get(@Nonnull String key) { @Override public void put(@Nonnull String key, @Nonnull String value) { - + throw new UnsupportedOperationException("The ChainedConfigurationRepository doesn't allow any changes."); } } diff --git a/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java b/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java index b8e309a..5820244 100644 --- a/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java +++ b/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java @@ -26,7 +26,7 @@ enum State { interface Base { - boolean isActive(); + Boolean isActive(); } @@ -34,7 +34,7 @@ interface Express extends Base { State stateDefault(); - int maxDeliveryDays(); + Integer maxDeliveryDays(); } /** @@ -48,7 +48,7 @@ public class MethodInvocationHandlerTest { public void testEnumValue() throws Throwable { final ConfigurationRepository repo = mock(ConfigurationRepository.class); - final Configuration configuration = new Configuration<>("express.state.default", DESCRIPTION, of(), "SHIPPING"); + final Configuration configuration = new Configuration<>("express.state.default", DESCRIPTION, of(), State.SHIPPING); when(repo.get(anyString())).thenReturn(Optional.of(configuration)); final ContextAwareConfigurationMethodInvocationHandler handler = createHandler(repo); @@ -58,7 +58,7 @@ public void testEnumValue() throws Throwable { } @Test - public void testIllegalEnumValue() throws Throwable { + public void testConfigHasWrongType() throws Throwable { final ConfigurationRepository repo = mock(ConfigurationRepository.class); final Configuration configuration = new Configuration<>("express.state.default", DESCRIPTION, of(), "42"); @@ -74,7 +74,7 @@ public void testIllegalEnumValue() throws Throwable { public void testPrimitiveType() throws Throwable { final ConfigurationRepository repo = mock(ConfigurationRepository.class); - final Configuration configuration = new Configuration<>("express.max.delivery.days", DESCRIPTION, of(), "3"); + final Configuration configuration = new Configuration<>("express.max.delivery.days", DESCRIPTION, of(), 3); when(repo.get(anyString())).thenReturn(Optional.of(configuration)); final ContextAwareConfigurationMethodInvocationHandler handler = createHandler(repo); @@ -87,7 +87,7 @@ public void testPrimitiveType() throws Throwable { public void testInheritedToggle() throws Throwable { final ConfigurationRepository repo = mock(ConfigurationRepository.class); - final Configuration configuration = new Configuration<>("express.is.active", DESCRIPTION, of(), Boolean.TRUE.toString()); + final Configuration configuration = new Configuration<>("express.is.active", DESCRIPTION, of(), true); when(repo.get(anyString())).thenReturn(Optional.of(configuration)); final ContextAwareConfigurationMethodInvocationHandler handler = createHandler(repo); From 9b1756f094034cdc84b524913ee882c8bec38e61 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Sat, 23 Sep 2023 07:43:25 +0200 Subject: [PATCH 03/16] handle config without matching class; simplify tests --- README.md | 2 +- .../AbstractConfigurationRepository.java | 22 ++- .../org/zalando/baigan/e2e/TestContext.java | 70 -------- .../S3ConfigurationRepositoryEnd2EndIT.java | 155 ++++++++++++++++++ .../SomeConfigObject.java | 2 +- .../SomeConfiguration.java} | 5 +- .../simples3repo/SimpleS3RepoEnd2EndIT.java | 87 ---------- .../SomeComplexConfiguration.java | 9 - .../SomeOtherConfigObject.java | 36 ---- .../StructuredS3RepoEnd2EndIT.java | 80 --------- 10 files changed, 179 insertions(+), 289 deletions(-) delete mode 100644 src/test/java/org/zalando/baigan/e2e/TestContext.java create mode 100644 src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java rename src/test/java/org/zalando/baigan/e2e/{structureds3repo => s3repo}/SomeConfigObject.java (95%) rename src/test/java/org/zalando/baigan/e2e/{simples3repo/SomePlainConfiguration.java => s3repo/SomeConfiguration.java} (51%) delete mode 100644 src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java delete mode 100644 src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java delete mode 100644 src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java delete mode 100644 src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java diff --git a/README.md b/README.md index 54c6264..ae5fb81 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,7 @@ curl -v -XPUT http://127.0.0.1:2379/v2/keys/express.feature.enabled -d value="$( ``` ## 0.18.0 + 0.19.0 + 0.19.1 releases -With certain JDK/JRE versions used, annotated configuration interfaces were not registered as beans. Be aware, that this issue does not occur when application code is being executed by a test runner or alike, only in production setups. Therefore we recommend using a higher version to avoid this. +With certain JDK/JRE versions used, annotated configuration interfaces were not registered as beans. Be aware, that this issue does not occur when application code is being executed by a test runner or alike, only in production setups. Therefore, we recommend using a higher version to avoid this. ## License diff --git a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java index 7ea95f5..ea71096 100644 --- a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java @@ -4,6 +4,8 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.proxy.BaiganConfigClasses; @@ -22,6 +24,8 @@ public abstract class AbstractConfigurationRepository implements ConfigurationRepository { + private final Logger LOG = LoggerFactory + .getLogger(AbstractConfigurationRepository.class); final ObjectMapper objectMapper; final BaiganConfigClasses baiganConfigClasses; @@ -35,7 +39,17 @@ protected List> getConfigurations(final String text) { try { List> rawConfigs = objectMapper.readValue(text, new TypeReference<>() { }); - return rawConfigs.stream().map(config -> deserializeConfig(config, findClass(config.getAlias()))).collect(toList()); + return rawConfigs.stream() + .map(config -> { + final Optional> typedConfig = findClass(config.getAlias()).map(targetClass -> deserializeConfig(config, targetClass)); + if (typedConfig.isEmpty()) { + LOG.info("Alias {} does not match any method in a class annotated with @BaiganConfig.", config.getAlias()); + } + return typedConfig; + }) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(toList()); } catch ( IOException e) { throw new UncheckedIOException("Unable to deserialize the Configuration.", e); @@ -58,13 +72,15 @@ private Configuration deserializeConfig(Configuration config, C } } - private Class findClass(String alias) { + private Optional> findClass(String alias) { List> matchingClasses = baiganConfigClasses.getConfigTypesByKey().entrySet().stream() .filter(entry -> alias.equals(entry.getKey())) .map(Map.Entry::getValue).collect(toList()); if (matchingClasses.size() == 1) { - return matchingClasses.get(0); + return Optional.of(matchingClasses.get(0)); + } else if (matchingClasses.isEmpty()) { + return Optional.empty(); } else { throw new RuntimeException("Did not find exactly one matching BaiganConfig for alias " + alias + " in " + baiganConfigClasses.getConfigTypesByKey() + ": matching classes " + matchingClasses); } diff --git a/src/test/java/org/zalando/baigan/e2e/TestContext.java b/src/test/java/org/zalando/baigan/e2e/TestContext.java deleted file mode 100644 index a30b119..0000000 --- a/src/test/java/org/zalando/baigan/e2e/TestContext.java +++ /dev/null @@ -1,70 +0,0 @@ -package org.zalando.baigan.e2e; - -import com.amazonaws.auth.AWSStaticCredentialsProvider; -import com.amazonaws.auth.BasicAWSCredentials; -import com.amazonaws.client.builder.AwsClientBuilder; -import com.amazonaws.regions.Regions; -import com.amazonaws.services.kms.AWSKMS; -import com.amazonaws.services.kms.AWSKMSClientBuilder; -import com.amazonaws.services.s3.AmazonS3; -import com.amazonaws.services.s3.AmazonS3ClientBuilder; -import com.amazonaws.services.s3.model.AmazonS3Exception; -import com.amazonaws.services.s3.model.CreateBucketRequest; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.Configuration; -import org.testcontainers.containers.localstack.LocalStackContainer; -import org.testcontainers.junit.jupiter.Container; -import org.testcontainers.junit.jupiter.Testcontainers; -import org.testcontainers.utility.DockerImageName; -import org.zalando.baigan.BaiganSpringContext; - -import static org.testcontainers.containers.localstack.LocalStackContainer.Service.KMS; -import static org.testcontainers.containers.localstack.LocalStackContainer.Service.S3; - -@ComponentScan(basePackageClasses = {BaiganSpringContext.class}) -@Testcontainers -public class TestContext { - - public static final String S3_CONFIG_BUCKET = "some-bucket"; - public static final String S3_CONFIG_KEY = "some-key"; - - @Container - private static final LocalStackContainer localstack = new LocalStackContainer( - DockerImageName.parse("localstack/localstack:2.1.0") - ).withServices(S3, KMS).withEnv("DEFAULT_REGION", Regions.EU_CENTRAL_1.getName()); - - @Bean - AWSKMS kms() { - localstack.start(); - return AWSKMSClientBuilder.standard().withEndpointConfiguration( - new AwsClientBuilder.EndpointConfiguration( - localstack.getEndpointOverride(KMS).toString(), localstack.getRegion() - ) - ).build(); - } - - @Bean - AmazonS3 amazonS3() { - localstack.start(); - AmazonS3 s3 = AmazonS3ClientBuilder.standard().withEndpointConfiguration( - new AwsClientBuilder.EndpointConfiguration( - localstack.getEndpointOverride(S3).toString(), localstack.getRegion() - ) - ).withCredentials( - new AWSStaticCredentialsProvider( - new BasicAWSCredentials(localstack.getAccessKey(), localstack.getSecretKey()) - ) - ).build(); - - try { - s3.createBucket(new CreateBucketRequest(S3_CONFIG_BUCKET, localstack.getRegion())); - } catch (AmazonS3Exception e) { - if (!e.getErrorCode().equals("BucketAlreadyOwnedByYou")) { - throw e; - } - } - - return s3; - } -} diff --git a/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java new file mode 100644 index 0000000..80341a2 --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java @@ -0,0 +1,155 @@ +package org.zalando.baigan.e2e.s3repo; + +import com.amazonaws.auth.AWSStaticCredentialsProvider; +import com.amazonaws.auth.BasicAWSCredentials; +import com.amazonaws.client.builder.AwsClientBuilder; +import com.amazonaws.regions.Regions; +import com.amazonaws.services.kms.AWSKMS; +import com.amazonaws.services.kms.AWSKMSClientBuilder; +import com.amazonaws.services.s3.AmazonS3; +import com.amazonaws.services.s3.AmazonS3ClientBuilder; +import com.amazonaws.services.s3.model.AmazonS3Exception; +import com.amazonaws.services.s3.model.CreateBucketRequest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.testcontainers.containers.localstack.LocalStackContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.utility.DockerImageName; +import org.zalando.baigan.BaiganSpringContext; +import org.zalando.baigan.annotation.ConfigurationServiceScan; +import org.zalando.baigan.proxy.BaiganConfigClasses; +import org.zalando.baigan.service.aws.S3ConfigurationRepository; +import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; + +import java.util.concurrent.ScheduledThreadPoolExecutor; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.testcontainers.containers.localstack.LocalStackContainer.Service.KMS; +import static org.testcontainers.containers.localstack.LocalStackContainer.Service.S3; + +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = {S3ConfigurationRepositoryEnd2EndIT.RepoConfig.class}) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class S3ConfigurationRepositoryEnd2EndIT { + + public static final String S3_CONFIG_BUCKET = "some-bucket"; + public static final String S3_CONFIG_KEY = "some-key"; + + @Autowired + private AmazonS3 s3; + + @Autowired + private SomeConfiguration someConfiguration; + + @Autowired + private ScheduledThreadPoolExecutor executor; + + @Test + public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException { + assertThat(someConfiguration.someConfig(), nullValue()); + assertThat(someConfiguration.isThisTrue(), nullValue()); + assertThat(someConfiguration.someValue(), nullValue()); + + s3.putObject( + S3_CONFIG_BUCKET, + S3_CONFIG_KEY, + "[{\"alias\": \"some.configuration.some.value\", \"defaultValue\": \"some value\"}]" + ); + Thread.sleep(1100); + assertThat(someConfiguration.someConfig(), nullValue()); + assertThat(someConfiguration.isThisTrue(), nullValue()); + assertThat(someConfiguration.someValue(), equalTo("some value")); + + s3.putObject( + S3_CONFIG_BUCKET, + S3_CONFIG_KEY, + "[{ \"alias\": \"some.configuration.some.config\", \"defaultValue\": {\"config_key\":\"a value\"}}," + + "{ \"alias\": \"some.non.existing.config\", \"defaultValue\": {\"other_config_key\":\"other value\"}}," + + "{ \"alias\": \"some.configuration.is.this.true\", \"defaultValue\": true}, " + + "{ \"alias\": \"some.configuration.some.value\", \"defaultValue\": \"some value\"}]" + ); + Thread.sleep(1100); + assertThat(someConfiguration.someConfig(), equalTo(new SomeConfigObject("a value"))); + assertThat(someConfiguration.isThisTrue(), equalTo(true)); + assertThat(someConfiguration.someValue(), equalTo("some value")); + } + + @AfterAll + public void cleanup() { + executor.shutdownNow(); + } + + @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.s3repo") + @Testcontainers + @ComponentScan(basePackageClasses = {BaiganSpringContext.class}) + static class RepoConfig { + + @Bean(destroyMethod = "shutdownNow") + ScheduledThreadPoolExecutor baiganRefresherPoolExecutor(){ + return new ScheduledThreadPoolExecutor(1); + } + + @Bean + S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, BaiganConfigClasses baiganConfigClasses, ScheduledThreadPoolExecutor executorService) { + amazonS3.putObject(S3_CONFIG_BUCKET, S3_CONFIG_KEY, "[]"); + return new S3ConfigurationRepositoryBuilder() + .bucketName(S3_CONFIG_BUCKET) + .key(S3_CONFIG_KEY) + .s3Client(amazonS3) + .kmsClient(kms) + .refreshIntervalInSeconds(1) + .executor(executorService) + .baiganConfigClasses(baiganConfigClasses) + .build(); + } + + @Container + private static final LocalStackContainer localstack = new LocalStackContainer( + DockerImageName.parse("localstack/localstack:2.1.0") + ).withServices(S3, KMS).withEnv("DEFAULT_REGION", Regions.EU_CENTRAL_1.getName()); + + @Bean + AWSKMS kms() { + localstack.start(); + return AWSKMSClientBuilder.standard().withEndpointConfiguration( + new AwsClientBuilder.EndpointConfiguration( + localstack.getEndpointOverride(KMS).toString(), localstack.getRegion() + ) + ).build(); + } + + @Bean + AmazonS3 amazonS3() { + localstack.start(); + AmazonS3 s3 = AmazonS3ClientBuilder.standard().withEndpointConfiguration( + new AwsClientBuilder.EndpointConfiguration( + localstack.getEndpointOverride(S3).toString(), localstack.getRegion() + ) + ).withCredentials( + new AWSStaticCredentialsProvider( + new BasicAWSCredentials(localstack.getAccessKey(), localstack.getSecretKey()) + ) + ).build(); + + try { + s3.createBucket(new CreateBucketRequest(S3_CONFIG_BUCKET, localstack.getRegion())); + } catch (AmazonS3Exception e) { + if (!e.getErrorCode().equals("BucketAlreadyOwnedByYou")) { + throw e; + } + } + + return s3; + } + } +} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeConfigObject.java b/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfigObject.java similarity index 95% rename from src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeConfigObject.java rename to src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfigObject.java index 78988dc..07badd4 100644 --- a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeConfigObject.java +++ b/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfigObject.java @@ -1,4 +1,4 @@ -package org.zalando.baigan.e2e.structureds3repo; +package org.zalando.baigan.e2e.s3repo; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; diff --git a/src/test/java/org/zalando/baigan/e2e/simples3repo/SomePlainConfiguration.java b/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfiguration.java similarity index 51% rename from src/test/java/org/zalando/baigan/e2e/simples3repo/SomePlainConfiguration.java rename to src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfiguration.java index 139e5ff..fd20bfa 100644 --- a/src/test/java/org/zalando/baigan/e2e/simples3repo/SomePlainConfiguration.java +++ b/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfiguration.java @@ -1,9 +1,10 @@ -package org.zalando.baigan.e2e.simples3repo; +package org.zalando.baigan.e2e.s3repo; import org.zalando.baigan.annotation.BaiganConfig; @BaiganConfig -public interface SomePlainConfiguration { +public interface SomeConfiguration { + SomeConfigObject someConfig(); String someValue(); Boolean isThisTrue(); } diff --git a/src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java deleted file mode 100644 index cb3cc39..0000000 --- a/src/test/java/org/zalando/baigan/e2e/simples3repo/SimpleS3RepoEnd2EndIT.java +++ /dev/null @@ -1,87 +0,0 @@ -package org.zalando.baigan.e2e.simples3repo; - -import com.amazonaws.services.kms.AWSKMS; -import com.amazonaws.services.s3.AmazonS3; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestInstance; -import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit.jupiter.SpringExtension; -import org.zalando.baigan.annotation.ConfigurationServiceScan; -import org.zalando.baigan.e2e.TestContext; -import org.zalando.baigan.proxy.BaiganConfigClasses; -import org.zalando.baigan.service.aws.S3ConfigurationRepository; -import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; - -import java.util.concurrent.ScheduledThreadPoolExecutor; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.nullValue; - -@ExtendWith(SpringExtension.class) -@ContextConfiguration(classes = {TestContext.class, SimpleS3RepoEnd2EndIT.RepoConfig.class}) -@TestInstance(TestInstance.Lifecycle.PER_CLASS) -public class SimpleS3RepoEnd2EndIT { - - @Autowired - private AmazonS3 s3; - - @Autowired - private SomePlainConfiguration someConfiguration; - - @Autowired - private ScheduledThreadPoolExecutor executor; - - @Test - public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException { - assertThat(someConfiguration.someValue(), nullValue()); - assertThat(someConfiguration.isThisTrue(), nullValue()); - s3.putObject(TestContext.S3_CONFIG_BUCKET, TestContext.S3_CONFIG_KEY, "[{ \"alias\": \"some.plain.configuration.some.value\", \"defaultValue\": \"a value\"}]"); - Thread.sleep(1100); - assertThat(someConfiguration.someValue(), equalTo("a value")); - assertThat(someConfiguration.isThisTrue(), nullValue()); - s3.putObject( - TestContext.S3_CONFIG_BUCKET, - TestContext.S3_CONFIG_KEY, - "[{ \"alias\": \"some.plain.configuration.is.this.true\", \"defaultValue\": true}, " + - "{ \"alias\": \"some.plain.configuration.some.value\", \"defaultValue\": \"a value\"}]" - ); - Thread.sleep(1100); - assertThat(someConfiguration.someValue(), equalTo("a value")); - assertThat(someConfiguration.isThisTrue(), equalTo(true)); - } - - @AfterAll - public void cleanup() { - executor.shutdownNow(); - } - - @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.simples3repo") - static class RepoConfig { - - @Bean(destroyMethod = "shutdownNow") - ScheduledThreadPoolExecutor baiganRefresherPoolExecutor(){ - return new ScheduledThreadPoolExecutor(1); - } - - @Bean - S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, BaiganConfigClasses baiganConfigClasses, ScheduledThreadPoolExecutor executor) { - amazonS3.putObject(TestContext.S3_CONFIG_BUCKET, TestContext.S3_CONFIG_KEY, "[]"); - - return new S3ConfigurationRepositoryBuilder() - .bucketName(TestContext.S3_CONFIG_BUCKET) - .key(TestContext.S3_CONFIG_KEY) - .s3Client(amazonS3) - .kmsClient(kms) - .refreshIntervalInSeconds(1) - .executor(executor) - .baiganConfigClasses(baiganConfigClasses) - .build(); - } - - } -} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java deleted file mode 100644 index 91adc16..0000000 --- a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeComplexConfiguration.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.zalando.baigan.e2e.structureds3repo; - -import org.zalando.baigan.annotation.BaiganConfig; - -@BaiganConfig -public interface SomeComplexConfiguration { - SomeConfigObject someConfig(); - SomeOtherConfigObject someOtherConfig(); -} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java b/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java deleted file mode 100644 index 2e8efcf..0000000 --- a/src/test/java/org/zalando/baigan/e2e/structureds3repo/SomeOtherConfigObject.java +++ /dev/null @@ -1,36 +0,0 @@ -package org.zalando.baigan.e2e.structureds3repo; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; - -import java.util.Objects; -import java.util.StringJoiner; - -public class SomeOtherConfigObject { - private final String anotherConfigKey; - - @JsonCreator - public SomeOtherConfigObject(@JsonProperty("other_config_key") String anotherConfigKey) { - this.anotherConfigKey = anotherConfigKey; - } - - @Override - public String toString() { - return new StringJoiner(", ", SomeOtherConfigObject.class.getSimpleName() + "[", "]") - .add("anotherConfigKey='" + anotherConfigKey + "'") - .toString(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - SomeOtherConfigObject config = (SomeOtherConfigObject) o; - return Objects.equals(anotherConfigKey, config.anotherConfigKey); - } - - @Override - public int hashCode() { - return Objects.hash(anotherConfigKey); - } -} diff --git a/src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java deleted file mode 100644 index 6ec4f5b..0000000 --- a/src/test/java/org/zalando/baigan/e2e/structureds3repo/StructuredS3RepoEnd2EndIT.java +++ /dev/null @@ -1,80 +0,0 @@ -package org.zalando.baigan.e2e.structureds3repo; - -import com.amazonaws.services.kms.AWSKMS; -import com.amazonaws.services.s3.AmazonS3; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestInstance; -import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit.jupiter.SpringExtension; -import org.zalando.baigan.annotation.ConfigurationServiceScan; -import org.zalando.baigan.e2e.TestContext; -import org.zalando.baigan.proxy.BaiganConfigClasses; -import org.zalando.baigan.service.aws.S3ConfigurationRepository; -import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; - -import java.util.concurrent.ScheduledThreadPoolExecutor; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.nullValue; -import static org.zalando.baigan.e2e.TestContext.S3_CONFIG_BUCKET; -import static org.zalando.baigan.e2e.TestContext.S3_CONFIG_KEY; - -@ExtendWith(SpringExtension.class) -@ContextConfiguration(classes = {TestContext.class, StructuredS3RepoEnd2EndIT.RepoConfig.class}) -@TestInstance(TestInstance.Lifecycle.PER_CLASS) -public class StructuredS3RepoEnd2EndIT { - - @Autowired - private AmazonS3 s3; - - @Autowired - private SomeComplexConfiguration someConfiguration; - - @Autowired - private ScheduledThreadPoolExecutor executor; - - @Test - public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException { - assertThat(someConfiguration.someConfig(), nullValue()); - s3.putObject( - S3_CONFIG_BUCKET, - S3_CONFIG_KEY, - "[{ \"alias\": \"some.complex.configuration.some.config\", \"defaultValue\": {\"config_key\":\"a value\"}}," + - "{ \"alias\": \"some.complex.configuration.some.other.config\", \"defaultValue\": {\"other_config_key\":\"other value\"}}]"); - Thread.sleep(1100); - assertThat(someConfiguration.someConfig(), equalTo(new SomeConfigObject("a value"))); - } - - @AfterAll - public void cleanup() { - executor.shutdownNow(); - } - - @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.structureds3repo") - static class RepoConfig { - - @Bean(destroyMethod = "shutdownNow") - ScheduledThreadPoolExecutor baiganRefresherPoolExecutor(){ - return new ScheduledThreadPoolExecutor(1); - } - - @Bean - S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, BaiganConfigClasses baiganConfigClasses, ScheduledThreadPoolExecutor executorService) { - amazonS3.putObject(TestContext.S3_CONFIG_BUCKET, TestContext.S3_CONFIG_KEY, "[]"); - return new S3ConfigurationRepositoryBuilder() - .bucketName(TestContext.S3_CONFIG_BUCKET) - .key(TestContext.S3_CONFIG_KEY) - .s3Client(amazonS3) - .kmsClient(kms) - .refreshIntervalInSeconds(1) - .executor(executorService) - .baiganConfigClasses(baiganConfigClasses) - .build(); - } - } -} From e41f24aae761d62a05a3d0223afd3066a11b3d42 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Wed, 27 Sep 2023 22:02:12 +0200 Subject: [PATCH 04/16] remove AbstractConfigurationRepository and use composition instead of inheritance The inheritance implied by the AbstractConfigurationRepository makes it harder to test its logic and that of the repositories that use it. This commit changes it to composition, turning the abstract class into a service that is used by the repositories. The EtcdConfigurationRepository did not actually use any of the functionality of the abstract class, so it does not have the new service as component. Also, this commit increases the test coverage. --- .../org/zalando/baigan/model/Condition.java | 25 ++- .../zalando/baigan/model/Configuration.java | 24 +++ .../org/zalando/baigan/model/EndsWith.java | 21 +++ .../java/org/zalando/baigan/model/Equals.java | 22 +++ .../java/org/zalando/baigan/model/In.java | 21 +++ ...pository.java => ConfigurationParser.java} | 31 ++-- .../service/EtcdConfigurationRepository.java | 6 +- .../FileSystemConfigurationRepository.java | 25 ++- .../aws/S3ConfigurationRepository.java | 10 +- .../baigan/service/github/GitCacheLoader.java | 80 ++++----- .../github/GitConfigurationRepository.java | 10 +- .../{s3repo => configs}/SomeConfigObject.java | 2 +- .../SomeConfiguration.java | 2 +- ...ystemConfigurationRepositoryEnd2EndIT.java | 80 +++++++++ .../S3ConfigurationRepositoryEnd2EndIT.java | 4 +- .../service/ConfigurationParserTest.java | 161 ++++++++++++++++++ .../service/github/GitCacheLoaderTest.java | 109 +++++++----- 17 files changed, 490 insertions(+), 143 deletions(-) rename src/main/java/org/zalando/baigan/service/{AbstractConfigurationRepository.java => ConfigurationParser.java} (71%) rename src/test/java/org/zalando/baigan/e2e/{s3repo => configs}/SomeConfigObject.java (96%) rename src/test/java/org/zalando/baigan/e2e/{s3repo => configs}/SomeConfiguration.java (82%) create mode 100644 src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java create mode 100644 src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java diff --git a/src/main/java/org/zalando/baigan/model/Condition.java b/src/main/java/org/zalando/baigan/model/Condition.java index cb12300..ef8f0c5 100644 --- a/src/main/java/org/zalando/baigan/model/Condition.java +++ b/src/main/java/org/zalando/baigan/model/Condition.java @@ -17,6 +17,8 @@ package org.zalando.baigan.model; import java.io.Serializable; +import java.util.Objects; +import java.util.StringJoiner; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -63,4 +65,25 @@ public ConditionType getConditionType() { return conditionType; } -} \ No newline at end of file + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Condition condition = (Condition) o; + return Objects.equals(paramName, condition.paramName) && Objects.equals(conditionType, condition.conditionType) && Objects.equals(value, condition.value); + } + + @Override + public int hashCode() { + return Objects.hash(paramName, conditionType, value); + } + + @Override + public String toString() { + return new StringJoiner(", ", Condition.class.getSimpleName() + "[", "]") + .add("paramName='" + paramName + "'") + .add("conditionType=" + conditionType) + .add("value=" + value) + .toString(); + } +} diff --git a/src/main/java/org/zalando/baigan/model/Configuration.java b/src/main/java/org/zalando/baigan/model/Configuration.java index 3ddb1e4..db2586d 100644 --- a/src/main/java/org/zalando/baigan/model/Configuration.java +++ b/src/main/java/org/zalando/baigan/model/Configuration.java @@ -17,7 +17,9 @@ package org.zalando.baigan.model; import java.io.Serializable; +import java.util.Objects; import java.util.Set; +import java.util.StringJoiner; import com.fasterxml.jackson.annotation.JsonProperty; @@ -67,4 +69,26 @@ public Type getDefaultValue() { return defaultValue; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Configuration that = (Configuration) o; + return Objects.equals(alias, that.alias) && Objects.equals(description, that.description) && Objects.equals(conditions, that.conditions) && Objects.equals(defaultValue, that.defaultValue); + } + + @Override + public int hashCode() { + return Objects.hash(alias, description, conditions, defaultValue); + } + + @Override + public String toString() { + return new StringJoiner(", ", Configuration.class.getSimpleName() + "[", "]") + .add("alias='" + alias + "'") + .add("description='" + description + "'") + .add("conditions=" + conditions) + .add("defaultValue=" + defaultValue) + .toString(); + } } diff --git a/src/main/java/org/zalando/baigan/model/EndsWith.java b/src/main/java/org/zalando/baigan/model/EndsWith.java index 8e7084d..1519529 100644 --- a/src/main/java/org/zalando/baigan/model/EndsWith.java +++ b/src/main/java/org/zalando/baigan/model/EndsWith.java @@ -16,7 +16,9 @@ package org.zalando.baigan.model; +import java.util.Objects; import java.util.Set; +import java.util.StringJoiner; import org.springframework.util.StringUtils; @@ -52,4 +54,23 @@ public boolean eval(final String forValue) { return false; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + EndsWith endsWith = (EndsWith) o; + return Objects.equals(endsWithValue, endsWith.endsWithValue); + } + + @Override + public int hashCode() { + return Objects.hash(endsWithValue); + } + + @Override + public String toString() { + return new StringJoiner(", ", EndsWith.class.getSimpleName() + "[", "]") + .add("endsWithValue=" + endsWithValue) + .toString(); + } } diff --git a/src/main/java/org/zalando/baigan/model/Equals.java b/src/main/java/org/zalando/baigan/model/Equals.java index 3108966..3fd3b43 100644 --- a/src/main/java/org/zalando/baigan/model/Equals.java +++ b/src/main/java/org/zalando/baigan/model/Equals.java @@ -19,6 +19,9 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.Objects; +import java.util.StringJoiner; + /** * Implementation of ConditionType that evaluates to true if the context param * matches the configured value.. @@ -43,4 +46,23 @@ public boolean eval(final String forValue) { return onValue.equalsIgnoreCase(forValue); } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Equals equals = (Equals) o; + return Objects.equals(onValue, equals.onValue); + } + + @Override + public int hashCode() { + return Objects.hash(onValue); + } + + @Override + public String toString() { + return new StringJoiner(", ", Equals.class.getSimpleName() + "[", "]") + .add("onValue='" + onValue + "'") + .toString(); + } } diff --git a/src/main/java/org/zalando/baigan/model/In.java b/src/main/java/org/zalando/baigan/model/In.java index 259feb0..cc2e826 100644 --- a/src/main/java/org/zalando/baigan/model/In.java +++ b/src/main/java/org/zalando/baigan/model/In.java @@ -16,7 +16,9 @@ package org.zalando.baigan.model; +import java.util.Objects; import java.util.Set; +import java.util.StringJoiner; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -45,4 +47,23 @@ public boolean eval(final String forValue) { return inValue.contains(forValue); } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + In in = (In) o; + return Objects.equals(inValue, in.inValue); + } + + @Override + public int hashCode() { + return Objects.hash(inValue); + } + + @Override + public String toString() { + return new StringJoiner(", ", In.class.getSimpleName() + "[", "]") + .add("inValue=" + inValue) + .toString(); + } } diff --git a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java similarity index 71% rename from src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java rename to src/main/java/org/zalando/baigan/service/ConfigurationParser.java index ea71096..55780d0 100644 --- a/src/main/java/org/zalando/baigan/service/AbstractConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java @@ -14,7 +14,6 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; @@ -22,26 +21,32 @@ import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; -public abstract class AbstractConfigurationRepository implements ConfigurationRepository { +public class ConfigurationParser { private final Logger LOG = LoggerFactory - .getLogger(AbstractConfigurationRepository.class); + .getLogger(ConfigurationParser.class); final ObjectMapper objectMapper; final BaiganConfigClasses baiganConfigClasses; - protected AbstractConfigurationRepository(final BaiganConfigClasses baiganConfigClasses, final ObjectMapper objectMapper) { + // TODO define package structure so this is not public + public ConfigurationParser(final BaiganConfigClasses baiganConfigClasses, final ObjectMapper objectMapper) { this.baiganConfigClasses = requireNonNull(baiganConfigClasses, "baiganConfigClasses has to be not null. Get them from the bean of the same name."); this.objectMapper = requireNonNull(objectMapper, "objectMapper has to be not null."); } @Nonnull - protected List> getConfigurations(final String text) { + public List> getConfigurations(final String text) { + if (text == null || text.isEmpty()) { + LOG.warn("Input to parse is empty: {}", text); + return List.of(); + } try { List> rawConfigs = objectMapper.readValue(text, new TypeReference<>() { }); return rawConfigs.stream() .map(config -> { - final Optional> typedConfig = findClass(config.getAlias()).map(targetClass -> deserializeConfig(config, targetClass)); + final Optional> typedConfig = Optional.ofNullable(baiganConfigClasses.getConfigTypesByKey().get(config.getAlias())) + .map(targetClass -> deserializeConfig(config, targetClass)); if (typedConfig.isEmpty()) { LOG.info("Alias {} does not match any method in a class annotated with @BaiganConfig.", config.getAlias()); } @@ -71,18 +76,4 @@ private Configuration deserializeConfig(Configuration config, C throw new RuntimeException(e); } } - - private Optional> findClass(String alias) { - List> matchingClasses = baiganConfigClasses.getConfigTypesByKey().entrySet().stream() - .filter(entry -> alias.equals(entry.getKey())) - .map(Map.Entry::getValue).collect(toList()); - - if (matchingClasses.size() == 1) { - return Optional.of(matchingClasses.get(0)); - } else if (matchingClasses.isEmpty()) { - return Optional.empty(); - } else { - throw new RuntimeException("Did not find exactly one matching BaiganConfig for alias " + alias + " in " + baiganConfigClasses.getConfigTypesByKey() + ": matching classes " + matchingClasses); - } - } } diff --git a/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java index f44a80e..6ce14b0 100644 --- a/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java @@ -19,8 +19,8 @@ /** * @author mchand */ -// TODO E2E test -public class EtcdConfigurationRepository extends AbstractConfigurationRepository { +// TODO Upgrade to v3 and add E2E test +public class EtcdConfigurationRepository { private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final String ETCD_URL_ENV_NAME = "ETCD_URL"; @@ -30,14 +30,12 @@ public class EtcdConfigurationRepository extends AbstractConfigurationRepository @VisibleForTesting public EtcdConfigurationRepository(final EtcdClient etcdClient, final BaiganConfigClasses baiganConfigClasses) { - super(baiganConfigClasses, objectMapper); checkArgument(etcdClient != null); this.etcdClient = etcdClient; } public EtcdConfigurationRepository(final BaiganConfigClasses baiganConfigClasses) { - super(baiganConfigClasses, objectMapper); etcdClient = new EtcdClient(getUrl()); } diff --git a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java index a92cd9d..ef9d6ec 100644 --- a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java @@ -2,7 +2,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.guava.GuavaModule; -import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -32,24 +31,24 @@ * @author mchand */ // TODO E2E test -public class FileSystemConfigurationRepository extends AbstractConfigurationRepository { +public class FileSystemConfigurationRepository implements ConfigurationRepository { private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final Logger LOG = LoggerFactory.getLogger(FileSystemConfigurationRepository.class); - private final LoadingCache> cachedConfigurations; + private final ConfigurationParser configurationParser; + private final LoadingCache>> cachedConfigurations; private final String fileName; - @VisibleForTesting - FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds, final BaiganConfigClasses baiganConfigClasses) { - super(baiganConfigClasses, objectMapper); + public FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds, final BaiganConfigClasses baiganConfigClasses) { + this.configurationParser = new ConfigurationParser(baiganConfigClasses, objectMapper); this.fileName = fileName; cachedConfigurations = CacheBuilder.newBuilder() .refreshAfterWrite(refreshIntervalInSeconds, TimeUnit.SECONDS) .build(new CacheLoader<>() { @Override - public Map load(String filename) { + public Map> load(String filename) { try { return loadConfigurations(filename); } catch (final Exception e) { @@ -59,8 +58,8 @@ public Map load(String filename) { } @Override - public ListenableFuture> reload( - String key, Map oldValue) + public ListenableFuture>> reload( + String key, Map> oldValue) throws Exception { LOG.info("Reloading the configuration from file [{}]", key); return super.reload(key, oldValue); @@ -85,13 +84,13 @@ public void put(@Nonnull String key, @Nonnull String value) { } - protected Map loadConfigurations(String filename) { + protected Map> loadConfigurations(String filename) { final String configurationText = loadResource(filename); - final Collection> configurations = getConfigurations( + final Collection> configurations = configurationParser.getConfigurations( configurationText); - final ImmutableMap.Builder builder = ImmutableMap.builder(); - for (Configuration each : configurations) { + final ImmutableMap.Builder> builder = ImmutableMap.builder(); + for (Configuration each : configurations) { builder.put(each.getAlias(), each); } diff --git a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java index ca05bdf..6f099a2 100644 --- a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java @@ -8,7 +8,8 @@ import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.proxy.BaiganConfigClasses; -import org.zalando.baigan.service.AbstractConfigurationRepository; +import org.zalando.baigan.service.ConfigurationParser; +import org.zalando.baigan.service.ConfigurationRepository; import javax.annotation.Nonnull; import java.util.List; @@ -20,10 +21,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -public class S3ConfigurationRepository extends AbstractConfigurationRepository { +public class S3ConfigurationRepository implements ConfigurationRepository { private static final Logger LOG = LoggerFactory.getLogger(S3ConfigurationRepository.class); + private final ConfigurationParser configurationParser; private final S3FileLoader s3Loader; private final long refreshInterval; private final ScheduledThreadPoolExecutor executor; @@ -33,7 +35,6 @@ public class S3ConfigurationRepository extends AbstractConfigurationRepository { final long refreshInterval, final ScheduledThreadPoolExecutor executor, final AmazonS3 s3Client, final AWSKMS kmsClient, final BaiganConfigClasses baiganConfigClasses, final ObjectMapper objectMapper) { - super(baiganConfigClasses, objectMapper); checkNotNull(bucketName, "bucketName is required"); checkNotNull(key, "key is required"); checkArgument(refreshInterval >= 0, "refreshInterval has to be >= 0"); @@ -41,6 +42,7 @@ public class S3ConfigurationRepository extends AbstractConfigurationRepository { checkNotNull(s3Client, "s3Client is required"); checkNotNull(kmsClient, "kmsClient is required"); + this.configurationParser = new ConfigurationParser(baiganConfigClasses, objectMapper); this.refreshInterval = refreshInterval; this.executor = executor; this.s3Loader = new S3FileLoader(bucketName, key, s3Client, kmsClient); @@ -53,7 +55,7 @@ public class S3ConfigurationRepository extends AbstractConfigurationRepository { protected void loadConfigurations() { final String configurationText = s3Loader.loadContent(); - final List> configurations = getConfigurations(configurationText); + final List> configurations = configurationParser.getConfigurations(configurationText); final ImmutableMap.Builder> builder = ImmutableMap.builder(); for (final Configuration configuration : configurations) { builder.put(configuration.getAlias(), configuration); diff --git a/src/main/java/org/zalando/baigan/service/github/GitCacheLoader.java b/src/main/java/org/zalando/baigan/service/github/GitCacheLoader.java index 4f6c7fd..95109b0 100644 --- a/src/main/java/org/zalando/baigan/service/github/GitCacheLoader.java +++ b/src/main/java/org/zalando/baigan/service/github/GitCacheLoader.java @@ -1,35 +1,31 @@ package org.zalando.baigan.service.github; -import java.io.IOException; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.concurrent.Callable; -import java.util.concurrent.Executors; - -import org.eclipse.egit.github.core.RepositoryContents; -import org.eclipse.egit.github.core.RepositoryId; -import org.eclipse.egit.github.core.client.GitHubClient; -import org.eclipse.egit.github.core.service.ContentsService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.zalando.baigan.model.Configuration; - -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.cache.CacheLoader; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; +import org.eclipse.egit.github.core.RepositoryContents; +import org.eclipse.egit.github.core.RepositoryId; +import org.eclipse.egit.github.core.client.GitHubClient; +import org.eclipse.egit.github.core.service.ContentsService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.service.ConfigurationParser; import javax.annotation.Nonnull; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.Callable; +import java.util.concurrent.Executors; import static com.google.common.util.concurrent.MoreExecutors.listeningDecorator; -import static org.apache.commons.codec.binary.Base64.*; +import static org.apache.commons.codec.binary.Base64.decodeBase64; /** * This class implements the {@link CacheLoader} offering Configuration loading @@ -39,7 +35,7 @@ * */ public class GitCacheLoader - extends CacheLoader> { + extends CacheLoader>> { private static final Logger LOG = LoggerFactory .getLogger(GitCacheLoader.class); @@ -48,6 +44,7 @@ public class GitCacheLoader private GitConfig config; + private final ConfigurationParser configurationParser; private final ListeningExecutorService executorService = listeningDecorator(Executors.newFixedThreadPool(1)); private ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); @@ -60,18 +57,19 @@ private static ContentsService buildContentsService(@Nonnull final GitConfig git return new ContentsService(client); } - public GitCacheLoader(@Nonnull final GitConfig gitConfig) { - this(gitConfig, buildContentsService(gitConfig)); + public GitCacheLoader(@Nonnull final GitConfig gitConfig, @Nonnull final ConfigurationParser configurationParser) { + this(gitConfig, buildContentsService(gitConfig), configurationParser); } @VisibleForTesting - GitCacheLoader(GitConfig gitConfig, ContentsService contentsService) { + GitCacheLoader(GitConfig gitConfig, ContentsService contentsService, ConfigurationParser configurationParser) { + this.configurationParser = configurationParser; this.config = gitConfig; this.contentsService = contentsService; } @Override - public Map load(String key) throws Exception { + public Map> load(String key) throws Exception { final RepositoryContents contents = getContentsForFile(key); if (contents != null) { return updateContent(contents); @@ -80,33 +78,33 @@ public Map load(String key) throws Exception { return ImmutableMap.of(); } - private Map updateContent( + private Map> updateContent( @Nonnull final RepositoryContents contents) { final String contentsSha = contents.getSha(); LOG.info("Loading the new repository contents [ SHA:{} ; NAME:{} ] ", contentsSha, contents.getPath()); - final List configurations = getConfigurations(getTextContent(contents)); + final List> configurations = configurationParser.getConfigurations(getTextContent(contents)); latestSha = contentsSha; - final ImmutableMap.Builder builder = ImmutableMap.builder(); - for (final Configuration each : configurations) { + final ImmutableMap.Builder> builder = ImmutableMap.builder(); + for (final Configuration each : configurations) { builder.put(each.getAlias(), each); } return builder.build(); } - public ListenableFuture> reload(final String key, - final Map oldValue) throws Exception { + public ListenableFuture>> reload(final String key, + final Map> oldValue) throws Exception { return createFuture(key, oldValue); } - private ListenableFuture> createFuture( + private ListenableFuture>> createFuture( final String sourceFile, - final Map oldValue) { + final Map> oldValue) { - final Callable> callable = () -> { + final Callable>> callable = () -> { final RepositoryContents contents = getContentsForFile(sourceFile); // If the contents is null, return old value, this is to // preserve in case Github is down. @@ -141,20 +139,4 @@ private String getTextContent(final RepositoryContents content) { final String stringContent = content.getContent(); return new String(decodeBase64(stringContent.getBytes())); } - - private List getConfigurations(final String text) { - try { - return objectMapper.readValue(text, - new TypeReference>() { - }); - } catch (IOException e) { - LOG.warn( - "Exception while deserializing the Configuration from the Github repository contents." + - "Please check to see if if matches the Configuration schema at " + - "https://github.com/zalando/baigan-config.", - e); - } - return ImmutableList.of(); - } - -} \ No newline at end of file +} diff --git a/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java index 171ef82..6c9e6ce 100644 --- a/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/github/GitConfigurationRepository.java @@ -8,7 +8,8 @@ import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.proxy.BaiganConfigClasses; -import org.zalando.baigan.service.AbstractConfigurationRepository; +import org.zalando.baigan.service.ConfigurationParser; +import org.zalando.baigan.service.ConfigurationRepository; import javax.annotation.Nonnull; import java.util.Map; @@ -26,19 +27,18 @@ */ // TODO remove or add E2E test @Deprecated -public class GitConfigurationRepository extends AbstractConfigurationRepository { +public class GitConfigurationRepository implements ConfigurationRepository { private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final Logger LOG = LoggerFactory.getLogger(GitConfigurationRepository.class); - private final LoadingCache> cachedConfigurations; + private final LoadingCache>> cachedConfigurations; private final GitConfig gitConfig; public GitConfigurationRepository(long refreshIntervalInMinutes, GitConfig gitConfig, BaiganConfigClasses baiganConfigClasses) { - super(baiganConfigClasses, objectMapper); this.gitConfig = gitConfig; cachedConfigurations = CacheBuilder.newBuilder() .refreshAfterWrite(refreshIntervalInMinutes, TimeUnit.MINUTES) - .build(new GitCacheLoader(gitConfig)); + .build(new GitCacheLoader(gitConfig, new ConfigurationParser(baiganConfigClasses, objectMapper))); } @Nonnull diff --git a/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfigObject.java b/src/test/java/org/zalando/baigan/e2e/configs/SomeConfigObject.java similarity index 96% rename from src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfigObject.java rename to src/test/java/org/zalando/baigan/e2e/configs/SomeConfigObject.java index 07badd4..f9e992e 100644 --- a/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfigObject.java +++ b/src/test/java/org/zalando/baigan/e2e/configs/SomeConfigObject.java @@ -1,4 +1,4 @@ -package org.zalando.baigan.e2e.s3repo; +package org.zalando.baigan.e2e.configs; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; diff --git a/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfiguration.java b/src/test/java/org/zalando/baigan/e2e/configs/SomeConfiguration.java similarity index 82% rename from src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfiguration.java rename to src/test/java/org/zalando/baigan/e2e/configs/SomeConfiguration.java index fd20bfa..1711a38 100644 --- a/src/test/java/org/zalando/baigan/e2e/s3repo/SomeConfiguration.java +++ b/src/test/java/org/zalando/baigan/e2e/configs/SomeConfiguration.java @@ -1,4 +1,4 @@ -package org.zalando.baigan.e2e.s3repo; +package org.zalando.baigan.e2e.configs; import org.zalando.baigan.annotation.BaiganConfig; diff --git a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java new file mode 100644 index 0000000..f07a3cd --- /dev/null +++ b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java @@ -0,0 +1,80 @@ +package org.zalando.baigan.e2e.filerepo; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.zalando.baigan.BaiganSpringContext; +import org.zalando.baigan.annotation.ConfigurationServiceScan; +import org.zalando.baigan.e2e.configs.SomeConfigObject; +import org.zalando.baigan.e2e.configs.SomeConfiguration; +import org.zalando.baigan.proxy.BaiganConfigClasses; +import org.zalando.baigan.service.FileSystemConfigurationRepository; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = {FileSystemConfigurationRepositoryEnd2EndIT.RepoConfig.class}) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class FileSystemConfigurationRepositoryEnd2EndIT { + + @Autowired + private SomeConfiguration someConfiguration; + + @Autowired + private Path configFile; + + @Test + public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException, IOException { + assertThat(someConfiguration.someConfig(), nullValue()); + assertThat(someConfiguration.isThisTrue(), nullValue()); + assertThat(someConfiguration.someValue(), nullValue()); + + Files.writeString(configFile, "[{\"alias\": \"some.configuration.some.value\", \"defaultValue\": \"some value\"}]"); + Thread.sleep(1100); + assertThat(someConfiguration.someConfig(), nullValue()); + assertThat(someConfiguration.isThisTrue(), nullValue()); + assertThat(someConfiguration.someValue(), equalTo("some value")); + + Files.writeString(configFile, "[{ \"alias\": \"some.configuration.some.config\", \"defaultValue\": {\"config_key\":\"a value\"}}," + + "{ \"alias\": \"some.non.existing.config\", \"defaultValue\": {\"other_config_key\":\"other value\"}}," + + "{ \"alias\": \"some.configuration.is.this.true\", \"defaultValue\": true}, " + + "{ \"alias\": \"some.configuration.some.value\", \"defaultValue\": \"some value\"}]" + ); + Thread.sleep(1100); + assertThat(someConfiguration.someConfig(), equalTo(new SomeConfigObject("a value"))); + assertThat(someConfiguration.isThisTrue(), equalTo(true)); + assertThat(someConfiguration.someValue(), equalTo("some value")); + } + + @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.configs") + @Testcontainers + @ComponentScan(basePackageClasses = {BaiganSpringContext.class}) + static class RepoConfig { + + @Bean + FileSystemConfigurationRepository configurationRepository(Path configFile, BaiganConfigClasses baiganConfigClasses) { + return new FileSystemConfigurationRepository(configFile.toString(), 1, baiganConfigClasses); + } + + @Bean("configFile") + Path configFile() { + try { + return Files.createTempFile("config", "json"); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + } +} diff --git a/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java index 80341a2..dbdd4ce 100644 --- a/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java @@ -25,6 +25,8 @@ import org.testcontainers.utility.DockerImageName; import org.zalando.baigan.BaiganSpringContext; import org.zalando.baigan.annotation.ConfigurationServiceScan; +import org.zalando.baigan.e2e.configs.SomeConfigObject; +import org.zalando.baigan.e2e.configs.SomeConfiguration; import org.zalando.baigan.proxy.BaiganConfigClasses; import org.zalando.baigan.service.aws.S3ConfigurationRepository; import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; @@ -89,7 +91,7 @@ public void cleanup() { executor.shutdownNow(); } - @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.s3repo") + @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.configs") @Testcontainers @ComponentScan(basePackageClasses = {BaiganSpringContext.class}) static class RepoConfig { diff --git a/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java new file mode 100644 index 0000000..34ae38b --- /dev/null +++ b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java @@ -0,0 +1,161 @@ +package org.zalando.baigan.service; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; +import org.zalando.baigan.model.Condition; +import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.model.Equals; +import org.zalando.baigan.proxy.BaiganConfigClasses; + +import java.io.UncheckedIOException; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.StringJoiner; + +import static java.util.stream.Collectors.toList; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThrows; + +public class ConfigurationParserTest { + + private static final ObjectMapper objectMapper = new ObjectMapper(); + + private static final BaiganConfigClasses stringConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", String.class)); + private static final ConfigurationParser parser = new ConfigurationParser(stringConfigClasses, objectMapper); + + @Test + public void whenInputContainsKeyForKnownType_shouldParseConfiguration() { + final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}]"; + + final List> parsedConfigs = parser.getConfigurations(input); + + final List> expectedConfigs = List.of( + new Configuration<>("some.config.some.key", null, Set.of(), "someValue") + ); + + assertThat(parsedConfigs, equalTo(expectedConfigs)); + } + + @Test + public void whenInputContainsTwoConfigs_shouldParseAllConfigsConfiguration() { + final BaiganConfigClasses structuredConfigClasses = new BaiganConfigClasses(Map.of( + "some.config.some.key", String.class, + "some.struct.config", StructuredConfig.class + )); + final ConfigurationParser parser = new ConfigurationParser(structuredConfigClasses, objectMapper); + + final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}," + + "{\"alias\":\"some.struct.config\",\"defaultValue\":{\"someConfig\":\"some value\",\"someOtherConfig\":1}}]"; + + final List> parsedConfigs = parser.getConfigurations(input); + + final List> expectedConfigs = List.of( + new Configuration<>("some.config.some.key", null, Set.of(), "someValue"), + new Configuration<>("some.struct.config", null, Set.of(), new StructuredConfig("some value", 1)) + ); + + assertThat(parsedConfigs, equalTo(expectedConfigs)); + } + + @Test + public void whenInputIsNullOrEmpty_shouldReturnEmptyList() { + assertThat(parser.getConfigurations(null), equalTo(List.of())); + assertThat(parser.getConfigurations(""), equalTo(List.of())); + } + + @Test + public void whenInputIsEmptyArray_shouldReturnEmptyList() { + assertThat(parser.getConfigurations("[]"), equalTo(List.of())); + } + + @Test + public void whenConfigKeyInInputHasNoClassMapping_shouldIgnoreKey() { + final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}," + + "{\"alias\":\"some.missing.config.key\",\"defaultValue\":\"some other value\"}]"; + + final List> parsedConfigs = parser.getConfigurations(input); + + assertThat(parsedConfigs.stream().map(config -> (String) config.getDefaultValue()).collect(toList()), equalTo(List.of("someValue"))); + } + + @Test + public void whenInputCannotBeParsedToJson_shouldThrowException() { + final String input = "some invalid input"; + + assertThrows(UncheckedIOException.class, () -> parser.getConfigurations(input)); + } + + @Test + public void whenInputIsStructuredConfigWithConditions_shouldDeserializeTypedConfigurationWithConditions() { + final BaiganConfigClasses structuredConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", StructuredConfig.class)); + final ConfigurationParser parser = new ConfigurationParser(structuredConfigClasses, objectMapper); + + final String input = "[{\"alias\":\"some.config.some.key\",\"description\":\"a description\"," + + "\"defaultValue\":{\"someConfig\":\"some value\",\"someOtherConfig\":1}," + + "\"conditions\":[{\"paramName\":\"some param name\",\"conditionType\":{\"type\":\"Equals\",\"onValue\":\"some value\"}," + + "\"value\":{\"someConfig\":\"some conditional value\",\"someOtherConfig\":-1}}]}]"; + + final List> parsedConfigs = parser.getConfigurations(input); + + final Configuration expectedConfig = new Configuration<>( + "some.config.some.key", + "a description", + Set.of(new Condition<>("some param name", new Equals("some value"), new StructuredConfig("some conditional value", -1))), + new StructuredConfig("some value", 1) + ); + + assertThat(parsedConfigs, equalTo(List.of(expectedConfig))); + } + + @Test + public void whenDefaultValueCannotBeParsed_shouldThrowException() { + final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":{}}]"; + + assertThrows(RuntimeException.class, () -> parser.getConfigurations(input)); + } + + @Test + public void whenConditionValueCannotBeParsed_shouldThrowException() { + final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"some value\"," + + "\"conditions\":[{\"paramName\":\"some param name\",\"conditionType\":{\"type\":\"Equals\",\"onValue\":\"some value\"},\"value\":{}}]}]"; + + assertThrows(RuntimeException.class, () -> parser.getConfigurations(input)); + } + + private static class StructuredConfig { + private final String someConfig; + private final int someOtherConfig; + + @JsonCreator + private StructuredConfig(@JsonProperty("someConfig") String someConfig, @JsonProperty("someOtherConfig") int someOtherConfig) { + this.someConfig = someConfig; + this.someOtherConfig = someOtherConfig; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + StructuredConfig that = (StructuredConfig) o; + return someOtherConfig == that.someOtherConfig && Objects.equals(someConfig, that.someConfig); + } + + @Override + public int hashCode() { + return Objects.hash(someConfig, someOtherConfig); + } + + @Override + public String toString() { + return new StringJoiner(", ", StructuredConfig.class.getSimpleName() + "[", "]") + .add("someConfig='" + someConfig + "'") + .add("someOtherConfig=" + someOtherConfig) + .toString(); + } + } +} diff --git a/src/test/java/org/zalando/baigan/service/github/GitCacheLoaderTest.java b/src/test/java/org/zalando/baigan/service/github/GitCacheLoaderTest.java index 10b9484..eb3b623 100644 --- a/src/test/java/org/zalando/baigan/service/github/GitCacheLoaderTest.java +++ b/src/test/java/org/zalando/baigan/service/github/GitCacheLoaderTest.java @@ -5,32 +5,45 @@ import org.apache.commons.codec.binary.Base64; import org.eclipse.egit.github.core.RepositoryContents; import org.eclipse.egit.github.core.service.ContentsService; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.zalando.baigan.model.Configuration; import org.mockito.junit.jupiter.MockitoExtension; +import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.service.ConfigurationParser; import java.io.IOException; import java.security.MessageDigest; +import java.util.List; import java.util.Map; +import java.util.Set; -import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public class GitCacheLoaderTest { - private String testConfiguration1 = "[{ \"alias\": \"express.feature.toggle\", \"description\": \"Feature toggle\", \"defaultValue\": false, \"conditions\": [ { " - + " \"value\": true, \"conditionType\": { \"onValue\": \"3\", \"type\": \"Equals\" }, \"paramName\": \"appdomain\" } ] }]"; + private static final String config1Json = "[{ \"alias\": \"express.feature.toggle\", \"description\": \"Feature toggle\", \"defaultValue\": false}]"; + private static final String config2Json = "[{ \"alias\": \"express.feature.toggle\", \"description\": \"Feature toggle\", \"defaultValue\": false}," + + "{ \"alias\": \"some.other.config\", \"description\": \"Other config\", \"defaultValue\": \"a value\"}]"; + private static final Configuration expressFeatureToggle = new Configuration<>("express.feature.toggle", "Feature toggle", Set.of(), false); + private static final Configuration someOtherConfig = new Configuration<>("some.other.config", "Other config", Set.of(), "a value"); + + private final ConfigurationParser configurationParser = mock(ConfigurationParser.class); - private String testConfiguration2 = "[{ \"alias\": \"express.feature.toggle\", \"description\": \"Feature toggle\", \"defaultValue\": false, \"conditions\": [ { " - + " \"value\": true, \"conditionType\": { \"onValue\": \"3\", \"type\": \"Equals\" }, \"paramName\": \"appdomain\" } ] }," - + "{ \"alias\": \"express.feature.serviceUrl\", \"description\": \"Feature Service Url\", \"defaultValue\": \"\", \"conditions\": [ { " - + " \"value\": \"http://express.trucks.zalan.do\", \"conditionType\": { \"onValue\": \"1\", \"type\": \"Equals\" }, " - + " \"paramName\": \"appdomain\" } ] }]"; + + @BeforeEach + public void setup() { + when(configurationParser.getConfigurations(config1Json)).thenReturn(List.of(expressFeatureToggle)); + when(configurationParser.getConfigurations(config2Json)).thenReturn(List.of(expressFeatureToggle, someOtherConfig)); + } @Test public void testLoad() throws Exception { @@ -39,19 +52,18 @@ public void testLoad() throws Exception { "somerepo", "master", "somefile", "aoth_token"); final ContentsService contentService = mock(ContentsService.class); - final GitCacheLoader loader = new GitCacheLoader(config, - contentService); + final GitCacheLoader loader = new GitCacheLoader(config, contentService, configurationParser); + + final RepositoryContents repositoryContents = createRepositoryContents(config1Json); when(contentService.getContents(any(), eq("staging.json"), eq("master"))) .thenReturn(ImmutableList - .of(createRepositoryContents(testConfiguration1))); - - final Map configurations = loader.load("staging.json"); - assertThat(configurations.size(), equalTo(1)); - assertThat(configurations.get("express.feature.toggle"), notNullValue()); + .of(repositoryContents)); + final Map> configurations = loader.load("staging.json"); + assertThat(configurations, equalTo(Map.of("express.feature.toggle", expressFeatureToggle))); } @Test @@ -61,36 +73,29 @@ public void testReload() throws Exception { "somerepo", "master", "somefile", "aoth_token"); final ContentsService contentService = mock(ContentsService.class); - final GitCacheLoader loader = new GitCacheLoader(config, - contentService); + final GitCacheLoader loader = new GitCacheLoader(config, contentService, configurationParser); when(contentService.getContents(any(), eq("staging.json"), eq("master"))) .thenReturn(ImmutableList - .of(createRepositoryContents(testConfiguration1))); + .of(createRepositoryContents(config1Json))); - Map configurations = loader.load("staging.json"); - assertThat(configurations.size(), equalTo(1)); - assertThat(configurations.get("express.feature.toggle"), notNullValue()); + Map> configurations = loader.load("staging.json"); + assertThat(configurations, equalTo(Map.of("express.feature.toggle", expressFeatureToggle))); when(contentService.getContents(any(), eq("staging.json"), eq("master"))) .thenReturn(ImmutableList - .of(createRepositoryContents(testConfiguration2))); + .of(createRepositoryContents(config2Json))); - final ListenableFuture> configurations2Future = loader + final ListenableFuture>> configurations2Future = loader .reload("staging.json", configurations); - final Map configurations2 = configurations2Future + final Map> configurations2 = configurations2Future .get(); - assertThat(configurations2, not(configurations)); - - assertThat(configurations2.size(), equalTo(2)); - assertThat(configurations.get("express.feature.toggle"), notNullValue()); - - assertThat(configurations2.get("express.feature.serviceUrl"), notNullValue()); + assertThat(configurations2, equalTo(Map.of("express.feature.toggle", expressFeatureToggle, "some.other.config", someOtherConfig))); } @Test @@ -100,47 +105,47 @@ public void testForegoReloadForUnchanged() throws Exception { "somerepo", "master", "somefile", "aoth_token"); final ContentsService contentService = mock(ContentsService.class); - final GitCacheLoader loader = new GitCacheLoader(config, contentService); + final GitCacheLoader loader = new GitCacheLoader(config, contentService, configurationParser); when(contentService.getContents(any(), eq("staging.json"), eq("master"))) .thenReturn(ImmutableList - .of(createRepositoryContents(testConfiguration2))); + .of(createRepositoryContents(config2Json))); - final Map configurations1 = loader.load("staging.json"); + final Map> configurations1 = loader.load("staging.json"); when(contentService.getContents(any(), eq("staging.json"), eq("master"))) .thenReturn(ImmutableList - .of(createRepositoryContents(testConfiguration2))); + .of(createRepositoryContents(config2Json))); - final ListenableFuture> configurations2Future = loader + final ListenableFuture>> configurations2Future = loader .reload("staging.json", configurations1); - final Map configurations2 = configurations2Future.get(); + final Map> configurations2 = configurations2Future.get(); assertThat(configurations2, equalTo(configurations1)); } @Test - public void testReloadWithIOExceptionInConcentService() throws Exception { + public void testReloadWithIOExceptionInContentService() throws Exception { final GitConfig config = new GitConfig("somehost", "someowner", "somerepo", "master", "somefile", "aoth_token"); final ContentsService contentService = mock(ContentsService.class); - final GitCacheLoader loader = new GitCacheLoader(config, contentService); + final GitCacheLoader loader = new GitCacheLoader(config, contentService, configurationParser); when( contentService.getContents(any(), eq("staging.json"), eq("master"))) .thenReturn(ImmutableList - .of(createRepositoryContents(testConfiguration1))); + .of(createRepositoryContents(config1Json))); - Map configurations = loader.load("staging.json"); + Map> configurations = loader.load("staging.json"); assertThat(configurations.size(), equalTo(1)); assertThat(configurations.get("express.feature.toggle"), notNullValue()); @@ -151,14 +156,30 @@ public void testReloadWithIOExceptionInConcentService() throws Exception { eq("staging.json"), eq("master")); - final ListenableFuture> configurations2Future = loader + final ListenableFuture>> configurations2Future = loader .reload("staging.json", configurations); - final Map configurations2 = configurations2Future.get(); + final Map> configurations2 = configurations2Future.get(); assertThat(configurations2, equalTo(configurations)); } + @Test + public void whenContentServiceFailsOnInitialLoad_shouldInitializeWithEmptyMap() throws Exception { + final GitConfig config = new GitConfig("somehost", "someowner", + "somerepo", "master", "somefile", "aoth_token"); + final ContentsService contentService = mock(ContentsService.class); + + final GitCacheLoader loader = new GitCacheLoader(config, contentService, configurationParser); + + doThrow(new IOException()).when(contentService).getContents(any(), + eq("staging.json"), + eq("master")); + + final Map> configurations = loader.load("staging.json"); + assertThat(configurations, equalTo(Map.of())); + } + private RepositoryContents createRepositoryContents(final String text) throws Exception { final RepositoryContents contents = new RepositoryContents(); byte[] base64OfConfig1 = Base64.encodeBase64(text.getBytes()); From de5bd8e7f2b2766ee17b1d4daad820673151f235 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Fri, 29 Sep 2023 11:34:08 +0200 Subject: [PATCH 05/16] increase test coverage --- ...figurationBeanDefinitionRegistrarTest.java | 28 +++- ...figurationMethodInvocationHandlerTest.java | 127 ++++++++++++++++++ 2 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandlerTest.java diff --git a/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java b/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java index 03e0c5b..1279f4b 100644 --- a/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java +++ b/src/test/java/org/zalando/baigan/context/ConfigurationBeanDefinitionRegistrarTest.java @@ -34,6 +34,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -42,22 +43,19 @@ */ public class ConfigurationBeanDefinitionRegistrarTest { + private final AnnotationMetadata metaData = Mockito.mock(AnnotationMetadata.class); + private final BeanDefinitionRegistry registry = Mockito.mock(BeanDefinitionRegistry.class); + private final ConfigurationBeanDefinitionRegistrar registrar = new ConfigurationBeanDefinitionRegistrar(); + @Test public void testRegistration() { - final ConfigurationBeanDefinitionRegistrar registrar = new ConfigurationBeanDefinitionRegistrar(); - - final AnnotationMetadata metaData = Mockito - .mock(AnnotationMetadata.class); Mockito.when(metaData.getAnnotationAttributes( ConfigurationServiceScan.class.getName())) .thenReturn(ImmutableMap.of("value", new String[]{"org.zalando.baigan.context"}, "basePackages", new String[]{})); - final BeanDefinitionRegistry registry = Mockito - .mock(BeanDefinitionRegistry.class); - final ArgumentCaptor beanDefinition = ArgumentCaptor .forClass(AbstractBeanDefinition.class); final ArgumentCaptor beanName = ArgumentCaptor @@ -78,6 +76,22 @@ public void testRegistration() { } + @Test + public void whenNothingAnnotatedWithConfigurationServiceScan_shouldThrowException() { + Mockito.when(metaData.getAnnotationAttributes( + ConfigurationServiceScan.class.getName())) + .thenReturn(null); + + assertThrows(IllegalArgumentException.class, () -> registrar.registerBeanDefinitions(metaData, registry)); + + Mockito.when(metaData.getAnnotationAttributes( + ConfigurationServiceScan.class.getName())) + .thenReturn(ImmutableMap.of()); + + assertThrows(IllegalArgumentException.class, () -> registrar.registerBeanDefinitions(metaData, registry)); + + } + } @BaiganConfig diff --git a/src/test/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandlerTest.java b/src/test/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandlerTest.java new file mode 100644 index 0000000..d8701c7 --- /dev/null +++ b/src/test/java/org/zalando/baigan/proxy/handler/ContextAwareConfigurationMethodInvocationHandlerTest.java @@ -0,0 +1,127 @@ +package org.zalando.baigan.proxy.handler; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.BeanFactory; +import org.zalando.baigan.context.ContextProviderRetriever; +import org.zalando.baigan.model.Configuration; +import org.zalando.baigan.provider.ContextProvider; +import org.zalando.baigan.service.ConditionsProcessor; +import org.zalando.baigan.service.ConfigurationRepository; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ContextAwareConfigurationMethodInvocationHandlerTest { + + private static final String key = "test.interface.get.some.value"; + private static final String expectedConfigValue = "some value"; + private static final Configuration config = new Configuration<>(key, "description", Set.of(), expectedConfigValue); + + private final ConfigurationRepository repository = mock(ConfigurationRepository.class); + private final ConditionsProcessor conditionsProcessor = mock(ConditionsProcessor.class); + private final ContextProviderRetriever contextProviderRetriever = mock(ContextProviderRetriever.class); + private final BeanFactory beanFactory = mock(BeanFactory.class); + private final ContextAwareConfigurationMethodInvocationHandler handler = new ContextAwareConfigurationMethodInvocationHandler(); + + @BeforeEach + public void setup() { + when(beanFactory.getBean(ConfigurationRepository.class)).thenReturn(repository); + when(beanFactory.getBean(ConditionsProcessor.class)).thenReturn(conditionsProcessor); + when(beanFactory.getBean(ContextProviderRetriever.class)).thenReturn(contextProviderRetriever); + handler.setBeanFactory(beanFactory); + } + + @Test + public void whenConfigurationRepositoryReturnsEmpty_shouldReturnNull() { + when(repository.get(key)).thenReturn(Optional.empty()); + final Object result = handler.handleInvocation((TestInterface) () -> null, TestInterface.class.getDeclaredMethods()[0], new Object[0]); + assertThat(result, nullValue()); + } + + @Test + public void whenContextIsEmpty_shouldReturnValueFromConditionsProcessorCalledWithEmptyContext() { + final Configuration configWithWrongType = new Configuration<>(key, "description", Set.of(), 1); + when(repository.get(key)).thenReturn(Optional.of(configWithWrongType)); + when(contextProviderRetriever.getContextParameterKeys()).thenReturn(Set.of()); + when(conditionsProcessor.process(configWithWrongType, Map.of())).thenReturn(1); + final Object result = handler.handleInvocation((TestInterface) () -> null, TestInterface.class.getDeclaredMethods()[0], new Object[0]); + assertThat(result, nullValue()); + } + + @Test + public void whenConfigReturnTypeDoesNotMatchMethodReturnType_shouldReturnNull() { + when(repository.get(key)).thenReturn(Optional.of(config)); + when(contextProviderRetriever.getContextParameterKeys()).thenReturn(Set.of()); + when(conditionsProcessor.process(config, Map.of())).thenReturn(expectedConfigValue); + final Object result = handler.handleInvocation((TestInterface) () -> null, TestInterface.class.getDeclaredMethods()[0], new Object[0]); + assertThat(result, equalTo(expectedConfigValue)); + } + + @Test + public void whenContextProvidersExist_shouldReturnValueFromConditionsProcessorCalledWithResultingContext() { + when(repository.get(key)).thenReturn(Optional.of(config)); + + final String param1 = "param1"; + final String param2 = "param2"; + when(contextProviderRetriever.getContextParameterKeys()).thenReturn(Set.of(param1, param2)); + + final ContextProvider contextProvider1 = mock(ContextProvider.class); + when(contextProvider1.getContextParam(param1)).thenReturn("value1"); + when(contextProviderRetriever.getProvidersFor(param1)).thenReturn(Set.of(contextProvider1)); + + final ContextProvider contextProvider2 = mock(ContextProvider.class); + when(contextProvider2.getContextParam(param2)).thenReturn("value2"); + when(contextProviderRetriever.getProvidersFor(param2)).thenReturn(Set.of(contextProvider2)); + + when(conditionsProcessor.process(config, Map.of(param1, "value1", param2, "value2"))).thenReturn(expectedConfigValue); + final Object result = handler.handleInvocation((TestInterface) () -> null, TestInterface.class.getDeclaredMethods()[0], new Object[0]); + assertThat(result, equalTo(expectedConfigValue)); + } + + @Test + public void whenContextProvidersAreEmpty_shouldReturnValueFromConditionsProcessorCalledWithEmptyContext() { + when(repository.get(key)).thenReturn(Optional.of(config)); + + final String param = "param"; + when(contextProviderRetriever.getContextParameterKeys()).thenReturn(Set.of(param)); + when(contextProviderRetriever.getProvidersFor(param)).thenReturn(List.of()); + + when(conditionsProcessor.process(config, Map.of())).thenReturn(expectedConfigValue); + final Object result = handler.handleInvocation((TestInterface) () -> null, TestInterface.class.getDeclaredMethods()[0], new Object[0]); + assertThat(result, equalTo(expectedConfigValue)); + } + + + // FIXME this behavior seems strange + @Test + public void whenThereAreMultipleContextProvidersForOneParam_shouldConsiderOnlyTheFirstContextProvider() { + when(repository.get(key)).thenReturn(Optional.of(config)); + + final String param = "param"; + when(contextProviderRetriever.getContextParameterKeys()).thenReturn(Set.of(param)); + + final ContextProvider contextProvider1 = mock(ContextProvider.class); + when(contextProvider1.getContextParam(param)).thenReturn("value1"); + + final ContextProvider contextProvider2 = mock(ContextProvider.class); + when(contextProvider2.getContextParam("some ignored param")).thenReturn("some ignored value"); + when(contextProviderRetriever.getProvidersFor(param)).thenReturn(List.of(contextProvider1, contextProvider2)); + + when(conditionsProcessor.process(config, Map.of(param, "value1"))).thenReturn(expectedConfigValue); + final Object result = handler.handleInvocation((TestInterface) () -> null, TestInterface.class.getDeclaredMethods()[0], new Object[0]); + assertThat(result, equalTo(expectedConfigValue)); + } + + interface TestInterface { + String getSomeValue(); + } +} From b7fff3cca7eaf3ed1c37dd013884b31ee7d1f7e4 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Fri, 6 Oct 2023 08:49:36 +0200 Subject: [PATCH 06/16] process generics in config types correctly This commit adds support for handling generics in the retunr type of a config method. Instead of parsing `Map>` to a `Map>>`, it is now correctly processed to the expected type. This works by using `Type` instead of `Class` in `BaiganConfigClasses` and actually using the `AnnotatedType` instead of the plain type when constructing the bean in `ConfigurationBeanRegistrar`. --- .../baigan/proxy/BaiganConfigClasses.java | 11 +++++---- .../ConfigurationBeanDefinitionRegistrar.java | 10 ++++---- .../baigan/service/ConfigurationParser.java | 7 +++--- .../FileSystemConfigurationRepository.java | 3 +-- .../baigan/e2e/configs/SomeConfiguration.java | 5 ++++ ...ystemConfigurationRepositoryEnd2EndIT.java | 21 +++++++++++++++++ .../service/ConfigurationParserTest.java | 23 ++++++++++++++++++- ...FileSystemConfigurationRepositoryTest.java | 3 ++- 8 files changed, 66 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java index 0b956da..bcd959f 100644 --- a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java +++ b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java @@ -2,28 +2,29 @@ import org.springframework.stereotype.Component; +import java.lang.reflect.Type; import java.util.Map; @Component public class BaiganConfigClasses { - private Map> configTypesByKey; + private Map configTypesByKey; public BaiganConfigClasses() {} - public BaiganConfigClasses(Map> configTypesByKey) { + public BaiganConfigClasses(Map configTypesByKey) { this.configTypesByKey = configTypesByKey; } - public void setConfigTypesByKey(Map> configTypesByKey) { + public void setConfigTypesByKey(Map configTypesByKey) { configTypesByKey.forEach((key, value) -> { - if (value.isPrimitive()) { + if (value.getClass().isPrimitive()) { throw new IllegalArgumentException("Config " + key + " has an illegal return type " + value + ". Primitives are not supported as return type."); } }); this.configTypesByKey = configTypesByKey; } - public Map> getConfigTypesByKey() { + public Map getConfigTypesByKey() { return configTypesByKey; } } diff --git a/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java b/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java index e1d579c..d038555 100644 --- a/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java +++ b/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java @@ -31,7 +31,7 @@ import org.zalando.baigan.annotation.BaiganConfig; import org.zalando.baigan.annotation.ConfigurationServiceScan; -import java.lang.reflect.Method; +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -106,8 +106,8 @@ private void createAndRegisterBeanDefinitions(final Set packages, } } } - Map> configTypesByKey = baiganConfigClasses.stream().flatMap(clazz -> - Arrays.stream(clazz.getMethods()).map(method -> new ConfigType(createKey(clazz, method), method.getReturnType())) + Map configTypesByKey = baiganConfigClasses.stream().flatMap(clazz -> + Arrays.stream(clazz.getMethods()).map(method -> new ConfigType(createKey(clazz, method), method.getAnnotatedReturnType().getType())) ).collect(toMap(c -> c.key, c -> c.type)); GenericBeanDefinition beanDefinition = new GenericBeanDefinition(); beanDefinition.setBeanClass(BaiganConfigClasses.class); @@ -153,9 +153,9 @@ protected boolean isCandidateComponent(AnnotatedBeanDefinition beanDefinition) { private static class ConfigType { private final String key; - private final Class type; + private final Type type; - public ConfigType(String key, Class type) { + public ConfigType(String key, Type type) { this.key = key; this.type = type; } diff --git a/src/main/java/org/zalando/baigan/service/ConfigurationParser.java b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java index 55780d0..f5aa8f0 100644 --- a/src/main/java/org/zalando/baigan/service/ConfigurationParser.java +++ b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java @@ -13,6 +13,7 @@ import javax.annotation.Nonnull; import java.io.IOException; import java.io.UncheckedIOException; +import java.lang.reflect.Type; import java.util.List; import java.util.Optional; import java.util.Set; @@ -61,16 +62,16 @@ public List> getConfigurations(final String text) { } } - private Configuration deserializeConfig(Configuration config, Class targetClass) { + private Configuration deserializeConfig(Configuration config, Type targetClass) { Set> typedConditions = Optional.ofNullable(config.getConditions()).orElse(Set.of()).stream().map(c -> { try { - return new Condition<>(c.getParamName(), c.getConditionType(), objectMapper.treeToValue(c.getValue(), targetClass)); + return new Condition<>(c.getParamName(), c.getConditionType(), objectMapper.treeToValue(c.getValue(), objectMapper.constructType(targetClass))); } catch (JsonProcessingException e) { throw new RuntimeException(e); } }).collect(toSet()); try { - T typedDefaultValue = objectMapper.treeToValue(config.getDefaultValue(), targetClass); + T typedDefaultValue = objectMapper.treeToValue(config.getDefaultValue(), objectMapper.constructType(targetClass)); return new Configuration<>(config.getAlias(), config.getDescription(), typedConditions, typedDefaultValue); } catch (JsonProcessingException e) { throw new RuntimeException(e); diff --git a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java index ef9d6ec..846dad2 100644 --- a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java @@ -30,7 +30,6 @@ * * @author mchand */ -// TODO E2E test public class FileSystemConfigurationRepository implements ConfigurationRepository { private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); @@ -41,7 +40,7 @@ public class FileSystemConfigurationRepository implements ConfigurationRepositor private final String fileName; public FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds, final BaiganConfigClasses baiganConfigClasses) { - this.configurationParser = new ConfigurationParser(baiganConfigClasses, objectMapper); + this.configurationParser = new ConfigurationParser(baiganConfigClasses, objectMapper); this.fileName = fileName; cachedConfigurations = CacheBuilder.newBuilder() diff --git a/src/test/java/org/zalando/baigan/e2e/configs/SomeConfiguration.java b/src/test/java/org/zalando/baigan/e2e/configs/SomeConfiguration.java index 1711a38..e6890f1 100644 --- a/src/test/java/org/zalando/baigan/e2e/configs/SomeConfiguration.java +++ b/src/test/java/org/zalando/baigan/e2e/configs/SomeConfiguration.java @@ -2,9 +2,14 @@ import org.zalando.baigan.annotation.BaiganConfig; +import java.util.List; +import java.util.Map; +import java.util.UUID; + @BaiganConfig public interface SomeConfiguration { SomeConfigObject someConfig(); String someValue(); Boolean isThisTrue(); + Map> topLevelGenerics(); } diff --git a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java index f07a3cd..9e0057b 100644 --- a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java @@ -19,6 +19,9 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.UUID; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -58,6 +61,24 @@ public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfiguratio assertThat(someConfiguration.someValue(), equalTo("some value")); } + @Test + public void givenS3Configuration_whenInvalidConfigurationIsProvided_thenTheConfigurationIsNotUpdated() { + // TODO + } + + @Test + public void givenS3Configuration_whenConfigurationTypeIsGeneric_thenDeserializesProperly() throws IOException, InterruptedException { + Files.writeString(configFile, "[{\"alias\": \"some.configuration.top.level.generics\",\"defaultValue\": {" + + "\"a8a23682-1623-450b-8817-50c98827ea4e\": [{\"config_key\":\"A\"}]," + + "\"76ced443-6555-4748-a22e-8700f3864e59\": [{\"config_key\":\"B\"}]}" + + "}]"); + Thread.sleep(1100); + assertThat(someConfiguration.topLevelGenerics(), equalTo(Map.of( + UUID.fromString("a8a23682-1623-450b-8817-50c98827ea4e"), List.of(new SomeConfigObject("A")), + UUID.fromString("76ced443-6555-4748-a22e-8700f3864e59"), List.of(new SomeConfigObject("B")) + ))); + } + @ConfigurationServiceScan(basePackages = "org.zalando.baigan.e2e.configs") @Testcontainers @ComponentScan(basePackageClasses = {BaiganSpringContext.class}) diff --git a/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java index 34ae38b..26082dc 100644 --- a/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java +++ b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java @@ -15,6 +15,7 @@ import java.util.Objects; import java.util.Set; import java.util.StringJoiner; +import java.util.UUID; import static java.util.stream.Collectors.toList; import static org.hamcrest.MatcherAssert.assertThat; @@ -127,7 +128,27 @@ public void whenConditionValueCannotBeParsed_shouldThrowException() { assertThrows(RuntimeException.class, () -> parser.getConfigurations(input)); } - private static class StructuredConfig { + @Test + public void whenConfigurationTypeHasGenericsWithAnnotatedType_shouldParseCorrectly() throws NoSuchMethodException { + final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":{" + + "\"a8a23682-1623-450b-8817-50c98827ea4e\": [{\"someConfig\":\"A\",\"someOtherConfig\":1}]," + + "\"76ced443-6555-4748-a22e-8700f3864e59\": [{\"someConfig\":\"B\"}]}" + + "}]"; + + final BaiganConfigClasses stringConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", ParameterizedConfig.class.getMethod("getConfig").getAnnotatedReturnType().getType())); + final ConfigurationParser parser = new ConfigurationParser(stringConfigClasses, objectMapper); + + assertThat(parser.getConfigurations(input).get(0).getDefaultValue(), equalTo(Map.of( + UUID.fromString("a8a23682-1623-450b-8817-50c98827ea4e"), List.of(new StructuredConfig("A", 1)), + UUID.fromString("76ced443-6555-4748-a22e-8700f3864e59"), List.of(new StructuredConfig("B", 0)) + ))); + } + + interface ParameterizedConfig { + Map> getConfig(); + } + + static class StructuredConfig { private final String someConfig; private final int someOtherConfig; diff --git a/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java b/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java index 26e73e7..d07bab3 100644 --- a/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java +++ b/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java @@ -3,6 +3,7 @@ import org.junit.jupiter.api.Test; import org.zalando.baigan.proxy.BaiganConfigClasses; +import java.lang.reflect.Type; import java.util.Map; import static org.hamcrest.CoreMatchers.equalTo; @@ -12,7 +13,7 @@ public class FileSystemConfigurationRepositoryTest { @Test public void testReadConfigurationsFromFile() { - final Map> configTypesByKey = Map.of( + final Map configTypesByKey = Map.of( "express.feature.toggle", Boolean.class, "express.feature.service.url", String.class, "pession.sync.feature.toggle", Boolean.class From 4c645eeaea7d4e72f27f6b6dd050c1019c2f1646 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Fri, 6 Oct 2023 08:52:16 +0200 Subject: [PATCH 07/16] fix to use generic return type instead of annotated return type --- .../baigan/proxy/ConfigurationBeanDefinitionRegistrar.java | 2 +- .../org/zalando/baigan/service/ConfigurationParserTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java b/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java index d038555..12e001e 100644 --- a/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java +++ b/src/main/java/org/zalando/baigan/proxy/ConfigurationBeanDefinitionRegistrar.java @@ -107,7 +107,7 @@ private void createAndRegisterBeanDefinitions(final Set packages, } } Map configTypesByKey = baiganConfigClasses.stream().flatMap(clazz -> - Arrays.stream(clazz.getMethods()).map(method -> new ConfigType(createKey(clazz, method), method.getAnnotatedReturnType().getType())) + Arrays.stream(clazz.getMethods()).map(method -> new ConfigType(createKey(clazz, method), method.getGenericReturnType())) ).collect(toMap(c -> c.key, c -> c.type)); GenericBeanDefinition beanDefinition = new GenericBeanDefinition(); beanDefinition.setBeanClass(BaiganConfigClasses.class); diff --git a/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java index 26082dc..ca3a952 100644 --- a/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java +++ b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java @@ -129,13 +129,13 @@ public void whenConditionValueCannotBeParsed_shouldThrowException() { } @Test - public void whenConfigurationTypeHasGenericsWithAnnotatedType_shouldParseCorrectly() throws NoSuchMethodException { + public void whenConfigurationTypeHasGenerics_shouldParseCorrectly() throws NoSuchMethodException { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":{" + "\"a8a23682-1623-450b-8817-50c98827ea4e\": [{\"someConfig\":\"A\",\"someOtherConfig\":1}]," + "\"76ced443-6555-4748-a22e-8700f3864e59\": [{\"someConfig\":\"B\"}]}" + "}]"; - final BaiganConfigClasses stringConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", ParameterizedConfig.class.getMethod("getConfig").getAnnotatedReturnType().getType())); + final BaiganConfigClasses stringConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", ParameterizedConfig.class.getMethod("getConfig").getGenericReturnType())); final ConfigurationParser parser = new ConfigurationParser(stringConfigClasses, objectMapper); assertThat(parser.getConfigurations(input).get(0).getDefaultValue(), equalTo(Map.of( From 08046509dca95fa1f343babb156e197590ae3c40 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Mon, 9 Oct 2023 23:15:03 +0200 Subject: [PATCH 08/16] simplify repo construction Repositories no longer require a `BaiganConfigClasses` instance upon construction. Instead, this class is hidden by a `ConfigTypeProvider` and a `ConfigurationParser` bean is provided to a `ConfigurationRepository` by the latter implementing `ApplicationContextAware`. --- .../baigan/proxy/BaiganConfigClasses.java | 9 +--- .../baigan/proxy/ConfigTypeProvider.java | 21 ++++++++++ .../baigan/service/ConfigurationParser.java | 18 ++++---- .../service/ConfigurationRepository.java | 8 +++- .../service/EtcdConfigurationRepository.java | 5 +-- .../FileSystemConfigurationRepository.java | 27 +++++++----- ...eSystemConfigurationRepositoryBuilder.java | 18 +------- .../aws/S3ConfigurationRepository.java | 41 ++++++++++--------- .../aws/S3ConfigurationRepositoryBuilder.java | 24 +---------- ...ystemConfigurationRepositoryEnd2EndIT.java | 12 ++++-- .../S3ConfigurationRepositoryEnd2EndIT.java | 4 +- .../service/ConfigurationParserTest.java | 37 +++++++++-------- ...FileSystemConfigurationRepositoryTest.java | 24 ----------- 13 files changed, 111 insertions(+), 137 deletions(-) create mode 100644 src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java delete mode 100644 src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java diff --git a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java index bcd959f..f72a12f 100644 --- a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java +++ b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java @@ -1,20 +1,13 @@ package org.zalando.baigan.proxy; -import org.springframework.stereotype.Component; - import java.lang.reflect.Type; import java.util.Map; -@Component -public class BaiganConfigClasses { +class BaiganConfigClasses { private Map configTypesByKey; public BaiganConfigClasses() {} - public BaiganConfigClasses(Map configTypesByKey) { - this.configTypesByKey = configTypesByKey; - } - public void setConfigTypesByKey(Map configTypesByKey) { configTypesByKey.forEach((key, value) -> { if (value.getClass().isPrimitive()) { diff --git a/src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java b/src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java new file mode 100644 index 0000000..c628047 --- /dev/null +++ b/src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java @@ -0,0 +1,21 @@ +package org.zalando.baigan.proxy; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +import java.lang.reflect.Type; + +@Component +public class ConfigTypeProvider { + + private final BaiganConfigClasses baiganConfigClasses; + + @Autowired + ConfigTypeProvider(BaiganConfigClasses baiganConfigClasses) { + this.baiganConfigClasses = baiganConfigClasses; + } + + public Type getType(final String configKey) { + return baiganConfigClasses.getConfigTypesByKey().get(configKey); + } +} diff --git a/src/main/java/org/zalando/baigan/service/ConfigurationParser.java b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java index f5aa8f0..69b5c2f 100644 --- a/src/main/java/org/zalando/baigan/service/ConfigurationParser.java +++ b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java @@ -6,9 +6,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; -import org.zalando.baigan.proxy.BaiganConfigClasses; +import org.zalando.baigan.proxy.ConfigTypeProvider; import javax.annotation.Nonnull; import java.io.IOException; @@ -18,21 +20,21 @@ import java.util.Optional; import java.util.Set; -import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; +@Component public class ConfigurationParser { private final Logger LOG = LoggerFactory .getLogger(ConfigurationParser.class); final ObjectMapper objectMapper; - final BaiganConfigClasses baiganConfigClasses; + final ConfigTypeProvider configTypeProvider; - // TODO define package structure so this is not public - public ConfigurationParser(final BaiganConfigClasses baiganConfigClasses, final ObjectMapper objectMapper) { - this.baiganConfigClasses = requireNonNull(baiganConfigClasses, "baiganConfigClasses has to be not null. Get them from the bean of the same name."); - this.objectMapper = requireNonNull(objectMapper, "objectMapper has to be not null."); + @Autowired + public ConfigurationParser(final ConfigTypeProvider configTypeProvider, final Optional objectMapper) { + this.configTypeProvider = configTypeProvider; + this.objectMapper = objectMapper.orElseGet(ObjectMapper::new); } @Nonnull @@ -46,7 +48,7 @@ public List> getConfigurations(final String text) { }); return rawConfigs.stream() .map(config -> { - final Optional> typedConfig = Optional.ofNullable(baiganConfigClasses.getConfigTypesByKey().get(config.getAlias())) + final Optional> typedConfig = Optional.ofNullable(configTypeProvider.getType(config.getAlias())) .map(targetClass -> deserializeConfig(config, targetClass)); if (typedConfig.isEmpty()) { LOG.info("Alias {} does not match any method in a class annotated with @BaiganConfig.", config.getAlias()); diff --git a/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java index cae30c4..d6a3cb8 100644 --- a/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java @@ -1,6 +1,9 @@ package org.zalando.baigan.service; import com.google.common.base.Optional; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.zalando.baigan.model.Configuration; import javax.annotation.Nonnull; @@ -9,7 +12,7 @@ * @author mchand */ -public interface ConfigurationRepository { +public interface ConfigurationRepository extends ApplicationContextAware { /** * This method required Guava's Optional which will be deprecated in favor of Java's Optional @@ -28,4 +31,7 @@ default Optional getConfig(@Nonnull final String key) { void put(@Nonnull final String key, @Nonnull final String value); + @Override + default void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + } } diff --git a/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java index 6ce14b0..70e8829 100644 --- a/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/EtcdConfigurationRepository.java @@ -8,7 +8,6 @@ import org.slf4j.LoggerFactory; import org.zalando.baigan.etcd.service.EtcdClient; import org.zalando.baigan.model.Configuration; -import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; import java.io.IOException; @@ -29,13 +28,13 @@ public class EtcdConfigurationRepository { private EtcdClient etcdClient; @VisibleForTesting - public EtcdConfigurationRepository(final EtcdClient etcdClient, final BaiganConfigClasses baiganConfigClasses) { + public EtcdConfigurationRepository(final EtcdClient etcdClient) { checkArgument(etcdClient != null); this.etcdClient = etcdClient; } - public EtcdConfigurationRepository(final BaiganConfigClasses baiganConfigClasses) { + public EtcdConfigurationRepository() { etcdClient = new EtcdClient(getUrl()); } diff --git a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java index 70796cc..5d48ff6 100644 --- a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepository.java @@ -1,7 +1,5 @@ package org.zalando.baigan.service; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -9,8 +7,10 @@ import com.google.common.util.concurrent.ListenableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.zalando.baigan.model.Configuration; -import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; import java.io.IOException; @@ -30,18 +30,23 @@ * * @author mchand */ -public class FileSystemConfigurationRepository implements ConfigurationRepository { +public class FileSystemConfigurationRepository implements ConfigurationRepository, ApplicationContextAware { - private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final Logger LOG = LoggerFactory.getLogger(FileSystemConfigurationRepository.class); - private final ConfigurationParser configurationParser; - private final LoadingCache>> cachedConfigurations; + private ConfigurationParser configurationParser; + private LoadingCache>> cachedConfigurations; private final String fileName; + private final long refreshIntervalInSeconds; - FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds, final BaiganConfigClasses baiganConfigClasses) { - this.configurationParser = new ConfigurationParser(baiganConfigClasses, objectMapper); + FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds) { this.fileName = fileName; + this.refreshIntervalInSeconds = refreshIntervalInSeconds; + } + + @Override + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + this.configurationParser = applicationContext.getBean(ConfigurationParser.class); cachedConfigurations = CacheBuilder.newBuilder() .refreshAfterWrite(refreshIntervalInSeconds, TimeUnit.SECONDS) @@ -58,8 +63,8 @@ public Map> load(String filename) { @Override public ListenableFuture>> reload( - String key, Map> oldValue) - throws Exception { + String key, Map> oldValue) + throws Exception { LOG.info("Reloading the configuration from file [{}]", key); return super.reload(key, oldValue); } diff --git a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryBuilder.java index 52087ee..5e8f207 100644 --- a/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryBuilder.java @@ -1,20 +1,16 @@ package org.zalando.baigan.service; -import org.zalando.baigan.proxy.BaiganConfigClasses; - import static java.util.Objects.requireNonNull; /** * A builder to construct a {@link FileSystemConfigurationRepository}. *

- * Requires that at least {@link FileSystemConfigurationRepository::filePath} and - * {@link BaiganConfigClasses} are specified. + * Requires that at least {@link FileSystemConfigurationRepository::filePath} is specified. */ public class FileSystemConfigurationRepositoryBuilder { private String filePath; private long refreshIntervalInSeconds = 60L; - private BaiganConfigClasses baiganConfigClasses; /** * @param filePath The path to the file that contains the configuration data. @@ -33,18 +29,8 @@ public FileSystemConfigurationRepositoryBuilder refreshIntervalInSeconds(long re return this; } - /** - * @param baiganConfigClasses Contains the list of classes annotated with {@link org.zalando.baigan.annotation.BaiganConfig}. - * This is typically set as the Spring bean named "baiganConfigClasses" provided by the library. - */ - public FileSystemConfigurationRepositoryBuilder baiganConfigClasses(BaiganConfigClasses baiganConfigClasses) { - this.baiganConfigClasses = baiganConfigClasses; - return this; - } - public FileSystemConfigurationRepository build() { requireNonNull(filePath, "filePath must not be null"); - requireNonNull(baiganConfigClasses, "baiganConfigClasses must not be null"); - return new FileSystemConfigurationRepository(filePath, refreshIntervalInSeconds, baiganConfigClasses); + return new FileSystemConfigurationRepository(filePath, refreshIntervalInSeconds); } } diff --git a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java index 6f099a2..52c2703 100644 --- a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepository.java @@ -2,12 +2,13 @@ import com.amazonaws.services.kms.AWSKMS; import com.amazonaws.services.s3.AmazonS3; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.zalando.baigan.model.Configuration; -import org.zalando.baigan.proxy.BaiganConfigClasses; import org.zalando.baigan.service.ConfigurationParser; import org.zalando.baigan.service.ConfigurationRepository; @@ -21,11 +22,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -public class S3ConfigurationRepository implements ConfigurationRepository { +public class S3ConfigurationRepository implements ConfigurationRepository, ApplicationContextAware { private static final Logger LOG = LoggerFactory.getLogger(S3ConfigurationRepository.class); - private final ConfigurationParser configurationParser; + private ConfigurationParser configurationParser; private final S3FileLoader s3Loader; private final long refreshInterval; private final ScheduledThreadPoolExecutor executor; @@ -33,8 +34,7 @@ public class S3ConfigurationRepository implements ConfigurationRepository { S3ConfigurationRepository(@Nonnull final String bucketName, @Nonnull final String key, final long refreshInterval, final ScheduledThreadPoolExecutor executor, - final AmazonS3 s3Client, final AWSKMS kmsClient, final BaiganConfigClasses baiganConfigClasses, - final ObjectMapper objectMapper) { + final AmazonS3 s3Client, final AWSKMS kmsClient) { checkNotNull(bucketName, "bucketName is required"); checkNotNull(key, "key is required"); checkArgument(refreshInterval >= 0, "refreshInterval has to be >= 0"); @@ -42,18 +42,32 @@ public class S3ConfigurationRepository implements ConfigurationRepository { checkNotNull(s3Client, "s3Client is required"); checkNotNull(kmsClient, "kmsClient is required"); - this.configurationParser = new ConfigurationParser(baiganConfigClasses, objectMapper); this.refreshInterval = refreshInterval; this.executor = executor; this.s3Loader = new S3FileLoader(bucketName, key, s3Client, kmsClient); + } + @Override + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + this.configurationParser = applicationContext.getBean(ConfigurationParser.class); loadConfigurations(); if (refreshInterval > 0) { setupRefresh(); } } - protected void loadConfigurations() { + @Nonnull + @Override + public Optional get(@Nonnull String key) { + return Optional.ofNullable(configurationsMap.get(key)); + } + + @Override + public void put(@Nonnull String key, @Nonnull String value) { + throw new UnsupportedOperationException("The S3ConfigurationRepository doesn't allow any changes."); + } + + private void loadConfigurations() { final String configurationText = s3Loader.loadContent(); final List> configurations = configurationParser.getConfigurations(configurationText); final ImmutableMap.Builder> builder = ImmutableMap.builder(); @@ -77,15 +91,4 @@ private void setupRefresh() { TimeUnit.SECONDS ); } - - @Nonnull - @Override - public Optional get(@Nonnull String key) { - return Optional.ofNullable(configurationsMap.get(key)); - } - - @Override - public void put(@Nonnull String key, @Nonnull String value) { - throw new UnsupportedOperationException("The S3ConfigurationRepository doesn't allow any changes."); - } } diff --git a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java index 8b3abdd..50fec72 100644 --- a/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/service/aws/S3ConfigurationRepositoryBuilder.java @@ -4,8 +4,6 @@ import com.amazonaws.services.kms.AWSKMSClientBuilder; import com.amazonaws.services.s3.AmazonS3; import com.amazonaws.services.s3.AmazonS3ClientBuilder; -import com.fasterxml.jackson.databind.ObjectMapper; -import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; import java.util.concurrent.ScheduledThreadPoolExecutor; @@ -18,7 +16,6 @@ * Must specify non-null values for * - {@link S3ConfigurationRepositoryBuilder#bucketName} * - {@link S3ConfigurationRepositoryBuilder#key} - * - {@link S3ConfigurationRepositoryBuilder#baiganConfigClasses} *

* The latter is typically set as the Spring bean named "baiganConfigClasses" provided by the library. */ @@ -30,8 +27,6 @@ public class S3ConfigurationRepositoryBuilder { private long refreshIntervalInSeconds = 60; private String bucketName; private String key; - private BaiganConfigClasses baiganConfigClasses; - private ObjectMapper objectMapper; /** * @param s3Client The S3 client to be used to fetch the configuration file. @@ -87,20 +82,6 @@ public S3ConfigurationRepositoryBuilder executor(ScheduledThreadPoolExecutor exe return this; } - /** - * @param baiganConfigClasses Contains the list of classes annotated with {@link org.zalando.baigan.annotation.BaiganConfig}. - * This is typically set as the Spring bean named "baiganConfigClasses" provided by the library. - */ - public S3ConfigurationRepositoryBuilder baiganConfigClasses(@Nonnull BaiganConfigClasses baiganConfigClasses) { - this.baiganConfigClasses = checkNotNull(baiganConfigClasses); - return this; - } - - public S3ConfigurationRepositoryBuilder objectMapper(ObjectMapper objectMapper) { - this.objectMapper = objectMapper; - return this; - } - public S3ConfigurationRepository build() { if (executor == null) { executor = new ScheduledThreadPoolExecutor(1); @@ -111,10 +92,7 @@ public S3ConfigurationRepository build() { if (kmsClient == null) { kmsClient = AWSKMSClientBuilder.defaultClient(); } - if (objectMapper == null) { - objectMapper = new ObjectMapper(); - } - return new S3ConfigurationRepository(bucketName, key, refreshIntervalInSeconds, executor, s3Client, kmsClient, baiganConfigClasses, objectMapper); + return new S3ConfigurationRepository(bucketName, key, refreshIntervalInSeconds, executor, s3Client, kmsClient); } } diff --git a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java index b799f7c..31e5c9d 100644 --- a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java @@ -1,5 +1,6 @@ package org.zalando.baigan.e2e.filerepo; +import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; @@ -13,7 +14,6 @@ import org.zalando.baigan.annotation.ConfigurationServiceScan; import org.zalando.baigan.e2e.configs.SomeConfigObject; import org.zalando.baigan.e2e.configs.SomeConfiguration; -import org.zalando.baigan.proxy.BaiganConfigClasses; import org.zalando.baigan.service.FileSystemConfigurationRepository; import org.zalando.baigan.service.FileSystemConfigurationRepositoryBuilder; @@ -79,7 +79,7 @@ public void givenAConfigurationFile_whenTheFileIsUpdatedWithInvalidConfig_thenTh } @Test - public void givenS3Configuration_whenConfigurationTypeIsGeneric_thenDeserializesProperly() throws IOException, InterruptedException { + public void givenAConfigurationFile_whenConfigurationTypeIsGeneric_thenDeserializesProperly() throws IOException, InterruptedException { Files.writeString(configFile, "[{\"alias\": \"some.configuration.top.level.generics\",\"defaultValue\": {" + "\"a8a23682-1623-450b-8817-50c98827ea4e\": [{\"config_key\":\"A\"}]," + "\"76ced443-6555-4748-a22e-8700f3864e59\": [{\"config_key\":\"B\"}]}" + @@ -97,11 +97,10 @@ public void givenS3Configuration_whenConfigurationTypeIsGeneric_thenDeserializes static class RepoConfig { @Bean - FileSystemConfigurationRepository configurationRepository(Path configFile, BaiganConfigClasses baiganConfigClasses) { + FileSystemConfigurationRepository configurationRepository(Path configFile) { return new FileSystemConfigurationRepositoryBuilder() .fileName(configFile.toString()) .refreshIntervalInSeconds(1) - .baiganConfigClasses(baiganConfigClasses) .build(); } @@ -115,5 +114,10 @@ Path configFile() { throw new RuntimeException(e); } } + + @Bean + ObjectMapper objectMapper() { + return new ObjectMapper(); + } } } diff --git a/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java index 626d4f0..6a9bca4 100644 --- a/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java @@ -27,7 +27,6 @@ import org.zalando.baigan.annotation.ConfigurationServiceScan; import org.zalando.baigan.e2e.configs.SomeConfigObject; import org.zalando.baigan.e2e.configs.SomeConfiguration; -import org.zalando.baigan.proxy.BaiganConfigClasses; import org.zalando.baigan.service.aws.S3ConfigurationRepository; import org.zalando.baigan.service.aws.S3ConfigurationRepositoryBuilder; @@ -124,7 +123,7 @@ ScheduledThreadPoolExecutor baiganRefresherPoolExecutor(){ } @Bean - S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, BaiganConfigClasses baiganConfigClasses, ScheduledThreadPoolExecutor executorService) { + S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, ScheduledThreadPoolExecutor executorService) { amazonS3.putObject(S3_CONFIG_BUCKET, S3_CONFIG_KEY, "[]"); return new S3ConfigurationRepositoryBuilder() .bucketName(S3_CONFIG_BUCKET) @@ -133,7 +132,6 @@ S3ConfigurationRepository configurationRepository(AmazonS3 amazonS3, AWSKMS kms, .kmsClient(kms) .refreshIntervalInSeconds(1) .executor(executorService) - .baiganConfigClasses(baiganConfigClasses) .build(); } diff --git a/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java index ca3a952..93a360d 100644 --- a/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java +++ b/src/test/java/org/zalando/baigan/service/ConfigurationParserTest.java @@ -2,12 +2,11 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.model.Equals; -import org.zalando.baigan.proxy.BaiganConfigClasses; +import org.zalando.baigan.proxy.ConfigTypeProvider; import java.io.UncheckedIOException; import java.util.List; @@ -17,22 +16,25 @@ import java.util.StringJoiner; import java.util.UUID; +import static java.util.Optional.empty; import static java.util.stream.Collectors.toList; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class ConfigurationParserTest { - private static final ObjectMapper objectMapper = new ObjectMapper(); - - private static final BaiganConfigClasses stringConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", String.class)); - private static final ConfigurationParser parser = new ConfigurationParser(stringConfigClasses, objectMapper); + private final ConfigTypeProvider configTypeProvider = mock(ConfigTypeProvider.class); + private final ConfigurationParser parser = new ConfigurationParser(configTypeProvider, empty()); @Test public void whenInputContainsKeyForKnownType_shouldParseConfiguration() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}]"; + when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); + final List> parsedConfigs = parser.getConfigurations(input); final List> expectedConfigs = List.of( @@ -44,15 +46,12 @@ public void whenInputContainsKeyForKnownType_shouldParseConfiguration() { @Test public void whenInputContainsTwoConfigs_shouldParseAllConfigsConfiguration() { - final BaiganConfigClasses structuredConfigClasses = new BaiganConfigClasses(Map.of( - "some.config.some.key", String.class, - "some.struct.config", StructuredConfig.class - )); - final ConfigurationParser parser = new ConfigurationParser(structuredConfigClasses, objectMapper); - final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}," + "{\"alias\":\"some.struct.config\",\"defaultValue\":{\"someConfig\":\"some value\",\"someOtherConfig\":1}}]"; + when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); + when(configTypeProvider.getType("some.struct.config")).thenReturn(StructuredConfig.class); + final List> parsedConfigs = parser.getConfigurations(input); final List> expectedConfigs = List.of( @@ -79,6 +78,8 @@ public void whenConfigKeyInInputHasNoClassMapping_shouldIgnoreKey() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}," + "{\"alias\":\"some.missing.config.key\",\"defaultValue\":\"some other value\"}]"; + when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); + final List> parsedConfigs = parser.getConfigurations(input); assertThat(parsedConfigs.stream().map(config -> (String) config.getDefaultValue()).collect(toList()), equalTo(List.of("someValue"))); @@ -93,14 +94,13 @@ public void whenInputCannotBeParsedToJson_shouldThrowException() { @Test public void whenInputIsStructuredConfigWithConditions_shouldDeserializeTypedConfigurationWithConditions() { - final BaiganConfigClasses structuredConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", StructuredConfig.class)); - final ConfigurationParser parser = new ConfigurationParser(structuredConfigClasses, objectMapper); - final String input = "[{\"alias\":\"some.config.some.key\",\"description\":\"a description\"," + "\"defaultValue\":{\"someConfig\":\"some value\",\"someOtherConfig\":1}," + "\"conditions\":[{\"paramName\":\"some param name\",\"conditionType\":{\"type\":\"Equals\",\"onValue\":\"some value\"}," + "\"value\":{\"someConfig\":\"some conditional value\",\"someOtherConfig\":-1}}]}]"; + when(configTypeProvider.getType("some.config.some.key")).thenReturn(StructuredConfig.class); + final List> parsedConfigs = parser.getConfigurations(input); final Configuration expectedConfig = new Configuration<>( @@ -117,6 +117,8 @@ public void whenInputIsStructuredConfigWithConditions_shouldDeserializeTypedConf public void whenDefaultValueCannotBeParsed_shouldThrowException() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":{}}]"; + when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); + assertThrows(RuntimeException.class, () -> parser.getConfigurations(input)); } @@ -125,6 +127,8 @@ public void whenConditionValueCannotBeParsed_shouldThrowException() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"some value\"," + "\"conditions\":[{\"paramName\":\"some param name\",\"conditionType\":{\"type\":\"Equals\",\"onValue\":\"some value\"},\"value\":{}}]}]"; + when(configTypeProvider.getType("some.config.some.key")).thenReturn(StructuredConfig.class); + assertThrows(RuntimeException.class, () -> parser.getConfigurations(input)); } @@ -135,8 +139,7 @@ public void whenConfigurationTypeHasGenerics_shouldParseCorrectly() throws NoSuc "\"76ced443-6555-4748-a22e-8700f3864e59\": [{\"someConfig\":\"B\"}]}" + "}]"; - final BaiganConfigClasses stringConfigClasses = new BaiganConfigClasses(Map.of("some.config.some.key", ParameterizedConfig.class.getMethod("getConfig").getGenericReturnType())); - final ConfigurationParser parser = new ConfigurationParser(stringConfigClasses, objectMapper); + when(configTypeProvider.getType("some.config.some.key")).thenReturn(ParameterizedConfig.class.getMethod("getConfig").getGenericReturnType()); assertThat(parser.getConfigurations(input).get(0).getDefaultValue(), equalTo(Map.of( UUID.fromString("a8a23682-1623-450b-8817-50c98827ea4e"), List.of(new StructuredConfig("A", 1)), diff --git a/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java b/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java deleted file mode 100644 index d07bab3..0000000 --- a/src/test/java/org/zalando/baigan/service/FileSystemConfigurationRepositoryTest.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.zalando.baigan.service; - -import org.junit.jupiter.api.Test; -import org.zalando.baigan.proxy.BaiganConfigClasses; - -import java.lang.reflect.Type; -import java.util.Map; - -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; - -public class FileSystemConfigurationRepositoryTest { - - @Test - public void testReadConfigurationsFromFile() { - final Map configTypesByKey = Map.of( - "express.feature.toggle", Boolean.class, - "express.feature.service.url", String.class, - "pession.sync.feature.toggle", Boolean.class - ); - final ConfigurationRepository repo = new FileSystemConfigurationRepository("src/test/resources/example.json", 180, new BaiganConfigClasses(configTypesByKey)); - assertThat(repo.get("express.feature.toggle").get().getDefaultValue(), equalTo(false)); - } -} From b1827b87996ca39c819d9aaad2fb22923f0b2cc7 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Tue, 10 Oct 2023 12:58:23 +0200 Subject: [PATCH 09/16] ConfigurationRepository should not extend ApplicationContextAware --- .../org/zalando/baigan/service/ConfigurationRepository.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java b/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java index d6a3cb8..1848243 100644 --- a/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/service/ConfigurationRepository.java @@ -12,7 +12,7 @@ * @author mchand */ -public interface ConfigurationRepository extends ApplicationContextAware { +public interface ConfigurationRepository { /** * This method required Guava's Optional which will be deprecated in favor of Java's Optional @@ -30,8 +30,4 @@ default Optional getConfig(@Nonnull final String key) { java.util.Optional get(@Nonnull final String key); void put(@Nonnull final String key, @Nonnull final String value); - - @Override - default void setApplicationContext(ApplicationContext applicationContext) throws BeansException { - } } From 6111e9ce2869e4f9a3820761c7c0aee55008ac31 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Tue, 10 Oct 2023 13:07:27 +0200 Subject: [PATCH 10/16] qualify ObjectMapper bean --- .../java/org/zalando/baigan/service/ConfigurationParser.java | 3 ++- .../filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/zalando/baigan/service/ConfigurationParser.java b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java index 69b5c2f..81783aa 100644 --- a/src/main/java/org/zalando/baigan/service/ConfigurationParser.java +++ b/src/main/java/org/zalando/baigan/service/ConfigurationParser.java @@ -7,6 +7,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Component; import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; @@ -32,7 +33,7 @@ public class ConfigurationParser { final ConfigTypeProvider configTypeProvider; @Autowired - public ConfigurationParser(final ConfigTypeProvider configTypeProvider, final Optional objectMapper) { + public ConfigurationParser(final ConfigTypeProvider configTypeProvider, @Qualifier("baiganObjectMapper") final Optional objectMapper) { this.configTypeProvider = configTypeProvider; this.objectMapper = objectMapper.orElseGet(ObjectMapper::new); } diff --git a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java index 31e5c9d..bf40016 100644 --- a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java @@ -115,7 +115,7 @@ Path configFile() { } } - @Bean + @Bean(name = "baiganObjectMapper") ObjectMapper objectMapper() { return new ObjectMapper(); } From a1958c696344699450bbc08fb689e1b4fc388c91 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Sun, 19 Nov 2023 22:05:37 +0100 Subject: [PATCH 11/16] add ConfigurationParser to RepositoryFactory --- .../baigan/proxy/BaiganConfigClasses.java | 2 +- .../baigan/proxy/ConfigTypeProvider.java | 21 --------- .../repository/ConfigurationParser.java | 42 +++++++++++------ .../EtcdConfigurationRepository.java | 31 ++++--------- .../EtcdConfigurationRepositoryBuilder.java | 7 +-- .../FileSystemConfigurationRepository.java | 21 +++------ ...eSystemConfigurationRepositoryBuilder.java | 6 ++- .../baigan/repository/RepositoryFactory.java | 38 +++++---------- .../repository/S3ConfigurationRepository.java | 24 ++++------ .../S3ConfigurationRepositoryBuilder.java | 12 +++-- ...ystemConfigurationRepositoryEnd2EndIT.java | 2 +- .../S3ConfigurationRepositoryEnd2EndIT.java | 7 ++- .../repository/ConfigurationParserTest.java | 46 ++++++++++--------- 13 files changed, 111 insertions(+), 148 deletions(-) delete mode 100644 src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java diff --git a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java index f72a12f..823554d 100644 --- a/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java +++ b/src/main/java/org/zalando/baigan/proxy/BaiganConfigClasses.java @@ -3,7 +3,7 @@ import java.lang.reflect.Type; import java.util.Map; -class BaiganConfigClasses { +public class BaiganConfigClasses { private Map configTypesByKey; public BaiganConfigClasses() {} diff --git a/src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java b/src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java deleted file mode 100644 index c628047..0000000 --- a/src/main/java/org/zalando/baigan/proxy/ConfigTypeProvider.java +++ /dev/null @@ -1,21 +0,0 @@ -package org.zalando.baigan.proxy; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Component; - -import java.lang.reflect.Type; - -@Component -public class ConfigTypeProvider { - - private final BaiganConfigClasses baiganConfigClasses; - - @Autowired - ConfigTypeProvider(BaiganConfigClasses baiganConfigClasses) { - this.baiganConfigClasses = baiganConfigClasses; - } - - public Type getType(final String configKey) { - return baiganConfigClasses.getConfigTypesByKey().get(configKey); - } -} diff --git a/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java b/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java index 16785dc..97a138f 100644 --- a/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java +++ b/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java @@ -11,7 +11,7 @@ import org.springframework.stereotype.Component; import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; -import org.zalando.baigan.proxy.ConfigTypeProvider; +import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; import java.io.IOException; @@ -21,6 +21,7 @@ import java.util.Optional; import java.util.Set; +import static java.util.Optional.empty; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; @@ -30,16 +31,16 @@ public class ConfigurationParser { private final Logger LOG = LoggerFactory .getLogger(ConfigurationParser.class); final ObjectMapper objectMapper; - final ConfigTypeProvider configTypeProvider; + final BaiganConfigClasses baiganConfigClasses; @Autowired - public ConfigurationParser(final ConfigTypeProvider configTypeProvider, @Qualifier("baiganObjectMapper") final Optional objectMapper) { - this.configTypeProvider = configTypeProvider; + public ConfigurationParser(final BaiganConfigClasses baiganConfigClasses, @Qualifier("baiganObjectMapper") final Optional objectMapper) { + this.baiganConfigClasses = baiganConfigClasses; this.objectMapper = objectMapper.orElseGet(ObjectMapper::new); } @Nonnull - public List> getConfigurations(final String text) { + public List> parseConfigurations(final String text) { if (text == null || text.isEmpty()) { LOG.warn("Input to parse is empty: {}", text); return List.of(); @@ -48,14 +49,7 @@ public List> getConfigurations(final String text) { List> rawConfigs = objectMapper.readValue(text, new TypeReference<>() { }); return rawConfigs.stream() - .map(config -> { - final Optional> typedConfig = Optional.ofNullable(configTypeProvider.getType(config.getAlias())) - .map(targetClass -> deserializeConfig(config, targetClass)); - if (typedConfig.isEmpty()) { - LOG.info("Alias {} does not match any method in a class annotated with @BaiganConfig.", config.getAlias()); - } - return typedConfig; - }) + .map(this::convertToTypedConfig) .filter(Optional::isPresent) .map(Optional::get) .collect(toList()); @@ -65,6 +59,28 @@ public List> getConfigurations(final String text) { } } + public Optional> parseConfiguration(final String text) { + if (text == null || text.isEmpty()) { + LOG.warn("Input to parse is empty: {}", text); + return empty(); + } + try { + Configuration rawConfig = objectMapper.readValue(text, new TypeReference<>(){}); + return convertToTypedConfig(rawConfig); + } catch (JsonProcessingException e) { + throw new UncheckedIOException(e); + } + } + + private Optional> convertToTypedConfig(final Configuration jsonConfig) { + final Optional> typedConfig = Optional.ofNullable(baiganConfigClasses.getConfigTypesByKey().get(jsonConfig.getAlias())) + .map(targetClass -> deserializeConfig(jsonConfig, targetClass)); + if (typedConfig.isEmpty()) { + LOG.info("Alias {} does not match any method in a class annotated with @BaiganConfig.", jsonConfig.getAlias()); + } + return typedConfig; + } + private Configuration deserializeConfig(Configuration config, Type targetClass) { Set> typedConditions = Optional.ofNullable(config.getConditions()).orElse(Set.of()).stream().map(c -> { try { diff --git a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java index f482088..fa47c1b 100644 --- a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java @@ -1,8 +1,5 @@ package org.zalando.baigan.repository; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.datatype.guava.GuavaModule; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -10,7 +7,6 @@ import org.zalando.baigan.repository.etcd.service.EtcdClient; import javax.annotation.Nonnull; -import java.io.IOException; import java.util.Optional; import static com.google.common.base.Preconditions.checkArgument; @@ -22,13 +18,13 @@ // TODO Upgrade to v3 public class EtcdConfigurationRepository implements ConfigurationRepository { - private static final ObjectMapper objectMapper = new ObjectMapper().registerModule(new GuavaModule()); private static final String CONFIG_PATH_PREFIX = "/v2/keys/"; - private static final Logger LOG = LoggerFactory.getLogger(EtcdConfigurationRepository.class); private final EtcdClient etcdClient; + private final ConfigurationParser configurationParser; - EtcdConfigurationRepository(final String etcdUrl) { - etcdClient = new EtcdClient(etcdUrl); + EtcdConfigurationRepository(final String etcdUrl, final ConfigurationParser configurationParser) { + this.etcdClient = new EtcdClient(etcdUrl); + this.configurationParser = configurationParser; } @Override @@ -40,19 +36,10 @@ public void put(@Nonnull final String key, @Nonnull final String value) { @Override @Nonnull public Optional get(@Nonnull final String key) { - try { - checkArgument(!Strings.isNullOrEmpty(key), - "Attempt to get configuration for an empty key !"); - final Optional optionalConfig = etcdClient.get(CONFIG_PATH_PREFIX + key); - - if (optionalConfig.isPresent()) { - return Optional.of(objectMapper.readValue(optionalConfig.get(), - Configuration.class)); - } - - } catch (IOException e) { - LOG.warn("Error while loading configuration for key: " + key, e); - } - return Optional.empty(); + checkArgument(!Strings.isNullOrEmpty(key), + "Attempt to get configuration for an empty key !"); + final Optional optionalConfig = etcdClient.get(CONFIG_PATH_PREFIX + key); + + return optionalConfig.flatMap(configurationParser::parseConfiguration); } } diff --git a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java index fccaa90..fc9c5d7 100644 --- a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java @@ -9,10 +9,11 @@ */ public class EtcdConfigurationRepositoryBuilder { + private final ConfigurationParser configurationParser; private String etcdUrl; - EtcdConfigurationRepositoryBuilder() { - + EtcdConfigurationRepositoryBuilder(final ConfigurationParser configurationParser) { + this.configurationParser = configurationParser; } public EtcdConfigurationRepositoryBuilder etcdUrl(final String etcdUrl) { @@ -22,6 +23,6 @@ public EtcdConfigurationRepositoryBuilder etcdUrl(final String etcdUrl) { public EtcdConfigurationRepository build() { requireNonNull(etcdUrl, "etcdUrl must not be null"); - return new EtcdConfigurationRepository(etcdUrl); + return new EtcdConfigurationRepository(etcdUrl, configurationParser); } } diff --git a/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepository.java b/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepository.java index 4fcc215..5d28b74 100644 --- a/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepository.java @@ -7,9 +7,6 @@ import com.google.common.util.concurrent.ListenableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.BeansException; -import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; import org.zalando.baigan.model.Configuration; import javax.annotation.Nonnull; @@ -31,23 +28,17 @@ * * @author mchand */ -public class FileSystemConfigurationRepository implements ConfigurationRepository, ApplicationContextAware { +public class FileSystemConfigurationRepository implements ConfigurationRepository { private static final Logger LOG = LoggerFactory.getLogger(FileSystemConfigurationRepository.class); - private ConfigurationParser configurationParser; - private LoadingCache>> cachedConfigurations; + private final ConfigurationParser configurationParser; + private final LoadingCache>> cachedConfigurations; private final String fileName; - private final long refreshIntervalInSeconds; - FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds) { + FileSystemConfigurationRepository(final String fileName, long refreshIntervalInSeconds, final ConfigurationParser configurationParser) { this.fileName = fileName; - this.refreshIntervalInSeconds = refreshIntervalInSeconds; - } - - @Override - public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { - this.configurationParser = applicationContext.getBean(ConfigurationParser.class); + this.configurationParser = configurationParser; cachedConfigurations = CacheBuilder.newBuilder() .refreshAfterWrite(refreshIntervalInSeconds, TimeUnit.SECONDS) @@ -91,7 +82,7 @@ public void put(@Nonnull String key, @Nonnull String value) { protected Map> loadConfigurations(String filename) { final String configurationText = loadResource(filename); - final Collection> configurations = configurationParser.getConfigurations(configurationText); + final Collection> configurations = configurationParser.parseConfigurations(configurationText); final ImmutableMap.Builder> builder = ImmutableMap.builder(); for (Configuration each : configurations) { diff --git a/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java index 9b3bc7f..314a0d1 100644 --- a/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java @@ -11,8 +11,10 @@ public class FileSystemConfigurationRepositoryBuilder { private String filePath; private long refreshIntervalInSeconds = 60L; + private final ConfigurationParser configurationParser; - FileSystemConfigurationRepositoryBuilder() { + FileSystemConfigurationRepositoryBuilder(final ConfigurationParser configurationParser) { + this.configurationParser = configurationParser; } /** @@ -34,6 +36,6 @@ public FileSystemConfigurationRepositoryBuilder refreshIntervalInSeconds(long re public FileSystemConfigurationRepository build() { requireNonNull(filePath, "filePath must not be null"); - return new FileSystemConfigurationRepository(filePath, refreshIntervalInSeconds); + return new FileSystemConfigurationRepository(filePath, refreshIntervalInSeconds, configurationParser); } } diff --git a/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java b/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java index 0d7a0b9..8a5bfdf 100644 --- a/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java +++ b/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java @@ -1,46 +1,30 @@ package org.zalando.baigan.repository; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; -/** - * This class is provided as a Spring Bean. It allows creating {@link ConfigurationRepository} instances. - * Each method returns a builder instance that can be used to configure the repository. - */ @Component public class RepositoryFactory { - /** - * Allows creating a {@link S3ConfigurationRepository}. - * - * @return {@link S3ConfigurationRepositoryBuilder} Builder to create the repository - */ + private final ConfigurationParser configurationParser; + + @Autowired + public RepositoryFactory(final ConfigurationParser configurationParser) { + this.configurationParser = configurationParser; + } + public S3ConfigurationRepositoryBuilder s3ConfigurationRepository() { - return new S3ConfigurationRepositoryBuilder(); + return new S3ConfigurationRepositoryBuilder(configurationParser); } - /** - * Allows creating a {@link FileSystemConfigurationRepository}. - * - * @return {@link FileSystemConfigurationRepositoryBuilder} Builder to create the repository - */ public FileSystemConfigurationRepositoryBuilder fileSystemConfigurationRepository() { - return new FileSystemConfigurationRepositoryBuilder(); + return new FileSystemConfigurationRepositoryBuilder(configurationParser); } - /** - * Allows creating a {@link EtcdConfigurationRepository}. - * - * @return {@link EtcdConfigurationRepositoryBuilder} Builder to create the repository - */ public EtcdConfigurationRepositoryBuilder etcdConfigurationRepository() { - return new EtcdConfigurationRepositoryBuilder(); + return new EtcdConfigurationRepositoryBuilder(configurationParser); } - /** - * Allows creating a {@link ChainedConfigurationRepository}. - * - * @return {@link ChainedConfigurationRepositoryBuilder} Builder to create the repository - */ public ChainedConfigurationRepositoryBuilder chainedConfigurationRepository() { return new ChainedConfigurationRepositoryBuilder(); } diff --git a/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepository.java b/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepository.java index e4c3cbc..0f2df2f 100644 --- a/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepository.java @@ -5,9 +5,6 @@ import com.google.common.collect.ImmutableMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.BeansException; -import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.repository.aws.S3FileLoader; @@ -15,7 +12,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import static com.google.common.base.Preconditions.checkArgument; @@ -25,19 +22,19 @@ * A {@link ConfigurationRepository} implementation that loads the configuration from an S3 bucket in regular * intervals. It can read KMS-encrypted configuration files. */ -public class S3ConfigurationRepository implements ConfigurationRepository, ApplicationContextAware { +public class S3ConfigurationRepository implements ConfigurationRepository { private static final Logger LOG = LoggerFactory.getLogger(S3ConfigurationRepository.class); - private ConfigurationParser configurationParser; + private final ConfigurationParser configurationParser; private final S3FileLoader s3Loader; private final long refreshInterval; - private final ScheduledThreadPoolExecutor executor; + private final ScheduledExecutorService executor; private volatile Map> configurationsMap = ImmutableMap.of(); S3ConfigurationRepository(@Nonnull final String bucketName, @Nonnull final String key, - final long refreshInterval, final ScheduledThreadPoolExecutor executor, - final AmazonS3 s3Client, final AWSKMS kmsClient) { + final long refreshInterval, final ScheduledExecutorService executor, + final AmazonS3 s3Client, final AWSKMS kmsClient, ConfigurationParser configurationParser) { checkNotNull(bucketName, "bucketName is required"); checkNotNull(key, "key is required"); checkArgument(refreshInterval >= 0, "refreshInterval has to be >= 0"); @@ -48,11 +45,8 @@ public class S3ConfigurationRepository implements ConfigurationRepository, Appli this.refreshInterval = refreshInterval; this.executor = executor; this.s3Loader = new S3FileLoader(bucketName, key, s3Client, kmsClient); - } + this.configurationParser = configurationParser; - @Override - public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { - this.configurationParser = applicationContext.getBean(ConfigurationParser.class); loadConfigurations(); if (refreshInterval > 0) { setupRefresh(); @@ -71,13 +65,15 @@ public void put(@Nonnull String key, @Nonnull String value) { } private void loadConfigurations() { + LOG.info("Loading configurations from S3 file"); final String configurationText = s3Loader.loadContent(); - final List> configurations = configurationParser.getConfigurations(configurationText); + final List> configurations = configurationParser.parseConfigurations(configurationText); final ImmutableMap.Builder> builder = ImmutableMap.builder(); for (final Configuration configuration : configurations) { builder.put(configuration.getAlias(), configuration); } configurationsMap = builder.build(); + LOG.info("Configuration now: {}", configurationsMap); } private void setupRefresh() { diff --git a/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java index 0b51881..4e777ea 100644 --- a/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java @@ -6,6 +6,7 @@ import com.amazonaws.services.s3.AmazonS3ClientBuilder; import javax.annotation.Nonnull; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import static com.google.common.base.Preconditions.checkNotNull; @@ -17,18 +18,19 @@ * - {@link S3ConfigurationRepositoryBuilder#bucketName} * - {@link S3ConfigurationRepositoryBuilder#key} *

- * The latter is typically set as the Spring bean named "baiganConfigClasses" provided by the library. */ public class S3ConfigurationRepositoryBuilder { - private ScheduledThreadPoolExecutor executor; + private ScheduledExecutorService executor; private AmazonS3 s3Client; private AWSKMS kmsClient; private long refreshIntervalInSeconds = 60; private String bucketName; private String key; + private final ConfigurationParser configurationParser; - S3ConfigurationRepositoryBuilder() { + public S3ConfigurationRepositoryBuilder(final ConfigurationParser configurationParser) { + this.configurationParser = configurationParser; } /** @@ -80,7 +82,7 @@ public S3ConfigurationRepositoryBuilder refreshIntervalInSeconds(final long refr * @param executor The {@link ScheduledThreadPoolExecutor} used to run the configuration refresh. If this is not * specified, a new {@link ScheduledThreadPoolExecutor} with a single thread is used. */ - public S3ConfigurationRepositoryBuilder executor(ScheduledThreadPoolExecutor executor) { + public S3ConfigurationRepositoryBuilder executor(ScheduledExecutorService executor) { this.executor = executor; return this; } @@ -96,6 +98,6 @@ public S3ConfigurationRepository build() { kmsClient = AWSKMSClientBuilder.defaultClient(); } - return new S3ConfigurationRepository(bucketName, key, refreshIntervalInSeconds, executor, s3Client, kmsClient); + return new S3ConfigurationRepository(bucketName, key, refreshIntervalInSeconds, executor, s3Client, kmsClient, configurationParser); } } diff --git a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java index d7a4e37..d526d4a 100644 --- a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java @@ -40,7 +40,7 @@ public class FileSystemConfigurationRepositoryEnd2EndIT { private Path configFile; @Test - public void givenAConfiguraionFile_whenConfigurationIsChanged_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException, IOException { + public void givenAConfigurationFile_whenConfigurationIsChanged_thenConfigurationBeanReturnsNewConfigAfterRefreshTime() throws InterruptedException, IOException { assertThat(someConfiguration.isThisTrue(), nullValue()); assertThat(someConfiguration.someValue(), nullValue()); assertThat(someConfiguration.someConfig(), nullValue()); diff --git a/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java index 2f018da..1ffb685 100644 --- a/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/s3repo/S3ConfigurationRepositoryEnd2EndIT.java @@ -61,6 +61,8 @@ public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfiguratio assertThat(someConfiguration.isThisTrue(), nullValue()); assertThat(someConfiguration.someValue(), nullValue()); assertThat(someConfiguration.someConfig(), nullValue()); + assertThat(someConfiguration.configList(), nullValue()); + assertThat(someConfiguration.topLevelGenerics(), nullValue()); s3.putObject( S3_CONFIG_BUCKET, @@ -71,6 +73,7 @@ public void givenS3Configuration_whenConfigurationIsChangedOnS3_thenConfiguratio assertThat(someConfiguration.isThisTrue(), nullValue()); assertThat(someConfiguration.someValue(), equalTo("some value")); assertThat(someConfiguration.someConfig(), nullValue()); + assertThat(someConfiguration.configList(), nullValue()); s3.putObject( S3_CONFIG_BUCKET, @@ -127,10 +130,10 @@ ScheduledThreadPoolExecutor baiganRefresherPoolExecutor() { @Bean S3ConfigurationRepository configurationRepository( + RepositoryFactory repositoryFactory, AmazonS3 amazonS3, AWSKMS kms, - ScheduledThreadPoolExecutor executorService, - RepositoryFactory repositoryFactory + ScheduledThreadPoolExecutor executorService ) { amazonS3.putObject(S3_CONFIG_BUCKET, S3_CONFIG_KEY, "[]"); return repositoryFactory.s3ConfigurationRepository() diff --git a/src/test/java/org/zalando/baigan/repository/ConfigurationParserTest.java b/src/test/java/org/zalando/baigan/repository/ConfigurationParserTest.java index cb8fc46..3a6eea9 100644 --- a/src/test/java/org/zalando/baigan/repository/ConfigurationParserTest.java +++ b/src/test/java/org/zalando/baigan/repository/ConfigurationParserTest.java @@ -6,7 +6,7 @@ import org.zalando.baigan.model.Condition; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.model.Equals; -import org.zalando.baigan.proxy.ConfigTypeProvider; +import org.zalando.baigan.proxy.BaiganConfigClasses; import java.io.UncheckedIOException; import java.util.List; @@ -26,16 +26,16 @@ public class ConfigurationParserTest { - private final ConfigTypeProvider configTypeProvider = mock(ConfigTypeProvider.class); - private final ConfigurationParser parser = new ConfigurationParser(configTypeProvider, empty()); + private final BaiganConfigClasses baiganConfigClasses = mock(BaiganConfigClasses.class); + private final ConfigurationParser parser = new ConfigurationParser(baiganConfigClasses, empty()); @Test public void whenInputContainsKeyForKnownType_shouldParseConfiguration() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}]"; - when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); + when(baiganConfigClasses.getConfigTypesByKey()).thenReturn(Map.of("some.config.some.key", String.class)); - final List> parsedConfigs = parser.getConfigurations(input); + final List> parsedConfigs = parser.parseConfigurations(input); final List> expectedConfigs = List.of( new Configuration<>("some.config.some.key", null, Set.of(), "someValue") @@ -49,10 +49,12 @@ public void whenInputContainsTwoConfigs_shouldParseAllConfigsConfiguration() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}," + "{\"alias\":\"some.struct.config\",\"defaultValue\":{\"someConfig\":\"some value\",\"someOtherConfig\":1}}]"; - when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); - when(configTypeProvider.getType("some.struct.config")).thenReturn(StructuredConfig.class); + when(baiganConfigClasses.getConfigTypesByKey()).thenReturn(Map.of( + "some.config.some.key", String.class, + "some.struct.config", StructuredConfig.class + )); - final List> parsedConfigs = parser.getConfigurations(input); + final List> parsedConfigs = parser.parseConfigurations(input); final List> expectedConfigs = List.of( new Configuration<>("some.config.some.key", null, Set.of(), "someValue"), @@ -64,13 +66,13 @@ public void whenInputContainsTwoConfigs_shouldParseAllConfigsConfiguration() { @Test public void whenInputIsNullOrEmpty_shouldReturnEmptyList() { - assertThat(parser.getConfigurations(null), equalTo(List.of())); - assertThat(parser.getConfigurations(""), equalTo(List.of())); + assertThat(parser.parseConfigurations(null), equalTo(List.of())); + assertThat(parser.parseConfigurations(""), equalTo(List.of())); } @Test public void whenInputIsEmptyArray_shouldReturnEmptyList() { - assertThat(parser.getConfigurations("[]"), equalTo(List.of())); + assertThat(parser.parseConfigurations("[]"), equalTo(List.of())); } @Test @@ -78,9 +80,9 @@ public void whenConfigKeyInInputHasNoClassMapping_shouldIgnoreKey() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"someValue\"}," + "{\"alias\":\"some.missing.config.key\",\"defaultValue\":\"some other value\"}]"; - when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); + when(baiganConfigClasses.getConfigTypesByKey()).thenReturn(Map.of("some.config.some.key", String.class)); - final List> parsedConfigs = parser.getConfigurations(input); + final List> parsedConfigs = parser.parseConfigurations(input); assertThat(parsedConfigs.stream().map(config -> (String) config.getDefaultValue()).collect(toList()), equalTo(List.of("someValue"))); } @@ -89,7 +91,7 @@ public void whenConfigKeyInInputHasNoClassMapping_shouldIgnoreKey() { public void whenInputCannotBeParsedToJson_shouldThrowException() { final String input = "some invalid input"; - assertThrows(UncheckedIOException.class, () -> parser.getConfigurations(input)); + assertThrows(UncheckedIOException.class, () -> parser.parseConfigurations(input)); } @Test @@ -99,9 +101,9 @@ public void whenInputIsStructuredConfigWithConditions_shouldDeserializeTypedConf "\"conditions\":[{\"paramName\":\"some param name\",\"conditionType\":{\"type\":\"Equals\",\"onValue\":\"some value\"}," + "\"value\":{\"someConfig\":\"some conditional value\",\"someOtherConfig\":-1}}]}]"; - when(configTypeProvider.getType("some.config.some.key")).thenReturn(StructuredConfig.class); + when(baiganConfigClasses.getConfigTypesByKey()).thenReturn(Map.of("some.config.some.key", StructuredConfig.class)); - final List> parsedConfigs = parser.getConfigurations(input); + final List> parsedConfigs = parser.parseConfigurations(input); final Configuration expectedConfig = new Configuration<>( "some.config.some.key", @@ -117,9 +119,9 @@ public void whenInputIsStructuredConfigWithConditions_shouldDeserializeTypedConf public void whenDefaultValueCannotBeParsed_shouldThrowException() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":{}}]"; - when(configTypeProvider.getType("some.config.some.key")).thenReturn(String.class); + when(baiganConfigClasses.getConfigTypesByKey()).thenReturn(Map.of("some.config.some.key", String.class)); - assertThrows(RuntimeException.class, () -> parser.getConfigurations(input)); + assertThrows(RuntimeException.class, () -> parser.parseConfigurations(input)); } @Test @@ -127,9 +129,9 @@ public void whenConditionValueCannotBeParsed_shouldThrowException() { final String input = "[{\"alias\":\"some.config.some.key\",\"defaultValue\":\"some value\"," + "\"conditions\":[{\"paramName\":\"some param name\",\"conditionType\":{\"type\":\"Equals\",\"onValue\":\"some value\"},\"value\":{}}]}]"; - when(configTypeProvider.getType("some.config.some.key")).thenReturn(StructuredConfig.class); + when(baiganConfigClasses.getConfigTypesByKey()).thenReturn(Map.of("some.config.some.key", StructuredConfig.class)); - assertThrows(RuntimeException.class, () -> parser.getConfigurations(input)); + assertThrows(RuntimeException.class, () -> parser.parseConfigurations(input)); } @Test @@ -139,9 +141,9 @@ public void whenConfigurationTypeHasGenerics_shouldParseCorrectly() throws NoSuc "\"76ced443-6555-4748-a22e-8700f3864e59\": [{\"someConfig\":\"B\"}]}" + "}]"; - when(configTypeProvider.getType("some.config.some.key")).thenReturn(ParameterizedConfig.class.getMethod("getConfig").getGenericReturnType()); + when(baiganConfigClasses.getConfigTypesByKey()).thenReturn(Map.of("some.config.some.key", ParameterizedConfig.class.getMethod("getConfig").getGenericReturnType())); - assertThat(parser.getConfigurations(input).get(0).getDefaultValue(), equalTo(Map.of( + assertThat(parser.parseConfigurations(input).get(0).getDefaultValue(), equalTo(Map.of( UUID.fromString("a8a23682-1623-450b-8817-50c98827ea4e"), List.of(new StructuredConfig("A", 1)), UUID.fromString("76ced443-6555-4748-a22e-8700f3864e59"), List.of(new StructuredConfig("B", 0)) ))); From c9945e8996b19587f0c9aa9f9d2af21ff7b37897 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Tue, 21 Nov 2023 08:54:22 +0100 Subject: [PATCH 12/16] configurable ObjectMapper; bring back Java doc --- .../repository/ConfigurationParser.java | 6 ++++- .../EtcdConfigurationRepositoryBuilder.java | 18 +++++++++++++- ...eSystemConfigurationRepositoryBuilder.java | 16 +++++++++++++ .../baigan/repository/RepositoryFactory.java | 24 +++++++++++++++++++ .../S3ConfigurationRepositoryBuilder.java | 13 ++++++++++ ...ystemConfigurationRepositoryEnd2EndIT.java | 12 +++++----- 6 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java b/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java index 97a138f..cafa6a6 100644 --- a/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java +++ b/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java @@ -30,7 +30,7 @@ public class ConfigurationParser { private final Logger LOG = LoggerFactory .getLogger(ConfigurationParser.class); - final ObjectMapper objectMapper; + ObjectMapper objectMapper; final BaiganConfigClasses baiganConfigClasses; @Autowired @@ -72,6 +72,10 @@ public Optional> parseConfiguration(final String text) { } } + void setObjectMapper(final ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + } + private Optional> convertToTypedConfig(final Configuration jsonConfig) { final Optional> typedConfig = Optional.ofNullable(baiganConfigClasses.getConfigTypesByKey().get(jsonConfig.getAlias())) .map(targetClass -> deserializeConfig(jsonConfig, targetClass)); diff --git a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java index fc9c5d7..199a8e7 100644 --- a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepositoryBuilder.java @@ -1,5 +1,7 @@ package org.zalando.baigan.repository; +import com.fasterxml.jackson.databind.ObjectMapper; + import static java.util.Objects.requireNonNull; /** @@ -9,8 +11,9 @@ */ public class EtcdConfigurationRepositoryBuilder { - private final ConfigurationParser configurationParser; private String etcdUrl; + private ObjectMapper objectMapper; + private final ConfigurationParser configurationParser; EtcdConfigurationRepositoryBuilder(final ConfigurationParser configurationParser) { this.configurationParser = configurationParser; @@ -21,8 +24,21 @@ public EtcdConfigurationRepositoryBuilder etcdUrl(final String etcdUrl) { return this; } + /** + * @param objectMapper The {@link ObjectMapper} used to parse the configurations. + */ + public EtcdConfigurationRepositoryBuilder objectMapper(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + return this; + } + public EtcdConfigurationRepository build() { requireNonNull(etcdUrl, "etcdUrl must not be null"); + + if (objectMapper != null) { + configurationParser.setObjectMapper(objectMapper); + } + return new EtcdConfigurationRepository(etcdUrl, configurationParser); } } diff --git a/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java index 314a0d1..e376328 100644 --- a/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/repository/FileSystemConfigurationRepositoryBuilder.java @@ -1,5 +1,7 @@ package org.zalando.baigan.repository; +import com.fasterxml.jackson.databind.ObjectMapper; + import static java.util.Objects.requireNonNull; /** @@ -11,6 +13,7 @@ public class FileSystemConfigurationRepositoryBuilder { private String filePath; private long refreshIntervalInSeconds = 60L; + private ObjectMapper objectMapper; private final ConfigurationParser configurationParser; FileSystemConfigurationRepositoryBuilder(final ConfigurationParser configurationParser) { @@ -34,8 +37,21 @@ public FileSystemConfigurationRepositoryBuilder refreshIntervalInSeconds(long re return this; } + /** + * @param objectMapper The {@link ObjectMapper} used to parse the configurations. + */ + public FileSystemConfigurationRepositoryBuilder objectMapper(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + return this; + } + public FileSystemConfigurationRepository build() { requireNonNull(filePath, "filePath must not be null"); + + if (objectMapper != null) { + configurationParser.setObjectMapper(objectMapper); + } + return new FileSystemConfigurationRepository(filePath, refreshIntervalInSeconds, configurationParser); } } diff --git a/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java b/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java index 8a5bfdf..65bd233 100644 --- a/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java +++ b/src/main/java/org/zalando/baigan/repository/RepositoryFactory.java @@ -3,6 +3,10 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; +/** + * This class is provided as a Spring Bean. It allows creating {@link ConfigurationRepository} instances. + * Each method returns a builder instance that can be used to configure the repository. + */ @Component public class RepositoryFactory { @@ -13,18 +17,38 @@ public RepositoryFactory(final ConfigurationParser configurationParser) { this.configurationParser = configurationParser; } + /** + * Allows creating a {@link S3ConfigurationRepository}. + * + * @return {@link S3ConfigurationRepositoryBuilder} Builder to create the repository + */ public S3ConfigurationRepositoryBuilder s3ConfigurationRepository() { return new S3ConfigurationRepositoryBuilder(configurationParser); } + /** + * Allows creating a {@link FileSystemConfigurationRepository}. + * + * @return {@link FileSystemConfigurationRepositoryBuilder} Builder to create the repository + */ public FileSystemConfigurationRepositoryBuilder fileSystemConfigurationRepository() { return new FileSystemConfigurationRepositoryBuilder(configurationParser); } + /** + * Allows creating a {@link EtcdConfigurationRepository}. + * + * @return {@link EtcdConfigurationRepositoryBuilder} Builder to create the repository + */ public EtcdConfigurationRepositoryBuilder etcdConfigurationRepository() { return new EtcdConfigurationRepositoryBuilder(configurationParser); } + /** + * Allows creating a {@link ChainedConfigurationRepository}. + * + * @return {@link ChainedConfigurationRepositoryBuilder} Builder to create the repository + */ public ChainedConfigurationRepositoryBuilder chainedConfigurationRepository() { return new ChainedConfigurationRepositoryBuilder(); } diff --git a/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java b/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java index 4e777ea..3d3d6af 100644 --- a/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java +++ b/src/main/java/org/zalando/baigan/repository/S3ConfigurationRepositoryBuilder.java @@ -4,6 +4,7 @@ import com.amazonaws.services.kms.AWSKMSClientBuilder; import com.amazonaws.services.s3.AmazonS3; import com.amazonaws.services.s3.AmazonS3ClientBuilder; +import com.fasterxml.jackson.databind.ObjectMapper; import javax.annotation.Nonnull; import java.util.concurrent.ScheduledExecutorService; @@ -27,6 +28,7 @@ public class S3ConfigurationRepositoryBuilder { private long refreshIntervalInSeconds = 60; private String bucketName; private String key; + private ObjectMapper objectMapper; private final ConfigurationParser configurationParser; public S3ConfigurationRepositoryBuilder(final ConfigurationParser configurationParser) { @@ -87,6 +89,14 @@ public S3ConfigurationRepositoryBuilder executor(ScheduledExecutorService execut return this; } + /** + * @param objectMapper The {@link ObjectMapper} used to parse the configurations. + */ + public S3ConfigurationRepositoryBuilder objectMapper(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + return this; + } + public S3ConfigurationRepository build() { if (executor == null) { executor = new ScheduledThreadPoolExecutor(1); @@ -97,6 +107,9 @@ public S3ConfigurationRepository build() { if (kmsClient == null) { kmsClient = AWSKMSClientBuilder.defaultClient(); } + if (objectMapper != null) { + configurationParser.setObjectMapper(objectMapper); + } return new S3ConfigurationRepository(bucketName, key, refreshIntervalInSeconds, executor, s3Client, kmsClient, configurationParser); } diff --git a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java index d526d4a..c4ff18d 100644 --- a/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java +++ b/src/test/java/org/zalando/baigan/e2e/filerepo/FileSystemConfigurationRepositoryEnd2EndIT.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.UUID; +import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -54,7 +55,10 @@ public void givenAConfigurationFile_whenConfigurationIsChanged_thenConfiguration Files.writeString(configFile, "[{ \"alias\": \"some.non.existing.config\", \"defaultValue\": \"an irrelevant value\"}," + "{ \"alias\": \"some.configuration.is.this.true\", \"defaultValue\": true}, " + "{ \"alias\": \"some.configuration.some.value\", \"defaultValue\": \"some value\"}, " + - "{ \"alias\": \"some.configuration.some.config\", \"defaultValue\": {\"config_key\":\"a value\"}}, " + + "{ \"alias\": \"some.configuration.some.config\", \"defaultValue\": {" + + "\"config_key\":\"a value\"," + + "\"extra_field\": \"objectMapper configured to not fail for unknown properties\"" + + "}}, " + "{ \"alias\": \"some.configuration.config.list\", \"defaultValue\": [\"A\",\"B\"]}]" ); Thread.sleep(1100); @@ -103,6 +107,7 @@ FileSystemConfigurationRepository configurationRepository(Path configFile, Repos return repositoryFactory.fileSystemConfigurationRepository() .fileName(configFile.toString()) .refreshIntervalInSeconds(1) + .objectMapper(new ObjectMapper().configure(FAIL_ON_UNKNOWN_PROPERTIES, false)) .build(); } @@ -116,10 +121,5 @@ Path configFile() { throw new RuntimeException(e); } } - - @Bean(name = "baiganObjectMapper") - ObjectMapper objectMapper() { - return new ObjectMapper(); - } } } From cfc1ef11b39fae6ada61d5230650c40a289cc893 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Tue, 21 Nov 2023 10:08:00 +0100 Subject: [PATCH 13/16] documentation --- README.md | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 90ddc32..be50a84 100644 --- a/README.md +++ b/README.md @@ -51,14 +51,21 @@ And the _@ConfigurationServiceScan_ annotation hints the Baigan registrar to loo @BaiganConfig public interface ExpressFeature { - public Boolean enabled(); + Boolean enabled(); - public String serviceUrl(); + String serviceUrl(); + SomeStructuredConfigClass complexConfiguration(); + + List configList(); + + Map> nestedGenericConfiguration(); } ``` -**Note**: Primitives are not supported as return types. +The individual methods may have arbitrary classes as return types, in particular complex structured types are supported, including Generics. + +**Note**: Primitives are not supported as return types as they cannot be null and therefore cannot express a missing configuration value. The above example code enables the application to inject _ExpressFeature_ spring bean into any other Spring bean: @@ -86,13 +93,20 @@ This is done using the Spring Bean of type `RepositoryFactory`, which allows cre types. The following example shows how to configure a filesystem based repository. ```Java - @Bean - public ConfigurationRepository configurationRepository(RepositoryFactory factory) { - return factory.fileSystemConfigurationRepository() - .fileName("configs.json"); + @Configuration + public class ApplicationConfiguration { + + @Bean + public ConfigurationRepository configurationRepository(RepositoryFactory factory){ + return factory.fileSystemConfigurationRepository() + .fileName("configs.json"); + } } ``` +Check the documentation of the builders for details on how to configure the repositories. In particular, all +repositories can be configured with a Jackson `ObjectMapper` used to deserialize the configuration. + ### Creating configurations Baigan configurations follow a specific schema and can be stored on any of the supported repositories. From 1b88cc3c246b1a1fc7c736f77d1a0812e564fbf4 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Tue, 21 Nov 2023 10:20:18 +0100 Subject: [PATCH 14/16] simplify code --- .../repository/ConfigurationParser.java | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java b/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java index cafa6a6..3ab5acf 100644 --- a/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java +++ b/src/main/java/org/zalando/baigan/repository/ConfigurationParser.java @@ -14,7 +14,6 @@ import org.zalando.baigan.proxy.BaiganConfigClasses; import javax.annotation.Nonnull; -import java.io.IOException; import java.io.UncheckedIOException; import java.lang.reflect.Type; import java.util.List; @@ -41,32 +40,28 @@ public ConfigurationParser(final BaiganConfigClasses baiganConfigClasses, @Quali @Nonnull public List> parseConfigurations(final String text) { - if (text == null || text.isEmpty()) { - LOG.warn("Input to parse is empty: {}", text); - return List.of(); - } - try { - List> rawConfigs = objectMapper.readValue(text, new TypeReference<>() { - }); - return rawConfigs.stream() - .map(this::convertToTypedConfig) - .filter(Optional::isPresent) - .map(Optional::get) - .collect(toList()); - } catch ( - IOException e) { - throw new UncheckedIOException("Unable to deserialize the Configuration.", e); - } + List> rawConfigs = parseConfigText(text, new TypeReference>>() { + }).orElse(List.of()); + return rawConfigs.stream() + .map(this::convertToTypedConfig) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(toList()); } public Optional> parseConfiguration(final String text) { + Optional> rawConfig = parseConfigText(text, new TypeReference<>() { + }); + return rawConfig.flatMap(this::convertToTypedConfig); + } + + private Optional parseConfigText(final String text, TypeReference type) { if (text == null || text.isEmpty()) { - LOG.warn("Input to parse is empty: {}", text); + LOG.warn("Input to parse is empty: {}", text); return empty(); } try { - Configuration rawConfig = objectMapper.readValue(text, new TypeReference<>(){}); - return convertToTypedConfig(rawConfig); + return Optional.of(objectMapper.readValue(text, type)); } catch (JsonProcessingException e) { throw new UncheckedIOException(e); } @@ -94,7 +89,7 @@ private Configuration deserializeConfig(Configuration config, T } }).collect(toSet()); try { - T typedDefaultValue = objectMapper.treeToValue(config.getDefaultValue(), objectMapper.constructType(targetClass)); + T typedDefaultValue = objectMapper.treeToValue(config.getDefaultValue(), objectMapper.constructType(targetClass)); return new Configuration<>(config.getAlias(), config.getDescription(), typedConditions, typedDefaultValue); } catch (JsonProcessingException e) { throw new RuntimeException(e); From b5fe18059a3c51f1ce02e30e61547bb8c6941832 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Tue, 21 Nov 2023 10:26:03 +0100 Subject: [PATCH 15/16] javadoc --- .../repository/EtcdConfigurationRepository.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java index fa47c1b..557813f 100644 --- a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java @@ -1,19 +1,17 @@ package org.zalando.baigan.repository; import com.google.common.base.Strings; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.repository.etcd.service.EtcdClient; import javax.annotation.Nonnull; +import java.io.UncheckedIOException; import java.util.Optional; import static com.google.common.base.Preconditions.checkArgument; /** * A {@link ConfigurationRepository} implementation that loads the configuration from an etcd server. - * The configuration is not cached and is fetched from etcd on every request. */ // TODO Upgrade to v3 public class EtcdConfigurationRepository implements ConfigurationRepository { @@ -29,10 +27,19 @@ public class EtcdConfigurationRepository implements ConfigurationRepository { @Override public void put(@Nonnull final String key, @Nonnull final String value) { - throw new UnsupportedOperationException( - "The put operation is not yet supported."); + throw new UnsupportedOperationException("The put operation is not yet supported."); } + /** + * Gets the configuration value from an etcd server. + * The configuration is not cached and is fetched from etcd on every request. + * + * @param key The key for which the configuration is to be fetched. + * @return The configuration for the given key, if present. Empty, if the key + * does not exist in etcd or fetching it from etcd fails. + * + * @throws UncheckedIOException if the configuration cannot be parsed. + */ @Override @Nonnull public Optional get(@Nonnull final String key) { From 44922c22b5ae06995e4b338070a59d223c1a8923 Mon Sep 17 00:00:00 2001 From: lwilhelm Date: Tue, 21 Nov 2023 10:55:35 +0100 Subject: [PATCH 16/16] return empty on failed parsing for EtcdConfigurationRepository While relatively silently accepting failed parsing of configuration is problematic, this is how the EtcdConfigurationRepository behaved in the past. So it makes sense to not change the behavior here as the main change of the whole PR is not concerned with this. --- .../EtcdConfigurationRepository.java | 14 ++++-- .../context/MethodInvocationHandlerTest.java | 2 +- src/test/resources/example.json | 44 ------------------- 3 files changed, 11 insertions(+), 49 deletions(-) delete mode 100644 src/test/resources/example.json diff --git a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java index 557813f..7c9c41b 100644 --- a/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java +++ b/src/main/java/org/zalando/baigan/repository/EtcdConfigurationRepository.java @@ -1,6 +1,8 @@ package org.zalando.baigan.repository; import com.google.common.base.Strings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.zalando.baigan.model.Configuration; import org.zalando.baigan.repository.etcd.service.EtcdClient; @@ -17,6 +19,7 @@ public class EtcdConfigurationRepository implements ConfigurationRepository { private static final String CONFIG_PATH_PREFIX = "/v2/keys/"; + private static final Logger LOG = LoggerFactory.getLogger(EtcdConfigurationRepository.class); private final EtcdClient etcdClient; private final ConfigurationParser configurationParser; @@ -36,9 +39,7 @@ public void put(@Nonnull final String key, @Nonnull final String value) { * * @param key The key for which the configuration is to be fetched. * @return The configuration for the given key, if present. Empty, if the key - * does not exist in etcd or fetching it from etcd fails. - * - * @throws UncheckedIOException if the configuration cannot be parsed. + * does not exist in etcd, fetching it from etcd fails, or the configuration cannot be parsed. */ @Override @Nonnull @@ -47,6 +48,11 @@ public Optional get(@Nonnull final String key) { "Attempt to get configuration for an empty key !"); final Optional optionalConfig = etcdClient.get(CONFIG_PATH_PREFIX + key); - return optionalConfig.flatMap(configurationParser::parseConfiguration); + try { + return optionalConfig.flatMap(configurationParser::parseConfiguration); + } catch (UncheckedIOException e) { + LOG.warn("Error while loading configuration for key: " + key, e); + return Optional.empty(); + } } } diff --git a/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java b/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java index 0e6c44f..8e38d1c 100644 --- a/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java +++ b/src/test/java/org/zalando/baigan/context/MethodInvocationHandlerTest.java @@ -71,7 +71,7 @@ public void testConfigHasWrongType() throws Throwable { } @Test - public void testPrimitiveType() throws Throwable { + public void testBoxedPrimitiveType() throws Throwable { final ConfigurationRepository repo = mock(ConfigurationRepository.class); final Configuration configuration = new Configuration<>("express.max.delivery.days", DESCRIPTION, of(), 3); diff --git a/src/test/resources/example.json b/src/test/resources/example.json deleted file mode 100644 index 5451c84..0000000 --- a/src/test/resources/example.json +++ /dev/null @@ -1,44 +0,0 @@ -[{ - "alias": "express.feature.toggle", - "description": "Feature toggle", - "defaultValue": false, - "conditions": [ - { - "value": true, - "conditionType": { - "onValue": "3", - "type": "Equals" - }, - "paramName": "appdomain" - } - ] -}, -{ - "alias": "express.feature.service.url", - "description": "Feature Service Url", - "defaultValue": "", - "conditions": [ - { - "value": "http://express.trucks.zalan.do", - "conditionType": { - "onValue": "2", - "type": "Equals" - }, - "paramName": "appdomain" - } - ] - }, - { - "alias": "pession.sync.feature.toggle", - "description": "Feature toggle", - "defaultValue": false, - "conditions": [{ - "value": true, - "conditionType": { - "inValue": ["24", "15"], - "type": "In" - }, - "paramName": "appdomain" - }] - } -]