Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Improved signal serialization #1365

Merged
merged 5 commits into from
May 2, 2019
Merged

Conversation

willemolding
Copy link
Collaborator

PR summary

This changes the format of the JSON formatted signal being sent over the websocket. Prior to this the signals were debug formatted Rust which is difficult to handle in Javascript. This changes them to fully formed JSON.

This required implementing Serialize on all structs that can compose an Action and also adding the 'serde_support' feature to snowflake.

Before:

{
	"signal": {
		"Internal": "SignalZomeFunctionCall(ZomeFnCall { id: ProcessUniqueId { prefix: 7, offset: 3 }, zome_name: \"community\", cap: CapabilityRequest { cap_token: HashString(\"QmfQSbSCpjs9zctrmVueYoKRuZTxWu1sSEQmKXx5QnTACB\"), provenance: Provenance(HashString(\"HcSCJC97Yu987nhupguu3vBF3HHVtbu4m7rZpgGF7qWwiiink586Ih539kHns7r\"), Signature(\"g1Yb0AtQsiyKD/TMcxN4fOXmA2/yD08ub6kN5i2lCvKQ2NOd0V5eZKpqt4YJ9GV1fxQrM2acYcRGK0WAZEgTAw==\")) }, fn_name: \"get_community_address_by_slug\", parameters: JsonString(\"{\\\"slug\\\":\\\"hylo-holo-engineering\\\"}\") })"
	},
	"instance_id": "hylo-chat"
}

After:

{
	"signal": {
		"signal_type":"Internal",
		"action": {
			"action_type":"SignalZomeFunctionCall",
			"data":{
				"id": {"prefix":7,"offset":3},
				"zome_name":"community",
				"cap": {
					"cap_token":"QmfQSbSCpjs9zctrmVueYoKRuZTxWu1sSEQmKXx5QnTACB",
					"provenance":["HcSCJC97Yu987nhupguu3vBF3HHVtbu4m7rZpgGF7qWwiiink586Ih539kHns7r","g1Yb0AtQsiyKD/TMcxN4fOXmA2/yD08ub6kN5i2lCvKQ2NOd0V5eZKpqt4YJ9GV1fxQrM2acYcRGK0WAZEgTAw=="]
				},
			"fn_name": "get_community_address_by_slug",
			"parameters":"{\"slug\":\"hylo-holo-engineering\"}"
			}
		},
		"id":{"prefix":7,"offset":4}
	},
	"instance_id":"hylo-chat"
}

changelog

Please check one of the following, relating to the CHANGELOG

  • this is a code change that effects some consumer (e.g. zome developers) of holochain core so there is an 'Unreleased' CHANGELOG item, with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)
  • this is not a code change, or doesn't effect anyone outside holochain core development

Copy link
Collaborator

@maackle maackle left a comment

Choose a reason for hiding this comment

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

Awesome, that was easier than I imagined 😅 I added custom serialization to Signal at first because I didn't know how deep I'd have to go with derive(Serialize) for Action but it turned out not too bad.

Kind of on a related note, what do you think of #1364? Now that Internal signals are visible externally, maybe this PR is a good time to rename it to something other than Internal, like Trace?

@Connoropolous
Copy link
Collaborator

that rename sounds interesting @maackle
We can add it as a follow up

Copy link
Member

@zippy zippy left a comment

Choose a reason for hiding this comment

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

woot

Copy link
Collaborator

@lucksus lucksus left a comment

Choose a reason for hiding this comment

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

Great! Yeah, was looking into this as well when wrapping up @maackle PR and started adding those Serialize derivations. I stopped when I though I had to make every action and their arguments serializable as well, but seems that is not needed if you use the adjacently tagged representation?

@@ -88,7 +88,8 @@ impl Hash for ActionWrapper {
}

/// All Actions for the Holochain Instance Store, according to Redux pattern.
#[derive(Clone, PartialEq, Debug)]
#[derive(Clone, PartialEq, Debug, Serialize)]
#[serde(tag = "action_type", content = "data")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aha!
👏 👏 👏

@Connoropolous Connoropolous merged commit cbd91ef into develop May 2, 2019
@maackle
Copy link
Collaborator

maackle commented May 2, 2019

Added #1378 as a followup

@zippy zippy deleted the improved-signal-serialization branch October 4, 2019 18:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants