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 a5109761..3cf0b794 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 @@ -82,15 +82,19 @@ public class ConfigWatch implements Closeable, ApplicationEventPublisherAware { Response> response = this.consul.getKVValues(context, new QueryParams(2, currentIndex)); - Long newIndex = response.getConsulIndex(); + // if response.value == null, response was a 404, otherwise it was a 200 + // reducing churn if there wasn't anything + if (response.getValue() != null && !response.getValue().isEmpty()) { + Long newIndex = response.getConsulIndex(); - if (newIndex != null && !newIndex.equals(currentIndex)) { - // don't publish the same index again, don't publish the first time (-1) so index can be primed - if (!this.consulIndexes.containsValue(newIndex) && !currentIndex.equals(-1L)) { - RefreshEventData data = new RefreshEventData(context, currentIndex, newIndex); - this.publisher.publishEvent(new RefreshEvent(this, data, data.toString())); + if (newIndex != null && !newIndex.equals(currentIndex)) { + // don't publish the same index again, don't publish the first time (-1) so index can be primed + if (!this.consulIndexes.containsValue(newIndex) && !currentIndex.equals(-1L)) { + RefreshEventData data = new RefreshEventData(context, currentIndex, newIndex); + this.publisher.publishEvent(new RefreshEvent(this, data, data.toString())); + } + this.consulIndexes.put(context, newIndex); } - this.consulIndexes.put(context, newIndex); } } catch (Exception e) { @@ -115,6 +119,10 @@ 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/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java new file mode 100644 index 00000000..37d2f0b2 --- /dev/null +++ b/spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java @@ -0,0 +1,80 @@ +/* + * Copyright 2013-2016 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 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.junit.Test; +import org.springframework.cloud.endpoint.event.RefreshEvent; +import org.springframework.context.ApplicationEventPublisher; + +import java.util.Arrays; +import java.util.List; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author Spencer Gibb + */ +public class ConfigWatchTests { + + @Test + public void watchPublishesEvent() { + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + + setupWatch(eventPublisher, new GetValue()); + + verify(eventPublisher, times(1)).publishEvent(any(RefreshEvent.class)); + } + + @Test + public void watchWithNullValueDoesNotPublishEvent() { + ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class); + + setupWatch(eventPublisher, null); + + verify(eventPublisher, never()).publishEvent(any(RefreshEvent.class)); + } + + private void setupWatch(ApplicationEventPublisher eventPublisher, GetValue getValue) { + ConsulClient consul = mock(ConsulClient.class); + List getValues = null; + + if (getValue != null) { + getValues = Arrays.asList(getValue); + } + + Response> response = new Response<>(getValues, 1L, false, 1L); + when(consul.getKVValues(eq("/app/"), any(QueryParams.class))).thenReturn(response); + + ConfigWatch watch = new ConfigWatch(new ConsulConfigProperties(), Arrays.asList("/app/"), consul); + watch.setApplicationEventPublisher(eventPublisher); + watch.getConsulIndexes().put("/app/", 0L); + watch.start(); + + watch.watchConfigKeyValues(); + } + +}