Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add API for deserializing data envelopes from JSON #395

Merged
merged 2 commits into from
May 9, 2024
Merged

Add API for deserializing data envelopes from JSON #395

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59a654b
Add API for deserializing envelopes from JSON
Neverlord Mar 14, 2024
1bc0d93
Implement new decode function for the JSON format
Neverlord Mar 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libbroker/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ set(BROKER_TEST_SRC
broker/data.test.cc
broker/detail/peer_status_map.test.cc
broker/domain_options.test.cc
broker/envelope.test.cc
broker/error.test.cc
broker/filter_type.test.cc
broker/format/bin.test.cc
Expand Down
5 changes: 5 additions & 0 deletions libbroker/broker/data_envelope.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@
#include "broker/error.hh"
#include "broker/expected.hh"
#include "broker/format/bin.hh"
#include "broker/internal/json.hh"
#include "broker/internal/native.hh"
#include "broker/internal/type_id.hh"
#include "broker/topic.hh"

#include <caf/binary_serializer.hpp>
#include <caf/byte_buffer.hpp>
#include <caf/expected.hpp>
#include <caf/json_object.hpp>
#include <caf/json_value.hpp>

using namespace std::literals;

Expand Down
37 changes: 37 additions & 0 deletions libbroker/broker/envelope.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "broker/error.hh"
#include "broker/expected.hh"
#include "broker/format/bin.hh"
#include "broker/internal/json.hh"
#include "broker/internal/logger.hh"
#include "broker/internal/type_id.hh"
#include "broker/p2p_message_type.hh"
Expand All @@ -20,6 +21,9 @@
#include <caf/byte_buffer.hpp>
#include <caf/detail/ieee_754.hpp>
#include <caf/detail/network_order.hpp>
#include <caf/expected.hpp>
#include <caf/json_object.hpp>
#include <caf/json_value.hpp>

namespace broker {

Expand Down Expand Up @@ -148,6 +152,39 @@ expected<envelope_ptr> envelope::deserialize(const std::byte* data,
}
}

expected<envelope_ptr> envelope::deserialize_json(const char* data,
size_t size) {
// Parse the JSON text into a JSON object.
auto val = caf::json_value::parse_shallow(std::string_view{data, size});
if (!val)
return error{ec::invalid_json};
auto obj = val->to_object();
// Type-checking.
if (obj.value("type").to_string() != "data-message")
Copy link
Contributor

@awelzel awelzel Mar 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I realize I'm looking at parsing one layer further in: The idea was JSON contains only the serialized event vector without the data-message and topic.

In my use-case, "type" and "topic" are in message headers, not within the payload and the payload isn't meant to be structured like the websocket protocol.

Let me check if the JSON serialization in #391 even does what I thought it did.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you'd basically want to have a function that parses you just the variant from the JSON format? I'll add that later.

Copy link
Contributor

@awelzel awelzel Mar 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the following snippet:

        std::vector<char> cbuf;
        auto ev = broker::zeek::Event(event.HandlerName(), std::move(xs), broker::to_timestamp(event.timestamp));
        broker::format::json::v1::encode(ev.move_data(), std::back_inserter(cbuf));

Produces JSON as follows (where the object encoding a Zeek event is at the top-level), similarly how this works for the binary protocol shown in #391. There's no topic or type, just the "event vector directly".

[#1] Received on "zeek.cluster.discovery"
{"@data-type":"vector","data":[{"@data-type":"count","data":1},{"@data-type":"count","data":1},{"@data-type":"vector","data":[{"@data-type":"string","data":"Cluster::Backend::NATS::hello"},{"@data-type":"vector","data":[{"@data-type":"string","data":"proxy-1"},{"@data-type":"string","data":"nats_tinkyx1_2491709_NCcEAFMz82U3"}]},{"@data-type":"vector","data":[{"@data-type":"vector","data":[{"@data-type":"count","data":1},{"@data-type":"timestamp","data":"1970-01-01T01:00:00.000"}]}]}]}]}

And that is what I was hoping to parse. For the binary protocol that can be achieved via the following:

    auto r = broker::data_envelope::deserialize(broker::endpoint_id::nil(), broker::endpoint_id::nil(), 0, "", payload,
                                                payload_size);
    broker::zeek::Event ev(std::move(*r));

Am I looking for broker::data_envelop::deserialize_json(...) instead? Maybe that doesn't make overly much sense.

Now, the main motivation is to "just" re-use existing encoding/serialization functionality from broker for events. If you think that API doesn't make much sense to have, feel free to push back. On the other hand, there is an API for serialization, so it might make sense to offer one for de-serialization as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, the main motivation is to "just" re-use existing encoding/serialization functionality from broker for events. If you think that API doesn't make much sense to have, feel free to push back.

No push back at all, I just didn't think of that use case yet. 🙂

I'll add functions that do the JSON (de)-serialization for variant as well.

return error{ec::deserialization_failed};
// Read the topic.
auto topic = obj.value("topic").to_string();
if (topic.empty())
return error{ec::deserialization_failed};
auto stl_topic = std::string_view{topic.data(), topic.size()};
// Try to convert the JSON structure into our binary serialization format.
std::vector<std::byte> buf;
buf.reserve(512); // Allocate some memory to avoid small allocations.
if (auto err = internal::json::data_message_to_binary(obj, buf))
return err;
// Turn the binary data into a data envelope. TTL and sender/receiver are
// not part of the JSON representation, so we use defaults values.
auto res = data_envelope::deserialize(endpoint_id::nil(), endpoint_id::nil(),
defaults::ttl, stl_topic, buf.data(),
buf.size());
// Note: must manually "unbox" the expected to convert from
// expected<data_envelope_ptr> to expected<envelope_ptr>.
if (res)
return *res;
else
return res.error();
}

data_envelope_ptr envelope::as_data() const {
BROKER_ASSERT(type() == envelope_type::data);
return {new_ref, static_cast<const data_envelope*>(this)};
Expand Down
4 changes: 4 additions & 0 deletions libbroker/broker/envelope.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ public:
/// write format.
static expected<envelope_ptr> deserialize(const std::byte* data, size_t size);

/// Attempts to deserialize an envelope from the given message in Broker's
/// JSON format.
static expected<envelope_ptr> deserialize_json(const char* data, size_t size);

/// @pre `type == envelope_type::data`
data_envelope_ptr as_data() const;

Expand Down
127 changes: 127 additions & 0 deletions libbroker/broker/envelope.test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
#include "broker/data_envelope.hh"

#include "broker/broker-test.test.hh"

using namespace broker;
using namespace std::literals;

namespace {

// A data message that has one of everything.
constexpr std::string_view json = R"_({
"type": "data-message",
"topic": "/foo/bar",
"@data-type": "vector",
"data": [
{
"@data-type": "none",
"data": {}
},
{
"@data-type": "boolean",
"data": true
},
{
"@data-type": "count",
"data": 42
},
{
"@data-type": "integer",
"data": 23
},
{
"@data-type": "real",
"data": 12.48
},
{
"@data-type": "string",
"data": "this is a string"
},
{
"@data-type": "address",
"data": "2001:db8::"
},
{
"@data-type": "subnet",
"data": "255.255.255.0/24"
},
{
"@data-type": "port",
"data": "8080/tcp"
},
{
"@data-type": "timestamp",
"data": "2022-04-10T16:07:00.000"
},
{
"@data-type": "timespan",
"data": "23s"
},
{
"@data-type": "enum-value",
"data": "foo"
},
{
"@data-type": "set",
"data": [
{
"@data-type": "integer",
"data": 1
},
{
"@data-type": "integer",
"data": 2
},
{
"@data-type": "integer",
"data": 3
}
]
},
{
"@data-type": "table",
"data": [
{
"key": {
"@data-type": "string",
"data": "first-name"
},
"value": {
"@data-type": "string",
"data": "John"
}
},
{
"key": {
"@data-type": "string",
"data": "last-name"
},
"value": {
"@data-type": "string",
"data": "Doe"
}
}
]
}
]
})_";

} // namespace

TEST(JSON can be deserialized to a data message) {
auto maybe_envelope = envelope::deserialize_json(json.data(), json.size());
REQUIRE(maybe_envelope);
}

TEST(an invalid JSON payload results in an error) {
auto no_json = "this is not json!"sv;
auto maybe_envelope = envelope::deserialize_json(no_json.data(),
no_json.size());
CHECK(!maybe_envelope);
}

TEST(a JSON payload that does not contain a broker data results in an error) {
std::string_view obj = R"_({"foo": "bar"})_";
auto maybe_envelope = envelope::deserialize_json(obj.data(), obj.size());
CHECK(!maybe_envelope);
}
2 changes: 2 additions & 0 deletions libbroker/broker/error.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ enum class ec : uint8_t {
redundant_connection,
/// Broker encountered a
logic_error = 40,
/// Broker failed to parse a JSON object.
invalid_json,
};
// --ec-enum-end

Expand Down
Loading