diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConfigWatch.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConfigWatch.java index 18c7b50c..64c95d90 100644 --- a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConfigWatch.java +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConfigWatch.java @@ -17,27 +17,28 @@ package org.springframework.cloud.consul.config; import java.io.Closeable; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; + import javax.annotation.PostConstruct; +import org.springframework.cloud.endpoint.event.RefreshEvent; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.context.ApplicationEventPublisherAware; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; + import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.QueryParams; import com.ecwid.consul.v1.Response; import com.ecwid.consul.v1.kv.model.GetValue; -import org.springframework.cloud.endpoint.event.RefreshEvent; -import org.springframework.context.ApplicationEventPublisher; -import org.springframework.context.ApplicationEventPublisherAware; -import org.springframework.scheduling.annotation.Scheduled; -import org.springframework.util.StringUtils; +import static org.springframework.cloud.consul.config.ConsulConfigProperties.Format.FILES; import lombok.Data; import lombok.extern.apachecommons.CommonsLog; -import org.springframework.util.ReflectionUtils; - -import static org.springframework.cloud.consul.config.ConsulConfigProperties.Format.FILES; /** * @author Spencer Gibb @@ -46,17 +47,21 @@ import static org.springframework.cloud.consul.config.ConsulConfigProperties.For public class ConfigWatch implements Closeable, ApplicationEventPublisherAware { private final ConsulConfigProperties properties; - private final List contexts; private final ConsulClient consul; - private AtomicBoolean running = new AtomicBoolean(false); + private LinkedHashMap consulIndexes; + private final AtomicBoolean running = new AtomicBoolean(false); private ApplicationEventPublisher publisher; - private HashMap consulIndexes = new HashMap<>(); - private Boolean initialized = false; + private boolean firstTime = true; + @Deprecated public ConfigWatch(ConsulConfigProperties properties, List contexts, ConsulClient consul) { + this(properties, consul, new LinkedHashMap()); + } + + public ConfigWatch(ConsulConfigProperties properties, ConsulClient consul, LinkedHashMap initialIndexes) { this.properties = properties; - this.contexts = contexts; this.consul = consul; + this.consulIndexes = new LinkedHashMap<>(initialIndexes); } @Override @@ -72,7 +77,7 @@ public class ConfigWatch implements Closeable, ApplicationEventPublisherAware { @Scheduled(fixedDelayString = "${spring.cloud.consul.config.watch.delay:1000}") public void watchConfigKeyValues() { if (this.running.get()) { - for (String context : this.contexts) { + for (String context : this.consulIndexes.keySet()) { // turn the context into a Consul folder path (unless our config format are FILES) if (properties.getFormat() != FILES && !context.endsWith("/")) { @@ -112,7 +117,7 @@ public class ConfigWatch implements Closeable, ApplicationEventPublisherAware { } catch (Exception e) { // only fail fast on the initial query, otherwise just log the error - if (!initialized && this.properties.isFailFast()) { + if (firstTime && this.properties.isFailFast()) { log.error("Fail fast is set and there was an error reading configuration from consul."); ReflectionUtils.rethrowRuntimeException(e); } else if (log.isTraceEnabled()) { @@ -124,7 +129,7 @@ public class ConfigWatch implements Closeable, ApplicationEventPublisherAware { } } } - initialized = true; + firstTime = false; } @Override @@ -132,10 +137,6 @@ public class ConfigWatch implements Closeable, ApplicationEventPublisherAware { this.running.compareAndSet(true, false); } - /* for testing */ HashMap getConsulIndexes() { - return this.consulIndexes; - } - @Data static class RefreshEventData { private final String context; 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 ba89c359..61770dbf 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 @@ -40,7 +40,7 @@ public class ConsulConfigAutoConfiguration { @ConditionalOnProperty(name = "spring.cloud.consul.config.watch.enabled", matchIfMissing = true) public ConfigWatch configWatch(ConsulConfigProperties properties, ConsulPropertySourceLocator locator, ConsulClient consul) { - return new ConfigWatch(properties, locator.getContexts(), consul); + return new ConfigWatch(properties, consul, locator.getContextIndexes()); } } } diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySource.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySource.java index 2053ffe4..176c8181 100644 --- a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySource.java +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConsulPropertySource.java @@ -48,6 +48,8 @@ public class ConsulPropertySource extends EnumerablePropertySource private final Map properties = new LinkedHashMap<>(); + private Long initialIndex; + public ConsulPropertySource(String context, ConsulClient source, ConsulConfigProperties configProperties) { super(context, source); @@ -64,6 +66,8 @@ public class ConsulPropertySource extends EnumerablePropertySource Response> response = source.getKVValues(context, configProperties.getAclToken(), QueryParams.DEFAULT); + initialIndex = response.getConsulIndex(); + final List values = response.getValue(); ConsulConfigProperties.Format format = configProperties.getFormat(); switch (format) { @@ -76,6 +80,10 @@ public class ConsulPropertySource extends EnumerablePropertySource } } + public Long getInitialIndex() { + return initialIndex; + } + /** * Parses the properties in key value style i.e., values are expected to be either a * sub key or a constant 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 f9e8e29a..011f107a 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,7 +19,9 @@ package org.springframework.cloud.consul.config; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.cloud.bootstrap.config.PropertySourceLocator; @@ -35,10 +37,10 @@ import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.Response; import com.ecwid.consul.v1.kv.model.GetValue; -import lombok.extern.apachecommons.CommonsLog; - import static org.springframework.cloud.consul.config.ConsulConfigProperties.Format.FILES; +import lombok.extern.apachecommons.CommonsLog; + /** * @author Spencer Gibb */ @@ -46,21 +48,28 @@ import static org.springframework.cloud.consul.config.ConsulConfigProperties.For @CommonsLog public class ConsulPropertySourceLocator implements PropertySourceLocator { - private ConsulClient consul; + private final ConsulClient consul; - private ConsulConfigProperties properties; + private final ConsulConfigProperties properties; - private List contexts = new ArrayList<>(); + private final List contexts = new ArrayList<>(); + + private final LinkedHashMap contextIndex = new LinkedHashMap<>(); public ConsulPropertySourceLocator(ConsulClient consul, ConsulConfigProperties properties) { this.consul = consul; this.properties = properties; } + @Deprecated public List getContexts() { return contexts; } + public LinkedHashMap getContextIndexes() { + return contextIndex; + } + @Override @Retryable(interceptor = "consulRetryInterceptor") public PropertySource locate(Environment environment) { @@ -112,13 +121,14 @@ public class ConsulPropertySourceLocator implements PropertySourceLocator { 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); + propertySource = create(propertySourceContext, contextIndex); } if (propertySource != null) { composite.addPropertySource(propertySource); @@ -138,9 +148,14 @@ public class ConsulPropertySourceLocator implements PropertySourceLocator { return null; } - private ConsulPropertySource create(String context) { + private void addIndex(String propertySourceContext, Long consulIndex) { + contextIndex.put(propertySourceContext, consulIndex); + } + + private ConsulPropertySource create(String context, Map contextIndex) { ConsulPropertySource propertySource = new ConsulPropertySource(context, this.consul, this.properties); propertySource.init(); + addIndex(context, propertySource.getInitialIndex()); return propertySource; } diff --git a/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/PropertySourcesLocatedEvent.java b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/PropertySourcesLocatedEvent.java new file mode 100644 index 00000000..6eed107d --- /dev/null +++ b/spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/PropertySourcesLocatedEvent.java @@ -0,0 +1,43 @@ +/* + * Copyright 2013-2017 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 + * + * 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. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.consul.config; + +import org.springframework.context.ApplicationEvent; + +import java.util.LinkedHashMap; + +/** + * @author Spencer Gibb + */ +public class PropertySourcesLocatedEvent extends ApplicationEvent { + + private final LinkedHashMap contextsToIndexes; + + /** + * Create a new ApplicationEvent. + * + * @param source the object on which the event initially occurred (never {@code null}) + */ + public PropertySourcesLocatedEvent(Object source, LinkedHashMap contextsToIndexes) { + super(source); + this.contextsToIndexes = contextsToIndexes; + } + + public LinkedHashMap getContextsToIndexes() { + return contextsToIndexes; + } +} diff --git a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java index 299ce508..1bdd0251 100644 --- a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java +++ b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java @@ -27,6 +27,8 @@ import org.springframework.context.ApplicationEventPublisher; import org.springframework.util.StringUtils; import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import static org.mockito.Matchers.any; @@ -107,12 +109,33 @@ public class ConfigWatchTests { configProperties.setAclToken(aclToken); } - ConfigWatch watch = new ConfigWatch(configProperties, Arrays.asList(context), consul); + LinkedHashMap initialIndexes = new LinkedHashMap<>(); + initialIndexes.put(context, 0L); + ConfigWatch watch = new ConfigWatch(configProperties, consul, initialIndexes); watch.setApplicationEventPublisher(eventPublisher); - watch.getConsulIndexes().put(context, 0L); watch.start(); watch.watchConfigKeyValues(); } + @Test + public void firstCallDoesNotPublishEvent() { + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + configProperties.setFormat(FILES); + + GetValue getValue = new GetValue(); + String context = "/config/app.yml"; + ConsulClient consul = mock(ConsulClient.class); + List getValues = Collections.singletonList(getValue); + + Response> response = new Response<>(getValues, 1L, false, 1L); + when(consul.getKVValues(eq(context), anyString(), any(QueryParams.class))).thenReturn(response); + + ConfigWatch watch = new ConfigWatch(configProperties, consul, new LinkedHashMap()); + watch.setApplicationEventPublisher(eventPublisher); + + watch.watchConfigKeyValues(); + verify(eventPublisher, times(0)).publishEvent(any(RefreshEvent.class)); + } + } diff --git a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorTests.java b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorTests.java index 07a8cd43..bca935a4 100644 --- a/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorTests.java +++ b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConsulPropertySourceLocatorTests.java @@ -19,28 +19,26 @@ package org.springframework.cloud.consul.config; import java.util.UUID; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; - -import com.ecwid.consul.v1.ConsulClient; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.cloud.consul.ConsulProperties; -import org.springframework.cloud.context.refresh.ContextRefresher; -import org.springframework.cloud.context.scope.refresh.RefreshScope; -import org.springframework.cloud.endpoint.RefreshEndpoint; +import org.springframework.cloud.context.environment.EnvironmentChangeEvent; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.event.EventListener; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.test.annotation.DirtiesContext; +import com.ecwid.consul.v1.ConsulClient; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; /** @@ -48,6 +46,7 @@ import static org.junit.Assert.assertThat; */ @DirtiesContext public class ConsulPropertySourceLocatorTests { + private static final String APP_NAME = "testConsulPropertySourceLocator"; private static final String PREFIX = "_propertySourceLocatorTests_config__"; private static final String ROOT = PREFIX + UUID.randomUUID(); private static final String VALUE1 = "testPropVal"; @@ -56,38 +55,32 @@ public class ConsulPropertySourceLocatorTests { private static final String VALUE2 = "testPropVal2"; private static final String TEST_PROP2 = "testProp2"; private static final String KEY2 = ROOT + "/application/"+ TEST_PROP2; + private static final String TEST_PROP3 = "testProp3"; + private static final String KEY3 = ROOT + "/"+APP_NAME+"/"+ TEST_PROP3; private ConfigurableApplicationContext context; @Configuration @EnableAutoConfiguration static class Config { + @Bean - public RefreshEndpoint refreshEndpoint(ConfigurableApplicationContext context, - RefreshScope scope) { - RefreshEndpoint endpoint = new TestRefreshEndpoint(context, scope); - return endpoint; - } - } - - static class TestRefreshEndpoint extends RefreshEndpoint { - private CountDownLatch successLatch = new CountDownLatch(1); - private CountDownLatch toManyLatch = new CountDownLatch(1); - private AtomicInteger count = new AtomicInteger(); - - public TestRefreshEndpoint( ConfigurableApplicationContext context, RefreshScope scope) { - super(new ContextRefresher(context, scope)); + public CountDownLatch countDownLatch1() { + return new CountDownLatch(1); } - @Override - public synchronized String[] refresh() { - String[] keys = super.refresh(); - if (this.count.incrementAndGet() == 1) { - this.successLatch.countDown(); - } else { - this.toManyLatch.countDown(); + @Bean + public CountDownLatch countDownLatch2() { + return new CountDownLatch(1); + } + + @EventListener + public void handle(EnvironmentChangeEvent event) { + if (event.getKeys().contains(TEST_PROP)) { + countDownLatch1().countDown(); + } else if (event.getKeys().contains(TEST_PROP3)) { + countDownLatch2().countDown(); } - return keys; } } @@ -106,9 +99,9 @@ public class ConsulPropertySourceLocatorTests { this.context = new SpringApplicationBuilder(Config.class) .web(false) - .run("--SPRING_APPLICATION_NAME=testConsulPropertySourceLocator", + .run("--SPRING_APPLICATION_NAME="+ APP_NAME, "--spring.cloud.consul.config.prefix="+ROOT, - "spring.cloud.consul.config.watch.delay=1"); + "spring.cloud.consul.config.watch.delay=10"); this.client = context.getBean(ConsulClient.class); this.properties = context.getBean(ConsulProperties.class); @@ -128,21 +121,32 @@ public class ConsulPropertySourceLocatorTests { } @Test - @Ignore("failing on travis") public void propertyLoadedAndUpdated() throws Exception { String testProp = this.environment.getProperty(TEST_PROP); assertThat("testProp was wrong", testProp, is(equalTo(VALUE1))); this.client.setKVValue(KEY1, "testPropValUpdate"); - TestRefreshEndpoint endpoint = this.context.getBean(TestRefreshEndpoint.class); - boolean receivedEvent = endpoint.successLatch.await(3, TimeUnit.MINUTES); + CountDownLatch latch = this.context.getBean("countDownLatch1", CountDownLatch.class); + boolean receivedEvent = latch.await(15, TimeUnit.SECONDS); assertThat("listener didn't receive event", receivedEvent, is(true)); testProp = this.environment.getProperty(TEST_PROP); assertThat("testProp was wrong after update", testProp, is(equalTo("testPropValUpdate"))); + } - boolean receivedExtraEvent = endpoint.toManyLatch.await(15, TimeUnit.SECONDS); - assertThat("refresh called to many times", receivedExtraEvent, is(false)); + @Test + public void contextDoesNotExistThenExists() throws Exception { + String testProp = this.environment.getProperty(TEST_PROP3); + assertThat("testProp was wrong", testProp, is(nullValue())); + + this.client.setKVValue(KEY3, "testPropValInsert"); + + CountDownLatch latch = this.context.getBean("countDownLatch2", CountDownLatch.class); + boolean receivedEvent = latch.await(15, TimeUnit.SECONDS); + assertThat("listener didn't receive event", receivedEvent, is(true)); + + testProp = this.environment.getProperty(TEST_PROP3); + assertThat(TEST_PROP3 + " was wrong after update", testProp, is(equalTo("testPropValInsert"))); } }