From 414a2617498b0916bce1e23d747917728a520c4c Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 26 Nov 2024 13:45:21 +0200 Subject: [PATCH] Stop using null terminators on JSON (de)serialization. (#5353) [SC-58051](https://app.shortcut.com/tiledb-inc/story/58051/stop-using-null-terminators-on-json-de-serialization) This PR updates the JSON deserialization code to explicitly pass the message's length to capnproto, instead of appending a null terminator and relying on it to get the length. Correspondingly, it removes the null terminator in serialized JSON messages, which is technically a behavior breaking change but the null terminator gets already removed by both [the Go API](https://github.com/TileDB-Inc/TileDB-Go/blob/9ec3a9462135bf6005dcc06a1f44a6dc24afecba/buffer.go#L105-L107) and the REST server (see story), and the serialization APIs are documented as being for internal use. --- TYPE: NO_HISTORY --- test/regression/targets/sc-18250.cc | 9 ++--- test/src/unit-cppapi-array.cc | 38 ++++++++++++++++++- tiledb/sm/buffer/buffer.h | 16 -------- .../buffer/test/unit_serialization_buffer.cc | 14 ------- tiledb/sm/rest/rest_client_remote.cc | 35 ----------------- tiledb/sm/rest/rest_client_remote.h | 9 ----- tiledb/sm/serialization/array.cc | 20 +++------- tiledb/sm/serialization/array_schema.cc | 38 ++++++------------- .../serialization/array_schema_evolution.cc | 8 ++-- tiledb/sm/serialization/capnp_utils.h | 21 ++++++++++ tiledb/sm/serialization/config.cc | 7 +--- tiledb/sm/serialization/consolidation.cc | 18 ++++----- tiledb/sm/serialization/enumeration.cc | 10 ++--- tiledb/sm/serialization/fragment_info.cc | 14 +++---- tiledb/sm/serialization/fragments.cc | 14 ++----- tiledb/sm/serialization/group.cc | 24 +++++------- tiledb/sm/serialization/query.cc | 18 +++------ tiledb/sm/serialization/query_plan.cc | 9 ++--- tiledb/sm/serialization/vacuum.cc | 8 ++-- 19 files changed, 127 insertions(+), 203 deletions(-) diff --git a/test/regression/targets/sc-18250.cc b/test/regression/targets/sc-18250.cc index 7cccdf09cb2..0b3f69de952 100644 --- a/test/regression/targets/sc-18250.cc +++ b/test/regression/targets/sc-18250.cc @@ -5,7 +5,7 @@ #include -static const char* schema_str = R"rstr( +static constexpr std::string_view schema_str = R"rstr( {"arrayType":"dense","attributes":[{ "cellValNum":1,"compressor":"NO_COMPRESSION", "compressorLevel":-1,"name":"a1","type":"INT32"}], @@ -33,11 +33,10 @@ TEST_CASE( status = tiledb_buffer_alloc(ctx, &buf); REQUIRE(status == TILEDB_OK); - status = - tiledb_buffer_set_data(ctx, buf, (void*)schema_str, sizeof(schema_str)); + status = tiledb_buffer_set_data( + ctx, buf, (void*)schema_str.data(), schema_str.size()); REQUIRE(status == TILEDB_OK); - status = tiledb_deserialize_array_schema( - ctx, buf, tiledb_serialization_type_t(0), 0, &schema); + status = tiledb_deserialize_array_schema(ctx, buf, TILEDB_JSON, 0, &schema); REQUIRE(status == TILEDB_OK); } diff --git a/test/src/unit-cppapi-array.cc b/test/src/unit-cppapi-array.cc index 88c8ec291e0..f27ef994ba9 100644 --- a/test/src/unit-cppapi-array.cc +++ b/test/src/unit-cppapi-array.cc @@ -51,8 +51,9 @@ struct CPPArrayFx { static const unsigned d1_tile = 10; static const unsigned d2_tile = 5; - CPPArrayFx() - : ctx(vfs_test_setup_.ctx()) + CPPArrayFx(shared_ptr config = nullptr) + : vfs_test_setup_(config.get()) + , ctx(vfs_test_setup_.ctx()) , array_uri_{vfs_test_setup_.array_uri("cpp_unit_array")} { Domain domain(ctx); auto d1 = Dimension::create(ctx, "d1", {{-100, 100}}, d1_tile); @@ -83,6 +84,18 @@ struct CPPArrayFx { std::string array_uri_; }; +struct CPPArrayFxJsonSerialization : public CPPArrayFx { + static Config create_config() { + Config result; + result.set("rest.server_serialization_format", "JSON"); + return result; + } + + CPPArrayFxJsonSerialization() + : CPPArrayFx(create_config().ptr()) { + } +}; + TEST_CASE("Config", "[cppapi][config][non-rest]") { // Primarily to instansiate operator[]/= template tiledb::Config cfg; @@ -92,6 +105,27 @@ TEST_CASE("Config", "[cppapi][config][non-rest]") { CHECK((std::string)cfg["vfs.s3.use_virtual_addressing"] == "true"); } +TEST_CASE_METHOD( + CPPArrayFxJsonSerialization, + "C++ API: Arrays, REST JSON serialization", + "[cppapi][basic][rest][json]") { + SECTION("Dimensions") { + ArraySchema schema(ctx, array_uri_); + CHECK(schema.domain().ndim() == 2); + auto a = schema.domain().dimensions()[0].domain(); + auto b = schema.domain().dimensions()[1].domain(); + CHECK_THROWS(schema.domain().dimensions()[0].domain()); + CHECK(a.first == -100); + CHECK(a.second == 100); + CHECK(b.first == 0); + CHECK(b.second == 100); + CHECK_THROWS(schema.domain().dimensions()[0].tile_extent() == 10); + CHECK(schema.domain().dimensions()[0].tile_extent() == 10); + CHECK(schema.domain().dimensions()[1].tile_extent() == 5); + CHECK(schema.domain().cell_num() == 20301); + } +} + TEST_CASE_METHOD(CPPArrayFx, "C++ API: Arrays", "[cppapi][basic][rest]") { SECTION("Dimensions") { ArraySchema schema(ctx, array_uri_); diff --git a/tiledb/sm/buffer/buffer.h b/tiledb/sm/buffer/buffer.h index 0e3fd53f642..27048094b2d 100644 --- a/tiledb/sm/buffer/buffer.h +++ b/tiledb/sm/buffer/buffer.h @@ -556,22 +556,6 @@ class SerializationBuffer { buffer_ = iter; } - /** - * Assigns a new owned buffer to the object, and appends a null terminator. - */ - template - inline void assign_null_terminated(const Iter& iter) { - // Clear vector and deallocate its buffer. - buffer_owner_.clear(); - buffer_owner_.shrink_to_fit(); - // Reserve enough space for the data and the null terminator. - buffer_owner_.reserve( - std::distance(std::cbegin(iter), std::cend(iter)) + 1); - buffer_owner_.assign(std::cbegin(iter), std::cend(iter)); - buffer_owner_.push_back('\0'); - buffer_ = buffer_owner_; - } - /** * Gets the number of bytes in the buffer. */ diff --git a/tiledb/sm/buffer/test/unit_serialization_buffer.cc b/tiledb/sm/buffer/test/unit_serialization_buffer.cc index d3ab52b0b79..94c85389744 100644 --- a/tiledb/sm/buffer/test/unit_serialization_buffer.cc +++ b/tiledb/sm/buffer/test/unit_serialization_buffer.cc @@ -84,20 +84,6 @@ TEST_CASE( CHECK_THROWS(buff.owned_mutable_span()); } -TEST_CASE( - "SerializationBuffer: Test owned null-terminated buffer", - "[serialization_buffer][owned][null_terminated]") { - SerializationBuffer buff{ - tiledb::test::get_test_memory_tracker()->get_resource( - MemoryType::SERIALIZATION_BUFFER)}; - std::string_view data = "abcd"; - buff.assign_null_terminated(data); - span buff_span = buff; - CHECK(buff.size() == data.size() + 1); - CHECK(memcmp(buff_span.data(), data.data(), data.size()) == 0); - CHECK(static_cast>(buff)[buff.size() - 1] == '\0'); -} - TEST_CASE( "SerializationBuffer: Test memory tracking", "[serialization_buffer][memory_tracking]") { diff --git a/tiledb/sm/rest/rest_client_remote.cc b/tiledb/sm/rest/rest_client_remote.cc index 463b1cc59ca..28087b8ffd0 100644 --- a/tiledb/sm/rest/rest_client_remote.cc +++ b/tiledb/sm/rest/rest_client_remote.cc @@ -217,10 +217,6 @@ RestClientRemote::get_array_schema_from_rest(const URI& uri) { "Error getting array schema from REST; server returned no data.")), nullopt}; - // Ensure data has a null delimiter for cap'n proto if using JSON - RETURN_NOT_OK_TUPLE( - ensure_json_null_delimited_string(&returned_data), nullopt); - auto array_schema = serialization::array_schema_deserialize( serialization_type_, returned_data, memory_tracker_); @@ -271,8 +267,6 @@ RestClientRemote::post_array_schema_from_rest( "Error getting array schema from REST; server returned no data."); } - // Ensure data has a null delimiter for cap'n proto if using JSON - throw_if_not_ok(ensure_json_null_delimited_string(&returned_data)); return serialization::deserialize_load_array_schema_response( uri, config, serialization_type_, returned_data, memory_tracker_); } @@ -355,8 +349,6 @@ void RestClientRemote::post_array_from_rest( "Error getting array from REST; server returned no data."); } - // Ensure data has a null delimiter for cap'n proto if using JSON - throw_if_not_ok(ensure_json_null_delimited_string(&returned_data)); serialization::array_deserialize( array, serialization_type_, returned_data, resources, memory_tracker_); } @@ -489,9 +481,6 @@ Status RestClientRemote::get_array_non_empty_domain( Status_RestError("Error getting array non-empty domain " "from REST; server returned no data.")); - // Ensure data has a null delimiter for cap'n proto if using JSON - RETURN_NOT_OK(ensure_json_null_delimited_string(&returned_data)); - // Deserialize data returned return serialization::nonempty_domain_deserialize( array, returned_data, serialization_type_); @@ -527,8 +516,6 @@ Status RestClientRemote::get_array_metadata_from_rest( return LOG_STATUS(Status_RestError( "Error getting array metadata from REST; server returned no data.")); - // Ensure data has a null delimiter for cap'n proto if using JSON - RETURN_NOT_OK(ensure_json_null_delimited_string(&returned_data)); return serialization::metadata_deserialize( array->unsafe_metadata(), array->config(), @@ -613,8 +600,6 @@ RestClientRemote::post_enumerations_from_rest( "Error getting enumerations from REST; server returned no data."); } - // Ensure data has a null delimiter for cap'n proto if using JSON - throw_if_not_ok(ensure_json_null_delimited_string(&returned_data)); return serialization::deserialize_load_enumerations_response( array_schema, config, serialization_type_, returned_data, memory_tracker); } @@ -669,8 +654,6 @@ void RestClientRemote::post_query_plan_from_rest( "Error getting query plan from REST; server returned no data."); } - // Ensure data has a null delimiter for cap'n proto if using JSON - throw_if_not_ok(ensure_json_null_delimited_string(&returned_data)); query_plan = serialization::deserialize_query_plan_response( query, serialization_type_, returned_data); } @@ -1191,8 +1174,6 @@ Status RestClientRemote::get_query_est_result_sizes( "Error getting array metadata from REST; server returned no data.")); } - // Ensure data has a null delimiter for cap'n proto if using JSON - RETURN_NOT_OK(ensure_json_null_delimited_string(&returned_data)); return serialization::query_est_result_size_deserialize( query, serialization_type_, true, returned_data); } @@ -1266,8 +1247,6 @@ Status RestClientRemote::post_fragment_info_from_rest( return LOG_STATUS(Status_RestError( "Error getting fragment info from REST; server returned no data.")); - // Ensure data has a null delimiter for cap'n proto if using JSON - RETURN_NOT_OK(ensure_json_null_delimited_string(&returned_data)); return serialization::fragment_info_deserialize( fragment_info, serialization_type_, uri, returned_data, memory_tracker_); } @@ -1308,8 +1287,6 @@ Status RestClientRemote::post_group_metadata_from_rest( "Error getting group metadata from REST; server returned no data.")); } - // Ensure data has a null delimiter for cap'n proto if using JSON - RETURN_NOT_OK(ensure_json_null_delimited_string(&returned_data)); return serialization::metadata_deserialize( group->unsafe_metadata(), group->config(), @@ -1407,8 +1384,6 @@ Status RestClientRemote::post_group_from_rest(const URI& uri, Group* group) { return LOG_STATUS(Status_RestError( "Error getting group from REST; server returned no data.")); - // Ensure data has a null delimiter for cap'n proto if using JSON - RETURN_NOT_OK(ensure_json_null_delimited_string(&returned_data)); return serialization::group_details_deserialize( group, serialization_type_, returned_data); } @@ -1458,14 +1433,6 @@ void RestClientRemote::delete_group_from_rest(const URI& uri, bool recursive) { stats_, url, serialization_type_, &returned_data, cache_key)); } -Status RestClientRemote::ensure_json_null_delimited_string(Buffer* buffer) { - if (serialization_type_ == SerializationType::JSON && - buffer->value(buffer->size() - 1) != '\0') { - RETURN_NOT_OK(buffer->write("\0", sizeof(char))); - } - return Status::Ok(); -} - Status RestClientRemote::post_consolidation_to_rest( const URI& uri, const Config& config) { BufferList serialized{memory_tracker_}; @@ -1545,8 +1512,6 @@ RestClientRemote::post_consolidation_plan_from_rest( "Error getting query plan from REST; server returned no data."); } - // Ensure data has a null delimiter for cap'n proto if using JSON - throw_if_not_ok(ensure_json_null_delimited_string(&returned_data)); return serialization::deserialize_consolidation_plan_response( serialization_type_, returned_data); } diff --git a/tiledb/sm/rest/rest_client_remote.h b/tiledb/sm/rest/rest_client_remote.h index 3d823d97328..cf2c082596a 100644 --- a/tiledb/sm/rest/rest_client_remote.h +++ b/tiledb/sm/rest/rest_client_remote.h @@ -577,15 +577,6 @@ class RestClientRemote : public RestClient { * @return Returns the redirection URI if exists and empty string otherwise */ std::string redirect_uri(const std::string& cache_key); - - /** - * Cap'n proto requires JSON messages to be null terminated c-style strings. - * This function checks if using JSON that returned buffers are null delimited - * - * @param buffer of server message - * @return Status - */ - Status ensure_json_null_delimited_string(Buffer* buffer); }; } // namespace tiledb::sm diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index 4724cd7c36b..9a4ef23182e 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -407,7 +407,7 @@ Status metadata_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -447,12 +447,9 @@ Status metadata_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - builder); + utils::decode_json_message(serialized_buffer, builder); auto reader = builder.asReader(); // Deserialize @@ -515,7 +512,7 @@ Status array_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(ArrayBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -551,13 +548,10 @@ void array_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::Array::Builder array_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - array_builder); + utils::decode_json_message(serialized_buffer, array_builder); capnp::Array::Reader array_reader = array_builder.asReader(); array_from_capnp(array_reader, resources, array, true, memory_tracker); break; @@ -612,7 +606,7 @@ Status array_open_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(arrayOpenBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -652,9 +646,7 @@ Status array_open_deserialize( ::capnp::MallocMessageBuilder message_builder; capnp::ArrayOpen::Builder array_open_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - array_open_builder); + utils::decode_json_message(serialized_buffer, array_open_builder, json); capnp::ArrayOpen::Reader array_open_reader = array_open_builder.asReader(); RETURN_NOT_OK(array_open_from_capnp(array_open_reader, array)); diff --git a/tiledb/sm/serialization/array_schema.cc b/tiledb/sm/serialization/array_schema.cc index bc8baa186ba..59edb97ff3d 100644 --- a/tiledb/sm/serialization/array_schema.cc +++ b/tiledb/sm/serialization/array_schema.cc @@ -1201,7 +1201,7 @@ Status array_schema_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(arraySchemaBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -1238,12 +1238,9 @@ shared_ptr array_schema_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; capnp::ArraySchema::Builder array_schema_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - array_schema_builder); + utils::decode_json_message(serialized_buffer, array_schema_builder); array_schema_reader = array_schema_builder.asReader(); return array_schema_from_capnp( array_schema_reader, URI(), memory_tracker); @@ -1301,7 +1298,7 @@ Status nonempty_domain_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -1342,12 +1339,9 @@ Status nonempty_domain_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - builder); + utils::decode_json_message(serialized_buffer, builder); auto reader = builder.asReader(); // Deserialize @@ -1429,7 +1423,7 @@ Status nonempty_domain_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -1472,12 +1466,9 @@ Status nonempty_domain_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - builder); + utils::decode_json_message(serialized_buffer, builder); auto reader = builder.asReader(); // Deserialize @@ -1552,7 +1543,7 @@ Status nonempty_domain_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -1587,12 +1578,9 @@ Status nonempty_domain_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - builder); + utils::decode_json_message(serialized_buffer, builder); auto reader = builder.asReader(); // Deserialize @@ -1655,7 +1643,7 @@ void serialize_load_array_schema_request( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - data.assign_null_terminated(capnp_json); + data.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -1699,11 +1687,10 @@ LoadArraySchemaRequest deserialize_load_array_schema_request( try { switch (serialization_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode(kj::StringPtr(data.data()), builder); + utils::decode_json_message(data, builder); auto reader = builder.asReader(); return load_array_schema_request_from_capnp(reader); } @@ -1765,7 +1752,7 @@ void serialize_load_array_schema_response( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - data.assign_null_terminated(capnp_json); + data.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -1831,11 +1818,10 @@ deserialize_load_array_schema_response( try { switch (serialization_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode(kj::StringPtr(data.data()), builder); + utils::decode_json_message(data, builder); auto reader = builder.asReader(); return load_array_schema_response_from_capnp( uri, reader, memory_tracker); diff --git a/tiledb/sm/serialization/array_schema_evolution.cc b/tiledb/sm/serialization/array_schema_evolution.cc index efb82e24fcd..55e62b7b27a 100644 --- a/tiledb/sm/serialization/array_schema_evolution.cc +++ b/tiledb/sm/serialization/array_schema_evolution.cc @@ -258,7 +258,7 @@ Status array_schema_evolution_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(arraySchemaEvolutionBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -299,13 +299,11 @@ Status array_schema_evolution_deserialize( switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::ArraySchemaEvolution::Builder array_schema_evolution_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - array_schema_evolution_builder); + utils::decode_json_message( + serialized_buffer, array_schema_evolution_builder); capnp::ArraySchemaEvolution::Reader array_schema_evolution_reader = array_schema_evolution_builder.asReader(); decoded_array_schema_evolution = array_schema_evolution_from_capnp( diff --git a/tiledb/sm/serialization/capnp_utils.h b/tiledb/sm/serialization/capnp_utils.h index fca72497080..da057ffbda4 100644 --- a/tiledb/sm/serialization/capnp_utils.h +++ b/tiledb/sm/serialization/capnp_utils.h @@ -35,6 +35,8 @@ #ifdef TILEDB_SERIALIZATION +#include + #include "tiledb-rest.capnp.h" #include "tiledb/common/heap_memory.h" @@ -692,6 +694,25 @@ Status deserialize_coords( return Status::Ok(); } +/** + * Converts a JSON payload to a CAPNP message. + * + * @param buffer A span containing the message. + * @param message Generic CAPNP message builder. + * @param json Optional JsonCodec to customize the process. + */ +void decode_json_message( + span buffer, + auto& message_builder, + const ::capnp::JsonCodec& json = {}) { + // Trim null terminator if exists. + if (!buffer.empty() && buffer.back() == '\0') { + buffer = buffer.first(buffer.size() - 1); + } + + json.decode(kj::ArrayPtr(buffer.data(), buffer.size()), message_builder); +} + } // namespace utils } // namespace tiledb::sm::serialization diff --git a/tiledb/sm/serialization/config.cc b/tiledb/sm/serialization/config.cc index 3177a362263..26381e5358c 100644 --- a/tiledb/sm/serialization/config.cc +++ b/tiledb/sm/serialization/config.cc @@ -118,7 +118,7 @@ Status config_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(configBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -154,13 +154,10 @@ Status config_deserialize( switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::Config::Builder config_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - config_builder); + utils::decode_json_message(serialized_buffer, config_builder); capnp::Config::Reader config_reader = config_builder.asReader(); RETURN_NOT_OK(config_from_capnp(config_reader, &decoded_config)); break; diff --git a/tiledb/sm/serialization/consolidation.cc b/tiledb/sm/serialization/consolidation.cc index b09fdc14245..ed957e7c989 100644 --- a/tiledb/sm/serialization/consolidation.cc +++ b/tiledb/sm/serialization/consolidation.cc @@ -104,7 +104,7 @@ Status array_consolidation_request_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(ArrayConsolidationRequestBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -140,14 +140,12 @@ Status array_consolidation_request_deserialize( switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::ArrayConsolidationRequest::Builder array_consolidation_request_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - array_consolidation_request_builder); + utils::decode_json_message( + serialized_buffer, array_consolidation_request_builder); capnp::ArrayConsolidationRequest::Reader array_consolidation_request_reader = array_consolidation_request_builder.asReader(); @@ -254,7 +252,7 @@ void serialize_consolidation_plan_request( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - request.assign_null_terminated(capnp_json); + request.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -285,11 +283,10 @@ uint64_t deserialize_consolidation_plan_request( try { switch (serialization_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::ConsolidationPlanRequest::Builder builder = message_builder.initRoot(); - json.decode(kj::StringPtr(request.data()), builder); + utils::decode_json_message(request, builder); capnp::ConsolidationPlanRequest::Reader reader = builder.asReader(); return consolidation_plan_request_from_capnp(reader); } @@ -333,7 +330,7 @@ void serialize_consolidation_plan_response( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - response.assign_null_terminated(capnp_json); + response.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -364,11 +361,10 @@ std::vector> deserialize_consolidation_plan_response( try { switch (serialization_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::ConsolidationPlanResponse::Builder builder = message_builder.initRoot(); - json.decode(kj::StringPtr(response.data()), builder); + utils::decode_json_message(response, builder); capnp::ConsolidationPlanResponse::Reader reader = builder.asReader(); return consolidation_plan_response_from_capnp(reader); } diff --git a/tiledb/sm/serialization/enumeration.cc b/tiledb/sm/serialization/enumeration.cc index 507c8224166..69c3bd38939 100644 --- a/tiledb/sm/serialization/enumeration.cc +++ b/tiledb/sm/serialization/enumeration.cc @@ -236,7 +236,7 @@ void serialize_load_enumerations_request( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - request.assign_null_terminated(capnp_json); + request.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -267,11 +267,10 @@ std::vector deserialize_load_enumerations_request( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::LoadEnumerationsRequest::Builder builder = message_builder.initRoot(); - json.decode(kj::StringPtr(request.data()), builder); + utils::decode_json_message(request, builder); capnp::LoadEnumerationsRequest::Reader reader = builder.asReader(); return load_enumerations_request_from_capnp(reader); } @@ -317,7 +316,7 @@ void serialize_load_enumerations_response( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - response.assign_null_terminated(capnp_json); + response.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -353,11 +352,10 @@ deserialize_load_enumerations_response( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::LoadEnumerationsResponse::Builder builder = message_builder.initRoot(); - json.decode(kj::StringPtr(response.data(), response.size()), builder); + utils::decode_json_message(response, builder); capnp::LoadEnumerationsResponse::Reader reader = builder.asReader(); return load_enumerations_response_from_capnp( reader, array_schema, memory_tracker); diff --git a/tiledb/sm/serialization/fragment_info.cc b/tiledb/sm/serialization/fragment_info.cc index 96fba72589f..c7798bfe5cf 100644 --- a/tiledb/sm/serialization/fragment_info.cc +++ b/tiledb/sm/serialization/fragment_info.cc @@ -101,7 +101,7 @@ Status fragment_info_request_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(fragment_info_req_builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -143,9 +143,8 @@ Status fragment_info_request_deserialize( ::capnp::MallocMessageBuilder message_builder; capnp::FragmentInfoRequest::Builder fragment_info_req_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - fragment_info_req_builder); + utils::decode_json_message( + serialized_buffer, fragment_info_req_builder, json); capnp::FragmentInfoRequest::Reader fragment_info_req_reader = fragment_info_req_builder.asReader(); RETURN_NOT_OK(fragment_info_request_from_capnp( @@ -402,7 +401,7 @@ Status fragment_info_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -443,12 +442,9 @@ Status fragment_info_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - builder); + utils::decode_json_message(serialized_buffer, builder); auto reader = builder.asReader(); // Deserialize diff --git a/tiledb/sm/serialization/fragments.cc b/tiledb/sm/serialization/fragments.cc index fd2e715233d..410acc24879 100644 --- a/tiledb/sm/serialization/fragments.cc +++ b/tiledb/sm/serialization/fragments.cc @@ -92,7 +92,7 @@ void serialize_delete_fragments_timestamps_request( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -122,14 +122,11 @@ std::tuple deserialize_delete_fragments_timestamps_request( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder .initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - builder); + utils::decode_json_message(serialized_buffer, builder); auto reader = builder.asReader(); // Deserialize return fragments_timestamps_from_capnp(reader); @@ -214,7 +211,7 @@ void serialize_delete_fragments_list_request( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -245,13 +242,10 @@ std::vector deserialize_delete_fragments_list_request( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; auto builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - builder); + utils::decode_json_message(serialized_buffer, builder); auto reader = builder.asReader(); // Deserialize return fragments_list_from_capnp(array_uri, reader); diff --git a/tiledb/sm/serialization/group.cc b/tiledb/sm/serialization/group.cc index 5f6a47d586c..c2236630d3a 100644 --- a/tiledb/sm/serialization/group.cc +++ b/tiledb/sm/serialization/group.cc @@ -377,7 +377,7 @@ Status group_serialize( ::capnp::JsonCodec json; json.handleByAnnotation(); kj::String capnp_json = json.encode(groupBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -416,9 +416,7 @@ Status group_deserialize( ::capnp::MallocMessageBuilder message_builder; capnp::Group::Builder group_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - group_builder); + utils::decode_json_message(serialized_buffer, group_builder, json); capnp::Group::Reader group_reader = group_builder.asReader(); RETURN_NOT_OK(group_from_capnp(group_reader, group)); break; @@ -476,7 +474,7 @@ Status group_details_serialize( ::capnp::JsonCodec json; json.handleByAnnotation(); kj::String capnp_json = json.encode(groupDetailsBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -515,9 +513,8 @@ Status group_details_deserialize( ::capnp::MallocMessageBuilder message_builder; capnp::Group::GroupDetails::Builder group_details_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - group_details_builder); + utils::decode_json_message( + serialized_buffer, group_details_builder, json); capnp::Group::GroupDetails::Reader group_details_reader = group_details_builder.asReader(); RETURN_NOT_OK(group_details_from_capnp(group_details_reader, group)); @@ -578,7 +575,7 @@ Status group_update_serialize( ::capnp::JsonCodec json; json.handleByAnnotation(); kj::String capnp_json = json.encode(groupUpdateBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -617,9 +614,8 @@ Status group_update_deserialize( ::capnp::MallocMessageBuilder message_builder; capnp::GroupUpdate::Builder group_update_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - group_update_builder); + utils::decode_json_message( + serialized_buffer, group_update_builder, json); capnp::GroupUpdate::Reader group_reader = group_update_builder.asReader(); RETURN_NOT_OK(group_update_from_capnp(group_reader, group)); @@ -679,7 +675,7 @@ Status group_create_serialize( ::capnp::JsonCodec json; json.handleByAnnotation(); kj::String capnp_json = json.encode(group_create_builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -723,7 +719,7 @@ Status group_metadata_serialize( ::capnp::JsonCodec json; json.handleByAnnotation(); kj::String capnp_json = json.encode(group_metadata_builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { diff --git a/tiledb/sm/serialization/query.cc b/tiledb/sm/serialization/query.cc index a56e22ba6b0..b6590c7e387 100644 --- a/tiledb/sm/serialization/query.cc +++ b/tiledb/sm/serialization/query.cc @@ -2285,11 +2285,10 @@ Status array_from_query_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::Query::Builder query_builder = message_builder.initRoot(); - json.decode(kj::StringPtr(serialized_buffer.data()), query_builder); + utils::decode_json_message(serialized_buffer, query_builder); capnp::Query::Reader query_reader = query_builder.asReader(); // Deserialize array instance. array_from_capnp( @@ -2362,7 +2361,7 @@ Status query_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(query_builder); - serialized_buffer.emplace_buffer().assign_null_terminated(capnp_json); + serialized_buffer.emplace_buffer().assign(capnp_json); // TODO: At this point the buffer data should also be serialized. break; } @@ -2507,13 +2506,10 @@ Status do_query_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::Query::Builder query_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - query_builder); + utils::decode_json_message(serialized_buffer, query_builder); capnp::Query::Reader query_reader = query_builder.asReader(); return query_from_capnp( query_reader, @@ -2728,7 +2724,7 @@ Status query_est_result_size_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(est_result_size_builder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -2760,13 +2756,11 @@ Status query_est_result_size_deserialize( try { switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::EstimatedResultSize::Builder estimated_result_size_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - estimated_result_size_builder); + utils::decode_json_message( + serialized_buffer, estimated_result_size_builder); capnp::EstimatedResultSize::Reader estimated_result_size_reader = estimated_result_size_builder.asReader(); RETURN_NOT_OK(query_est_result_size_reader_from_capnp( diff --git a/tiledb/sm/serialization/query_plan.cc b/tiledb/sm/serialization/query_plan.cc index 059304f9ac1..7b089703bb3 100644 --- a/tiledb/sm/serialization/query_plan.cc +++ b/tiledb/sm/serialization/query_plan.cc @@ -174,7 +174,7 @@ void serialize_query_plan_request( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - request.assign_null_terminated(capnp_json); + request.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -213,7 +213,7 @@ void deserialize_query_plan_request( ::capnp::MallocMessageBuilder message_builder; capnp::QueryPlanRequest::Builder builder = message_builder.initRoot(); - json.decode(kj::StringPtr(request.data()), builder); + utils::decode_json_message(request, builder, json); capnp::QueryPlanRequest::Reader reader = builder.asReader(); query_plan_request_from_capnp(reader, compute_tp, query); break; @@ -259,7 +259,7 @@ void serialize_query_plan_response( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(builder); - response.assign_null_terminated(capnp_json); + response.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -292,11 +292,10 @@ QueryPlan deserialize_query_plan_response( try { switch (serialization_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::QueryPlanResponse::Builder builder = message_builder.initRoot(); - json.decode(kj::StringPtr(response.data()), builder); + utils::decode_json_message(response, builder); capnp::QueryPlanResponse::Reader reader = builder.asReader(); return query_plan_response_from_capnp(reader, query); } diff --git a/tiledb/sm/serialization/vacuum.cc b/tiledb/sm/serialization/vacuum.cc index 659a22a4d9e..5419cbdc574 100644 --- a/tiledb/sm/serialization/vacuum.cc +++ b/tiledb/sm/serialization/vacuum.cc @@ -85,7 +85,7 @@ Status array_vacuum_request_serialize( case SerializationType::JSON: { ::capnp::JsonCodec json; kj::String capnp_json = json.encode(ArrayVacuumRequestBuilder); - serialized_buffer.assign_null_terminated(capnp_json); + serialized_buffer.assign(capnp_json); break; } case SerializationType::CAPNP: { @@ -121,13 +121,11 @@ Status array_vacuum_request_deserialize( switch (serialize_type) { case SerializationType::JSON: { - ::capnp::JsonCodec json; ::capnp::MallocMessageBuilder message_builder; capnp::ArrayVacuumRequest::Builder array_vacuum_request_builder = message_builder.initRoot(); - json.decode( - kj::StringPtr(static_cast(serialized_buffer.data())), - array_vacuum_request_builder); + utils::decode_json_message( + serialized_buffer, array_vacuum_request_builder); capnp::ArrayVacuumRequest::Reader array_vacuum_request_reader = array_vacuum_request_builder.asReader(); RETURN_NOT_OK(array_vacuum_request_from_capnp(