From b630b6e420f413bcec83de47b73749e8552052ec Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Wed, 9 Sep 2026 09:24:00 +0530 Subject: [PATCH] Reduce Boilerplate using helpers --- cpp/src/arrow/extension/opaque.cc | 74 ++------------------------ cpp/src/arrow/extension/opaque_test.cc | 35 +++++------- 2 files changed, 17 insertions(+), 92 deletions(-) diff --git a/cpp/src/arrow/extension/opaque.cc b/cpp/src/arrow/extension/opaque.cc index 2dae9ef56788..57eb05b9e5c4 100644 --- a/cpp/src/arrow/extension/opaque.cc +++ b/cpp/src/arrow/extension/opaque.cc @@ -63,77 +63,11 @@ std::string OpaqueType::Serialize() const { Result> OpaqueType::Deserialize( std::shared_ptr storage_type, const std::string& serialized_data) const { - simdjson::padded_string padded_json(serialized_data); - simdjson::ondemand::parser parser; - - ARROW_ASSIGN_OR_RAISE(auto document, - internal::ResolveSimdjsonResult(parser.iterate(padded_json), - "Failed to parse JSON")); - - ARROW_ASSIGN_OR_RAISE(auto object, - internal::ResolveSimdjsonResult(document.get_object(), - "Failed to get JSON object")); - - std::string type_name; - std::string vendor_name; - bool has_type_name = false; - bool has_vendor_name = false; - - for (auto field_result : object) { - ARROW_ASSIGN_OR_RAISE(auto field, internal::ResolveSimdjsonResult( - field_result, "Failed to iterate JSON object")); - - ARROW_ASSIGN_OR_RAISE( - auto key, internal::ResolveSimdjsonResult(field.unescaped_key(), - "Failed to get JSON object key")); - - auto value = field.value(); - - if (key == "type_name") { - has_type_name = true; - - ARROW_ASSIGN_OR_RAISE(auto type, - internal::ResolveSimdjsonResult( - value.type(), "Failed to determine type_name JSON type")); - - if (type != simdjson::ondemand::json_type::string) { - return Status::Invalid( - "Invalid serialized JSON data for OpaqueType: type_name is not a string"); - } - - ARROW_ASSIGN_OR_RAISE( - auto name, - internal::ResolveSimdjsonResult(value.get_string(), "Failed to get type_name")); - type_name = std::string(name); - - } else if (key == "vendor_name") { - has_vendor_name = true; - - ARROW_ASSIGN_OR_RAISE( - auto type, internal::ResolveSimdjsonResult( - value.type(), "Failed to determine vendor_name JSON type")); - - if (type != simdjson::ondemand::json_type::string) { - return Status::Invalid( - "Invalid serialized JSON data for OpaqueType: vendor_name is not a string"); - } - - ARROW_ASSIGN_OR_RAISE(auto name, - internal::ResolveSimdjsonResult(value.get_string(), - "Failed to get vendor_name")); - vendor_name = std::string(name); - } - } - - if (!has_type_name) { - return Status::Invalid( - "Invalid serialized JSON data for OpaqueType: missing type_name"); - } + internal::JsonObjectParser parser; + RETURN_NOT_OK(parser.Parse(serialized_data)); - if (!has_vendor_name) { - return Status::Invalid( - "Invalid serialized JSON data for OpaqueType: missing vendor_name"); - } + ARROW_ASSIGN_OR_RAISE(auto type_name, parser.GetString("type_name")); + ARROW_ASSIGN_OR_RAISE(auto vendor_name, parser.GetString("vendor_name")); return opaque(std::move(storage_type), std::move(type_name), std::move(vendor_name)); } diff --git a/cpp/src/arrow/extension/opaque_test.cc b/cpp/src/arrow/extension/opaque_test.cc index ac093ddfe07b..7c7c2bfa4b0e 100644 --- a/cpp/src/arrow/extension/opaque_test.cc +++ b/cpp/src/arrow/extension/opaque_test.cc @@ -127,28 +127,19 @@ TEST(OpaqueType, Deserialize) { auto type = internal::checked_pointer_cast( extension::opaque(null(), "type", "vendor")); - EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("Failed to parse JSON"), - type->Deserialize(null(), R"()")); - EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, - testing::HasSubstr("Failed to get JSON object"), - type->Deserialize(null(), R"({)")); - EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, - testing::HasSubstr("Failed to get JSON object"), - type->Deserialize(null(), R"([])")); - EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, testing::HasSubstr("missing type_name"), - type->Deserialize(null(), R"({})")); - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, testing::HasSubstr("type_name is not a string"), - type->Deserialize(null(), R"({"type_name": 2, "vendor_name": ""})")); - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, testing::HasSubstr("type_name is not a string"), - type->Deserialize(null(), R"({"type_name": null, "vendor_name": ""})")); - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, testing::HasSubstr("vendor_name is not a string"), - type->Deserialize(null(), R"({"vendor_name": 2, "type_name": ""})")); - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, testing::HasSubstr("vendor_name is not a string"), - type->Deserialize(null(), R"({"vendor_name": null, "type_name": ""})")); + + ASSERT_RAISES(Invalid, type->Deserialize(null(), R"()")); + ASSERT_RAISES(Invalid, type->Deserialize(null(), R"({)")); + ASSERT_RAISES(TypeError, type->Deserialize(null(), R"([])")); + ASSERT_RAISES(KeyError, type->Deserialize(null(), R"({})")); + ASSERT_RAISES(TypeError, + type->Deserialize(null(), R"({"type_name": 2, "vendor_name": ""})")); + ASSERT_RAISES(TypeError, + type->Deserialize(null(), R"({"type_name": null, "vendor_name": ""})")); + ASSERT_RAISES(TypeError, + type->Deserialize(null(), R"({"vendor_name": 2, "type_name": ""})")); + ASSERT_RAISES(TypeError, + type->Deserialize(null(), R"({"vendor_name": null, "type_name": ""})")); } TEST(OpaqueType, MetadataRoundTrip) {