From 272259306926538af772829a2f0c9291aa998873 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Sat, 27 Sep 2025 18:31:51 +0800 Subject: [PATCH 1/5] refactor: Restructure catalog module --- example/demo_example.cc | 2 +- src/iceberg/CMakeLists.txt | 2 +- src/iceberg/catalog/CMakeLists.txt | 1 + src/iceberg/catalog/memory/CMakeLists.txt | 18 +++ .../catalog/{ => memory}/in_memory_catalog.cc | 2 +- .../catalog/{ => memory}/in_memory_catalog.h | 0 src/iceberg/test/CMakeLists.txt | 18 ++- src/iceberg/test/in_memory_catalog_test.cc | 2 +- src/iceberg/test/rest_catalog_test.cc | 111 ++++++++++++++++++ 9 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 src/iceberg/catalog/memory/CMakeLists.txt rename src/iceberg/catalog/{ => memory}/in_memory_catalog.cc (99%) rename src/iceberg/catalog/{ => memory}/in_memory_catalog.h (100%) create mode 100644 src/iceberg/test/rest_catalog_test.cc diff --git a/example/demo_example.cc b/example/demo_example.cc index d20f48c0f..c333c7971 100644 --- a/example/demo_example.cc +++ b/example/demo_example.cc @@ -21,7 +21,7 @@ #include "iceberg/arrow/arrow_file_io.h" #include "iceberg/avro/avro_register.h" -#include "iceberg/catalog/in_memory_catalog.h" +#include "iceberg/catalog/memory/in_memory_catalog.h" #include "iceberg/parquet/parquet_register.h" #include "iceberg/table.h" #include "iceberg/table_scan.h" diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index c9e665d4d..a2a648f9c 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -18,7 +18,7 @@ set(ICEBERG_INCLUDES "$" "$") set(ICEBERG_SOURCES - catalog/in_memory_catalog.cc + catalog/memory/in_memory_catalog.cc expression/expression.cc expression/expressions.cc expression/literal.cc diff --git a/src/iceberg/catalog/CMakeLists.txt b/src/iceberg/catalog/CMakeLists.txt index 9e11eee37..bcfb5f0cd 100644 --- a/src/iceberg/catalog/CMakeLists.txt +++ b/src/iceberg/catalog/CMakeLists.txt @@ -16,6 +16,7 @@ # under the License. iceberg_install_all_headers(iceberg/catalog) +add_subdirectory(memory) if(ICEBERG_BUILD_REST) add_subdirectory(rest) diff --git a/src/iceberg/catalog/memory/CMakeLists.txt b/src/iceberg/catalog/memory/CMakeLists.txt new file mode 100644 index 000000000..306a4a7ff --- /dev/null +++ b/src/iceberg/catalog/memory/CMakeLists.txt @@ -0,0 +1,18 @@ +# 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. + +iceberg_install_all_headers(iceberg/catalog/memory) diff --git a/src/iceberg/catalog/in_memory_catalog.cc b/src/iceberg/catalog/memory/in_memory_catalog.cc similarity index 99% rename from src/iceberg/catalog/in_memory_catalog.cc rename to src/iceberg/catalog/memory/in_memory_catalog.cc index 0215b9087..08a9822c2 100644 --- a/src/iceberg/catalog/in_memory_catalog.cc +++ b/src/iceberg/catalog/memory/in_memory_catalog.cc @@ -17,7 +17,7 @@ * under the License. */ -#include "iceberg/catalog/in_memory_catalog.h" +#include "iceberg/catalog/memory/in_memory_catalog.h" #include #include // IWYU pragma: keep diff --git a/src/iceberg/catalog/in_memory_catalog.h b/src/iceberg/catalog/memory/in_memory_catalog.h similarity index 100% rename from src/iceberg/catalog/in_memory_catalog.h rename to src/iceberg/catalog/memory/in_memory_catalog.h diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 1eac3aeca..2a496d5fa 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -15,13 +15,23 @@ # specific language governing permissions and limitations # under the License. +fetchcontent_declare(cpp-httplib + GIT_REPOSITORY https://github.com/yhirose/cpp-httplib.git + GIT_TAG 89c932f313c6437c38f2982869beacc89c2f2246 #release-0.26.0 +) + fetchcontent_declare(googletest GIT_REPOSITORY https://github.com/google/googletest.git GIT_TAG b514bdc898e2951020cbdca1304b75f5950d1f59 # release-1.15.2 FIND_PACKAGE_ARGS NAMES GTest) -fetchcontent_makeavailable(googletest) + +if(ICEBERG_BUILD_REST_CATALOG) + fetchcontent_makeavailable(cpp-httplib googletest) +else() + fetchcontent_makeavailable(googletest) +endif() set(ICEBERG_TEST_RESOURCES "${CMAKE_SOURCE_DIR}/src/iceberg/test/resources") @@ -136,3 +146,9 @@ if(ICEBERG_BUILD_BUNDLE) add_iceberg_test(scan_test USE_BUNDLE SOURCES file_scan_task_test.cc) endif() + +if(ICEBERG_BUILD_REST_CATALOG) + add_iceberg_test(rest_catalog_test SOURCES rest_catalog_test.cc) + target_link_libraries(rest_catalog_test PRIVATE iceberg_rest_catalog_static) + target_include_directories(rest_catalog_test PRIVATE ${cpp-httplib_SOURCE_DIR}) +endif() diff --git a/src/iceberg/test/in_memory_catalog_test.cc b/src/iceberg/test/in_memory_catalog_test.cc index 36d67cbe7..da1804b82 100644 --- a/src/iceberg/test/in_memory_catalog_test.cc +++ b/src/iceberg/test/in_memory_catalog_test.cc @@ -17,7 +17,7 @@ * under the License. */ -#include "iceberg/catalog/in_memory_catalog.h" +#include "iceberg/catalog/memory/in_memory_catalog.h" #include diff --git a/src/iceberg/test/rest_catalog_test.cc b/src/iceberg/test/rest_catalog_test.cc new file mode 100644 index 000000000..b10f8c53c --- /dev/null +++ b/src/iceberg/test/rest_catalog_test.cc @@ -0,0 +1,111 @@ +/* + * 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. + */ + +#include "iceberg/catalog/rest/rest_catalog.h" + +#include + +#include +#include + +#include +#include +#include + +namespace iceberg::catalog::rest { + +class RestCatalogIntegrationTest : public ::testing::Test { + protected: + void SetUp() override { + server_ = std::make_unique(); + port_ = server_->bind_to_any_port("127.0.0.1"); + + server_thread_ = std::thread([this]() { server_->listen_after_bind(); }); + } + + void TearDown() override { + server_->stop(); + if (server_thread_.joinable()) { + server_thread_.join(); + } + } + + std::unique_ptr server_; + int port_ = -1; + std::thread server_thread_; +}; + +TEST_F(RestCatalogIntegrationTest, GetConfigSuccessfully) { + server_->Get("/v1/config", [](const httplib::Request&, httplib::Response& res) { + res.status = 200; + res.set_content(R"({"warehouse": "s3://test-bucket"})", "application/json"); + }); + + std::string base_uri = "http://127.0.0.1:" + std::to_string(port_); + RestCatalog catalog(base_uri); + cpr::Response response = catalog.GetConfig(); + + ASSERT_EQ(response.error.code, cpr::ErrorCode::OK); + ASSERT_EQ(response.status_code, 200); + + auto json_body = nlohmann::json::parse(response.text); + EXPECT_EQ(json_body["warehouse"], "s3://test-bucket"); +} + +TEST_F(RestCatalogIntegrationTest, ListNamespacesReturnsMultipleResults) { + server_->Get("/v1/namespaces", [](const httplib::Request&, httplib::Response& res) { + res.status = 200; + res.set_content(R"({ + "namespaces": [ + ["accounting", "db"], + ["production", "db"] + ] + })", + "application/json"); + }); + + std::string base_uri = "http://127.0.0.1:" + std::to_string(port_); + RestCatalog catalog(base_uri); + cpr::Response response = catalog.ListNamespaces(); + + ASSERT_EQ(response.error.code, cpr::ErrorCode::OK); + ASSERT_EQ(response.status_code, 200); + + auto json_body = nlohmann::json::parse(response.text); + ASSERT_TRUE(json_body.contains("namespaces")); + EXPECT_EQ(json_body["namespaces"].size(), 2); + EXPECT_THAT(json_body["namespaces"][0][0], "accounting"); +} + +TEST_F(RestCatalogIntegrationTest, HandlesServerError) { + server_->Get("/v1/config", [](const httplib::Request&, httplib::Response& res) { + res.status = 500; + res.set_content("Internal Server Error", "text/plain"); + }); + + std::string base_uri = "http://127.0.0.1:" + std::to_string(port_); + RestCatalog catalog(base_uri); + cpr::Response response = catalog.GetConfig(); + + ASSERT_EQ(response.error.code, cpr::ErrorCode::OK); + ASSERT_EQ(response.status_code, 500); + ASSERT_EQ(response.text, "Internal Server Error"); +} + +} // namespace iceberg::catalog::rest From 2fe93470d633dd0021979e7c79fc16d21a905fa6 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Sat, 27 Sep 2025 18:40:15 +0800 Subject: [PATCH 2/5] add json dep for rest --- src/iceberg/catalog/rest/CMakeLists.txt | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index f18859cfc..460abc7ee 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -22,19 +22,28 @@ set(ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS) set(ICEBERG_REST_STATIC_INSTALL_INTERFACE_LIBS) set(ICEBERG_REST_SHARED_INSTALL_INTERFACE_LIBS) -list(APPEND ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS - "$,iceberg_static,iceberg_shared>" cpr::cpr) -list(APPEND ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS - "$,iceberg_shared,iceberg_static>" cpr::cpr) +list(APPEND + ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS + "$,iceberg_static,iceberg_shared>" + cpr::cpr + nlohmann_json::nlohmann_json) +list(APPEND + ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS + "$,iceberg_shared,iceberg_static>" + cpr::cpr + nlohmann_json::nlohmann_json) list(APPEND ICEBERG_REST_STATIC_INSTALL_INTERFACE_LIBS "$,Iceberg::iceberg_static,Iceberg::iceberg_shared>" - "$,Iceberg::cpr,cpr::cpr>") + "$,Iceberg::cpr,cpr::cpr>" + "$,Iceberg::nlohmann_json,nlohmann_json::nlohmann_json>" +) list(APPEND ICEBERG_REST_SHARED_INSTALL_INTERFACE_LIBS "$,Iceberg::iceberg_shared,Iceberg::iceberg_static>" - "$,Iceberg::cpr,cpr::cpr>") - + "$,Iceberg::cpr,cpr::cpr>" + "$,Iceberg::nlohmann_json,nlohmann_json::nlohmann_json>" +) add_iceberg_lib(iceberg_rest SOURCES ${ICEBERG_REST_SOURCES} From 2060b4d20843a707237df15ecdb4d7333fa7fa41 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Sun, 28 Sep 2025 16:44:14 +0800 Subject: [PATCH 3/5] fix --- src/iceberg/test/CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 2a496d5fa..0a54a57c3 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -27,7 +27,7 @@ fetchcontent_declare(googletest NAMES GTest) -if(ICEBERG_BUILD_REST_CATALOG) +if(ICEBERG_BUILD_REST) fetchcontent_makeavailable(cpp-httplib googletest) else() fetchcontent_makeavailable(googletest) @@ -147,8 +147,8 @@ if(ICEBERG_BUILD_BUNDLE) endif() -if(ICEBERG_BUILD_REST_CATALOG) +if(ICEBERG_BUILD_REST) add_iceberg_test(rest_catalog_test SOURCES rest_catalog_test.cc) - target_link_libraries(rest_catalog_test PRIVATE iceberg_rest_catalog_static) + target_link_libraries(rest_catalog_test PRIVATE iceberg_rest_static) target_include_directories(rest_catalog_test PRIVATE ${cpp-httplib_SOURCE_DIR}) endif() From b6dd502e4779bfccd2f0e38c99fe8ca4c25eee80 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Mon, 29 Sep 2025 16:51:52 +0800 Subject: [PATCH 4/5] fix review --- src/iceberg/catalog/rest/CMakeLists.txt | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index 460abc7ee..b4f823d7f 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -22,28 +22,18 @@ set(ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS) set(ICEBERG_REST_STATIC_INSTALL_INTERFACE_LIBS) set(ICEBERG_REST_SHARED_INSTALL_INTERFACE_LIBS) -list(APPEND - ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS - "$,iceberg_static,iceberg_shared>" - cpr::cpr - nlohmann_json::nlohmann_json) -list(APPEND - ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS - "$,iceberg_shared,iceberg_static>" - cpr::cpr - nlohmann_json::nlohmann_json) +list(APPEND ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS + "$,iceberg_static,iceberg_shared>" cpr::cpr) +list(APPEND ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS + "$,iceberg_shared,iceberg_static>" cpr::cpr) list(APPEND ICEBERG_REST_STATIC_INSTALL_INTERFACE_LIBS "$,Iceberg::iceberg_static,Iceberg::iceberg_shared>" - "$,Iceberg::cpr,cpr::cpr>" - "$,Iceberg::nlohmann_json,nlohmann_json::nlohmann_json>" -) + "$,Iceberg::cpr,cpr::cpr>") list(APPEND ICEBERG_REST_SHARED_INSTALL_INTERFACE_LIBS "$,Iceberg::iceberg_shared,Iceberg::iceberg_static>" - "$,Iceberg::cpr,cpr::cpr>" - "$,Iceberg::nlohmann_json,nlohmann_json::nlohmann_json>" -) + "$,Iceberg::cpr,cpr::cpr>") add_iceberg_lib(iceberg_rest SOURCES ${ICEBERG_REST_SOURCES} From 957f9d7b24676d30a65475ada75176d71e45a230 Mon Sep 17 00:00:00 2001 From: Li Feiyang Date: Tue, 30 Sep 2025 10:54:08 +0800 Subject: [PATCH 5/5] fix review --- src/iceberg/catalog/CMakeLists.txt | 1 + src/iceberg/catalog/rest/CMakeLists.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/src/iceberg/catalog/CMakeLists.txt b/src/iceberg/catalog/CMakeLists.txt index bcfb5f0cd..71675b5c0 100644 --- a/src/iceberg/catalog/CMakeLists.txt +++ b/src/iceberg/catalog/CMakeLists.txt @@ -16,6 +16,7 @@ # under the License. iceberg_install_all_headers(iceberg/catalog) + add_subdirectory(memory) if(ICEBERG_BUILD_REST) diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index b4f823d7f..f18859cfc 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -34,6 +34,7 @@ list(APPEND ICEBERG_REST_SHARED_INSTALL_INTERFACE_LIBS "$,Iceberg::iceberg_shared,Iceberg::iceberg_static>" "$,Iceberg::cpr,cpr::cpr>") + add_iceberg_lib(iceberg_rest SOURCES ${ICEBERG_REST_SOURCES}