This is an automated email from the ASF dual-hosted git repository.
gangwu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 37bc3890 refactor: Use RestCatalogProperties by value instead of
unique_ptr (#575)
37bc3890 is described below
commit 37bc3890ceba37ae714ea9a5f3103b2612bab7b6
Author: lishuxu <[email protected]>
AuthorDate: Wed Feb 25 18:22:45 2026 +0800
refactor: Use RestCatalogProperties by value instead of unique_ptr (#575)
---
src/iceberg/catalog/rest/catalog_properties.cc | 13 ++--
src/iceberg/catalog/rest/catalog_properties.h | 10 +--
src/iceberg/catalog/rest/rest_catalog.cc | 15 +++--
src/iceberg/catalog/rest/rest_catalog.h | 8 +--
src/iceberg/test/config_test.cc | 86 +++++++++++++-------------
src/iceberg/test/rest_catalog_test.cc | 6 +-
6 files changed, 65 insertions(+), 73 deletions(-)
diff --git a/src/iceberg/catalog/rest/catalog_properties.cc
b/src/iceberg/catalog/rest/catalog_properties.cc
index 4d956837..9f12c492 100644
--- a/src/iceberg/catalog/rest/catalog_properties.cc
+++ b/src/iceberg/catalog/rest/catalog_properties.cc
@@ -23,15 +23,14 @@
namespace iceberg::rest {
-std::unique_ptr<RestCatalogProperties>
RestCatalogProperties::default_properties() {
- return std::unique_ptr<RestCatalogProperties>(new RestCatalogProperties());
+RestCatalogProperties RestCatalogProperties::default_properties() {
+ return RestCatalogProperties();
}
-std::unique_ptr<RestCatalogProperties> RestCatalogProperties::FromMap(
- const std::unordered_map<std::string, std::string>& properties) {
- auto rest_catalog_config =
- std::unique_ptr<RestCatalogProperties>(new RestCatalogProperties());
- rest_catalog_config->configs_ = properties;
+RestCatalogProperties RestCatalogProperties::FromMap(
+ std::unordered_map<std::string, std::string> properties) {
+ RestCatalogProperties rest_catalog_config;
+ rest_catalog_config.configs_ = std::move(properties);
return rest_catalog_config;
}
diff --git a/src/iceberg/catalog/rest/catalog_properties.h
b/src/iceberg/catalog/rest/catalog_properties.h
index d351b50f..be054dfa 100644
--- a/src/iceberg/catalog/rest/catalog_properties.h
+++ b/src/iceberg/catalog/rest/catalog_properties.h
@@ -19,7 +19,6 @@
#pragma once
-#include <memory>
#include <string>
#include <unordered_map>
@@ -51,11 +50,11 @@ class ICEBERG_REST_EXPORT RestCatalogProperties
inline static constexpr std::string_view kHeaderPrefix = "header.";
/// \brief Create a default RestCatalogProperties instance.
- static std::unique_ptr<RestCatalogProperties> default_properties();
+ static RestCatalogProperties default_properties();
/// \brief Create a RestCatalogProperties instance from a map of key-value
pairs.
- static std::unique_ptr<RestCatalogProperties> FromMap(
- const std::unordered_map<std::string, std::string>& properties);
+ static RestCatalogProperties FromMap(
+ std::unordered_map<std::string, std::string> properties);
/// \brief Returns HTTP headers to be added to every request.
std::unordered_map<std::string, std::string> ExtractHeaders() const;
@@ -63,9 +62,6 @@ class ICEBERG_REST_EXPORT RestCatalogProperties
/// \brief Get the URI of the REST catalog server.
/// \return The URI if configured, or an error if not set or empty.
Result<std::string_view> Uri() const;
-
- private:
- RestCatalogProperties() = default;
};
} // namespace iceberg::rest
diff --git a/src/iceberg/catalog/rest/rest_catalog.cc
b/src/iceberg/catalog/rest/rest_catalog.cc
index 8cc7be75..94c6b1e4 100644
--- a/src/iceberg/catalog/rest/rest_catalog.cc
+++ b/src/iceberg/catalog/rest/rest_catalog.cc
@@ -130,7 +130,7 @@ Result<std::shared_ptr<RestCatalog>> RestCatalog::Make(
ICEBERG_ASSIGN_OR_RAISE(auto server_config,
FetchServerConfig(*paths, config, *init_session));
- std::unique_ptr<RestCatalogProperties> final_config =
RestCatalogProperties::FromMap(
+ RestCatalogProperties final_config = RestCatalogProperties::FromMap(
MergeConfigs(server_config.defaults, config.configs(),
server_config.overrides));
std::unordered_set<Endpoint> endpoints;
@@ -145,22 +145,21 @@ Result<std::shared_ptr<RestCatalog>> RestCatalog::Make(
}
// Update resource paths based on the final config
- ICEBERG_ASSIGN_OR_RAISE(auto final_uri, final_config->Uri());
+ ICEBERG_ASSIGN_OR_RAISE(auto final_uri, final_config.Uri());
ICEBERG_ASSIGN_OR_RAISE(
paths, ResourcePaths::Make(std::string(TrimTrailingSlash(final_uri)),
-
final_config->Get(RestCatalogProperties::kPrefix)));
+
final_config.Get(RestCatalogProperties::kPrefix)));
- auto client = std::make_unique<HttpClient>(final_config->ExtractHeaders());
+ auto client = std::make_unique<HttpClient>(final_config.ExtractHeaders());
ICEBERG_ASSIGN_OR_RAISE(auto catalog_session,
- auth_manager->CatalogSession(*client,
final_config->configs()));
+ auth_manager->CatalogSession(*client,
final_config.configs()));
return std::shared_ptr<RestCatalog>(new RestCatalog(
std::move(final_config), std::move(file_io), std::move(client),
std::move(paths),
std::move(endpoints), std::move(auth_manager),
std::move(catalog_session)));
}
-RestCatalog::RestCatalog(std::unique_ptr<RestCatalogProperties> config,
- std::shared_ptr<FileIO> file_io,
+RestCatalog::RestCatalog(RestCatalogProperties config, std::shared_ptr<FileIO>
file_io,
std::unique_ptr<HttpClient> client,
std::unique_ptr<ResourcePaths> paths,
std::unordered_set<Endpoint> endpoints,
@@ -170,7 +169,7 @@
RestCatalog::RestCatalog(std::unique_ptr<RestCatalogProperties> config,
file_io_(std::move(file_io)),
client_(std::move(client)),
paths_(std::move(paths)),
- name_(config_->Get(RestCatalogProperties::kName)),
+ name_(config_.Get(RestCatalogProperties::kName)),
supported_endpoints_(std::move(endpoints)),
auth_manager_(std::move(auth_manager)),
catalog_session_(std::move(catalog_session)) {
diff --git a/src/iceberg/catalog/rest/rest_catalog.h
b/src/iceberg/catalog/rest/rest_catalog.h
index d498d9c7..5cc61eae 100644
--- a/src/iceberg/catalog/rest/rest_catalog.h
+++ b/src/iceberg/catalog/rest/rest_catalog.h
@@ -24,6 +24,7 @@
#include <unordered_set>
#include "iceberg/catalog.h"
+#include "iceberg/catalog/rest/catalog_properties.h"
#include "iceberg/catalog/rest/endpoint.h"
#include "iceberg/catalog/rest/iceberg_rest_export.h"
#include "iceberg/catalog/rest/type_fwd.h"
@@ -104,9 +105,8 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog,
const std::string& metadata_file_location) override;
private:
- RestCatalog(std::unique_ptr<RestCatalogProperties> config,
- std::shared_ptr<FileIO> file_io, std::unique_ptr<HttpClient>
client,
- std::unique_ptr<ResourcePaths> paths,
+ RestCatalog(RestCatalogProperties config, std::shared_ptr<FileIO> file_io,
+ std::unique_ptr<HttpClient> client,
std::unique_ptr<ResourcePaths> paths,
std::unordered_set<Endpoint> endpoints,
std::unique_ptr<auth::AuthManager> auth_manager,
std::shared_ptr<auth::AuthSession> catalog_session);
@@ -119,7 +119,7 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog,
const std::string& location,
const std::unordered_map<std::string, std::string>& properties, bool
stage_create);
- std::unique_ptr<RestCatalogProperties> config_;
+ RestCatalogProperties config_;
std::shared_ptr<FileIO> file_io_;
std::unique_ptr<HttpClient> client_;
std::unique_ptr<ResourcePaths> paths_;
diff --git a/src/iceberg/test/config_test.cc b/src/iceberg/test/config_test.cc
index b7f378bf..36aa9f30 100644
--- a/src/iceberg/test/config_test.cc
+++ b/src/iceberg/test/config_test.cc
@@ -65,9 +65,7 @@ class TestConfig : public ConfigBase<TestConfig> {
EnumToString, StringToEnum};
inline static const Entry<double> kDoubleConfig{"double_config", 3.14};
- static std::unique_ptr<TestConfig> default_properties() {
- return std::unique_ptr<TestConfig>(new TestConfig());
- }
+ static TestConfig default_properties() { return TestConfig(); }
private:
TestConfig() = default;
@@ -77,55 +75,55 @@ TEST(ConfigTest, BasicOperations) {
auto config = TestConfig::default_properties();
// Test default values
- ASSERT_EQ(config->Get(TestConfig::kStringConfig),
std::string("default_value"));
- ASSERT_EQ(config->Get(TestConfig::kIntConfig), 25);
- ASSERT_EQ(config->Get(TestConfig::kBoolConfig), false);
- ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
- ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 3.14);
+ ASSERT_EQ(config.Get(TestConfig::kStringConfig),
std::string("default_value"));
+ ASSERT_EQ(config.Get(TestConfig::kIntConfig), 25);
+ ASSERT_EQ(config.Get(TestConfig::kBoolConfig), false);
+ ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
+ ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 3.14);
// Test setting values
- config->Set(TestConfig::kStringConfig, std::string("new_value"));
- config->Set(TestConfig::kIntConfig, 100);
- config->Set(TestConfig::kBoolConfig, true);
- config->Set(TestConfig::kEnumConfig, TestEnum::VALUE2);
- config->Set(TestConfig::kDoubleConfig, 2.99);
-
- ASSERT_EQ(config->Get(TestConfig::kStringConfig), "new_value");
- ASSERT_EQ(config->Get(TestConfig::kIntConfig), 100);
- ASSERT_EQ(config->Get(TestConfig::kBoolConfig), true);
- ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE2);
- ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 2.99);
+ config.Set(TestConfig::kStringConfig, std::string("new_value"));
+ config.Set(TestConfig::kIntConfig, 100);
+ config.Set(TestConfig::kBoolConfig, true);
+ config.Set(TestConfig::kEnumConfig, TestEnum::VALUE2);
+ config.Set(TestConfig::kDoubleConfig, 2.99);
+
+ ASSERT_EQ(config.Get(TestConfig::kStringConfig), "new_value");
+ ASSERT_EQ(config.Get(TestConfig::kIntConfig), 100);
+ ASSERT_EQ(config.Get(TestConfig::kBoolConfig), true);
+ ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE2);
+ ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 2.99);
// Test setting values again
- config->Set(TestConfig::kStringConfig, std::string("newer_value"));
- config->Set(TestConfig::kIntConfig, 200);
- config->Set(TestConfig::kBoolConfig, false);
- config->Set(TestConfig::kEnumConfig, TestEnum::VALUE1);
- config->Set(TestConfig::kDoubleConfig, 3.99);
-
- ASSERT_EQ(config->Get(TestConfig::kStringConfig), "newer_value");
- ASSERT_EQ(config->Get(TestConfig::kIntConfig), 200);
- ASSERT_EQ(config->Get(TestConfig::kBoolConfig), false);
- ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
- ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 3.99);
+ config.Set(TestConfig::kStringConfig, std::string("newer_value"));
+ config.Set(TestConfig::kIntConfig, 200);
+ config.Set(TestConfig::kBoolConfig, false);
+ config.Set(TestConfig::kEnumConfig, TestEnum::VALUE1);
+ config.Set(TestConfig::kDoubleConfig, 3.99);
+
+ ASSERT_EQ(config.Get(TestConfig::kStringConfig), "newer_value");
+ ASSERT_EQ(config.Get(TestConfig::kIntConfig), 200);
+ ASSERT_EQ(config.Get(TestConfig::kBoolConfig), false);
+ ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
+ ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 3.99);
// Test unsetting a value
- config->Unset(TestConfig::kIntConfig);
- config->Unset(TestConfig::kEnumConfig);
- config->Unset(TestConfig::kDoubleConfig);
- ASSERT_EQ(config->Get(TestConfig::kIntConfig), 25);
- ASSERT_EQ(config->Get(TestConfig::kStringConfig), "newer_value");
- ASSERT_EQ(config->Get(TestConfig::kBoolConfig), false);
- ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
- ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 3.14);
+ config.Unset(TestConfig::kIntConfig);
+ config.Unset(TestConfig::kEnumConfig);
+ config.Unset(TestConfig::kDoubleConfig);
+ ASSERT_EQ(config.Get(TestConfig::kIntConfig), 25);
+ ASSERT_EQ(config.Get(TestConfig::kStringConfig), "newer_value");
+ ASSERT_EQ(config.Get(TestConfig::kBoolConfig), false);
+ ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
+ ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 3.14);
// Test resetting all values
- config->Reset();
- ASSERT_EQ(config->Get(TestConfig::kStringConfig), "default_value");
- ASSERT_EQ(config->Get(TestConfig::kIntConfig), 25);
- ASSERT_EQ(config->Get(TestConfig::kBoolConfig), false);
- ASSERT_EQ(config->Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
- ASSERT_EQ(config->Get(TestConfig::kDoubleConfig), 3.14);
+ config.Reset();
+ ASSERT_EQ(config.Get(TestConfig::kStringConfig), "default_value");
+ ASSERT_EQ(config.Get(TestConfig::kIntConfig), 25);
+ ASSERT_EQ(config.Get(TestConfig::kBoolConfig), false);
+ ASSERT_EQ(config.Get(TestConfig::kEnumConfig), TestEnum::VALUE1);
+ ASSERT_EQ(config.Get(TestConfig::kDoubleConfig), 3.14);
}
} // namespace iceberg
diff --git a/src/iceberg/test/rest_catalog_test.cc
b/src/iceberg/test/rest_catalog_test.cc
index e52c87ee..d4a9477b 100644
--- a/src/iceberg/test/rest_catalog_test.cc
+++ b/src/iceberg/test/rest_catalog_test.cc
@@ -133,12 +133,12 @@ class RestCatalogIntegrationTest : public ::testing::Test
{
Result<std::shared_ptr<RestCatalog>> CreateCatalog() {
auto config = RestCatalogProperties::default_properties();
config
- ->Set(RestCatalogProperties::kUri,
- std::format("{}:{}", kLocalhostUri, kRestCatalogPort))
+ .Set(RestCatalogProperties::kUri,
+ std::format("{}:{}", kLocalhostUri, kRestCatalogPort))
.Set(RestCatalogProperties::kName, std::string(kCatalogName))
.Set(RestCatalogProperties::kWarehouse, std::string(kWarehouseName));
auto file_io = std::make_shared<test::StdFileIO>();
- return RestCatalog::Make(*config, std::make_shared<test::StdFileIO>());
+ return RestCatalog::Make(config, std::make_shared<test::StdFileIO>());
}
// Helper function to create a default schema for testing
