allow key deletion to trigger configuration refresh

spring-cloud-consul-binder/src/test/java/org/springframework/cloud/consul/binder/config/ConsulBinderConfigurationTests.java

@@ -34,6 +34,9 @@
  */
 public class ConsulBinderConfigurationTests {
 
+	/**
+	 * exception
+	 */
 	@Rule
 	public ExpectedException exception = ExpectedException.none();
 
@@ -55,6 +58,9 @@ public void consulDisabledDisablesBinder() {
 
 	interface Events {
 
+		/**
+		 * @return the channel
+		 */
 		// @Output
 		MessageChannel purchases();
 

spring-cloud-consul-binder/src/test/java/org/springframework/cloud/consul/binder/test/producer/TestProducer.java

@@ -93,6 +93,9 @@ public boolean partitionStrategyInvoked() {
 
 	public static class StubPartitionSelectorStrategy implements PartitionSelectorStrategy {
 
+		/**
+		 * invoked
+		 */
 		public volatile boolean invoked = false;
 
 		@Override

spring-cloud-consul-config/src/main/java/org/springframework/cloud/consul/config/ConfigWatch.java

@@ -57,7 +57,7 @@ public class ConfigWatch implements ApplicationEventPublisherAware, SmartLifecyc
 
 	private final AtomicBoolean running = new AtomicBoolean(false);
 
-	private LinkedHashMap<String, Long> consulIndexes;
+	private final LinkedHashMap<String, Long> consulIndexes;
 
 	private ApplicationEventPublisher publisher;
 
@@ -150,41 +150,32 @@ public void watchConfigKeyValues() {
 
 				// use the consul ACL token if found
 				String aclToken = this.properties.getAclToken();
-				if (StringUtils.isEmpty(aclToken)) {
+				if (!StringUtils.hasLength(aclToken)) {
 					aclToken = null;
 				}
 
 				Response<List<GetValue>> response = this.consul.getKVValues(context, aclToken,
 						new QueryParams(this.properties.getWatch().getWaitTime(), currentIndex));
-
-				// 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)) {
-							if (log.isTraceEnabled()) {
-								log.trace("Context " + context + " has new index " + newIndex);
-							}
-							RefreshEventData data = new RefreshEventData(context, currentIndex, newIndex);
-							this.publisher.publishEvent(new RefreshEvent(this, data, data.toString()));
-						}
-						else if (log.isTraceEnabled()) {
-							log.trace("Event for index already published for context " + context);
+				// if response.value == null, response was a 404, a key is deleted
+				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)) {
+						if (log.isTraceEnabled()) {
+							log.trace("Context " + context + " has new index " + newIndex);
 						}
-						this.consulIndexes.put(context, newIndex);
+						RefreshEventData data = new RefreshEventData(context, currentIndex, newIndex);
+						this.publisher.publishEvent(new RefreshEvent(this, data, data.toString()));
 					}
 					else if (log.isTraceEnabled()) {
-						log.trace("Same index for context " + context);
+						log.trace("Event for index already published for context " + context);
 					}
+					this.consulIndexes.put(context, newIndex);
 				}
 				else if (log.isTraceEnabled()) {
-					log.trace("No value for context " + context);
+					log.trace("Same index for context " + context);
 				}
-
 			}
 			catch (Exception e) {
 				// only fail fast on the initial query, otherwise just log the error

spring-cloud-consul-config/src/test/java/org/springframework/cloud/consul/config/ConfigWatchTests.java

@@ -39,7 +39,6 @@
 import static org.mockito.Mockito.atLeastOnce;
 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;
 import static org.springframework.cloud.consul.config.ConsulConfigProperties.Format.FILES;
@@ -52,7 +51,7 @@ public class ConfigWatchTests {
 	private ConsulConfigProperties configProperties;
 
 	@Before
-	public void setUp() throws Exception {
+	public void setUp() {
 		this.configProperties = new ConsulConfigProperties();
 	}
 
@@ -62,6 +61,11 @@ public void watchPublishesEventWithAcl() {
 
 		setupWatch(eventPublisher, new GetValue(), "/app/", "2ee647bd-bd69-4118-9f34-b9a6e9e60746");
 
+		// there are two threads to publish events here in this UT, the unit test main
+		// thread and the thread started by
+		// config.start(). If you set a breakpoint or have a slow machine, you will see
+		// the test cases fail if we
+		// times(1) here. To work around this problem we use atLeastOnce() here.
 		verify(eventPublisher, atLeastOnce()).publishEvent(any(RefreshEvent.class));
 	}
 
@@ -71,16 +75,16 @@ public void watchPublishesEvent() {
 
 		setupWatch(eventPublisher, new GetValue(), "/app/");
 
-		verify(eventPublisher, times(1)).publishEvent(any(RefreshEvent.class));
+		verify(eventPublisher, atLeastOnce()).publishEvent(any(RefreshEvent.class));
 	}
 
 	@Test
-	public void watchWithNullValueDoesNotPublishEvent() {
+	public void watchForDeletedKeyPublishesEvent() {
 		ApplicationEventPublisher eventPublisher = mock(ApplicationEventPublisher.class);
 
 		setupWatch(eventPublisher, null, "/app/");
 
-		verify(eventPublisher, never()).publishEvent(any(RefreshEvent.class));
+		verify(eventPublisher, atLeastOnce()).publishEvent(any(RefreshEvent.class));
 	}
 
 	@Test
@@ -135,11 +139,11 @@ public void firstCallDoesNotPublishEvent() {
 		Response<List<GetValue>> response = new Response<>(getValues, 1L, false, 1L);
 		when(consul.getKVValues(eq(context), anyString(), any(QueryParams.class))).thenReturn(response);
 
-		ConfigWatch watch = new ConfigWatch(this.configProperties, consul, new LinkedHashMap<String, Long>());
+		ConfigWatch watch = new ConfigWatch(this.configProperties, consul, new LinkedHashMap<>());
 		watch.setApplicationEventPublisher(eventPublisher);
 
 		watch.watchConfigKeyValues();
-		verify(eventPublisher, times(0)).publishEvent(any(RefreshEvent.class));
+		verify(eventPublisher, never()).publishEvent(any(RefreshEvent.class));
 	}
 
 }

spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/test/ConsulTestcontainers.java

@@ -35,6 +35,9 @@ public class ConsulTestcontainers implements ApplicationContextInitializer<Confi
 
 	static final Logger logger = LoggerFactory.getLogger(ConsulTestcontainers.class);
 
+	/**
+	 * the consul server
+	 */
 	public static GenericContainer<?> consul = new GenericContainer<>("consul:1.7.2")
 			.withLogConsumer(new Slf4jLogConsumer(logger).withSeparateOutputStreams())
 			.waitingFor(Wait.forHttp("/v1/status/leader")).withExposedPorts(8500)

spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulHeartbeatTaskTests.java

@@ -35,7 +35,7 @@
 import static org.mockito.Mockito.verify;
 
 /**
- * Test for ConsulHeartbeatTask
+ * Test for ConsulHeartbeatTask.
  *
  * @author Toshiaki Maki
  */

spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoServiceRegistrationRetryTests.java

@@ -47,9 +47,15 @@
 @ContextConfiguration(initializers = ConsulTestcontainers.class)
 public class ConsulAutoServiceRegistrationRetryTests {
 
+	/**
+	 * the exception
+	 */
 	@Rule
 	public ExpectedException exception = ExpectedException.none();
 
+	/**
+	 * the output
+	 */
 	@Rule
 	public OutputCaptureRule output = new OutputCaptureRule();