use ToJson for Snapshot serialization

Author: zhjwpku

src/iceberg/json_internal.cc

@@ -304,7 +304,7 @@ nlohmann::json SchemaToJson(const Schema& schema) {
   return json;
 }
 
-nlohmann::json SnapshotRefToJson(const SnapshotRef& ref) {
+nlohmann::json ToJson(const SnapshotRef& ref) {
   nlohmann::json json;
   json[kSnapshotId] = ref.snapshot_id;
   json[kType] = SnapshotRefTypeToString(ref.type());
@@ -320,14 +320,17 @@ nlohmann::json SnapshotRefToJson(const SnapshotRef& ref) {
   return json;
 }
 
-nlohmann::json SnapshotToJson(const Snapshot& snapshot) {
+nlohmann::json ToJson(const Snapshot& snapshot) {
   nlohmann::json json;
   json[kSnapshotId] = snapshot.snapshot_id;
   SetOptionalField(json, kParentSnapshotId, snapshot.parent_snapshot_id);
   json[kSequenceNumber] = snapshot.sequence_number;
   json[kTimestampMs] = snapshot.timestamp_ms;
   json[kManifestList] = snapshot.manifest_list;
-  json[kSummary] = snapshot.summary;
+  // If there is an operation, write the summary map
+  if (snapshot.operation().has_value()) {
+    json[kSummary] = snapshot.summary;
+  }
   SetOptionalField(json, kSchemaId, snapshot.schema_id);
   return json;
 }

src/iceberg/json_internal.h

@@ -88,17 +88,17 @@ nlohmann::json TypeToJson(const Type& type);
 /// \return The JSON representation of the field.
 nlohmann::json FieldToJson(const SchemaField& field);
 
-/// \brief Convert an Iceberg SnapshotRef to JSON.
+/// \brief Serializes a `SnapshotRef` object to JSON.
 ///
-/// \param[in] snapshot_ref The Iceberg snapshot reference to convert.
-/// \return The JSON representation of the snapshot reference.
-nlohmann::json SnapshotRefToJson(const SnapshotRef& snapshot_ref);
+/// \param[in] snapshot_ref The `SnapshotRef` object to be serialized.
+/// \return A JSON object representing the `SnapshotRef`.
+nlohmann::json ToJson(const SnapshotRef& snapshot_ref);
 
-/// \brief Convert an Iceberg Snapshot to JSON.
+/// \brief Serializes a `Snapshot` object to JSON.
 ///
-/// \param[in] snapshot The Iceberg snapshot to convert.
-/// \return The JSON representation of the snapshot.
-nlohmann::json SnapshotToJson(const Snapshot& snapshot);
+/// \param[in] snapshot The `Snapshot` object to be serialized.
+/// \return A JSON object representing the `snapshot`.
+nlohmann::json ToJson(const Snapshot& snapshot);
 
 /// \brief Convert JSON to an Iceberg Schema.
 ///
@@ -166,16 +166,16 @@ Result<std::unique_ptr<PartitionField>> PartitionFieldFromJson(
 Result<std::unique_ptr<PartitionSpec>> PartitionSpecFromJson(
     const std::shared_ptr<Schema>& schema, const nlohmann::json& json);
 
-/// \brief Convert JSON to an Iceberg SnapshotRef.
+/// \brief Deserializes a JSON object into a `SnapshotRef` object.
 ///
-/// \param[in] json The JSON representation of the snapshot reference.
-/// \return The Iceberg snapshot reference or an error if the conversion fails.
+/// \param[in] json The JSON object representing a `SnapshotRef`.
+/// \return A `SnapshotRef` object or an error if the conversion fails.
 Result<std::unique_ptr<SnapshotRef>> SnapshotRefFromJson(const nlohmann::json& json);
 
-/// \brief Convert JSON to an Iceberg Snapshot.
+/// \brief Deserializes a JSON object into a `Snapshot` object.
 ///
 /// \param[in] json The JSON representation of the snapshot.
-/// \return The Iceberg snapshot or an error if the conversion fails.
+/// \return A `Snapshot` object or an error if the conversion fails.
 Result<std::unique_ptr<Snapshot>> SnapshotFromJson(const nlohmann::json& json);
 
 }  // namespace iceberg

test/json_internal_test.cc

@@ -25,6 +25,7 @@
 #include <iceberg/result.h>
 #include <nlohmann/json.hpp>
 
+#include "gmock/gmock.h"
 #include "iceberg/partition_spec.h"
 #include "iceberg/schema.h"
 #include "iceberg/snapshot.h"
@@ -56,6 +57,16 @@ Result<std::unique_ptr<PartitionField>> FromJsonHelper(const nlohmann::json& jso
   return PartitionFieldFromJson(json);
 }
 
+template <>
+Result<std::unique_ptr<SnapshotRef>> FromJsonHelper(const nlohmann::json& json) {
+  return SnapshotRefFromJson(json);
+}
+
+template <>
+Result<std::unique_ptr<Snapshot>> FromJsonHelper(const nlohmann::json& json) {
+  return SnapshotFromJson(json);
+}
+
 // Helper function to reduce duplication in testing
 template <typename T>
 void TestJsonConversion(const T& obj, const nlohmann::json& expected_json) {
@@ -117,7 +128,8 @@ TEST(JsonPartitionTest, PartitionFieldFromJsonMissingField) {
 
   auto result = PartitionFieldFromJson(invalid_json);
   EXPECT_FALSE(result.has_value());
-  EXPECT_EQ(result.error().kind, ErrorKind::kJsonParseError);
+  EXPECT_THAT(result, IsError(ErrorKind::kJsonParseError));
+  EXPECT_THAT(result, HasErrorMessage("Missing 'source-id'"));
 }
 
 TEST(JsonPartitionTest, PartitionSpec) {
@@ -162,12 +174,7 @@ TEST(JsonInternalTest, SnapshotRefBranch) {
           "max-snapshot-age-ms":123456789,
           "max-ref-age-ms":987654321})"_json;
 
-  auto json = SnapshotRefToJson(ref);
-  EXPECT_EQ(expected_json, json) << "JSON conversion mismatch.";
-
-  auto obj_ex = SnapshotRefFromJson(expected_json);
-  EXPECT_TRUE(obj_ex.has_value()) << "Failed to deserialize JSON.";
-  EXPECT_EQ(ref, *obj_ex.value()) << "Deserialized object mismatch.";
+  TestJsonConversion(ref, expected_json);
 }
 
 TEST(JsonInternalTest, SnapshotRefTag) {
@@ -179,12 +186,7 @@ TEST(JsonInternalTest, SnapshotRefTag) {
           "type":"tag",
           "max-ref-age-ms":54321})"_json;
 
-  auto json = SnapshotRefToJson(ref);
-  EXPECT_EQ(expected_json, json) << "JSON conversion mismatch.";
-
-  auto obj_ex = SnapshotRefFromJson(expected_json);
-  EXPECT_TRUE(obj_ex.has_value()) << "Failed to deserialize JSON.";
-  EXPECT_EQ(ref, *obj_ex.value()) << "Deserialized object mismatch.";
+  TestJsonConversion(ref, expected_json);
 }
 
 TEST(JsonInternalTest, Snapshot) {
@@ -213,12 +215,7 @@ TEST(JsonInternalTest, Snapshot) {
           },
           "schema-id":42})"_json;
 
-  auto json = SnapshotToJson(snapshot);
-  EXPECT_EQ(expected_json, json) << "JSON conversion mismatch.";
-
-  auto obj_ex = SnapshotFromJson(expected_json);
-  EXPECT_TRUE(obj_ex.has_value()) << "Failed to deserialize JSON.";
-  EXPECT_EQ(snapshot, *obj_ex.value()) << "Deserialized object mismatch.";
+  TestJsonConversion(snapshot, expected_json);
 }
 
 TEST(JsonInternalTest, SnapshotFromJsonWithInvalidSummary) {