From dea3a30caec6320783aaadeaa4c34123afb95bbc Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 22 Jan 2021 12:20:27 +0100 Subject: [PATCH] Allow optional top level properties in messages Add optional arguments to Request and Event to control which properties are required to be present in the payload. This allows gracefully extending the protocol, reducing overhead for optional data, and leaving properties out for additional behaviour. --- docs/writing-plugins.md | 34 +++++++++++++++++++++++++++++----- packages/lib/link/messages.js | 35 ++++++++++++++++++++++++++++++----- test/lib/link/messages.js | 6 ++++++ 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md index 211f7aab..ae5ab831 100644 --- a/docs/writing-plugins.md +++ b/docs/writing-plugins.md @@ -381,8 +381,10 @@ the event, for example the following could be defined in `info.js`: type: "foo_frobber:start_frobnication", links: ["master-slave", "slave-instance"], forwardTo: "instance", + eventRequired: ["frobnication_type"], eventProperties: { "frobnication_type": { type: "string" }, + "urgent": { type: "boolean" }, }, }), }, @@ -390,8 +392,8 @@ the event, for example the following could be defined in `info.js`: This specifies an event that can be sent from the master to a slave, and from a slave to an instance. It also specifies that the event must contain the property `frobnication_type`, with a string value in the -data payload. It will also be forwarded by slaves to a specific -instance. +data payload and that it may optionally contain a boolean `urgent` +property. It will also be forwarded by slaves to a specific instance. The following properties are recognized by the Event constructor: @@ -434,12 +436,19 @@ it's sent to, but not back from where it came from. Currently, only that slave except for the instance it came from. from an instance will cause it to be broadcast to all instances of +#### eventRequired + +By default all properties defined in `eventProperties` are required to +be present in the payload for the event. This can be overriden by +specifying an array of properties which are required here. This is +equivalent to using the `required` keyword in JSON schema. + #### eventProperties Object with properties mapping to a JSON schema of that property that specifies what's valid to send in the event. This is equivalent to using the `properties` keyword in JSON schema, except that the -properties specified are implicitly required and additional properties +properties specified are by default required and additional properties are not allowed. See [this guide][guide] for an introduction to writing JSON schemas. @@ -466,8 +475,10 @@ the event. For example, the following could be defined in `info.js`: type: "foo_frobber:report_frobnication", links: ["master-slave", "slave-instance"], forwardTo: "instance", + requestRequired: ["verbosity"], requestProperties: { "verbosity": { type: "integer" }, + "special": { type: "boolean" }, }, responseProperties: { "report": { @@ -481,7 +492,8 @@ the event. For example, the following could be defined in `info.js`: This specifies a request that can be sent from the master to a slave, and from a slave to an instance. The request data must contain the property `verbosity` with an integer number as the value, as well as the -`instance_id` property (implied by `forwardTo: "instance"`), and the +`instance_id` property (implied by `forwardTo: "instance"`) and it may +also contain a boolean `special` property. It also defines that the response sent must contain a `report` property mapping to an array of strings. When received by a slave, it will also be forwarded to the instance specified by `instance_id`. @@ -520,15 +532,27 @@ instance, or `"instance"` to indicate it should be forwarded to the instances specified by the `instance_id` request property. This works by using a default handler for the request by the links that forward it. +#### requestRequired + +By default all properties defined in `requestProperties` are required to +be present in the payload for the request. This can be overriden by +specifying an array of properties which are required here. This is +equivalent to using the `required` keyword in JSON schema. + #### requestProperties Object with properties mapping to a JSON schema of that property that specifies what's valid to send in the request. This is equivalent to using the `properties` keyword in JSON schema, except that the -properties specified are implicitly required and additional properties +properties specified are by default required and additional properties are not allowed. See [this guide][guide] for an introduction to writing JSON schemas +#### responseRequired + +Same as the requestRequired only for the response sent back by the +target. + #### responseProperties Same as the requestProperties only for the response sent back by the diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 283a395f..aea0e040 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -70,16 +70,24 @@ class Request extends Message { * Optional target to forward this request to. 'instance' add * instance_id into the requestProperties and forward to the given * instance. 'master' forwards it to the master server. + * @param {?Array} requestRequired - + * List of properties that are required to be present in the data + * payload of this request. Defaults to all. * @param {Object} requestProperties - * Mapping of property values to JSON schema specifications for the * properties that are valid in the data payload of this requst. + * @param {?Array} responseRequired - + * List of properties that are required to be present in the data + * payload of the repsonse to this request. Defaults to all. * @param {Object} responseProperties - * Mapping of property values to JSON schema specifications for the * properties that are valid in the data payload of the response to * this requst. */ constructor({ - type, permission, links, forwardTo = null, requestProperties = {}, responseProperties = {}, + type, permission, links, forwardTo = null, + requestRequired = null, requestProperties = {}, + responseRequired = null, responseProperties = {}, }) { super(); this.type = type; @@ -98,7 +106,12 @@ class Request extends Message { throw new Error(`permission is not allowed on ${this.type} request as it is not over control-master link`); } + if (!requestRequired) { + requestRequired = Object.keys(requestProperties); + } + if (forwardTo === "instance") { + requestRequired = ["instance_id", ...requestRequired]; requestProperties = { "instance_id": { type: "integer" }, ...requestProperties, @@ -114,12 +127,16 @@ class Request extends Message { "type": { const: this.requestType }, "data": { additionalProperties: false, - required: Object.keys(requestProperties), + required: requestRequired, properties: requestProperties, }, }, }); + if (!responseRequired) { + responseRequired = Object.keys(responseProperties); + } + this._responseValidator = libSchema.compile({ $schema: "http://json-schema.org/draft-07/schema#", properties: { @@ -128,7 +145,7 @@ class Request extends Message { anyOf: [ { additionalProperties: false, - required: ["seq", ...Object.keys(responseProperties)], + required: ["seq", ...responseRequired], properties: { "seq": { type: "integer" }, ...responseProperties, @@ -760,11 +777,14 @@ class Event extends Message { * links that are downstream, so sending an event set to instance * broadcast that's sent to a slave will only be broadcast to that * slave's instances. + * @param {?Array} eventRequired - + * List of properties required to be present in the event payload. + * Defaults to all. * @param {Object} eventProperties - * Mapping of property values to JSON schema specifications for the * properties that are valid in the data payload of this event. */ - constructor({ type, links, forwardTo = null, broadcastTo = null, eventProperties = {} }) { + constructor({ type, links, forwardTo = null, broadcastTo = null, eventRequired = null, eventProperties = {} }) { super(); this.type = type; this.links = links; @@ -772,7 +792,12 @@ class Event extends Message { this.broadcastTo = broadcastTo; this.handlerSuffix = "EventHandler"; + if (!eventRequired) { + eventRequired = Object.keys(eventProperties); + } + if (forwardTo === "instance") { + eventRequired = ["instance_id", ...eventRequired]; eventProperties = { "instance_id": { type: "integer" }, ...eventProperties, @@ -793,7 +818,7 @@ class Event extends Message { "type": { const: this.eventType }, "data": { additionalProperties: false, - required: Object.keys(eventProperties), + required: eventRequired, properties: eventProperties, }, }, diff --git a/test/lib/link/messages.js b/test/lib/link/messages.js index 97ad641e..b2005273 100644 --- a/test/lib/link/messages.js +++ b/test/lib/link/messages.js @@ -17,11 +17,15 @@ describe("lib/link/messages", function() { let testRequest = new libLink.Request({ type: "test", links: ["source-target"], + requestRequired: ["test"], requestProperties: { "test": { type: "string" }, + "optional": { type: "number" }, }, + responseRequired: ["test"], responseProperties: { "test": { type: "string" }, + "optional": { type: "number" }, }, }); @@ -127,8 +131,10 @@ describe("lib/link/messages", function() { let testEvent = new libLink.Event({ type: "test", links: ["source-target"], + eventRequired: ["test"], eventProperties: { "test": { type: "string" }, + "optional": { type: "number" }, }, });