Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding getScheme on DefaultServiceInstance. #1168

Merged
merged 3 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2022 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.
Expand All @@ -18,6 +18,7 @@

import java.net.URI;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

Expand All @@ -28,15 +29,20 @@
* @author Tim Ysewyn
* @author Charu Covindane
* @author Neil Powell
* @author Olga Maciaszek-Sharma
*/
public class DefaultServiceInstance implements ServiceInstance {

private final List<String> secureSchemes = List.of("https", "wss");

private String instanceId;

private String serviceId;

private String host;

private String scheme;

private int port;

private boolean secure;
Expand All @@ -45,6 +51,7 @@ public class DefaultServiceInstance implements ServiceInstance {

private URI uri;


/**
* @param instanceId the id of the instance.
* @param serviceId the id of the service.
Expand All @@ -55,12 +62,27 @@ public class DefaultServiceInstance implements ServiceInstance {
*/
public DefaultServiceInstance(String instanceId, String serviceId, String host, int port, boolean secure,
Map<String, String> metadata) {
this(instanceId, serviceId, host, port, secure, metadata, secure ? "https" : "http");
}

/**
* @param instanceId the id of the instance.
* @param serviceId the id of the service.
* @param host the host where the service instance can be found.
* @param port the port on which the service is running.
* @param secure indicates whether or not the connection needs to be secure.
* @param metadata a map containing metadata.
* @param scheme the protocol used to connect to the service instance.
*/
public DefaultServiceInstance(String instanceId, String serviceId, String host, int port, boolean secure,
Map<String, String> metadata, String scheme) {
this.instanceId = instanceId;
this.serviceId = serviceId;
this.host = host;
this.port = port;
this.secure = secure;
this.metadata = metadata;
this.scheme = scheme;
}

/**
Expand All @@ -77,14 +99,27 @@ public DefaultServiceInstance(String instanceId, String serviceId, String host,
public DefaultServiceInstance() {
}

/**
* @param instanceId the id of the instance.
* @param serviceId the id of the service.
* @param host the host where the service instance can be found.
* @param port the port on which the service is running.
* @param secure indicates whether or not the connection needs to be secure.
* @param scheme the protocol used to connect to the service instance.
*/
public DefaultServiceInstance(String instanceId, String serviceId, String host, int port, boolean secure,
String scheme) {
this(instanceId, serviceId, host, port, secure, new LinkedHashMap<>(), scheme);
}

/**
* Creates a URI from the given ServiceInstance's host:port.
* @param instance the ServiceInstance.
* @return URI of the form (secure)?https:http + "host:port". Scheme port default used
* if port not set.
*/
public static URI getUri(ServiceInstance instance) {
String scheme = (instance.isSecure()) ? "https" : "http";
String scheme = instance.getScheme();
int port = instance.getPort();
if (port <= 0) {
port = (instance.isSecure()) ? 443 : 80;
Expand Down Expand Up @@ -148,8 +183,8 @@ 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)) {
scheme = this.uri.getScheme();
if (secureSchemes.contains(scheme)) {
this.secure = true;
}
}
Expand Down Expand Up @@ -179,4 +214,9 @@ public int hashCode() {
return Objects.hash(instanceId, serviceId, host, port, secure, metadata);
}

@Override
public String getScheme() {
return scheme;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2022 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.
Expand Down Expand Up @@ -87,4 +87,8 @@ public void setInstance(String serviceId, String host, int port) {
local = new DefaultServiceInstance(null, serviceId, host, port, false);
}

public void setInstance(String serviceId, String host, int port, String scheme) {
local = new DefaultServiceInstance(null, serviceId, host, port, false, scheme);
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2022 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.
Expand Down Expand Up @@ -32,6 +32,7 @@
* Tests for mapping properties to instances in {@link SimpleDiscoveryClient}.
*
* @author Biju Kunjummen
* @author Olga Maciaszek-Sharma
*/

@SpringBootTest(properties = { "spring.application.name=service0",
Expand Down Expand Up @@ -73,6 +74,14 @@ public void testDiscoveryClientShouldResolveSimpleValues() {
then(s1.getPort()).isEqualTo(8080);
then(s1.getUri()).isEqualTo(URI.create("http://s11:8080"));
then(s1.isSecure()).isEqualTo(false);
then(s1.getScheme()).isEqualTo("http");

ServiceInstance s2 = this.discoveryClient.getInstances("service1").get(1);
then(s2.getHost()).isEqualTo("s12");
then(s2.getPort()).isEqualTo(8443);
then(s2.getUri()).isEqualTo(URI.create("https://s12:8443"));
then(s2.isSecure()).isEqualTo(true);
then(s2.getScheme()).isEqualTo("https");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* @author Biju Kunjummen
* @author Charu Covindane
* @author Neil Powell
* @author Olga Maciaszek-Sharma
*/
public class SimpleDiscoveryClientTests {

Expand All @@ -47,7 +48,8 @@ public void setUp() {
DefaultServiceInstance service1Inst1 = new DefaultServiceInstance(null, null, "host1", 8080, false);
DefaultServiceInstance service1Inst2 = new DefaultServiceInstance(null, null, "host2", 0, true);
DefaultServiceInstance service1Inst3 = new DefaultServiceInstance(null, null, "host3", 0, false);
map.put("service1", Arrays.asList(service1Inst1, service1Inst2, service1Inst3));
DefaultServiceInstance service1Inst4 = new DefaultServiceInstance(null, null, "host4", 8443, true);
map.put("service1", Arrays.asList(service1Inst1, service1Inst2, service1Inst3, service1Inst4));
simpleDiscoveryProperties.setInstances(map);
simpleDiscoveryProperties.afterPropertiesSet();
this.simpleDiscoveryClient = new SimpleDiscoveryClient(simpleDiscoveryProperties);
Expand All @@ -56,7 +58,7 @@ public void setUp() {
@Test
public void shouldBeAbleToRetrieveServiceDetailsByName() {
List<ServiceInstance> instances = this.simpleDiscoveryClient.getInstances("service1");
then(instances.size()).isEqualTo(3);
then(instances.size()).isEqualTo(4);
then(instances.get(0).getServiceId()).isEqualTo("service1");
then(instances.get(0).getHost()).isEqualTo("host1");
then(instances.get(0).getPort()).isEqualTo(8080);
Expand All @@ -77,6 +79,13 @@ public void shouldBeAbleToRetrieveServiceDetailsByName() {
then(instances.get(2).getUri()).isEqualTo(URI.create("http://host3:80"));
then(instances.get(2).isSecure()).isEqualTo(false);
then(instances.get(2).getMetadata()).isNotNull();

then(instances.get(3).getServiceId()).isEqualTo("service1");
then(instances.get(3).getHost()).isEqualTo("host4");
then(instances.get(3).getPort()).isEqualTo(8443);
then(instances.get(3).getUri()).isEqualTo(URI.create("https://host4:8443"));
then(instances.get(3).isSecure()).isEqualTo(true);
then(instances.get(3).getMetadata()).isNotNull();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ void shouldCheckInstanceWithProvidedHealthCheckPathWithRestTemplate() {
String serviceId = "ignored-service";
properties.getHealthCheck().getPath().put("ignored-service", "/health");
ServiceInstance serviceInstance = new DefaultServiceInstance("ignored-service-1", serviceId, "127.0.0.1", port,
false);
false, "http");
listSupplier = new HealthCheckServiceInstanceListSupplier(
ServiceInstanceListSuppliers.from(serviceId, serviceInstance),
buildLoadBalancerClientFactory(serviceId, properties), healthCheckFunction(restTemplate));
Expand Down