Deprecates SimpleServiceInstance in favor of DefaultServiceInstance (#835)

* Initial Commit

* Added URI to DefaultServiceInstance

* Added default constructor

* Fixed PR Comments

spring-cloud-commons/src/main/java/org/springframework/cloud/client/DefaultServiceInstance.java

@@ -26,20 +26,23 @@ import java.util.Objects;
  *
  * @author Spencer Gibb
  * @author Tim Ysewyn
+ * @author Charu Covindane
  */
 public class DefaultServiceInstance implements ServiceInstance {
 
-	private final String instanceId;
+	private String instanceId;
 
-	private final String serviceId;
+	private String serviceId;
 
-	private final String host;
+	private String host;
 
-	private final int port;
+	private int port;
 
-	private final boolean secure;
+	private boolean secure;
 
-	private final Map<String, String> metadata;
+	private Map<String, String> metadata = new LinkedHashMap<>();
+
+	private URI uri;
 
 	/**
 	 * @param instanceId the id of the instance.
@@ -98,6 +101,9 @@ public class DefaultServiceInstance implements ServiceInstance {
 		this(serviceId, host, port, secure, new LinkedHashMap<>());
 	}
 
+	public DefaultServiceInstance() {
+	}
+
 	/**
 	 * Creates a URI from the given ServiceInstance's host:port.
 	 * @param instance the ServiceInstance.
@@ -145,6 +151,32 @@ public class DefaultServiceInstance implements ServiceInstance {
 		return this.secure;
 	}
 
+	public void setInstanceId(String instanceId) {
+		this.instanceId = instanceId;
+	}
+
+	public void setServiceId(String serviceId) {
+		this.serviceId = serviceId;
+	}
+
+	public void setHost(String host) {
+		this.host = host;
+	}
+
+	public void setPort(int port) {
+		this.port = port;
+	}
+
+	public void setUri(URI uri) {
+		this.uri = uri;
+		this.host = this.uri.getHost();
+		this.port = this.uri.getPort();
+		String scheme = this.uri.getScheme();
+		if ("https".equals(scheme)) {
+			this.secure = true;
+		}
+	}
+
 	@Override
 	public String toString() {
 		return "DefaultServiceInstance{" + "instanceId='" + this.instanceId + '\''

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClient.java

@@ -19,9 +19,9 @@ package org.springframework.cloud.client.discovery.simple;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
-import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties.SimpleServiceInstance;
 
 /**
  * A {@link org.springframework.cloud.client.discovery.DiscoveryClient} that will use the
@@ -29,6 +29,7 @@ import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperti
  *
  * @author Biju Kunjummen
  * @author Olga Maciaszek-Sharma
+ * @author Charu Covindane
  */
 public class SimpleDiscoveryClient implements DiscoveryClient {
 
@@ -46,7 +47,7 @@ public class SimpleDiscoveryClient implements DiscoveryClient {
 	@Override
 	public List<ServiceInstance> getInstances(String serviceId) {
 		List<ServiceInstance> serviceInstances = new ArrayList<>();
-		List<SimpleServiceInstance> serviceInstanceForService = this.simpleDiscoveryProperties
+		List<DefaultServiceInstance> serviceInstanceForService = this.simpleDiscoveryProperties
 				.getInstances().get(serviceId);
 
 		if (serviceInstanceForService != null) {

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClientAutoConfiguration.java

@@ -16,8 +16,6 @@
 
 package org.springframework.cloud.client.discovery.simple;
 
-import java.net.URI;
-
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.AutoConfigureBefore;
@@ -37,6 +35,7 @@ import org.springframework.core.annotation.Order;
  * Spring Boot auto-configuration for simple properties-based discovery client.
  *
  * @author Biju Kunjummen
+ * @author Charu Covindane
  */
 @Configuration(proxyBeanMethods = false)
 @AutoConfigureBefore({ NoopDiscoveryClientAutoConfiguration.class,
@@ -67,10 +66,9 @@ public class SimpleDiscoveryClientAutoConfiguration
 	public SimpleDiscoveryProperties simpleDiscoveryProperties(
 			@Value("${spring.application.name:application}") String serviceId) {
 		simple.getLocal().setServiceId(serviceId);
-		simple.getLocal()
-				.setUri(URI.create(
-						"http://" + this.inet.findFirstNonLoopbackHostInfo().getHostname()
-								+ ":" + findPort()));
+		simple.getLocal().setHost(this.inet.findFirstNonLoopbackHostInfo().getHostname());
+		simple.getLocal().setPort(findPort());
+
 		return simple;
 	}
 
@@ -95,10 +93,7 @@ public class SimpleDiscoveryClientAutoConfiguration
 	public void onApplicationEvent(WebServerInitializedEvent webServerInitializedEvent) {
 		this.port = webServerInitializedEvent.getWebServer().getPort();
 		if (this.port > 0) {
-			simple.getLocal()
-					.setUri(URI.create("http://"
-							+ this.inet.findFirstNonLoopbackHostInfo().getHostname() + ":"
-							+ this.port));
+			simple.getLocal().setPort(this.port);
 		}
 	}
 

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryProperties.java

@@ -25,6 +25,7 @@ import java.util.Map;
 import javax.annotation.PostConstruct;
 
 import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 
@@ -38,31 +39,33 @@ import org.springframework.cloud.client.discovery.DiscoveryClient;
  * @author Biju Kunjummen
  * @author Olga Maciaszek-Sharma
  * @author Tim Ysewyn
+ * @author Charu Covindane
  */
 
 @ConfigurationProperties(prefix = "spring.cloud.discovery.client.simple")
 public class SimpleDiscoveryProperties {
 
-	private Map<String, List<SimpleServiceInstance>> instances = new HashMap<>();
+	private Map<String, List<DefaultServiceInstance>> instances = new HashMap<>();
 
 	/**
 	 * The properties of the local instance (if it exists). Users should set these
 	 * properties explicitly if they are exporting data (e.g. metrics) that need to be
 	 * identified by the service instance.
 	 */
-	private SimpleServiceInstance local = new SimpleServiceInstance();
+	private DefaultServiceInstance local = new DefaultServiceInstance(null, null, null, 0,
+			false);
 
 	private int order = DiscoveryClient.DEFAULT_ORDER;
 
-	public Map<String, List<SimpleServiceInstance>> getInstances() {
+	public Map<String, List<DefaultServiceInstance>> getInstances() {
 		return this.instances;
 	}
 
-	public void setInstances(Map<String, List<SimpleServiceInstance>> instances) {
+	public void setInstances(Map<String, List<DefaultServiceInstance>> instances) {
 		this.instances = instances;
 	}
 
-	public SimpleServiceInstance getLocal() {
+	public DefaultServiceInstance getLocal() {
 		return this.local;
 	}
 
@@ -77,15 +80,22 @@ public class SimpleDiscoveryProperties {
 	@PostConstruct
 	public void init() {
 		for (String key : this.instances.keySet()) {
-			for (SimpleServiceInstance instance : this.instances.get(key)) {
+			for (DefaultServiceInstance instance : this.instances.get(key)) {
 				instance.setServiceId(key);
 			}
 		}
 	}
 
+	public void setInstance(String serviceId, String host, int port) {
+		local = new DefaultServiceInstance(null, serviceId, host, port, false);
+	}
+
 	/**
 	 * Basic implementation of {@link ServiceInstance}.
+	 *
+	 * @deprecated in favor of {@link DefaultServiceInstance}
 	 */
+	@Deprecated
 	public static class SimpleServiceInstance implements ServiceInstance {
 
 		/**

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/reactive/SimpleReactiveDiscoveryClientAutoConfiguration.java

@@ -16,8 +16,6 @@
 
 package org.springframework.cloud.client.discovery.simple.reactive;
 
-import java.net.URI;
-
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.actuate.health.ReactiveHealthIndicator;
@@ -44,6 +42,7 @@ import org.springframework.core.annotation.Order;
  * Spring Boot auto-configuration for simple properties-based reactive discovery client.
  *
  * @author Tim Ysewyn
+ * @author Charu Covindane
  * @since 2.2.0
  */
 @Configuration(proxyBeanMethods = false)
@@ -71,8 +70,8 @@ public class SimpleReactiveDiscoveryClientAutoConfiguration
 	@Bean
 	public SimpleReactiveDiscoveryProperties simpleReactiveDiscoveryProperties() {
 		simple.getLocal().setServiceId(serviceId);
-		simple.getLocal().setUri(URI.create("http://"
-				+ inet.findFirstNonLoopbackHostInfo().getHostname() + ":" + findPort()));
+		simple.getLocal().setHost(inet.findFirstNonLoopbackHostInfo().getHostname());
+		simple.getLocal().setPort(findPort());
 		return simple;
 	}
 
@@ -96,8 +95,7 @@ public class SimpleReactiveDiscoveryClientAutoConfiguration
 	public void onApplicationEvent(WebServerInitializedEvent webServerInitializedEvent) {
 		port = webServerInitializedEvent.getWebServer().getPort();
 		if (port > 0) {
-			simple.getLocal().setUri(URI.create("http://"
-					+ inet.findFirstNonLoopbackHostInfo().getHostname() + ":" + port));
+			simple.getLocal().setPort(port);
 		}
 	}
 

spring-cloud-commons/src/main/java/org/springframework/cloud/client/discovery/simple/reactive/SimpleReactiveDiscoveryProperties.java

@@ -27,6 +27,7 @@ import javax.annotation.PostConstruct;
 import reactor.core.publisher.Flux;
 
 import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient;
@@ -40,19 +41,20 @@ import static java.util.Collections.emptyList;
  * {@link org.springframework.cloud.client.discovery.composite.CompositeDiscoveryClient}.
  *
  * @author Tim Ysewyn
+ * @author Charu Covindane
  * @since 2.2.0
  */
 @ConfigurationProperties(prefix = "spring.cloud.discovery.client.simple")
 public class SimpleReactiveDiscoveryProperties {
 
-	private Map<String, List<SimpleServiceInstance>> instances = new HashMap<>();
+	private Map<String, List<DefaultServiceInstance>> instances = new HashMap<>();
 
 	/**
 	 * The properties of the local instance (if it exists). Users should set these
 	 * properties explicitly if they are exporting data (e.g. metrics) that need to be
 	 * identified by the service instance.
 	 */
-	private SimpleServiceInstance local = new SimpleServiceInstance();
+	private DefaultServiceInstance local = new DefaultServiceInstance();
 
 	private int order = DiscoveryClient.DEFAULT_ORDER;
 
@@ -60,15 +62,15 @@ public class SimpleReactiveDiscoveryProperties {
 		return Flux.fromIterable(instances.getOrDefault(service, emptyList()));
 	}
 
-	Map<String, List<SimpleServiceInstance>> getInstances() {
+	Map<String, List<DefaultServiceInstance>> getInstances() {
 		return instances;
 	}
 
-	public void setInstances(Map<String, List<SimpleServiceInstance>> instances) {
+	public void setInstances(Map<String, List<DefaultServiceInstance>> instances) {
 		this.instances = instances;
 	}
 
-	public SimpleServiceInstance getLocal() {
+	public DefaultServiceInstance getLocal() {
 		return this.local;
 	}
 
@@ -83,7 +85,7 @@ public class SimpleReactiveDiscoveryProperties {
 	@PostConstruct
 	public void init() {
 		for (String key : this.instances.keySet()) {
-			for (SimpleServiceInstance instance : this.instances.get(key)) {
+			for (DefaultServiceInstance instance : this.instances.get(key)) {
 				instance.setServiceId(key);
 			}
 		}
@@ -92,6 +94,7 @@ public class SimpleReactiveDiscoveryProperties {
 	/**
 	 * Basic implementation of {@link ServiceInstance}.
 	 */
+	@Deprecated
 	public static class SimpleServiceInstance implements ServiceInstance {
 
 		/**

spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/simple/SimpleDiscoveryClientTests.java

@@ -25,13 +25,14 @@ import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
 
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
-import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties.SimpleServiceInstance;
 
 import static org.assertj.core.api.BDDAssertions.then;
 
 /**
  * @author Biju Kunjummen
+ * @author Charu Covindane
  */
 public class SimpleDiscoveryClientTests {
 
@@ -41,11 +42,11 @@ public class SimpleDiscoveryClientTests {
 	public void setUp() {
 		SimpleDiscoveryProperties simpleDiscoveryProperties = new SimpleDiscoveryProperties();
 
-		Map<String, List<SimpleServiceInstance>> map = new HashMap<>();
-		SimpleServiceInstance service1Inst1 = new SimpleServiceInstance(
-				URI.create("http://host1:8080"));
-		SimpleServiceInstance service1Inst2 = new SimpleServiceInstance(
-				URI.create("https://host2:8443"));
+		Map<String, List<DefaultServiceInstance>> map = new HashMap<>();
+		DefaultServiceInstance service1Inst1 = new DefaultServiceInstance(null, null,
+				"host1", 8080, false);
+		DefaultServiceInstance service1Inst2 = new DefaultServiceInstance(null, null,
+				"host2", 8443, true);
 		map.put("service1", Arrays.asList(service1Inst1, service1Inst2));
 		simpleDiscoveryProperties.setInstances(map);
 		simpleDiscoveryProperties.init();

spring-cloud-commons/src/test/java/org/springframework/cloud/client/discovery/simple/reactive/SimpleReactiveDiscoveryClientTests.java

@@ -16,7 +16,6 @@
 
 package org.springframework.cloud.client.discovery.simple.reactive;
 
-import java.net.URI;
 import java.util.Arrays;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -24,23 +23,24 @@ import org.junit.jupiter.api.Test;
 import reactor.core.publisher.Flux;
 import reactor.test.StepVerifier;
 
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient;
-import org.springframework.cloud.client.discovery.simple.reactive.SimpleReactiveDiscoveryProperties.SimpleServiceInstance;
 
 import static java.util.Collections.singletonMap;
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
  * @author Tim Ysewyn
+ * @author Charu Covindane
  */
 public class SimpleReactiveDiscoveryClientTests {
 
-	private final SimpleServiceInstance service1Inst1 = new SimpleServiceInstance(
-			URI.create("http://host1:8080"));
+	private final DefaultServiceInstance service1Inst1 = new DefaultServiceInstance(null,
+			null, "host1", 8080, false);
 
-	private final SimpleServiceInstance service1Inst2 = new SimpleServiceInstance(
-			URI.create("https://host2:8443"));
+	private final DefaultServiceInstance service1Inst2 = new DefaultServiceInstance(null,
+			null, "host2", 8443, true);
 
 	private SimpleReactiveDiscoveryClient client;
 

spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunctionTests.java

@@ -31,11 +31,11 @@ import org.springframework.boot.SpringBootConfiguration;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.web.server.LocalServerPort;
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.client.discovery.EnableDiscoveryClient;
 import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties;
-import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties.SimpleServiceInstance;
 import org.springframework.cloud.client.loadbalancer.LoadBalancerClient;
 import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest;
 import org.springframework.context.annotation.Bean;
@@ -53,6 +53,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
 /**
  * @author Spencer Gibb
  * @author Ryan Baxter
+ * @author Charu Covindane
  */
 @RunWith(SpringRunner.class)
 @SpringBootTest(webEnvironment = RANDOM_PORT)
@@ -69,9 +70,10 @@ public class LoadBalancerExchangeFilterFunctionTests {
 
 	@Before
 	public void before() {
-		SimpleServiceInstance instance = new SimpleServiceInstance();
+		DefaultServiceInstance instance = new DefaultServiceInstance();
 		instance.setServiceId("testservice");
-		instance.setUri(URI.create("http://localhost:" + this.port));
+		instance.setHost("localhost");
+		instance.setPort(this.port);
 		this.properties.getInstances().put("testservice", Arrays.asList(instance));
 	}
 

spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/ReactorLoadBalancerExchangeFilterFunctionTests.java

@@ -16,7 +16,6 @@
 
 package org.springframework.cloud.client.loadbalancer.reactive;
 
-import java.net.URI;
 import java.util.Collections;
 import java.util.List;
 import java.util.Random;
@@ -32,6 +31,7 @@ import org.springframework.boot.SpringBootConfiguration;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.web.server.LocalServerPort;
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.client.discovery.EnableDiscoveryClient;
@@ -51,6 +51,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
  * Tests for {@link ReactorLoadBalancerExchangeFilterFunction}.
  *
  * @author Olga Maciaszek-Sharma
+ * @author Charu Covindane
  */
 @ExtendWith(SpringExtension.class)
 @SpringBootTest(webEnvironment = RANDOM_PORT)
@@ -67,9 +68,10 @@ class ReactorLoadBalancerExchangeFilterFunctionTests {
 
 	@BeforeEach
 	void setUp() {
-		SimpleDiscoveryProperties.SimpleServiceInstance instance = new SimpleDiscoveryProperties.SimpleServiceInstance();
+		DefaultServiceInstance instance = new DefaultServiceInstance();
 		instance.setServiceId("testservice");
-		instance.setUri(URI.create("http://localhost:" + this.port));
+		instance.setHost("localhost");
+		instance.setPort(this.port);
 		this.properties.getInstances().put("testservice",
 				Collections.singletonList(instance));
 	}

spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java

@@ -17,7 +17,6 @@
 package org.springframework.cloud.loadbalancer.blocking.client;
 
 import java.io.IOException;
-import java.net.URI;
 import java.util.Collections;
 import java.util.List;
 import java.util.Random;
@@ -31,6 +30,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.SpringBootConfiguration;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.cloud.client.DefaultServiceInstance;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties;
@@ -53,6 +53,7 @@ import static org.assertj.core.api.Assertions.fail;
  * Tests for {@link BlockingLoadBalancerClient}.
  *
  * @author Olga Maciaszek-Sharma
+ * @author Charu Covindane
  */
 @SpringBootTest
 @ExtendWith(SpringExtension.class)
@@ -66,10 +67,10 @@ class BlockingLoadBalancerClientTests {
 
 	@BeforeEach
 	void setUp() {
+		DefaultServiceInstance serviceInstance = new DefaultServiceInstance(null, null,
+				"test.example", 9999, true);
 		properties.getInstances().put("myservice",
-				Collections.singletonList(
-						new SimpleDiscoveryProperties.SimpleServiceInstance(
-								URI.create("https://test.example:9999"))));
+				Collections.singletonList(serviceInstance));
 	}
 
 	@Test