follow comments

Author: lijinglun

java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/LanceNamespace.java

@@ -13,7 +13,6 @@
  */
 package com.lancedb.lance.namespace;
 
-import com.lancedb.lance.namespace.conf.CoreKeys;
 import com.lancedb.lance.namespace.model.AlterTransactionRequest;
 import com.lancedb.lance.namespace.model.AlterTransactionResponse;
 import com.lancedb.lance.namespace.model.CountRowsRequest;
@@ -61,7 +60,8 @@
 /** TODO: add documentation */
 public interface LanceNamespace {
   static LanceNamespace create(String name, Map<String, String> properties, Object conf) {
-    String impl = properties.getOrDefault(CoreKeys.CATALOG_IMPL, CoreKeys.CATALOG_IMPL_DEFAULT);
+    String impl =
+        properties.getOrDefault(NamespaceProperties.NS_IMPL, NamespaceProperties.NS_IMPL_DEFAULT);
 
     LanceNamespace ns;
     try {
@@ -82,10 +82,10 @@ static LanceNamespace create(String name, Map<String, String> properties, Object
   }
 
   /**
-   * Initialize catalog with custom name and conf properties.
+   * Initialize namespace with custom name and conf properties.
    *
-   * @param name a custom name for the catalog
-   * @param properties catalog conf properties
+   * @param name a custom name for the namespace
+   * @param properties namespace conf properties
    */
   default void initialize(String name, Map<String, String> properties) {}
 

…ncedb/lance/namespace/conf/CoreKeys.java …lance/namespace/NamespaceProperties.javajava/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/conf/CoreKeys.java renamed to java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/NamespaceProperties.java java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/conf/CoreKeys.java renamed to java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/NamespaceProperties.java

@@ -11,10 +11,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.lancedb.lance.namespace.conf;
+package com.lancedb.lance.namespace;
 
-public class CoreKeys {
-  public static final String CATALOG_IMPL = "lance.namespace.catalog-impl";
-  public static final String CATALOG_IMPL_DEFAULT =
+public class NamespaceProperties {
+  public static final String NS_IMPL = "lance.namespace.impl";
+  public static final String NS_IMPL_DEFAULT =
       "com.lancedb.lance.namespace.hive.LanceHiveNamespace";
 }

java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/ObjectIdentifier.java

@@ -13,11 +13,12 @@
  */
 package com.lancedb.lance.namespace;
 
+import com.lancedb.lance.namespace.util.ValidationUtil;
+
 import com.google.common.collect.ImmutableList;
 
 import java.util.Arrays;
 import java.util.List;
-import java.util.stream.Collectors;
 
 public class ObjectIdentifier {
   private String[] levels;
@@ -27,11 +28,12 @@ private ObjectIdentifier(String[] levels) {
   }
 
   public static ObjectIdentifier of(List<String> levels) {
-    List<String> normalizedLevels =
-        levels.stream()
-            .filter(level -> level != null && !level.isEmpty())
-            .collect(Collectors.toList());
-    return new ObjectIdentifier(normalizedLevels.toArray(new String[0]));
+    levels.stream()
+        .forEach(
+            level ->
+                ValidationUtil.checkNotNullOrEmptyString(
+                    level, "Invalid namespace containing empty string %s", levels));
+    return new ObjectIdentifier(levels.toArray(new String[0]));
   }
 
   public String level(int pos) {

…cedb/lance/namespace/util/PageUtils.java …ncedb/lance/namespace/util/PageUtil.javajava/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/util/PageUtils.java renamed to java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/util/PageUtil.java java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/util/PageUtils.java renamed to java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/util/PageUtil.java

@@ -15,8 +15,8 @@
 
 import java.util.List;
 
-public class PageUtils {
-  private PageUtils() {}
+public class PageUtil {
+  private PageUtil() {}
 
   public static class Page {
     private List<String> items;

java/lance-namespace-core/src/test/java/com/lancedb/lance/namespace/TestObjectIdentifier.java

@@ -19,25 +19,33 @@
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class TestObjectIdentifier {
   @Test
   public void testObjectIdentifier() {
-    // Case 1: empty namespace
-    ObjectIdentifier oid = ObjectIdentifier.of(Lists.newArrayList());
-    assertEquals(0, oid.size());
-
+    // Case 1: invalid namespace
     List<String> ns = Lists.newArrayList();
     ns.add(null);
+    assertThrows(IllegalArgumentException.class, () -> ObjectIdentifier.of(ns));
+
+    ns.clear();
     ns.add("");
+    assertThrows(IllegalArgumentException.class, () -> ObjectIdentifier.of(ns));
+
+    assertThrows(
+        IllegalArgumentException.class, () -> ObjectIdentifier.of(Lists.newArrayList("a", "")));
+
+    // Case 2: empty namespace
+    ObjectIdentifier oid = ObjectIdentifier.of(Lists.newArrayList());
     assertEquals(0, oid.size());
 
-    oid = ObjectIdentifier.of(Lists.newArrayList("a", "", "b"));
+    oid = ObjectIdentifier.of(Lists.newArrayList("a", "b"));
     assertEquals(2, oid.size());
     assertEquals("a", oid.level(0));
     assertEquals("b", oid.level(1));
 
-    // Case 2: parse name
+    // Case 3: parse name
     oid = ObjectIdentifier.of(Lists.newArrayList());
     assertEquals("", oid.name());
 
@@ -47,7 +55,7 @@ public void testObjectIdentifier() {
     oid = ObjectIdentifier.of(Lists.newArrayList("a", "b"));
     assertEquals("b", oid.name());
 
-    // Case 3: parse parent
+    // Case 4: parse parent
     oid = ObjectIdentifier.of(Lists.newArrayList());
     assertEquals(Lists.newArrayList(), oid.parent());
 

java/lance-namespace-hms/pom.xml java/lance-namespace-hive/pom.xmljava/lance-namespace-hms/pom.xml renamed to java/lance-namespace-hive/pom.xml

@@ -13,9 +13,9 @@
     </parent>
 
 
-    <artifactId>lance-namespace-hms</artifactId>
+    <artifactId>lance-namespace-hive</artifactId>
     <name>${project.artifactId}</name>
-    <description>Lance Namespace HMS Impl</description>
+    <description>Lance Namespace Hive Impl</description>
     <packaging>jar</packaging>
 
     <dependencies>

…/lancedb/lance/namespace/ClientPool.java …/lancedb/lance/namespace/ClientPool.javajava/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/ClientPool.java renamed to java/lance-namespace-hive/src/main/java/com/lancedb/lance/namespace/ClientPool.java java/lance-namespace-hms/src/main/java/com/lancedb/lance/namespace/ClientPool.java renamed to java/lance-namespace-hive/src/main/java/com/lancedb/lance/namespace/ClientPool.java

@@ -11,9 +11,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.lancedb.lance.namespace.conf;
+package com.lancedb.lance.namespace;
 
-public class ConfKeys {
-  public static final String HMS_CLIENT_POOL_SIZE = "lance.namespace.hms.client-pool-size";
-  public static final String HMS_CLIENT_POOL_SIZE_DEFAULT = "3";
+public class HiveNamespaceProperties {
+  public static final String HIVE_CLIENT_POOL_SIZE = "lance.namespace.hive.client-pool-size";
+  public static final String HIVE_CLIENT_POOL_SIZE_DEFAULT = "3";
 }