Populate data_envelope with topic in json::decode()

[awelzel]

Hey @Neverlord - I'm attempting to use broker::format::json::v1::decode() to decode Zeek events sent by external clients, but implemented outside of the Broker layer. The Broker WebSocket format defines the "topic" a client publishes to, to be part of the top-level JSON object.

This PR changes decode() to optionally support extracting the topic, too. Does this feel too ad-hoc? I realize decode() can decode non-events, too, so maybe adding a separate helper would be the better approach?

It's this part from the internal/json_client.cc, but it has a parsed JSON representation available:

broker/libbroker/broker/internal/json_client.cc

Lines 140 to 142 in 5847b2a

	auto maybe_msg = data_envelope::deserialize(
	id, endpoint_id::nil(), defaults::ttl,
	caf::to_string(obj.value("topic").to_string()), buf.data(), buf.size());

[Neverlord]

It definitely feels off to me to leak application logic into the decoding layer. What's wrong with asking the variant after decoding?

variant res;
... decode ...
if (auto topic = res.to_table()["topic"sv]; topic.is_string()) {
  ... topic.to_string() ...
}

[awelzel]

What's wrong with asking the variant after decoding?

Have you tried? The json::data_message_to_binary() that converts from JSON to binary will discard topic of the top-level data-message. It only looks at the data and @data-type, so the resulting variant won't have that.

https://docs.zeek.org/projects/broker/en/current/web-socket.html#data-messages

It definitely feels off to me to leak application logic into the decoding layer.

So and so - the data-message as shown in the documentation does have a "topic" field, so maybe it's the name that's too generic.

[Neverlord]

It only looks at the data and @data-Type, so the resulting variant won't have that.

Ah, that's the extra context that I needed. It's been a while, thanks for jogging my memory. 🙂

I do dislike the output parameter, though. What if instead of hard-coding topic::reserved as the topic, we take the topic from the JSON (if the field is present)? That would make it available in the envelope and you could navigate to it via variant::shared_envelope().

[awelzel]

That would make it available in the envelope and you could navigate to it via variant::shared_envelope().

Yep, that works and looks nice! Re-pushed. Thanks!