You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ch...@apache.org on 2023/03/08 11:29:58 UTC

[kyuubi] branch master updated: [KYUUBI #4452] Strip the redundant leading and tailing slash of getZooKeeperNamespace.

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 68b34be49 [KYUUBI #4452] Strip the redundant leading and tailing slash of getZooKeeperNamespace.
68b34be49 is described below

commit 68b34be4926783c9e35c38969e07cb4f919e0532
Author: maruilei <ma...@58.com>
AuthorDate: Wed Mar 8 19:29:47 2023 +0800

    [KYUUBI #4452] Strip the redundant leading and tailing slash of getZooKeeperNamespace.
    
    ### _Why are the changes needed?_
    
    As described in the discussion:  <https://github.com/apache/kyuubi/discussions/4436>
    
    When the zk namespace contains leading and trailing slashes, the exception will cause the client connection to fail, and the exception prompt information is not clear.
    
    So we strip the redundant leading and tailing slashes of getZooKeeperNamespace and add exception stack information at the same time.
    
    ### _How was this patch tested?_
    - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
    
    - [ ] Add screenshots for manual tests if appropriate
    
    - [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #4454 from merrily01/zk-namespace.
    
    Closes #4452
    
    62290545e [maruilei] Address comments.
    cee43cd61 [maruilei] [KYUUBI #4452] Strip the redundant leading and tailing slash of getZooKeeperNamespace.
    
    Authored-by: maruilei <ma...@58.com>
    Signed-off-by: Cheng Pan <ch...@apache.org>
---
 .../jdbc/hive/ZooKeeperHiveClientHelper.java       |  5 +-
 .../jdbc/hive/ZooKeeperHiveClientHelperTest.java   | 59 ++++++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/ZooKeeperHiveClientHelper.java b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/ZooKeeperHiveClientHelper.java
index 349fc8dfb..41fadfa2f 100644
--- a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/ZooKeeperHiveClientHelper.java
+++ b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/ZooKeeperHiveClientHelper.java
@@ -17,6 +17,7 @@
 
 package org.apache.kyuubi.jdbc.hive;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
@@ -32,12 +33,14 @@ class ZooKeeperHiveClientHelper {
   // Pattern for key1=value1;key2=value2
   private static final Pattern kvPattern = Pattern.compile("([^=;]*)=([^;]*);?");
 
-  private static String getZooKeeperNamespace(JdbcConnectionParams connParams) {
+  @VisibleForTesting
+  protected static String getZooKeeperNamespace(JdbcConnectionParams connParams) {
     String zooKeeperNamespace =
         connParams.getSessionVars().get(JdbcConnectionParams.ZOOKEEPER_NAMESPACE);
     if ((zooKeeperNamespace == null) || (zooKeeperNamespace.isEmpty())) {
       zooKeeperNamespace = JdbcConnectionParams.ZOOKEEPER_DEFAULT_NAMESPACE;
     }
+    zooKeeperNamespace = zooKeeperNamespace.replaceAll("^/+", "").replaceAll("/+$", "");
     return zooKeeperNamespace;
   }
 
diff --git a/kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/ZooKeeperHiveClientHelperTest.java b/kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/ZooKeeperHiveClientHelperTest.java
new file mode 100644
index 000000000..d1fd78f47
--- /dev/null
+++ b/kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/ZooKeeperHiveClientHelperTest.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.apache.kyuubi.jdbc.hive;
+
+import static org.apache.kyuubi.jdbc.hive.Utils.extractURLComponents;
+import static org.junit.Assert.assertEquals;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Properties;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class ZooKeeperHiveClientHelperTest {
+
+  private String uri;
+
+  @Parameterized.Parameters
+  public static Collection<String[]> data() {
+    return Arrays.asList(
+        new String[][] {
+          {"jdbc:hive2://hostname:10018/db;zooKeeperNamespace=zookeeper/namespace"},
+          {"jdbc:hive2://hostname:10018/db;zooKeeperNamespace=/zookeeper/namespace"},
+          {"jdbc:hive2://hostname:10018/db;zooKeeperNamespace=zookeeper/namespace/"},
+          {"jdbc:hive2://hostname:10018/db;zooKeeperNamespace=/zookeeper/namespace/"},
+          {"jdbc:hive2://hostname:10018/db;zooKeeperNamespace=///zookeeper/namespace///"}
+        });
+  }
+
+  public ZooKeeperHiveClientHelperTest(String uri) {
+    this.uri = uri;
+  }
+
+  @Test
+  public void testGetZooKeeperNamespace() throws JdbcUriParseException {
+    JdbcConnectionParams jdbcConnectionParams = extractURLComponents(uri, new Properties());
+    assertEquals(
+        "zookeeper/namespace",
+        ZooKeeperHiveClientHelper.getZooKeeperNamespace(jdbcConnectionParams));
+  }
+}