Skip to content

Commit

Permalink
[TABLE SERVICE] add default port to service hosts
Browse files Browse the repository at this point in the history
Descriptions of the changes in this PR:

*Motivation*

Currently we have to add `4181` in the service uri in order to make it work.
Ideally it would be great that the system can fill the default port if port is missing.

*Changes*

add default port when parsing `ServiceURI`





Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes apache#1778 from sijie/add_default_port
  • Loading branch information
sijie authored Nov 2, 2018
1 parent 4992413 commit d13bffc
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.net.URI;
import java.util.List;
import java.util.stream.Collectors;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand Down Expand Up @@ -122,6 +123,7 @@ public class ServiceURI {
* Service string for bookkeeper service.
*/
public static final String SERVICE_BK = "bk";
public static final int SERVICE_BK_PORT = 4181;

/**
* The default local bk service uri.
Expand Down Expand Up @@ -160,7 +162,7 @@ public static ServiceURI create(String uriStr) {
public static ServiceURI create(URI uri) {
checkNotNull(uri, "service uri instance is null");

String serviceName = null;
String serviceName;
String[] serviceInfos = new String[0];
String scheme = uri.getScheme();
if (null != scheme) {
Expand All @@ -175,6 +177,8 @@ public static ServiceURI create(URI uri) {
serviceName = schemeParts[0];
serviceInfos = new String[schemeParts.length - 1];
System.arraycopy(schemeParts, 1, serviceInfos, 0, serviceInfos.length);
} else {
serviceName = null;
}

String userAndHostInformation = uri.getAuthority();
Expand All @@ -192,7 +196,10 @@ public static ServiceURI create(URI uri) {
serviceUser = null;
serviceHosts = splitter.splitToList(userAndHostInformation);
}
serviceHosts.forEach(host -> validateHostName(host));
serviceHosts = serviceHosts
.stream()
.map(host -> validateHostName(serviceName, host))
.collect(Collectors.toList());

String servicePath = uri.getPath();
checkArgument(null != servicePath,
Expand All @@ -207,7 +214,7 @@ public static ServiceURI create(URI uri) {
uri);
}

private static void validateHostName(String hostname) {
private static String validateHostName(String serviceName, String hostname) {
String[] parts = hostname.split(":");
if (parts.length >= 3) {
throw new IllegalArgumentException("Invalid hostname : " + hostname);
Expand All @@ -217,8 +224,12 @@ private static void validateHostName(String hostname) {
} catch (NumberFormatException nfe) {
throw new IllegalArgumentException("Invalid hostname : " + hostname);
}
return hostname;
} else if (parts.length == 1 && serviceName.toLowerCase().equals(SERVICE_BK)) {
return hostname + ":" + SERVICE_BK_PORT;
} else {
return hostname;
}

}

private final String serviceName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,19 @@ public void testMultipleHostsWithoutPorts() {
"bk",
new String[0],
null,
new String[] { "host1", "host2", "host3" },
new String[] { "host1:4181", "host2:4181", "host3:4181" },
"/path/to/namespace");
}

@Test
public void testMultipleHostsMixedPorts() {
String serviceUri = "bk://host1:3181,host2,host3:2181/path/to/namespace";
assertServiceUri(
serviceUri,
"bk",
new String[0],
null,
new String[] { "host1:3181", "host2:4181", "host3:2181" },
"/path/to/namespace");
}

Expand All @@ -166,7 +178,7 @@ public void testMultipleHostsMixed() {
"bk",
new String[0],
null,
new String[] { "host1:2181", "host2", "host3:2181" },
new String[] { "host1:2181", "host2:4181", "host3:2181" },
"/path/to/namespace");
}

Expand Down

0 comments on commit d13bffc

Please sign in to comment.