Skip to content

Commit 7a432e3

Browse files
committed
Allow empty commit table requests
1 parent 5249f51 commit 7a432e3

2 files changed

Lines changed: 46 additions & 35 deletions

File tree

src/iceberg/catalog/rest/json_serde.cc

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -902,25 +902,29 @@ Result<CommitTableRequest> CommitTableRequestFromJson(const nlohmann::json& json
902902
}
903903

904904
ICEBERG_ASSIGN_OR_RAISE(auto requirements_json,
905-
GetJsonValue<nlohmann::json>(json, kRequirements));
906-
if (!requirements_json.is_array()) {
907-
return JsonParseError("Expected '{}' to be an array, got {}", kRequirements,
908-
SafeDumpJson(requirements_json));
909-
}
910-
for (const auto& req_json : requirements_json) {
911-
ICEBERG_ASSIGN_OR_RAISE(auto requirement, TableRequirementFromJson(req_json));
912-
request.requirements.push_back(std::move(requirement));
905+
GetJsonValueOptional<nlohmann::json>(json, kRequirements));
906+
if (requirements_json.has_value()) {
907+
if (!requirements_json->is_array()) {
908+
return JsonParseError("Expected '{}' to be an array, got {}", kRequirements,
909+
SafeDumpJson(*requirements_json));
910+
}
911+
for (const auto& req_json : *requirements_json) {
912+
ICEBERG_ASSIGN_OR_RAISE(auto requirement, TableRequirementFromJson(req_json));
913+
request.requirements.push_back(std::move(requirement));
914+
}
913915
}
914916

915917
ICEBERG_ASSIGN_OR_RAISE(auto updates_json,
916-
GetJsonValue<nlohmann::json>(json, kUpdates));
917-
if (!updates_json.is_array()) {
918-
return JsonParseError("Expected '{}' to be an array, got {}", kUpdates,
919-
SafeDumpJson(updates_json));
920-
}
921-
for (const auto& update_json : updates_json) {
922-
ICEBERG_ASSIGN_OR_RAISE(auto update, TableUpdateFromJson(update_json));
923-
request.updates.push_back(std::move(update));
918+
GetJsonValueOptional<nlohmann::json>(json, kUpdates));
919+
if (updates_json.has_value()) {
920+
if (!updates_json->is_array()) {
921+
return JsonParseError("Expected '{}' to be an array, got {}", kUpdates,
922+
SafeDumpJson(*updates_json));
923+
}
924+
for (const auto& update_json : *updates_json) {
925+
ICEBERG_ASSIGN_OR_RAISE(auto update, TableUpdateFromJson(update_json));
926+
request.updates.push_back(std::move(update));
927+
}
924928
}
925929

926930
ICEBERG_RETURN_UNEXPECTED(request.Validate());

src/iceberg/test/rest_json_serde_test.cc

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,7 +1236,31 @@ INSTANTIATE_TEST_SUITE_P(
12361236
CommitTableRequestDeserializeParam{
12371237
.test_name = "MissingIdentifier",
12381238
.json_str = R"({"requirements":[],"updates":[]})",
1239-
.expected_model = {}}),
1239+
.expected_model = {}},
1240+
// Requirements field is missing (should deserialize to empty requirements)
1241+
CommitTableRequestDeserializeParam{
1242+
.test_name = "MissingRequirements",
1243+
.json_str =
1244+
R"({"identifier":{"namespace":["ns1"],"name":"table1"},"updates":[]})",
1245+
.expected_model = {.identifier = TableIdentifier{Namespace{{"ns1"}},
1246+
"table1"}}},
1247+
// Updates field is missing (should deserialize to empty updates)
1248+
CommitTableRequestDeserializeParam{
1249+
.test_name = "MissingUpdates",
1250+
.json_str =
1251+
R"({"identifier":{"namespace":["ns1"],"name":"table1"},"requirements":[]})",
1252+
.expected_model = {.identifier = TableIdentifier{Namespace{{"ns1"}},
1253+
"table1"}}},
1254+
// Null requirements and updates are treated as absent
1255+
CommitTableRequestDeserializeParam{
1256+
.test_name = "NullRequirementsAndUpdates",
1257+
.json_str =
1258+
R"({"identifier":{"namespace":["ns1"],"name":"table1"},"requirements":null,"updates":null})",
1259+
.expected_model = {.identifier = TableIdentifier{Namespace{{"ns1"}},
1260+
"table1"}}},
1261+
// Empty JSON object
1262+
CommitTableRequestDeserializeParam{
1263+
.test_name = "EmptyJson", .json_str = R"({})", .expected_model = {}}),
12401264
[](const ::testing::TestParamInfo<CommitTableRequestDeserializeParam>& info) {
12411265
return info.param.test_name;
12421266
});
@@ -1304,24 +1328,7 @@ INSTANTIATE_TEST_SUITE_P(
13041328
.test_name = "InvalidUpdatesNotArray",
13051329
.invalid_json_str =
13061330
R"({"identifier":{"namespace":["ns1"],"name":"table1"},"requirements":[],"updates":{"action":"assign-uuid","uuid":"2cc52516-5e73-41f2-b139-545d41a4e151"}})",
1307-
.expected_error_message = "Expected 'updates' to be an array"},
1308-
// Missing required requirements field
1309-
CommitTableRequestInvalidParam{
1310-
.test_name = "MissingRequirements",
1311-
.invalid_json_str =
1312-
R"({"identifier":{"namespace":["ns1"],"name":"table1"},"updates":[]})",
1313-
.expected_error_message = "Missing 'requirements'"},
1314-
// Missing required updates field
1315-
CommitTableRequestInvalidParam{
1316-
.test_name = "MissingUpdates",
1317-
.invalid_json_str =
1318-
R"({"identifier":{"namespace":["ns1"],"name":"table1"},"requirements":[]})",
1319-
.expected_error_message = "Missing 'updates'"},
1320-
// Empty JSON object
1321-
CommitTableRequestInvalidParam{
1322-
.test_name = "EmptyJson",
1323-
.invalid_json_str = R"({})",
1324-
.expected_error_message = "Missing 'requirements'"}),
1331+
.expected_error_message = "Expected 'updates' to be an array"}),
13251332
[](const ::testing::TestParamInfo<CommitTableRequestInvalidParam>& info) {
13261333
return info.param.test_name;
13271334
});

0 commit comments

Comments
 (0)