Skip to content

Commit 4d34bf8

Browse files
HeartLinkedHeartLinked
committed
fix review
1 parent e4628d1 commit 4d34bf8

File tree

4 files changed

+165
-129
lines changed

4 files changed

+165
-129
lines changed

‎src/iceberg/catalog/rest/json_internal.cc‎

Lines changed: 11 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
#include "iceberg/catalog/rest/json_internal.h"
2121

22-
#include <ranges>
2322
#include <string>
2423
#include <unordered_map>
2524
#include <utility>
@@ -29,10 +28,7 @@
2928

3029
#include "iceberg/catalog/rest/types.h"
3130
#include "iceberg/json_internal.h"
32-
#include "iceberg/partition_spec.h"
33-
#include "iceberg/schema.h"
34-
#include "iceberg/sort_order.h"
35-
#include "iceberg/table_metadata.h"
31+
#include "iceberg/table_identifier.h"
3632
#include "iceberg/util/json_util_internal.h"
3733
#include "iceberg/util/macros.h"
3834

@@ -66,7 +62,6 @@ constexpr std::string_view kIdentifiers = "identifiers";
6662

6763
} // namespace
6864

69-
// CreateNamespaceRequest
7065
nlohmann::json ToJson(const CreateNamespaceRequest& request) {
7166
nlohmann::json json;
7267
json[kNamespace] = request.namespace_.levels;
@@ -83,12 +78,10 @@ Result<CreateNamespaceRequest> CreateNamespaceRequestFromJson(
8378
GetJsonValue<std::vector<std::string>>(json, kNamespace));
8479
ICEBERG_ASSIGN_OR_RAISE(
8580
request.properties,
86-
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
87-
kProperties)));
81+
GetJsonValueOrDefault<decltype(request.properties)>(json, kProperties));
8882
return request;
8983
}
9084

91-
// UpdateNamespacePropertiesRequest
9285
nlohmann::json ToJson(const UpdateNamespacePropertiesRequest& request) {
9386
// Initialize as an empty object so that when all optional fields are absent we return
9487
// {} instead of null
@@ -108,65 +101,10 @@ Result<UpdateNamespacePropertiesRequest> UpdateNamespacePropertiesRequestFromJso
108101
ICEBERG_ASSIGN_OR_RAISE(
109102
request.removals, GetJsonValueOrDefault<std::vector<std::string>>(json, kRemovals));
110103
ICEBERG_ASSIGN_OR_RAISE(
111-
request.updates,
112-
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
113-
kUpdates)));
104+
request.updates, GetJsonValueOrDefault<decltype(request.updates)>(json, kUpdates));
114105
return request;
115106
}
116107

117-
// CreateTableRequest
118-
nlohmann::json ToJson(const CreateTableRequest& request) {
119-
nlohmann::json json;
120-
json[kName] = request.name;
121-
if (!request.location.empty()) {
122-
json[kLocation] = request.location;
123-
}
124-
json[kSchema] = ToJson(*request.schema);
125-
if (request.partition_spec) {
126-
json[kPartitionSpec] = ToJson(*request.partition_spec);
127-
}
128-
if (request.write_order) {
129-
json[kWriteOrder] = ToJson(*request.write_order);
130-
}
131-
SetOptionalField(json, kStageCreate, request.stage_create);
132-
if (!request.properties.empty()) {
133-
json[kProperties] = request.properties;
134-
}
135-
return json;
136-
}
137-
138-
Result<CreateTableRequest> CreateTableRequestFromJson(const nlohmann::json& json) {
139-
CreateTableRequest request;
140-
ICEBERG_ASSIGN_OR_RAISE(request.name, GetJsonValue<std::string>(json, kName));
141-
ICEBERG_ASSIGN_OR_RAISE(request.location,
142-
GetJsonValueOrDefault<std::string>(json, kLocation, ""));
143-
ICEBERG_ASSIGN_OR_RAISE(auto schema_json, GetJsonValue<nlohmann::json>(json, kSchema));
144-
ICEBERG_ASSIGN_OR_RAISE(request.schema, SchemaFromJson(schema_json));
145-
if (json.contains(kPartitionSpec)) {
146-
ICEBERG_ASSIGN_OR_RAISE(auto partition_spec_json,
147-
GetJsonValue<nlohmann::json>(json, kPartitionSpec));
148-
ICEBERG_ASSIGN_OR_RAISE(request.partition_spec,
149-
PartitionSpecFromJson(request.schema, partition_spec_json));
150-
} else {
151-
request.partition_spec = nullptr;
152-
}
153-
if (json.contains(kWriteOrder)) {
154-
ICEBERG_ASSIGN_OR_RAISE(auto write_order_json,
155-
GetJsonValue<nlohmann::json>(json, kWriteOrder));
156-
ICEBERG_ASSIGN_OR_RAISE(request.write_order, SortOrderFromJson(write_order_json));
157-
} else {
158-
request.write_order = nullptr;
159-
}
160-
ICEBERG_ASSIGN_OR_RAISE(request.stage_create,
161-
GetJsonValueOptional<bool>(json, kStageCreate));
162-
ICEBERG_ASSIGN_OR_RAISE(
163-
request.properties,
164-
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
165-
kProperties)));
166-
return request;
167-
}
168-
169-
// RegisterTableRequest
170108
nlohmann::json ToJson(const RegisterTableRequest& request) {
171109
nlohmann::json json;
172110
json[kName] = request.name;
@@ -182,14 +120,11 @@ Result<RegisterTableRequest> RegisterTableRequestFromJson(const nlohmann::json&
182120
ICEBERG_ASSIGN_OR_RAISE(request.name, GetJsonValue<std::string>(json, kName));
183121
ICEBERG_ASSIGN_OR_RAISE(request.metadata_location,
184122
GetJsonValue<std::string>(json, kMetadataLocation));
185-
// Default to false if not present
186-
ICEBERG_ASSIGN_OR_RAISE(auto overwrite_opt,
187-
GetJsonValueOptional<bool>(json, kOverwrite));
188-
request.overwrite = overwrite_opt.value_or(false);
123+
ICEBERG_ASSIGN_OR_RAISE(request.overwrite,
124+
GetJsonValueOrDefault<bool>(json, kOverwrite, false));
189125
return request;
190126
}
191127

192-
// RenameTableRequest
193128
nlohmann::json ToJson(const RenameTableRequest& request) {
194129
nlohmann::json json;
195130
json[kSource] = ToJson(request.source);
@@ -222,8 +157,8 @@ nlohmann::json ToJson(const LoadTableResult& result) {
222157

223158
Result<LoadTableResult> LoadTableResultFromJson(const nlohmann::json& json) {
224159
LoadTableResult result;
225-
ICEBERG_ASSIGN_OR_RAISE(result.metadata_location, GetJsonValueOrDefault<std::string>(
226-
json, kMetadataLocation, ""));
160+
ICEBERG_ASSIGN_OR_RAISE(result.metadata_location,
161+
GetJsonValueOrDefault<std::string>(json, kMetadataLocation));
227162
ICEBERG_ASSIGN_OR_RAISE(auto metadata_json,
228163
GetJsonValue<nlohmann::json>(json, kMetadata));
229164
ICEBERG_ASSIGN_OR_RAISE(result.metadata, TableMetadataFromJson(metadata_json));
@@ -233,7 +168,6 @@ Result<LoadTableResult> LoadTableResultFromJson(const nlohmann::json& json) {
233168
return result;
234169
}
235170

236-
// ListNamespacesResponse
237171
nlohmann::json ToJson(const ListNamespacesResponse& response) {
238172
nlohmann::json json;
239173
if (!response.next_page_token.empty()) {
@@ -251,7 +185,7 @@ Result<ListNamespacesResponse> ListNamespacesResponseFromJson(
251185
const nlohmann::json& json) {
252186
ListNamespacesResponse response;
253187
ICEBERG_ASSIGN_OR_RAISE(response.next_page_token,
254-
GetJsonValueOrDefault<std::string>(json, kNextPageToken, ""));
188+
GetJsonValueOrDefault<std::string>(json, kNextPageToken));
255189
ICEBERG_ASSIGN_OR_RAISE(auto namespaces_json,
256190
GetJsonValue<nlohmann::json>(json, kNamespaces));
257191
for (const auto& ns_json : namespaces_json) {
@@ -261,7 +195,6 @@ Result<ListNamespacesResponse> ListNamespacesResponseFromJson(
261195
return response;
262196
}
263197

264-
// CreateNamespaceResponse
265198
nlohmann::json ToJson(const CreateNamespaceResponse& response) {
266199
nlohmann::json json;
267200
json[kNamespace] = response.namespace_.levels;
@@ -278,12 +211,10 @@ Result<CreateNamespaceResponse> CreateNamespaceResponseFromJson(
278211
GetJsonValue<std::vector<std::string>>(json, kNamespace));
279212
ICEBERG_ASSIGN_OR_RAISE(
280213
response.properties,
281-
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
282-
kProperties)));
214+
GetJsonValueOrDefault<decltype(response.properties)>(json, kProperties));
283215
return response;
284216
}
285217

286-
// GetNamespaceResponse
287218
nlohmann::json ToJson(const GetNamespaceResponse& response) {
288219
nlohmann::json json;
289220
json[kNamespace] = response.namespace_.levels;
@@ -299,12 +230,10 @@ Result<GetNamespaceResponse> GetNamespaceResponseFromJson(const nlohmann::json&
299230
GetJsonValue<std::vector<std::string>>(json, kNamespace));
300231
ICEBERG_ASSIGN_OR_RAISE(
301232
response.properties,
302-
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
303-
kProperties)));
233+
GetJsonValueOrDefault<decltype(response.properties)>(json, kProperties));
304234
return response;
305235
}
306236

307-
// UpdateNamespacePropertiesResponse
308237
nlohmann::json ToJson(const UpdateNamespacePropertiesResponse& response) {
309238
nlohmann::json json;
310239
json[kUpdated] = response.updated;
@@ -327,7 +256,6 @@ Result<UpdateNamespacePropertiesResponse> UpdateNamespacePropertiesResponseFromJ
327256
return response;
328257
}
329258

330-
// ListTablesResponse
331259
nlohmann::json ToJson(const ListTablesResponse& response) {
332260
nlohmann::json json;
333261
if (!response.next_page_token.empty()) {
@@ -344,7 +272,7 @@ nlohmann::json ToJson(const ListTablesResponse& response) {
344272
Result<ListTablesResponse> ListTablesResponseFromJson(const nlohmann::json& json) {
345273
ListTablesResponse response;
346274
ICEBERG_ASSIGN_OR_RAISE(response.next_page_token,
347-
GetJsonValueOrDefault<std::string>(json, kNextPageToken, ""));
275+
GetJsonValueOrDefault<std::string>(json, kNextPageToken));
348276
ICEBERG_ASSIGN_OR_RAISE(auto identifiers_json,
349277
GetJsonValue<nlohmann::json>(json, kIdentifiers));
350278
for (const auto& id_json : identifiers_json) {

‎src/iceberg/catalog/rest/json_internal.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,6 @@ ICEBERG_REST_EXPORT nlohmann::json ToJson(const ListTablesResponse& response);
7777
ICEBERG_REST_EXPORT Result<ListTablesResponse> ListTablesResponseFromJson(
7878
const nlohmann::json& json);
7979

80-
/// \brief Serializes a `CreateTableRequest` object to JSON.
81-
ICEBERG_REST_EXPORT nlohmann::json ToJson(const CreateTableRequest& request);
82-
83-
/// \brief Deserializes a JSON object into a `CreateTableRequest` object.
84-
ICEBERG_REST_EXPORT Result<CreateTableRequest> CreateTableRequestFromJson(
85-
const nlohmann::json& json);
86-
8780
/// \brief Serializes a `LoadTableResult` object to JSON.
8881
ICEBERG_REST_EXPORT nlohmann::json ToJson(const LoadTableResult& result);
8982

‎src/iceberg/catalog/rest/meson.build‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,4 @@ iceberg_rest_dep = declare_dependency(
4646
meson.override_dependency('iceberg-rest', iceberg_rest_dep)
4747
pkg.generate(iceberg_rest_lib)
4848

49-
install_headers(
50-
['rest_catalog.h', 'types.h', 'json_internal.h'],
51-
subdir: 'iceberg/catalog/rest',
52-
)
49+
install_headers(['rest_catalog.h', 'types.h'], subdir: 'iceberg/catalog/rest')

0 commit comments

Comments
 (0)