[bookkeeper] branch master updated: [TABLE SERVICE] add default port to service hosts

[si...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new d13bffc  [TABLE SERVICE] add default port to service hosts
d13bffc is described below

commit d13bffc0a2853b8d2876c3314f6732eaf576534a
Author: Sijie Guo <gu...@gmail.com>
AuthorDate: Fri Nov 2 14:41:09 2018 -0700

    [TABLE SERVICE] add default port to service hosts
    
    
    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 <eo...@gmail.com>
    
    This closes #1778 from sijie/add_default_port
---
 .../org/apache/bookkeeper/common/net/ServiceURI.java  | 19 +++++++++++++++----
 .../apache/bookkeeper/common/net/ServiceURITest.java  | 16 ++++++++++++++--
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/net/ServiceURI.java b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/net/ServiceURI.java
index 4e2b158..3d419f3 100644
--- a/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/net/ServiceURI.java
+++ b/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/net/ServiceURI.java
@@ -27,6 +27,7 @@ import com.google.common.base.Strings;
 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;
@@ -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.
@@ -160,7 +162,7 @@ public class ServiceURI {
     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) {
@@ -175,6 +177,8 @@ public class ServiceURI {
             serviceName = schemeParts[0];
             serviceInfos = new String[schemeParts.length - 1];
             System.arraycopy(schemeParts, 1, serviceInfos, 0, serviceInfos.length);
+        } else {
+            serviceName = null;
         }
 
         String userAndHostInformation = uri.getAuthority();
@@ -192,7 +196,10 @@ public class ServiceURI {
             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,
@@ -207,7 +214,7 @@ public class ServiceURI {
             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);
@@ -217,8 +224,12 @@ public class ServiceURI {
             } 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;
diff --git a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/net/ServiceURITest.java b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/net/ServiceURITest.java
index 9982c43..2ec83df 100644
--- a/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/net/ServiceURITest.java
+++ b/bookkeeper-common/src/test/java/org/apache/bookkeeper/common/net/ServiceURITest.java
@@ -154,7 +154,19 @@ public class ServiceURITest {
             "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");
     }
 
@@ -166,7 +178,7 @@ public class ServiceURITest {
             "bk",
             new String[0],
             null,
-            new String[] { "host1:2181", "host2", "host3:2181" },
+            new String[] { "host1:2181", "host2:4181", "host3:2181" },
             "/path/to/namespace");
     }