From bd621e987f1a76844ad20ceebc2adbb72d97ff1d Mon Sep 17 00:00:00 2001
From: Niko <Nizh2008@gmail.com>
Date: Mon, 4 Mar 2019 23:41:35 +0800
Subject: [PATCH] Fix deregister services critical not working when using TTL
 (#500)

* Service health check ttl with critical timeout

fixes gh-490
---
 .../ConsulAutoRegistration.java               |  8 +--
 ...heckTtlDeregisterCriticalServiceTests.java | 64 +++++++++++++++++++
 2 files changed, 68 insertions(+), 4 deletions(-)
 create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistrationCheckTtlDeregisterCriticalServiceTests.java

diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java
index e2ae41d2..4ad5115b 100644
--- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java
+++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistration.java
@@ -223,6 +223,10 @@ public class ConsulAutoRegistration extends ConsulRegistration {
 	public static NewService.Check createCheck(Integer port,
 			HeartbeatProperties ttlConfig, ConsulDiscoveryProperties properties) {
 		NewService.Check check = new NewService.Check();
+		if (StringUtils.hasText(properties.getHealthCheckCriticalTimeout())) {
+			check.setDeregisterCriticalServiceAfter(
+					properties.getHealthCheckCriticalTimeout());
+		}
 		if (ttlConfig.isEnabled()) {
 			check.setTtl(ttlConfig.getTtl());
 			return check;
@@ -241,10 +245,6 @@ public class ConsulAutoRegistration extends ConsulRegistration {
 		check.setHeader(properties.getHealthCheckHeaders());
 		check.setInterval(properties.getHealthCheckInterval());
 		check.setTimeout(properties.getHealthCheckTimeout());
-		if (StringUtils.hasText(properties.getHealthCheckCriticalTimeout())) {
-			check.setDeregisterCriticalServiceAfter(
-					properties.getHealthCheckCriticalTimeout());
-		}
 		check.setTlsSkipVerify(properties.getHealthCheckTlsSkipVerify());
 		return check;
 	}
diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistrationCheckTtlDeregisterCriticalServiceTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistrationCheckTtlDeregisterCriticalServiceTests.java
new file mode 100644
index 00000000..7f81b5a1
--- /dev/null
+++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/serviceregistry/ConsulAutoRegistrationCheckTtlDeregisterCriticalServiceTests.java
@@ -0,0 +1,64 @@
+/*
+ * Copyright 2013-2019 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.serviceregistry;
+
+import com.ecwid.consul.v1.agent.model.NewService;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationConfiguration;
+import org.springframework.cloud.consul.ConsulAutoConfiguration;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.context.junit4.SpringRunner;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author Niko Tung
+ */
+@RunWith(SpringRunner.class)
+@SpringBootTest(classes = ConsulAutoRegistrationCheckTtlDeregisterCriticalServiceTests.TestConfig.class, properties = {
+		"spring.application.name=myConsulServiceRegistryHealthCheckTtlDeregisterCriticalServiceAfter-N",
+		"spring.cloud.consul.discovery.health-check-critical-timeout=1m",
+		"spring.cloud.consul.discovery.heartbeat.enabled=true" }, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+public class ConsulAutoRegistrationCheckTtlDeregisterCriticalServiceTests {
+
+	@Autowired
+	private ConsulRegistration registration;
+
+	@Test
+	public void contextLoads() {
+		NewService service = registration.getService();
+		assertThat("1m".equals(service.getCheck().getDeregisterCriticalServiceAfter()))
+				.as("Service with heartbeat check and deregister critical timeout registered")
+				.isTrue();
+	}
+
+	@Configuration
+	@EnableAutoConfiguration
+	@ImportAutoConfiguration({ AutoServiceRegistrationConfiguration.class,
+			ConsulAutoConfiguration.class,
+			ConsulAutoServiceRegistrationAutoConfiguration.class })
+	protected static class TestConfig {
+
+	}
+
+}