From 1840647458d849c372a36ac3c19b5d59085463d6 Mon Sep 17 00:00:00 2001
From: Spencer Gibb <spencer@gibb.us>
Date: Wed, 16 Aug 2017 00:53:40 -0600
Subject: [PATCH] Move `EurekaClient.close()` to `EurekaRegistration.close()`.

This moves the lifecycle of the eureka client of of EurekaServiceRegistry.

fixes gh-2099
---
 .../eureka/EurekaClientConfigBean.java        |  6 +-
 .../serviceregistry/EurekaRegistration.java   |  9 ++-
 .../EurekaServiceRegistry.java                |  5 +-
 .../EurekaClientAutoConfigurationTests.java   | 73 ++++++++++++++++++-
 .../EurekaServiceRegistryTests.java           | 60 +++++++++++++++
 5 files changed, 144 insertions(+), 9 deletions(-)
 create mode 100644 spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistryTests.java

diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java
index a5747d37..3e5ab858 100644
--- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java
+++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java
@@ -43,9 +43,6 @@ public class EurekaClientConfigBean implements EurekaClientConfig {
 
 	public static final String PREFIX = "eureka.client";
 
-	@Autowired(required = false)
-	PropertyResolver propertyResolver;
-
 	public static final String DEFAULT_URL = "http://localhost:8761" + DEFAULT_PREFIX
 			+ "/";
 
@@ -53,6 +50,9 @@ public class EurekaClientConfigBean implements EurekaClientConfig {
 
 	private static final int MINUTES = 60;
 
+	@Autowired(required = false)
+	PropertyResolver propertyResolver;
+
 	/**
 	 * Flag to indicate that the Eureka client is enabled.
 	 */
diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java
index c4ea82b0..e0afab9c 100644
--- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java
+++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaRegistration.java
@@ -17,6 +17,8 @@
 
 package org.springframework.cloud.netflix.eureka.serviceregistry;
 
+import java.io.Closeable;
+import java.io.IOException;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.commons.logging.Log;
@@ -39,7 +41,7 @@ import com.netflix.discovery.EurekaClientConfig;
 /**
  * @author Spencer Gibb
  */
-public class EurekaRegistration implements Registration {
+public class EurekaRegistration implements Registration, Closeable {
 	private static final Log log = LogFactory.getLog(EurekaRegistration.class);
 
 	private final EurekaClient eurekaClient;
@@ -159,4 +161,9 @@ public class EurekaRegistration implements Registration {
 	public int getNonSecurePort() {
 		return this.instanceConfig.getNonSecurePort();
 	}
+
+	@Override
+	public void close() throws IOException {
+		this.eurekaClient.shutdown();
+	}
 }
diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistry.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistry.java
index 950e57d1..de11522f 100644
--- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistry.java
+++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistry.java
@@ -67,8 +67,9 @@ public class EurekaServiceRegistry implements ServiceRegistry<EurekaRegistration
 
 			reg.getApplicationInfoManager().setInstanceStatus(InstanceInfo.InstanceStatus.DOWN);
 
-			//TODO: on deregister or on context shutdown
-			reg.getEurekaClient().shutdown();
+			//shutdown of eureka client should happen with EurekaRegistration.close()
+			//auto registration will create a bean which will be properly disposed
+			//manual registrations will need to call close()
 		}
 	}
 
diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java
index a1fa3933..00d8ab80 100644
--- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java
+++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java
@@ -17,25 +17,38 @@
 
 package org.springframework.cloud.netflix.eureka;
 
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
+
+import org.assertj.core.api.Assertions;
 import org.junit.After;
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.springframework.aop.scope.ScopedProxyFactoryBean;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.condition.SearchStrategy;
 import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.boot.test.util.EnvironmentTestUtils;
 import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration;
 import org.springframework.cloud.commons.util.UtilAutoConfiguration;
+import org.springframework.cloud.netflix.eureka.serviceregistry.EurekaRegistration;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 
+import com.netflix.appinfo.ApplicationInfoManager;
+import com.netflix.discovery.EurekaClient;
+import com.netflix.discovery.EurekaClientConfig;
 import com.netflix.discovery.shared.transport.jersey.EurekaJerseyClient;
 import com.sun.jersey.client.apache4.ApacheHttpClient4;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.springframework.boot.test.util.EnvironmentTestUtils.addEnvironment;
 
 /**
@@ -46,8 +59,8 @@ public class EurekaClientAutoConfigurationTests {
 	private AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
 
 	@After
-	public void init() {
-		if (this.context != null) {
+	public void after() {
+		if (this.context != null && this.context.isActive()) {
 			this.context.close();
 		}
 	}
@@ -292,6 +305,26 @@ public class EurekaClientAutoConfigurationTests {
 		this.context.getBean(EurekaHealthIndicator.class);
 	}
 
+	@Test
+	public void eurekaClientClosed() {
+		setupContext(TestEurekaClientConfiguration.class);
+		if (this.context != null) {
+			CountDownLatch latch = this.context.getBean(CountDownLatch.class);
+			this.context.close();
+			Assertions.assertThat(latch.getCount()).isEqualTo(0);
+		}
+	}
+
+	@Test
+	public void eurekaRegistrationClosed() throws IOException {
+		setupContext(TestEurekaRegistrationConfiguration.class);
+		if (this.context != null) {
+			EurekaRegistration registration = this.context.getBean(EurekaRegistration.class);
+			this.context.close();
+			verify(registration).close();
+		}
+	}
+
 	private void testNonSecurePort(String propName) {
 		addEnvironment(this.context, propName + ":8888");
 		setupContext();
@@ -305,8 +338,42 @@ public class EurekaClientAutoConfigurationTests {
 	@Configuration
 	@EnableConfigurationProperties
 	@Import({ UtilAutoConfiguration.class, EurekaClientAutoConfiguration.class })
-	protected static class TestConfiguration {
+	protected static class TestConfiguration { }
 
+	@Configuration
+	protected static class TestEurekaClientConfiguration {
+
+		@Bean
+		public CountDownLatch countDownLatch() {
+			return new CountDownLatch(1);
+		}
+
+		@Bean(destroyMethod = "shutdown")
+		@ConditionalOnMissingBean(value = EurekaClient.class, search = SearchStrategy.CURRENT)
+		public EurekaClient eurekaClient(ApplicationInfoManager manager, EurekaClientConfig config, ApplicationContext context) {
+			return new CloudEurekaClient(manager, config, null, context) {
+				@Override
+				public synchronized void shutdown() {
+					CountDownLatch latch = countDownLatch();
+					if (latch.getCount() == 1) {
+						latch.countDown();
+					}
+					super.shutdown();
+				}
+			};
+		}
+	}
+
+	@Configuration
+	protected static class TestEurekaRegistrationConfiguration {
+
+		@Bean
+		public EurekaRegistration eurekaRegistration(EurekaClient eurekaClient, CloudEurekaInstanceConfig instanceConfig, ApplicationInfoManager applicationInfoManager) {
+			return spy(EurekaRegistration.builder(instanceConfig)
+					.with(applicationInfoManager)
+					.with(eurekaClient)
+					.build());
+		}
 	}
 
 	@Configuration
diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistryTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistryTests.java
new file mode 100644
index 00000000..2ffe3189
--- /dev/null
+++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/serviceregistry/EurekaServiceRegistryTests.java
@@ -0,0 +1,60 @@
+/*
+ * 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.netflix.eureka.serviceregistry;
+
+import org.junit.Test;
+import org.springframework.cloud.commons.util.InetUtils;
+import org.springframework.cloud.commons.util.InetUtilsProperties;
+import org.springframework.cloud.netflix.eureka.CloudEurekaClient;
+import org.springframework.cloud.netflix.eureka.EurekaClientConfigBean;
+import org.springframework.cloud.netflix.eureka.EurekaInstanceConfigBean;
+import org.springframework.context.ApplicationEventPublisher;
+
+import com.netflix.appinfo.ApplicationInfoManager;
+import com.netflix.appinfo.InstanceInfo;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author Spencer Gibb
+ */
+public class EurekaServiceRegistryTests {
+
+	@Test
+	public void eurekaClientNotShutdownInDeregister() {
+		EurekaServiceRegistry registry = new EurekaServiceRegistry();
+
+		CloudEurekaClient eurekaClient = mock(CloudEurekaClient.class);
+		ApplicationInfoManager applicationInfoManager = mock(ApplicationInfoManager.class);
+
+		when(applicationInfoManager.getInfo()).thenReturn(mock(InstanceInfo.class));
+
+		EurekaRegistration registration = EurekaRegistration.builder(new EurekaInstanceConfigBean(new InetUtils(new InetUtilsProperties())))
+				.with(eurekaClient)
+				.with(applicationInfoManager)
+				.with(new EurekaClientConfigBean(), mock(ApplicationEventPublisher.class))
+				.build();
+
+		registry.deregister(registration);
+
+		verifyZeroInteractions(eurekaClient);
+	}
+
+}