diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigAutoConfiguration.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigAutoConfiguration.java index 5a1b00af..36256f9f 100644 --- a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigAutoConfiguration.java +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigAutoConfiguration.java @@ -21,7 +21,6 @@ import com.ecwid.consul.v1.ConsulClient; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; -import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.consul.ConditionalOnConsulEnabled; @@ -45,12 +44,6 @@ public class ConsulConfigAutoConfiguration { */ public static final String CONFIG_WATCH_TASK_SCHEDULER_NAME = "configWatchTaskScheduler"; - @Bean - @ConditionalOnMissingBean - public ConsulConfigProperties consulConfigProperties() { - return new ConsulConfigProperties(); - } - @Configuration(proxyBeanMethods = false) @ConditionalOnClass(RefreshEndpoint.class) @ConditionalOnProperty(name = "spring.cloud.consul.config.watch.enabled", matchIfMissing = true) diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigBootstrapConfiguration.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigBootstrapConfiguration.java index f1278bbc..4f080e60 100644 --- a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigBootstrapConfiguration.java +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigBootstrapConfiguration.java @@ -27,6 +27,8 @@ import org.springframework.cloud.consul.ConsulAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.core.env.Environment; +import org.springframework.util.StringUtils; /** * @author Spencer Gibb @@ -47,8 +49,12 @@ public class ConsulConfigBootstrapConfiguration { @Bean @ConditionalOnMissingBean - public ConsulConfigProperties consulConfigProperties() { - return new ConsulConfigProperties(); + public ConsulConfigProperties consulConfigProperties(Environment env) { + ConsulConfigProperties properties = new ConsulConfigProperties(); + if (StringUtils.isEmpty(properties.getName())) { + properties.setName(env.getProperty("spring.application.name", "application")); + } + return properties; } @Bean diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigDataLoader.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigDataLoader.java index c398c4ad..95e947fe 100644 --- a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigDataLoader.java +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulConfigDataLoader.java @@ -19,59 +19,34 @@ package org.springframework.cloud.consul.config; import java.util.Collections; import com.ecwid.consul.v1.ConsulClient; -import com.ecwid.consul.v1.Response; -import com.ecwid.consul.v1.kv.model.GetValue; import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.springframework.boot.context.config.ConfigData; import org.springframework.boot.context.config.ConfigDataLoader; import org.springframework.boot.context.config.ConfigDataLoaderContext; import org.springframework.boot.context.config.ConfigDataLocationNotFoundException; -import static org.springframework.cloud.consul.config.ConsulConfigProperties.Format.FILES; - public class ConsulConfigDataLoader implements ConfigDataLoader { - private static final Log log = LogFactory.getLog(ConsulConfigDataLoader.class); + private final Log log; + + public ConsulConfigDataLoader(Log log) { + this.log = log; + } @Override public ConfigData load(ConfigDataLoaderContext context, ConsulConfigDataLocation location) { try { ConsulClient consul = getBean(context, ConsulClient.class); - ConsulConfigProperties properties = location.getProperties(); - ConsulPropertySource propertySource = null; + ConsulConfigIndexes indexes = getBean(context, ConsulConfigIndexes.class); - if (properties.getFormat() == FILES) { - Response response = consul.getKVValue(location.getContext(), properties.getAclToken()); - addIndex(context, location, response.getConsulIndex()); - if (response.getValue() != null) { - ConsulFilesPropertySource filesPropertySource = new ConsulFilesPropertySource(location.getContext(), - consul, properties); - filesPropertySource.init(response.getValue()); - propertySource = filesPropertySource; - } - else if (!location.isOptional()) { - throw new ConfigDataLocationNotFoundException(location); - } - } - else { - propertySource = create(context, location); - } + ConsulPropertySource propertySource = location.getConsulPropertySources().createPropertySource( + location.getContext(), location.isOptional(), consul, indexes.getIndexes()::put); return new ConfigData(Collections.singletonList(propertySource)); } - catch (ConfigDataLocationNotFoundException e) { - throw e; - } catch (Exception e) { - if (location.getProperties().isFailFast() || !location.isOptional()) { - throw new ConfigDataLocationNotFoundException(location, e); - } - else { - log.warn("Unable to load consul config from " + location.getContext(), e); - } + throw new ConfigDataLocationNotFoundException(location, e); } - return null; } protected T getBean(ConfigDataLoaderContext context, Class type) { @@ -81,19 +56,4 @@ public class ConsulConfigDataLoader implements ConfigDataLoader FILES_SUFFIXES = Collections .unmodifiableList(Arrays.asList(".yml", ".yaml", ".properties")); + private final Log log; + + public ConsulConfigDataLocationResolver(Log log) { + this.log = log; + } + @Override public boolean isResolvable(ConfigDataLocationResolverContext context, String location) { if (!location.startsWith(PREFIX)) { @@ -82,18 +89,22 @@ public class ConsulConfigDataLocationResolver implements ConfigDataLocationResol ConsulConfigProperties properties = loadConfigProperties(context.getBinder()); + ConsulPropertySources consulPropertySources = new ConsulPropertySources(properties, log); + List contexts = (locationUri == null || CollectionUtils.isEmpty(locationUri.getPathSegments())) - ? getAutomaticContexts(profiles, properties) : getCustomContexts(locationUri, properties); + ? consulPropertySources.getAutomaticContexts(profiles.getAccepted()) + : getCustomContexts(locationUri, properties); registerBean(context, ConsulProperties.class, loadProperties(context.getBinder(), locationUri)); + registerAndPromoteBean(context, ConsulConfigProperties.class, () -> properties); + registerAndPromoteBean(context, ConsulClient.class, () -> createConsulClient(context)); registerAndPromoteBean(context, ConsulConfigIndexes.class, ConsulConfigDataIndexes::new); - return contexts.stream() - .map(propertySourceContext -> new ConsulConfigDataLocation(properties, propertySourceContext, optional)) - .collect(Collectors.toList()); + return contexts.stream().map(propertySourceContext -> new ConsulConfigDataLocation(propertySourceContext, + optional, properties, consulPropertySources)).collect(Collectors.toList()); } private List getCustomContexts(UriComponents uriComponents, ConsulConfigProperties properties) { @@ -118,48 +129,6 @@ public class ConsulConfigDataLocationResolver implements ConfigDataLocationResol return DIR_SUFFIXES; } - protected List getAutomaticContexts(Profiles profiles, ConsulConfigProperties properties) { - List contexts = new ArrayList<>(); - - String prefix = properties.getPrefix(); - String defaultContext = getContext(prefix, properties.getDefaultContext()); - for (String suffix : getSuffixes(properties)) { - contexts.add(defaultContext + suffix); - } - for (String suffix : getSuffixes(properties)) { - addProfiles(contexts, defaultContext, profiles, suffix, properties); - } - - // getName() defaults to ${spring.application.name} or application - String baseContext = getContext(prefix, properties.getName()); - - for (String suffix : getSuffixes(properties)) { - contexts.add(baseContext + suffix); - } - for (String suffix : getSuffixes(properties)) { - addProfiles(contexts, baseContext, profiles, suffix, properties); - } - // we build them backwards, first wins, so reverse - Collections.reverse(contexts); - return contexts; - } - - protected String getContext(String prefix, String context) { - if (StringUtils.isEmpty(prefix)) { - return context; - } - else { - return prefix + "/" + context; - } - } - - protected void addProfiles(List contexts, String baseContext, Profiles profiles, String suffix, - ConsulConfigProperties properties) { - for (String profile : profiles.getAccepted()) { - contexts.add(baseContext + properties.getProfileSeparator() + profile + suffix); - } - } - @Nullable protected UriComponents parseLocation(ConfigDataLocationResolverContext context, String location) { String uri = location.substring(PREFIX.length()); diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocator.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocator.java index 51578fee..f8ef97d6 100644 --- a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocator.java +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocator.java @@ -19,14 +19,10 @@ package org.springframework.cloud.consul.config; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import com.ecwid.consul.v1.ConsulClient; -import com.ecwid.consul.v1.Response; -import com.ecwid.consul.v1.kv.model.GetValue; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -37,10 +33,6 @@ import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; import org.springframework.core.env.PropertySource; import org.springframework.retry.annotation.Retryable; -import org.springframework.util.ReflectionUtils; -import org.springframework.util.StringUtils; - -import static org.springframework.cloud.consul.config.ConsulConfigProperties.Format.FILES; /** * @author Spencer Gibb @@ -85,76 +77,18 @@ public class ConsulPropertySourceLocator implements PropertySourceLocator, Consu if (environment instanceof ConfigurableEnvironment) { ConfigurableEnvironment env = (ConfigurableEnvironment) environment; - String appName = this.properties.getName(); - if (StringUtils.isEmpty(appName)) { - appName = env.getProperty("spring.application.name", "application"); - } + ConsulPropertySources sources = new ConsulPropertySources(properties, log); List profiles = Arrays.asList(env.getActiveProfiles()); - - String prefix = this.properties.getPrefix(); - - List suffixes = new ArrayList<>(); - if (this.properties.getFormat() != FILES) { - suffixes.add("/"); - } - else { - suffixes.add(".yml"); - suffixes.add(".yaml"); - suffixes.add(".properties"); - } - - String defaultContext = getContext(prefix, this.properties.getDefaultContext()); - - for (String suffix : suffixes) { - this.contexts.add(defaultContext + suffix); - } - for (String suffix : suffixes) { - addProfiles(this.contexts, defaultContext, profiles, suffix); - } - - String baseContext = getContext(prefix, appName); - - for (String suffix : suffixes) { - this.contexts.add(baseContext + suffix); - } - for (String suffix : suffixes) { - addProfiles(this.contexts, baseContext, profiles, suffix); - } - - Collections.reverse(this.contexts); + this.contexts.addAll(sources.getAutomaticContexts(profiles)); CompositePropertySource composite = new CompositePropertySource("consul"); for (String propertySourceContext : this.contexts) { - try { - ConsulPropertySource propertySource = null; - if (this.properties.getFormat() == FILES) { - Response response = this.consul.getKVValue(propertySourceContext, - this.properties.getAclToken()); - addIndex(propertySourceContext, response.getConsulIndex()); - if (response.getValue() != null) { - ConsulFilesPropertySource filesPropertySource = new ConsulFilesPropertySource( - propertySourceContext, this.consul, this.properties); - filesPropertySource.init(response.getValue()); - propertySource = filesPropertySource; - } - } - else { - propertySource = create(propertySourceContext, this.contextIndex); - } - if (propertySource != null) { - composite.addPropertySource(propertySource); - } - } - catch (Exception e) { - if (this.properties.isFailFast()) { - log.error("Fail fast is set and there was an error reading configuration from consul."); - ReflectionUtils.rethrowRuntimeException(e); - } - else { - log.warn("Unable to load consul config from " + propertySourceContext, e); - } + ConsulPropertySource propertySource = sources.createPropertySource(propertySourceContext, true, + this.consul, contextIndex::put); + if (propertySource != null) { + composite.addPropertySource(propertySource); } } @@ -163,30 +97,15 @@ public class ConsulPropertySourceLocator implements PropertySourceLocator, Consu return null; } - private String getContext(String prefix, String context) { - if (StringUtils.isEmpty(prefix)) { - return context; - } - else { - return prefix + "/" + context; - } - } - private void addIndex(String propertySourceContext, Long consulIndex) { this.contextIndex.put(propertySourceContext, consulIndex); } - private ConsulPropertySource create(String context, Map contextIndex) { + private ConsulPropertySource create(String context) { ConsulPropertySource propertySource = new ConsulPropertySource(context, this.consul, this.properties); propertySource.init(); addIndex(context, propertySource.getInitialIndex()); return propertySource; } - private void addProfiles(List contexts, String baseContext, List profiles, String suffix) { - for (String profile : profiles) { - contexts.add(baseContext + this.properties.getProfileSeparator() + profile + suffix); - } - } - } diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySources.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySources.java new file mode 100644 index 00000000..e6b38e72 --- /dev/null +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySources.java @@ -0,0 +1,162 @@ +/* + * Copyright 2015-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.consul.config; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.function.BiConsumer; + +import com.ecwid.consul.v1.ConsulClient; +import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.kv.model.GetValue; +import org.apache.commons.logging.Log; + +import org.springframework.util.StringUtils; + +import static org.springframework.cloud.consul.config.ConsulConfigProperties.Format.FILES; + +public class ConsulPropertySources { + + protected static final List DIR_SUFFIXES = Collections.singletonList("/"); + + protected static final List FILES_SUFFIXES = Collections + .unmodifiableList(Arrays.asList(".yml", ".yaml", ".properties")); + + private final ConsulConfigProperties properties; + + private final Log log; + + public ConsulPropertySources(ConsulConfigProperties properties, Log log) { + this.properties = properties; + this.log = log; + } + + public List getAutomaticContexts(List profiles) { + List contexts = new ArrayList<>(); + String prefix = properties.getPrefix(); + String defaultContext = getContext(prefix, properties.getDefaultContext()); + List suffixes = getSuffixes(); + for (String suffix : suffixes) { + contexts.add(defaultContext + suffix); + } + for (String suffix : suffixes) { + addProfiles(contexts, defaultContext, profiles, suffix); + } + + // getName() defaults to ${spring.application.name} or application + String baseContext = getContext(prefix, properties.getName()); + + for (String suffix : suffixes) { + contexts.add(baseContext + suffix); + } + for (String suffix : suffixes) { + addProfiles(contexts, baseContext, profiles, suffix); + } + // we build them backwards, first wins, so reverse + Collections.reverse(contexts); + return contexts; + } + + protected String getContext(String prefix, String context) { + if (StringUtils.isEmpty(prefix)) { + return context; + } + else { + return prefix + "/" + context; + } + } + + protected List getSuffixes() { + if (properties.getFormat() == FILES) { + return FILES_SUFFIXES; + } + return DIR_SUFFIXES; + } + + private void addProfiles(List contexts, String baseContext, List profiles, String suffix) { + for (String profile : profiles) { + contexts.add(baseContext + properties.getProfileSeparator() + profile + suffix); + } + } + + public ConsulPropertySource createPropertySource(String propertySourceContext, boolean optional, + ConsulClient consul, BiConsumer indexConsumer) { + try { + ConsulPropertySource propertySource = null; + + if (properties.getFormat() == FILES) { + Response response = consul.getKVValue(propertySourceContext, properties.getAclToken()); + indexConsumer.accept(propertySourceContext, response.getConsulIndex()); + if (response.getValue() != null) { + ConsulFilesPropertySource filesPropertySource = new ConsulFilesPropertySource(propertySourceContext, + consul, properties); + filesPropertySource.init(response.getValue()); + propertySource = filesPropertySource; + } + else if (!optional) { + throw new PropertySourceNotFoundException(propertySourceContext); + } + } + else { + propertySource = create(propertySourceContext, consul, indexConsumer); + } + return propertySource; + } + catch (PropertySourceNotFoundException e) { + throw e; + } + catch (Exception e) { + if (properties.isFailFast() || !optional) { + throw new PropertySourceNotFoundException(propertySourceContext, e); + } + else { + log.warn("Unable to load consul config from " + propertySourceContext, e); + } + } + return null; + } + + private ConsulPropertySource create(String context, ConsulClient consulClient, + BiConsumer indexConsumer) { + ConsulPropertySource propertySource = new ConsulPropertySource(context, consulClient, this.properties); + propertySource.init(); + indexConsumer.accept(context, propertySource.getInitialIndex()); + return propertySource; + } + + static class PropertySourceNotFoundException extends RuntimeException { + + private final String context; + + PropertySourceNotFoundException(String context) { + this.context = context; + } + + PropertySourceNotFoundException(String context, Exception cause) { + super(cause); + this.context = context; + } + + public String getContext() { + return this.context; + } + + } + +} diff --git a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulConfigDataLocationResolverTests.java b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulConfigDataLocationResolverTests.java index 171cdaa1..d1fb4f65 100644 --- a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulConfigDataLocationResolverTests.java +++ b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulConfigDataLocationResolverTests.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.function.Supplier; import java.util.stream.Collectors; +import org.apache.commons.logging.LogFactory; import org.junit.jupiter.api.Test; import org.springframework.boot.context.config.ConfigDataLocationResolverContext; @@ -39,7 +40,7 @@ public class ConsulConfigDataLocationResolverTests { @Test public void testParseLocation() { - ConsulConfigDataLocationResolver resolver = new ConsulConfigDataLocationResolver(); + ConsulConfigDataLocationResolver resolver = new ConsulConfigDataLocationResolver(LogFactory.getLog(getClass())); UriComponents uriComponents = resolver.parseLocation(null, "consul:myhost:8501/mypath1;/mypath2;/mypath3"); assertThat(uriComponents.toUri()).hasScheme("consul").hasHost("myhost").hasPort(8501) .hasPath("/mypath1;/mypath2;/mypath3"); @@ -89,7 +90,7 @@ public class ConsulConfigDataLocationResolverTests { } private ConsulConfigDataLocationResolver createResolver() { - ConsulConfigDataLocationResolver resolver = new ConsulConfigDataLocationResolver() { + return new ConsulConfigDataLocationResolver(LogFactory.getLog(getClass())) { @Override public void registerBean(ConfigDataLocationResolverContext context, Class type, T instance) { @@ -107,7 +108,6 @@ public class ConsulConfigDataLocationResolverTests { // do nothing } }; - return resolver; } } diff --git a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorRetryTests.java b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorRetryTests.java index dec4b796..07c868ab 100644 --- a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorRetryTests.java +++ b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorRetryTests.java @@ -17,7 +17,6 @@ package org.springframework.cloud.consul.config; import com.ecwid.consul.transport.TransportException; -import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -27,6 +26,8 @@ import org.springframework.boot.test.system.OutputCaptureRule; import org.springframework.context.annotation.Configuration; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.fail; /** * @author Spencer Gibb @@ -38,15 +39,15 @@ public class ConsulPropertySourceLocatorRetryTests { @Test public void testRetry() { - Assert.assertThrows(TransportException.class, () -> { + assertThatThrownBy(() -> { new SpringApplicationBuilder(Config.class).properties( "spring.application.name=testConsulPropertySourceLocatorRetry", "spring.config.use-legacy-processing=true", "spring.cloud.consul.host=53210a7c-4809-42cb-8b30-057d2db85fcc", "logging.level.org.springframework.retry=TRACE", "server.port=0", "spring.cloud.consul.port=65530", "spring.cloud.consul.retry.maxAttempts=1", "spring.cloud.consul.config.failFast=true").run(); - Assert.fail("Did not throw TransportException"); - }); + fail("Did not throw expected exception"); + }).hasCauseInstanceOf(TransportException.class); assertThat(output).contains("RetryContext retrieved"); }