(iceberg-rust) branch main updated: fix: Fix namespace identifier in url (#435)

[liurenjie1024]

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

liurenjie1024 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git


The following commit(s) were added to refs/heads/main by this push:
     new 0937f19  fix: Fix namespace identifier in url (#435)
0937f19 is described below

commit 0937f19d4c4b4ea14d423e6c78731904b4c9012e
Author: Renjie Liu <liurenjie2...@gmail.com>
AuthorDate: Fri Jul 5 15:25:16 2024 +0800

    fix: Fix namespace identifier in url (#435)
    
    * fix: Fix namespace identifier in url
    
    * Remove table encoding
---
 crates/catalog/rest/src/catalog.rs             | 11 ++++----
 crates/catalog/rest/tests/rest_catalog_test.rs | 35 ++++++++++++++++++++++++++
 crates/iceberg/src/catalog/mod.rs              |  7 +++---
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/crates/catalog/rest/src/catalog.rs 
b/crates/catalog/rest/src/catalog.rs
index a43862a..83cc0ec 100644
--- a/crates/catalog/rest/src/catalog.rs
+++ b/crates/catalog/rest/src/catalog.rs
@@ -26,7 +26,6 @@ use reqwest::header::{self, HeaderMap, HeaderName, 
HeaderValue};
 use reqwest::{Method, StatusCode, Url};
 use tokio::sync::OnceCell;
 use typed_builder::TypedBuilder;
-use urlencoding::encode;
 
 use crate::client::HttpClient;
 use crate::types::{
@@ -82,11 +81,11 @@ impl RestCatalogConfig {
     }
 
     fn namespace_endpoint(&self, ns: &NamespaceIdent) -> String {
-        self.url_prefixed(&["namespaces", &ns.encode_in_url()])
+        self.url_prefixed(&["namespaces", &ns.to_url_string()])
     }
 
     fn tables_endpoint(&self, ns: &NamespaceIdent) -> String {
-        self.url_prefixed(&["namespaces", &ns.encode_in_url(), "tables"])
+        self.url_prefixed(&["namespaces", &ns.to_url_string(), "tables"])
     }
 
     fn rename_table_endpoint(&self) -> String {
@@ -96,9 +95,9 @@ impl RestCatalogConfig {
     fn table_endpoint(&self, table: &TableIdent) -> String {
         self.url_prefixed(&[
             "namespaces",
-            &table.namespace.encode_in_url(),
+            &table.namespace.to_url_string(),
             "tables",
-            encode(&table.name).as_ref(),
+            &table.name,
         ])
     }
 
@@ -320,7 +319,7 @@ impl Catalog for RestCatalog {
             self.context().await?.config.namespaces_endpoint(),
         );
         if let Some(ns) = parent {
-            request = request.query(&[("parent", ns.encode_in_url())]);
+            request = request.query(&[("parent", ns.to_url_string())]);
         }
 
         let resp = self
diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs 
b/crates/catalog/rest/tests/rest_catalog_test.rs
index 3c22241..52f243d 100644
--- a/crates/catalog/rest/tests/rest_catalog_test.rs
+++ b/crates/catalog/rest/tests/rest_catalog_test.rs
@@ -375,3 +375,38 @@ fn assert_map_contains(map1: &HashMap<String, String>, 
map2: &HashMap<String, St
         assert_eq!(map2.get(k).unwrap(), v);
     }
 }
+
+#[tokio::test]
+async fn test_list_empty_multi_level_namespace() {
+    let fixture = 
set_test_fixture("test_list_empty_multi_level_namespace").await;
+
+    let ns_apple = Namespace::with_properties(
+        NamespaceIdent::from_strs(["a_a", "apple"]).unwrap(),
+        HashMap::from([
+            ("owner".to_string(), "ray".to_string()),
+            ("community".to_string(), "apache".to_string()),
+        ]),
+    );
+
+    // Currently this namespace doesn't exist, so it should return error.
+    assert!(fixture
+        .rest_catalog
+        .list_namespaces(Some(ns_apple.name()))
+        .await
+        .is_err());
+
+    // Create namespaces
+    fixture
+        .rest_catalog
+        .create_namespace(ns_apple.name(), ns_apple.properties().clone())
+        .await
+        .unwrap();
+
+    // List namespace
+    let nss = fixture
+        .rest_catalog
+        .list_namespaces(Some(&NamespaceIdent::from_strs(["a_a", 
"apple"]).unwrap()))
+        .await
+        .unwrap();
+    assert!(nss.is_empty());
+}
diff --git a/crates/iceberg/src/catalog/mod.rs 
b/crates/iceberg/src/catalog/mod.rs
index 172f335..4d7a47a 100644
--- a/crates/iceberg/src/catalog/mod.rs
+++ b/crates/iceberg/src/catalog/mod.rs
@@ -30,7 +30,6 @@ use std::fmt::Debug;
 use std::mem::take;
 use std::ops::Deref;
 use typed_builder::TypedBuilder;
-use urlencoding::encode;
 use uuid::Uuid;
 
 /// The catalog API for Iceberg Rust.
@@ -123,9 +122,9 @@ impl NamespaceIdent {
         Self::from_vec(iter.into_iter().map(|s| s.to_string()).collect())
     }
 
-    /// Returns url encoded format.
-    pub fn encode_in_url(&self) -> String {
-        encode(&self.as_ref().join("\u{1F}")).to_string()
+    /// Returns a string for used in url.
+    pub fn to_url_string(&self) -> String {
+        self.as_ref().join("\u{001f}")
     }
 
     /// Returns inner strings.