refactor: make SortOrder constructor private

src/iceberg/json_internal.cc

@@ -206,6 +206,19 @@ Result<std::unique_ptr<SortField>> SortFieldFromJson(const nlohmann::json& json)
                                      null_order);
 }
 
+Result<std::unique_ptr<SortOrder>> SortOrderFromJson(
+    const nlohmann::json& json, const std::shared_ptr<Schema>& current_schema) {
+  ICEBERG_ASSIGN_OR_RAISE(auto order_id, GetJsonValue<int32_t>(json, kOrderId));
+  ICEBERG_ASSIGN_OR_RAISE(auto fields, GetJsonValue<nlohmann::json>(json, kFields));
+
+  std::vector<SortField> sort_fields;
+  for (const auto& field_json : fields) {
+    ICEBERG_ASSIGN_OR_RAISE(auto sort_field, SortFieldFromJson(field_json));
+    sort_fields.push_back(std::move(*sort_field));
+  }
+  return SortOrder::Make(*current_schema, order_id, std::move(sort_fields));
+}
+
 Result<std::unique_ptr<SortOrder>> SortOrderFromJson(const nlohmann::json& json) {
   ICEBERG_ASSIGN_OR_RAISE(auto order_id, GetJsonValue<int32_t>(json, kOrderId));
   ICEBERG_ASSIGN_OR_RAISE(auto fields, GetJsonValue<nlohmann::json>(json, kFields));
@@ -215,7 +228,7 @@ Result<std::unique_ptr<SortOrder>> SortOrderFromJson(const nlohmann::json& json)
     ICEBERG_ASSIGN_OR_RAISE(auto sort_field, SortFieldFromJson(field_json));
     sort_fields.push_back(std::move(*sort_field));
   }
-  return std::make_unique<SortOrder>(order_id, std::move(sort_fields));
+  return SortOrder::Make(order_id, std::move(sort_fields));
 }
 
 nlohmann::json ToJson(const SchemaField& field) {
@@ -919,9 +932,11 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version,
 ///
 /// \param[in] json The JSON object to parse.
 /// \param[in] format_version The format version of the table.
+/// \param[in] current_schema The current schema.
 /// \param[out] default_sort_order_id The default sort order ID.
 /// \param[out] sort_orders The list of sort orders.
 Status ParseSortOrders(const nlohmann::json& json, int8_t format_version,
+                       const std::shared_ptr<Schema>& current_schema,
                        int32_t& default_sort_order_id,
                        std::vector<std::shared_ptr<SortOrder>>& sort_orders) {
   if (json.contains(kSortOrders)) {
@@ -930,7 +945,8 @@ Status ParseSortOrders(const nlohmann::json& json, int8_t format_version,
     ICEBERG_ASSIGN_OR_RAISE(auto sort_order_array,
                             GetJsonValue<nlohmann::json>(json, kSortOrders));
     for (const auto& sort_order_json : sort_order_array) {
-      ICEBERG_ASSIGN_OR_RAISE(auto sort_order, SortOrderFromJson(sort_order_json));
+      ICEBERG_ASSIGN_OR_RAISE(auto sort_order,
+                              SortOrderFromJson(sort_order_json, current_schema));
       sort_orders.push_back(std::move(sort_order));
     }
   } else {
@@ -1005,9 +1021,9 @@ Result<std::unique_ptr<TableMetadata>> TableMetadataFromJson(const nlohmann::jso
     }
   }
 
-  ICEBERG_RETURN_UNEXPECTED(ParseSortOrders(json, table_metadata->format_version,
-                                            table_metadata->default_sort_order_id,
-                                            table_metadata->sort_orders));
+  ICEBERG_RETURN_UNEXPECTED(ParseSortOrders(
+      json, table_metadata->format_version, current_schema,
+      table_metadata->default_sort_order_id, table_metadata->sort_orders));
 
   if (json.contains(kProperties)) {
     ICEBERG_ASSIGN_OR_RAISE(table_metadata->properties, FromJsonMap(json, kProperties));

src/iceberg/json_internal.h

@@ -69,10 +69,11 @@ ICEBERG_EXPORT nlohmann::json ToJson(const SortOrder& sort_order);
 /// Each `SortField` will be parsed using the `SortFieldFromJson` function.
 ///
 /// \param json The JSON object representing a `SortOrder`.
+/// \param current_schema The current schema associated with the sort order.
 /// \return An `expected` value containing either a `SortOrder` object or an error. If the
 /// JSON is malformed or missing expected fields, an error will be returned.
 ICEBERG_EXPORT Result<std::unique_ptr<SortOrder>> SortOrderFromJson(
-    const nlohmann::json& json);
+    const nlohmann::json& json, const std::shared_ptr<Schema>& current_schema);
 
 /// \brief Convert an Iceberg Schema to JSON.
 ///

src/iceberg/sort_order.cc

@@ -20,6 +20,7 @@
 #include "iceberg/sort_order.h"
 
 #include <format>
+#include <memory>
 #include <optional>
 #include <ranges>
 
@@ -36,8 +37,8 @@ SortOrder::SortOrder(int32_t order_id, std::vector<SortField> fields)
     : order_id_(order_id), fields_(std::move(fields)) {}
 
 const std::shared_ptr<SortOrder>& SortOrder::Unsorted() {
-  static const std::shared_ptr<SortOrder> unsorted =
-      std::make_shared<SortOrder>(kUnsortedOrderId, std::vector<SortField>{});
+  static const std::shared_ptr<SortOrder> unsorted = std::shared_ptr<SortOrder>(
+      new SortOrder(kUnsortedOrderId, std::vector<SortField>{}));
   return unsorted;
 }
 
@@ -113,7 +114,7 @@ Result<std::unique_ptr<SortOrder>> SortOrder::Make(const Schema& schema, int32_t
     return InvalidArgument("Sort order must have at least one sort field");
   }
 
-  auto sort_order = std::make_unique<SortOrder>(sort_id, std::move(fields));
+  auto sort_order = std::unique_ptr<SortOrder>(new SortOrder(sort_id, std::move(fields)));
   ICEBERG_RETURN_UNEXPECTED(sort_order->Validate(schema));
   return sort_order;
 }
@@ -128,7 +129,7 @@ Result<std::unique_ptr<SortOrder>> SortOrder::Make(int32_t sort_id,
     return InvalidArgument("Sort order must have at least one sort field");
   }
 
-  return std::make_unique<SortOrder>(sort_id, std::move(fields));
+  return std::unique_ptr<SortOrder>(new SortOrder(sort_id, std::move(fields)));
 }
 
 }  // namespace iceberg

src/iceberg/sort_order.h

@@ -41,8 +41,6 @@ class ICEBERG_EXPORT SortOrder : public util::Formattable {
   static constexpr int32_t kUnsortedOrderId = 0;
   static constexpr int32_t kInitialSortOrderId = 1;
 
-  SortOrder(int32_t order_id, std::vector<SortField> fields);
-
   /// \brief Get an unsorted sort order singleton.
   static const std::shared_ptr<SortOrder>& Unsorted();
 
@@ -95,6 +93,12 @@ class ICEBERG_EXPORT SortOrder : public util::Formattable {
                                                  std::vector<SortField> fields);
 
  private:
+  /// \brief Constructs a SortOrder instance.
+  /// \param order_id The sort order id.
+  /// \param fields The sort fields.
+  /// \note Use the static Make methods to create SortOrder instances.
+  SortOrder(int32_t order_id, std::vector<SortField> fields);
+
   /// \brief Compare two sort orders for equality.
   bool Equals(const SortOrder& other) const;
 

src/iceberg/test/json_internal_test.cc

@@ -49,11 +49,6 @@ Result<std::unique_ptr<SortField>> FromJsonHelper(const nlohmann::json& json) {
   return SortFieldFromJson(json);
 }
 
-template <>
-Result<std::unique_ptr<SortOrder>> FromJsonHelper(const nlohmann::json& json) {
-  return SortOrderFromJson(json);
-}
-
 template <>
 Result<std::unique_ptr<PartitionField>> FromJsonHelper(const nlohmann::json& json) {
   return PartitionFieldFromJson(json);
@@ -107,17 +102,26 @@ TEST(JsonInternalTest, SortField) {
 }
 
 TEST(JsonInternalTest, SortOrder) {
+  auto schema = std::make_shared<Schema>(
+      std::vector<SchemaField>{SchemaField(5, "region", iceberg::string(), false),
+                               SchemaField(7, "ts", iceberg::int64(), false)},
+      /*schema_id=*/100);
   auto identity_transform = Transform::Identity();
   SortField st_ts(5, identity_transform, SortDirection::kAscending, NullOrder::kFirst);
   SortField st_bar(7, identity_transform, SortDirection::kDescending, NullOrder::kLast);
-  SortOrder sort_order(100, {st_ts, st_bar});
-
+  ICEBERG_UNWRAP_OR_FAIL(auto sort_order, SortOrder::Make(100, {st_ts, st_bar}));
+  EXPECT_TRUE(sort_order->Validate(*schema));
   nlohmann::json expected_sort_order =
       R"({"order-id":100,"fields":[
           {"transform":"identity","source-id":5,"direction":"asc","null-order":"nulls-first"},
           {"transform":"identity","source-id":7,"direction":"desc","null-order":"nulls-last"}]})"_json;
 
-  TestJsonConversion(sort_order, expected_sort_order);
+  auto json = ToJson(*sort_order);
+  EXPECT_EQ(expected_sort_order, json) << "JSON conversion mismatch.";
+
+  // Specialize FromJson based on type (T)
+  ICEBERG_UNWRAP_OR_FAIL(auto obj_ex, SortOrderFromJson(expected_sort_order, schema));
+  EXPECT_EQ(*sort_order, *obj_ex) << "Deserialized object mismatch.";
 }
 
 TEST(JsonInternalTest, PartitionField) {

src/iceberg/test/metadata_serde_test.cc

@@ -148,12 +148,17 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) {
       std::vector<PartitionField>{PartitionField(/*source_id=*/1, /*field_id=*/1000, "x",
                                                  Transform::Identity())});
 
-  auto expected_sort_order = std::make_shared<SortOrder>(
-      /*order_id=*/3,
-      std::vector<SortField>{SortField(/*source_id=*/2, Transform::Identity(),
-                                       SortDirection::kAscending, NullOrder::kFirst),
-                             SortField(/*source_id=*/3, Transform::Bucket(4),
-                                       SortDirection::kDescending, NullOrder::kLast)});
+  ICEBERG_UNWRAP_OR_FAIL(
+      auto sort_order,
+      SortOrder::Make(*expected_schema_2,
+                      /*order_id=*/3,
+                      std::vector<SortField>{
+                          SortField(/*source_id=*/2, Transform::Identity(),
+                                    SortDirection::kAscending, NullOrder::kFirst),
+                          SortField(/*source_id=*/3, Transform::Bucket(4),
+                                    SortDirection::kDescending, NullOrder::kLast)}));
+
+  auto expected_sort_order = std::shared_ptr<SortOrder>(std::move(sort_order));
 
   auto expected_snapshot_1 = std::make_shared<Snapshot>(Snapshot{
       .snapshot_id = 3051729675574597004,
@@ -228,13 +233,18 @@ TEST(MetadataSerdeTest, DeserializeV2ValidMinimal) {
       std::vector<PartitionField>{PartitionField(/*source_id=*/1, /*field_id=*/1000, "x",
                                                  Transform::Identity())});
 
-  auto expected_sort_order = std::make_shared<SortOrder>(
-      /*order_id=*/3, std::vector<SortField>{
+  ICEBERG_UNWRAP_OR_FAIL(
+      auto sort_order,
+      SortOrder::Make(*expected_schema,
+                      /*order_id=*/3,
+                      std::vector<SortField>{
                           SortField(/*source_id=*/2, Transform::Identity(),
                                     SortDirection::kAscending, NullOrder::kFirst),
                           SortField(/*source_id=*/3, Transform::Bucket(4),
                                     SortDirection::kDescending, NullOrder::kLast),
-                      });
+                      }));
+
+  auto expected_sort_order = std::shared_ptr<SortOrder>(std::move(sort_order));
 
   TableMetadata expected{
       .format_version = 2,