Skip to content

Commit f249cc5

Browse files
HeartLinkedHeartLinked
committed
fix review(test todo
1 parent 8444ef8 commit f249cc5

File tree

9 files changed

+187
-250
lines changed

9 files changed

+187
-250
lines changed

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

Lines changed: 55 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@
1919

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

22+
#include <ranges>
2223
#include <string>
2324
#include <unordered_map>
2425
#include <utility>
2526
#include <vector>
2627

2728
#include <nlohmann/json.hpp>
2829

30+
#include "iceberg/catalog/rest/types.h"
2931
#include "iceberg/json_internal.h"
3032
#include "iceberg/partition_spec.h"
3133
#include "iceberg/schema.h"
@@ -62,27 +64,6 @@ constexpr std::string_view kMetadata = "metadata";
6264
constexpr std::string_view kConfig = "config";
6365
constexpr std::string_view kIdentifiers = "identifiers";
6466

65-
using MapType = std::unordered_map<std::string, std::string>;
66-
67-
/// Helper function to convert TableIdentifier to JSON
68-
nlohmann::json ToJson(const TableIdentifier& identifier) {
69-
nlohmann::json json;
70-
json[kNamespace] = identifier.ns.levels;
71-
json[kName] = identifier.name;
72-
return json;
73-
}
74-
75-
/// Helper function to parse TableIdentifier from JSON
76-
Result<TableIdentifier> TableIdentifierFromJson(const nlohmann::json& json) {
77-
TableIdentifier identifier;
78-
79-
ICEBERG_ASSIGN_OR_RAISE(identifier.ns.levels,
80-
GetJsonValue<std::vector<std::string>>(json, kNamespace));
81-
ICEBERG_ASSIGN_OR_RAISE(identifier.name, GetJsonValue<std::string>(json, kName));
82-
83-
return identifier;
84-
}
85-
8667
} // namespace
8768

8869
// CreateNamespaceRequest
@@ -98,110 +79,90 @@ nlohmann::json ToJson(const CreateNamespaceRequest& request) {
9879
Result<CreateNamespaceRequest> CreateNamespaceRequestFromJson(
9980
const nlohmann::json& json) {
10081
CreateNamespaceRequest request;
101-
102-
ICEBERG_ASSIGN_OR_RAISE(auto levels,
82+
ICEBERG_ASSIGN_OR_RAISE(request.namespace_.levels,
10383
GetJsonValue<std::vector<std::string>>(json, kNamespace));
104-
request.namespace_.levels = std::move(levels);
105-
106-
ICEBERG_ASSIGN_OR_RAISE(request.properties,
107-
GetJsonValueOrDefault<MapType>(json, kProperties));
108-
84+
ICEBERG_ASSIGN_OR_RAISE(
85+
request.properties,
86+
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
87+
kProperties)));
10988
return request;
11089
}
11190

11291
// UpdateNamespacePropertiesRequest
11392
nlohmann::json ToJson(const UpdateNamespacePropertiesRequest& request) {
93+
// Initialize as an empty object so that when all optional fields are absent we return
94+
// {} instead of null
11495
nlohmann::json json = nlohmann::json::object();
115-
11696
if (!request.removals.empty()) {
11797
json[kRemovals] = request.removals;
11898
}
11999
if (!request.updates.empty()) {
120100
json[kUpdates] = request.updates;
121101
}
122-
123102
return json;
124103
}
125104

126105
Result<UpdateNamespacePropertiesRequest> UpdateNamespacePropertiesRequestFromJson(
127106
const nlohmann::json& json) {
128107
UpdateNamespacePropertiesRequest request;
129-
130108
ICEBERG_ASSIGN_OR_RAISE(
131109
request.removals, GetJsonValueOrDefault<std::vector<std::string>>(json, kRemovals));
132-
ICEBERG_ASSIGN_OR_RAISE(request.updates,
133-
GetJsonValueOrDefault<MapType>(json, kUpdates));
134-
110+
ICEBERG_ASSIGN_OR_RAISE(
111+
request.updates,
112+
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
113+
kUpdates)));
135114
return request;
136115
}
137116

138117
// CreateTableRequest
139118
nlohmann::json ToJson(const CreateTableRequest& request) {
140119
nlohmann::json json;
141-
142120
json[kName] = request.name;
143-
144121
if (!request.location.empty()) {
145122
json[kLocation] = request.location;
146123
}
147-
148-
json[kSchema] = iceberg::ToJson(*request.schema);
149-
124+
json[kSchema] = ToJson(*request.schema);
150125
if (request.partition_spec) {
151-
json[kPartitionSpec] = iceberg::ToJson(*request.partition_spec);
126+
json[kPartitionSpec] = ToJson(*request.partition_spec);
152127
}
153-
154128
if (request.write_order) {
155-
json[kWriteOrder] = iceberg::ToJson(*request.write_order);
129+
json[kWriteOrder] = ToJson(*request.write_order);
156130
}
157-
158131
SetOptionalField(json, kStageCreate, request.stage_create);
159-
160132
if (!request.properties.empty()) {
161133
json[kProperties] = request.properties;
162134
}
163-
164135
return json;
165136
}
166137

167138
Result<CreateTableRequest> CreateTableRequestFromJson(const nlohmann::json& json) {
168139
CreateTableRequest request;
169-
170140
ICEBERG_ASSIGN_OR_RAISE(request.name, GetJsonValue<std::string>(json, kName));
171-
172-
ICEBERG_ASSIGN_OR_RAISE(auto location,
173-
GetJsonValueOptional<std::string>(json, kLocation));
174-
request.location = location.value_or("");
175-
141+
ICEBERG_ASSIGN_OR_RAISE(request.location,
142+
GetJsonValueOrDefault<std::string>(json, kLocation, ""));
176143
ICEBERG_ASSIGN_OR_RAISE(auto schema_json, GetJsonValue<nlohmann::json>(json, kSchema));
177-
ICEBERG_ASSIGN_OR_RAISE(auto schema_ptr, iceberg::SchemaFromJson(schema_json));
178-
request.schema = std::move(schema_ptr);
179-
144+
ICEBERG_ASSIGN_OR_RAISE(request.schema, SchemaFromJson(schema_json));
180145
if (json.contains(kPartitionSpec)) {
181146
ICEBERG_ASSIGN_OR_RAISE(auto partition_spec_json,
182147
GetJsonValue<nlohmann::json>(json, kPartitionSpec));
183-
ICEBERG_ASSIGN_OR_RAISE(
184-
request.partition_spec,
185-
iceberg::PartitionSpecFromJson(request.schema, partition_spec_json));
148+
ICEBERG_ASSIGN_OR_RAISE(request.partition_spec,
149+
PartitionSpecFromJson(request.schema, partition_spec_json));
186150
} else {
187151
request.partition_spec = nullptr;
188152
}
189-
190153
if (json.contains(kWriteOrder)) {
191154
ICEBERG_ASSIGN_OR_RAISE(auto write_order_json,
192155
GetJsonValue<nlohmann::json>(json, kWriteOrder));
193-
ICEBERG_ASSIGN_OR_RAISE(request.write_order,
194-
iceberg::SortOrderFromJson(write_order_json));
156+
ICEBERG_ASSIGN_OR_RAISE(request.write_order, SortOrderFromJson(write_order_json));
195157
} else {
196158
request.write_order = nullptr;
197159
}
198-
199160
ICEBERG_ASSIGN_OR_RAISE(request.stage_create,
200161
GetJsonValueOptional<bool>(json, kStageCreate));
201-
202-
ICEBERG_ASSIGN_OR_RAISE(request.properties,
203-
GetJsonValueOrDefault<MapType>(json, kProperties));
204-
162+
ICEBERG_ASSIGN_OR_RAISE(
163+
request.properties,
164+
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
165+
kProperties)));
205166
return request;
206167
}
207168

@@ -218,16 +179,13 @@ nlohmann::json ToJson(const RegisterTableRequest& request) {
218179

219180
Result<RegisterTableRequest> RegisterTableRequestFromJson(const nlohmann::json& json) {
220181
RegisterTableRequest request;
221-
222182
ICEBERG_ASSIGN_OR_RAISE(request.name, GetJsonValue<std::string>(json, kName));
223183
ICEBERG_ASSIGN_OR_RAISE(request.metadata_location,
224184
GetJsonValue<std::string>(json, kMetadataLocation));
225-
226185
// Default to false if not present
227186
ICEBERG_ASSIGN_OR_RAISE(auto overwrite_opt,
228187
GetJsonValueOptional<bool>(json, kOverwrite));
229188
request.overwrite = overwrite_opt.value_or(false);
230-
231189
return request;
232190
}
233191

@@ -241,85 +199,65 @@ nlohmann::json ToJson(const RenameTableRequest& request) {
241199

242200
Result<RenameTableRequest> RenameTableRequestFromJson(const nlohmann::json& json) {
243201
RenameTableRequest request;
244-
245202
ICEBERG_ASSIGN_OR_RAISE(auto source_json, GetJsonValue<nlohmann::json>(json, kSource));
246203
ICEBERG_ASSIGN_OR_RAISE(request.source, TableIdentifierFromJson(source_json));
247-
248204
ICEBERG_ASSIGN_OR_RAISE(auto dest_json,
249205
GetJsonValue<nlohmann::json>(json, kDestination));
250206
ICEBERG_ASSIGN_OR_RAISE(request.destination, TableIdentifierFromJson(dest_json));
251-
252207
return request;
253208
}
254209

255210
// LoadTableResult (used by CreateTableResponse, LoadTableResponse)
256211
nlohmann::json ToJson(const LoadTableResult& result) {
257212
nlohmann::json json;
258-
259-
if (result.metadata_location.has_value() && !result.metadata_location->empty()) {
260-
json[kMetadataLocation] = result.metadata_location.value();
213+
if (!result.metadata_location.empty()) {
214+
json[kMetadataLocation] = result.metadata_location;
261215
}
262-
263-
json[kMetadata] = iceberg::ToJson(*result.metadata);
264-
216+
json[kMetadata] = ToJson(*result.metadata);
265217
if (!result.config.empty()) {
266218
json[kConfig] = result.config;
267219
}
268-
269220
return json;
270221
}
271222

272223
Result<LoadTableResult> LoadTableResultFromJson(const nlohmann::json& json) {
273224
LoadTableResult result;
274-
275-
ICEBERG_ASSIGN_OR_RAISE(auto metadata_location,
276-
GetJsonValueOptional<std::string>(json, kMetadataLocation));
277-
result.metadata_location = metadata_location;
278-
225+
ICEBERG_ASSIGN_OR_RAISE(result.metadata_location, GetJsonValueOrDefault<std::string>(
226+
json, kMetadataLocation, ""));
279227
ICEBERG_ASSIGN_OR_RAISE(auto metadata_json,
280228
GetJsonValue<nlohmann::json>(json, kMetadata));
281-
ICEBERG_ASSIGN_OR_RAISE(auto metadata_ptr,
282-
iceberg::TableMetadataFromJson(metadata_json));
283-
result.metadata = std::move(metadata_ptr);
284-
285-
ICEBERG_ASSIGN_OR_RAISE(result.config, GetJsonValueOrDefault<MapType>(json, kConfig));
286-
229+
ICEBERG_ASSIGN_OR_RAISE(result.metadata, TableMetadataFromJson(metadata_json));
230+
ICEBERG_ASSIGN_OR_RAISE(
231+
result.config, (GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(
232+
json, kConfig)));
287233
return result;
288234
}
289235

290236
// ListNamespacesResponse
291237
nlohmann::json ToJson(const ListNamespacesResponse& response) {
292238
nlohmann::json json;
293-
294239
if (!response.next_page_token.empty()) {
295240
json[kNextPageToken] = response.next_page_token;
296241
}
297-
298242
nlohmann::json namespaces = nlohmann::json::array();
299243
for (const auto& ns : response.namespaces) {
300-
namespaces.push_back(ns.levels);
244+
namespaces.push_back(ToJson(ns));
301245
}
302246
json[kNamespaces] = std::move(namespaces);
303-
304247
return json;
305248
}
306249

307250
Result<ListNamespacesResponse> ListNamespacesResponseFromJson(
308251
const nlohmann::json& json) {
309252
ListNamespacesResponse response;
310-
311-
ICEBERG_ASSIGN_OR_RAISE(auto next_page_token,
312-
GetJsonValueOptional<std::string>(json, kNextPageToken));
313-
response.next_page_token = next_page_token.value_or("");
314-
315-
ICEBERG_ASSIGN_OR_RAISE(
316-
auto namespace_levels,
317-
GetJsonValue<std::vector<std::vector<std::string>>>(json, kNamespaces));
318-
response.namespaces.reserve(namespace_levels.size());
319-
for (auto& levels : namespace_levels) {
320-
response.namespaces.push_back(Namespace{.levels = std::move(levels)});
253+
ICEBERG_ASSIGN_OR_RAISE(response.next_page_token,
254+
GetJsonValueOrDefault<std::string>(json, kNextPageToken, ""));
255+
ICEBERG_ASSIGN_OR_RAISE(auto namespaces_json,
256+
GetJsonValue<nlohmann::json>(json, kNamespaces));
257+
for (const auto& ns_json : namespaces_json) {
258+
ICEBERG_ASSIGN_OR_RAISE(auto ns, NamespaceFromJson(ns_json));
259+
response.namespaces.push_back(std::move(ns));
321260
}
322-
323261
return response;
324262
}
325263

@@ -336,14 +274,12 @@ nlohmann::json ToJson(const CreateNamespaceResponse& response) {
336274
Result<CreateNamespaceResponse> CreateNamespaceResponseFromJson(
337275
const nlohmann::json& json) {
338276
CreateNamespaceResponse response;
339-
340-
ICEBERG_ASSIGN_OR_RAISE(auto levels,
277+
ICEBERG_ASSIGN_OR_RAISE(response.namespace_.levels,
341278
GetJsonValue<std::vector<std::string>>(json, kNamespace));
342-
response.namespace_.levels = std::move(levels);
343-
344-
ICEBERG_ASSIGN_OR_RAISE(response.properties,
345-
GetJsonValueOrDefault<MapType>(json, kProperties));
346-
279+
ICEBERG_ASSIGN_OR_RAISE(
280+
response.properties,
281+
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
282+
kProperties)));
347283
return response;
348284
}
349285

@@ -359,14 +295,12 @@ nlohmann::json ToJson(const GetNamespaceResponse& response) {
359295

360296
Result<GetNamespaceResponse> GetNamespaceResponseFromJson(const nlohmann::json& json) {
361297
GetNamespaceResponse response;
362-
363-
ICEBERG_ASSIGN_OR_RAISE(auto levels,
298+
ICEBERG_ASSIGN_OR_RAISE(response.namespace_.levels,
364299
GetJsonValue<std::vector<std::string>>(json, kNamespace));
365-
response.namespace_.levels = std::move(levels);
366-
367-
ICEBERG_ASSIGN_OR_RAISE(response.properties,
368-
GetJsonValueOrDefault<MapType>(json, kProperties));
369-
300+
ICEBERG_ASSIGN_OR_RAISE(
301+
response.properties,
302+
(GetJsonValueOrDefault<std::unordered_map<std::string, std::string>>(json,
303+
kProperties)));
370304
return response;
371305
}
372306

@@ -384,49 +318,39 @@ nlohmann::json ToJson(const UpdateNamespacePropertiesResponse& response) {
384318
Result<UpdateNamespacePropertiesResponse> UpdateNamespacePropertiesResponseFromJson(
385319
const nlohmann::json& json) {
386320
UpdateNamespacePropertiesResponse response;
387-
388321
ICEBERG_ASSIGN_OR_RAISE(response.updated,
389322
GetJsonValue<std::vector<std::string>>(json, kUpdated));
390323
ICEBERG_ASSIGN_OR_RAISE(response.removed,
391324
GetJsonValue<std::vector<std::string>>(json, kRemoved));
392325
ICEBERG_ASSIGN_OR_RAISE(
393326
response.missing, GetJsonValueOrDefault<std::vector<std::string>>(json, kMissing));
394-
395327
return response;
396328
}
397329

398330
// ListTablesResponse
399331
nlohmann::json ToJson(const ListTablesResponse& response) {
400332
nlohmann::json json;
401-
402333
if (!response.next_page_token.empty()) {
403334
json[kNextPageToken] = response.next_page_token;
404335
}
405-
406336
nlohmann::json identifiers_json = nlohmann::json::array();
407337
for (const auto& identifier : response.identifiers) {
408338
identifiers_json.push_back(ToJson(identifier));
409339
}
410340
json[kIdentifiers] = identifiers_json;
411-
412341
return json;
413342
}
414343

415344
Result<ListTablesResponse> ListTablesResponseFromJson(const nlohmann::json& json) {
416345
ListTablesResponse response;
417-
418-
ICEBERG_ASSIGN_OR_RAISE(auto next_page_token,
419-
GetJsonValueOptional<std::string>(json, kNextPageToken));
420-
response.next_page_token = next_page_token.value_or("");
421-
346+
ICEBERG_ASSIGN_OR_RAISE(response.next_page_token,
347+
GetJsonValueOrDefault<std::string>(json, kNextPageToken, ""));
422348
ICEBERG_ASSIGN_OR_RAISE(auto identifiers_json,
423349
GetJsonValue<nlohmann::json>(json, kIdentifiers));
424-
425350
for (const auto& id_json : identifiers_json) {
426351
ICEBERG_ASSIGN_OR_RAISE(auto identifier, TableIdentifierFromJson(id_json));
427352
response.identifiers.push_back(std::move(identifier));
428353
}
429-
430354
return response;
431355
}
432356

0 commit comments

Comments
 (0)