Skip to content

refactor: simplify test structure for rest catalog models #290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 4, 2025
+541 −746
Merged

refactor: simplify test structure for rest catalog models #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 33 additions & 39 deletions src/iceberg/catalog/rest/json_internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ constexpr std::string_view kIdentifiers = "identifiers";
nlohmann::json ToJson(const CreateNamespaceRequest& request) {
nlohmann::json json;
json[kNamespace] = request.namespace_.levels;
if (!request.properties.empty()) {
json[kProperties] = request.properties;
}
SetContainerField(json, kProperties, request.properties);
return json;
}

Expand All @@ -83,15 +81,9 @@ Result<CreateNamespaceRequest> CreateNamespaceRequestFromJson(
}

nlohmann::json ToJson(const UpdateNamespacePropertiesRequest& request) {
// Initialize as an empty object so that when all optional fields are absent we return
// {} instead of null
nlohmann::json json = nlohmann::json::object();
if (!request.removals.empty()) {
json[kRemovals] = request.removals;
}
if (!request.updates.empty()) {
json[kUpdates] = request.updates;
}
SetContainerField(json, kRemovals, request.removals);
SetContainerField(json, kUpdates, request.updates);
return json;
}

Expand Down Expand Up @@ -145,13 +137,9 @@ Result<RenameTableRequest> RenameTableRequestFromJson(const nlohmann::json& json
// LoadTableResult (used by CreateTableResponse, LoadTableResponse)
nlohmann::json ToJson(const LoadTableResult& result) {
nlohmann::json json;
if (!result.metadata_location.empty()) {
json[kMetadataLocation] = result.metadata_location;
}
SetOptionalStringField(json, kMetadataLocation, result.metadata_location);
json[kMetadata] = ToJson(*result.metadata);
if (!result.config.empty()) {
json[kConfig] = result.config;
}
SetContainerField(json, kConfig, result.config);
return json;
}

Expand All @@ -162,17 +150,14 @@ Result<LoadTableResult> LoadTableResultFromJson(const nlohmann::json& json) {
ICEBERG_ASSIGN_OR_RAISE(auto metadata_json,
GetJsonValue<nlohmann::json>(json, kMetadata));
ICEBERG_ASSIGN_OR_RAISE(result.metadata, TableMetadataFromJson(metadata_json));
ICEBERG_ASSIGN_OR_RAISE(
result.config, (GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(
json, kConfig)));
ICEBERG_ASSIGN_OR_RAISE(result.config,
GetJsonValueOrDefault<decltype(result.config)>(json, kConfig));
return result;
}

nlohmann::json ToJson(const ListNamespacesResponse& response) {
nlohmann::json json;
if (!response.next_page_token.empty()) {
json[kNextPageToken] = response.next_page_token;
}
SetOptionalStringField(json, kNextPageToken, response.next_page_token);
nlohmann::json namespaces = nlohmann::json::array();
for (const auto& ns : response.namespaces) {
namespaces.push_back(ToJson(ns));
Expand All @@ -198,9 +183,7 @@ Result<ListNamespacesResponse> ListNamespacesResponseFromJson(
nlohmann::json ToJson(const CreateNamespaceResponse& response) {
nlohmann::json json;
json[kNamespace] = response.namespace_.levels;
if (!response.properties.empty()) {
json[kProperties] = response.properties;
}
SetContainerField(json, kProperties, response.properties);
return json;
}

Expand All @@ -218,9 +201,7 @@ Result<CreateNamespaceResponse> CreateNamespaceResponseFromJson(
nlohmann::json ToJson(const GetNamespaceResponse& response) {
nlohmann::json json;
json[kNamespace] = response.namespace_.levels;
if (!response.properties.empty()) {
json[kProperties] = response.properties;
}
SetContainerField(json, kProperties, response.properties);
return json;
}

Expand All @@ -238,29 +219,25 @@ nlohmann::json ToJson(const UpdateNamespacePropertiesResponse& response) {
nlohmann::json json;
json[kUpdated] = response.updated;
json[kRemoved] = response.removed;
if (!response.missing.empty()) {
json[kMissing] = response.missing;
}
SetContainerField(json, kMissing, response.missing);
return json;
}

Result<UpdateNamespacePropertiesResponse> UpdateNamespacePropertiesResponseFromJson(
const nlohmann::json& json) {
UpdateNamespacePropertiesResponse response;
ICEBERG_ASSIGN_OR_RAISE(response.updated,
GetJsonValue<std::vector<std::string>>(json, kUpdated));
ICEBERG_ASSIGN_OR_RAISE(response.removed,
GetJsonValue<std::vector<std::string>>(json, kRemoved));
ICEBERG_ASSIGN_OR_RAISE(
response.updated, GetJsonValueOrDefault<std::vector<std::string>>(json, kUpdated));
ICEBERG_ASSIGN_OR_RAISE(
response.removed, GetJsonValueOrDefault<std::vector<std::string>>(json, kRemoved));
ICEBERG_ASSIGN_OR_RAISE(
response.missing, GetJsonValueOrDefault<std::vector<std::string>>(json, kMissing));
return response;
}

nlohmann::json ToJson(const ListTablesResponse& response) {
nlohmann::json json;
if (!response.next_page_token.empty()) {
json[kNextPageToken] = response.next_page_token;
}
SetOptionalStringField(json, kNextPageToken, response.next_page_token);
nlohmann::json identifiers_json = nlohmann::json::array();
for (const auto& identifier : response.identifiers) {
identifiers_json.push_back(ToJson(identifier));
Expand All @@ -282,4 +259,21 @@ Result<ListTablesResponse> ListTablesResponseFromJson(const nlohmann::json& json
return response;
}

#define ICEBERG_DEFINE_FROM_JSON(Model) \
template <> \
Result<Model> FromJson<Model>(const nlohmann::json& json) { \
return Model##FromJson(json); \
}

ICEBERG_DEFINE_FROM_JSON(ListNamespacesResponse)
ICEBERG_DEFINE_FROM_JSON(CreateNamespaceRequest)
ICEBERG_DEFINE_FROM_JSON(CreateNamespaceResponse)
ICEBERG_DEFINE_FROM_JSON(GetNamespaceResponse)
ICEBERG_DEFINE_FROM_JSON(UpdateNamespacePropertiesRequest)
ICEBERG_DEFINE_FROM_JSON(UpdateNamespacePropertiesResponse)
ICEBERG_DEFINE_FROM_JSON(ListTablesResponse)
ICEBERG_DEFINE_FROM_JSON(LoadTableResult)
ICEBERG_DEFINE_FROM_JSON(RegisterTableRequest)
ICEBERG_DEFINE_FROM_JSON(RenameTableRequest)

} // namespace iceberg::rest
97 changes: 26 additions & 71 deletions src/iceberg/catalog/rest/json_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,81 +21,36 @@

#include <nlohmann/json_fwd.hpp>

#include "iceberg/catalog/rest/iceberg_rest_export.h"
#include "iceberg/catalog/rest/types.h"
#include "iceberg/result.h"

namespace iceberg::rest {

/// \brief Serializes a `ListNamespacesResponse` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const ListNamespacesResponse& response);

/// \brief Deserializes a JSON object into a `ListNamespacesResponse` object.
ICEBERG_REST_EXPORT Result<ListNamespacesResponse> ListNamespacesResponseFromJson(
const nlohmann::json& json);

/// \brief Serializes a `CreateNamespaceRequest` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const CreateNamespaceRequest& request);

/// \brief Deserializes a JSON object into a `CreateNamespaceRequest` object.
ICEBERG_REST_EXPORT Result<CreateNamespaceRequest> CreateNamespaceRequestFromJson(
const nlohmann::json& json);

/// \brief Serializes a `CreateNamespaceResponse` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const CreateNamespaceResponse& response);

/// \brief Deserializes a JSON object into a `CreateNamespaceResponse` object.
ICEBERG_REST_EXPORT Result<CreateNamespaceResponse> CreateNamespaceResponseFromJson(
const nlohmann::json& json);

/// \brief Serializes a `GetNamespaceResponse` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const GetNamespaceResponse& response);

/// \brief Deserializes a JSON object into a `GetNamespaceResponse` object.
ICEBERG_REST_EXPORT Result<GetNamespaceResponse> GetNamespaceResponseFromJson(
const nlohmann::json& json);

/// \brief Serializes an `UpdateNamespacePropertiesRequest` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(
const UpdateNamespacePropertiesRequest& request);

/// \brief Deserializes a JSON object into an `UpdateNamespacePropertiesRequest` object.
ICEBERG_REST_EXPORT Result<UpdateNamespacePropertiesRequest>
UpdateNamespacePropertiesRequestFromJson(const nlohmann::json& json);

/// \brief Serializes an `UpdateNamespacePropertiesResponse` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(
const UpdateNamespacePropertiesResponse& response);

/// \brief Deserializes a JSON object into an `UpdateNamespacePropertiesResponse` object.
ICEBERG_REST_EXPORT Result<UpdateNamespacePropertiesResponse>
UpdateNamespacePropertiesResponseFromJson(const nlohmann::json& json);

/// \brief Serializes a `ListTablesResponse` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const ListTablesResponse& response);

/// \brief Deserializes a JSON object into a `ListTablesResponse` object.
ICEBERG_REST_EXPORT Result<ListTablesResponse> ListTablesResponseFromJson(
const nlohmann::json& json);

/// \brief Serializes a `LoadTableResult` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const LoadTableResult& result);

/// \brief Deserializes a JSON object into a `LoadTableResult` object.
ICEBERG_REST_EXPORT Result<LoadTableResult> LoadTableResultFromJson(
const nlohmann::json& json);

/// \brief Serializes a `RegisterTableRequest` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const RegisterTableRequest& request);

/// \brief Deserializes a JSON object into a `RegisterTableRequest` object.
ICEBERG_REST_EXPORT Result<RegisterTableRequest> RegisterTableRequestFromJson(
const nlohmann::json& json);

/// \brief Serializes a `RenameTableRequest` object to JSON.
ICEBERG_REST_EXPORT nlohmann::json ToJson(const RenameTableRequest& request);

/// \brief Deserializes a JSON object into a `RenameTableRequest` object.
ICEBERG_REST_EXPORT Result<RenameTableRequest> RenameTableRequestFromJson(
const nlohmann::json& json);
template <typename Model>
Result<Model> FromJson(const nlohmann::json& json);

#define ICEBERG_DECLARE_JSON_SERDE(Model) \
ICEBERG_REST_EXPORT Result<Model> Model##FromJson(const nlohmann::json& json); \
\
template <> \
ICEBERG_REST_EXPORT Result<Model> FromJson(const nlohmann::json& json); \
\
ICEBERG_REST_EXPORT nlohmann::json ToJson(const Model& model);

/// \note Don't forget to add `ICEBERG_DEFINE_FROM_JSON` to the end of
/// `json_internal.cc` to define the `FromJson` function for the model.
ICEBERG_DECLARE_JSON_SERDE(ListNamespacesResponse)
ICEBERG_DECLARE_JSON_SERDE(CreateNamespaceRequest)
ICEBERG_DECLARE_JSON_SERDE(CreateNamespaceResponse)
ICEBERG_DECLARE_JSON_SERDE(GetNamespaceResponse)
ICEBERG_DECLARE_JSON_SERDE(UpdateNamespacePropertiesRequest)
ICEBERG_DECLARE_JSON_SERDE(UpdateNamespacePropertiesResponse)
ICEBERG_DECLARE_JSON_SERDE(ListTablesResponse)
ICEBERG_DECLARE_JSON_SERDE(LoadTableResult)
ICEBERG_DECLARE_JSON_SERDE(RegisterTableRequest)
ICEBERG_DECLARE_JSON_SERDE(RenameTableRequest)

#undef ICEBERG_DECLARE_JSON_SERDE

} // namespace iceberg::rest
6 changes: 3 additions & 3 deletions src/iceberg/test/rest_catalog_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class RestCatalogIntegrationTest : public ::testing::Test {
std::thread server_thread_;
};

TEST_F(RestCatalogIntegrationTest, GetConfigSuccessfully) {
TEST_F(RestCatalogIntegrationTest, DISABLED_GetConfigSuccessfully) {
server_->Get("/v1/config", [](const httplib::Request&, httplib::Response& res) {
res.status = 200;
res.set_content(R"({"warehouse": "s3://test-bucket"})", "application/json");
Expand All @@ -68,7 +68,7 @@ TEST_F(RestCatalogIntegrationTest, GetConfigSuccessfully) {
EXPECT_EQ(json_body["warehouse"], "s3://test-bucket");
}

TEST_F(RestCatalogIntegrationTest, ListNamespacesReturnsMultipleResults) {
TEST_F(RestCatalogIntegrationTest, DISABLED_ListNamespacesReturnsMultipleResults) {
server_->Get("/v1/namespaces", [](const httplib::Request&, httplib::Response& res) {
res.status = 200;
res.set_content(R"({
Expand All @@ -93,7 +93,7 @@ TEST_F(RestCatalogIntegrationTest, ListNamespacesReturnsMultipleResults) {
EXPECT_THAT(json_body["namespaces"][0][0], "accounting");
}

TEST_F(RestCatalogIntegrationTest, HandlesServerError) {
TEST_F(RestCatalogIntegrationTest, DISABLED_HandlesServerError) {
server_->Get("/v1/config", [](const httplib::Request&, httplib::Response& res) {
res.status = 500;
res.set_content("Internal Server Error", "text/plain");
Expand Down
Loading
Loading