From 9a78440fe061fee0f7dd9c2c0f1c33266a9ef07f Mon Sep 17 00:00:00 2001 From: reluc Date: Tue, 16 Jan 2024 12:50:52 +0100 Subject: [PATCH 1/4] fix(core/interaction-output): allign value function implementation with spec Note that the exploreDirectory method has to be updated to reflect the new check for the presence of DataSchema in the value function. Tests have been updated too. In particular opcua required an ad hoc parsing. fix #1216 --- .../binding-http/test/http-server-test.ts | 2 +- .../test/full-opcua-thing-test.ts | 23 ++++++-- packages/core/src/interaction-output.ts | 54 ++++++++++--------- packages/core/src/wot-impl.ts | 22 ++++---- packages/core/test/DiscoveryTest.ts | 1 + 5 files changed, 61 insertions(+), 41 deletions(-) diff --git a/packages/binding-http/test/http-server-test.ts b/packages/binding-http/test/http-server-test.ts index e0e99ef2f..d96c956cd 100644 --- a/packages/binding-http/test/http-server-test.ts +++ b/packages/binding-http/test/http-server-test.ts @@ -160,7 +160,7 @@ class HttpServerTest { let test: DataSchemaValue; testThing.setPropertyReadHandler("test", (_) => Promise.resolve(test)); testThing.setPropertyWriteHandler("test", async (value) => { - test = await value.value(); + test = Buffer.from(await value.arrayBuffer()).toString("utf-8"); }); testThing.setActionHandler("try", async (input: WoT.InteractionOutput) => { diff --git a/packages/binding-opcua/test/full-opcua-thing-test.ts b/packages/binding-opcua/test/full-opcua-thing-test.ts index 3d3966683..12333dc78 100644 --- a/packages/binding-opcua/test/full-opcua-thing-test.ts +++ b/packages/binding-opcua/test/full-opcua-thing-test.ts @@ -49,6 +49,7 @@ const thingDescription: WoT.ThingDescription = { observable: true, readOnly: true, unit: "°C", + type: "number", "opcua:nodeId": { root: "i=84", path: "/Objects/1:MySensor/2:ParameterSet/1:Temperature" }, // Don't specifu type here as it could be multi form: type: [ "object", "number" ], forms: [ @@ -111,6 +112,7 @@ const thingDescription: WoT.ThingDescription = { description: "the temperature set point", observable: true, unit: "°C", + type: "number", // dont't forms: [ { @@ -358,10 +360,25 @@ describe("Full OPCUA Thing Test", () => { return { thing, servient }; } - async function doTest(thing: WoT.ConsumedThing, propertyName: string, localOptions: InteractionOptions) { + async function doTest( + thing: WoT.ConsumedThing, + propertyName: string, + localOptions: InteractionOptions, + forceParsing = false + ) { debug("------------------------------------------------------"); try { const content = await thing.readProperty(propertyName, localOptions); + if (forceParsing) { + // In opcua binding it is possible to return a special response that contains + // reacher details than the bare value. However this make the returned value + // not complaint with its data schema. Therefore we have to fallback to + // custom parsing. + const raw = await content.arrayBuffer(); + const json = JSON.parse(Buffer.from(raw).toString("utf-8")); + debug(json?.toString()); + return json; + } const json = await content.value(); debug(json?.toString()); return json; @@ -395,13 +412,13 @@ describe("Full OPCUA Thing Test", () => { const json1 = await doTest(thing, propertyName, { formIndex: 1 }); expect(json1).to.eql(25); - const json2 = await doTest(thing, propertyName, { formIndex: 2 }); + const json2 = await doTest(thing, propertyName, { formIndex: 2 }, true); expect(json2).to.eql({ Type: 11, Body: 25 }); expect(thingDescription.properties?.temperature.forms[3].contentType).eql( "application/opcua+json;type=DataValue" ); - const json3 = await doTest(thing, propertyName, { formIndex: 3 }); + const json3 = await doTest(thing, propertyName, { formIndex: 3 }, true); debug(json3?.toString()); expect((json3 as Record).Value).to.eql({ Type: 11, Body: 25 }); } finally { diff --git a/packages/core/src/interaction-output.ts b/packages/core/src/interaction-output.ts index 21b0b58d1..8d9596a06 100644 --- a/packages/core/src/interaction-output.ts +++ b/packages/core/src/interaction-output.ts @@ -16,7 +16,7 @@ import * as util from "util"; import * as WoT from "wot-typescript-definitions"; import { ContentSerdes } from "./content-serdes"; import { ProtocolHelpers } from "./core"; -import { DataSchemaError, NotSupportedError } from "./errors"; +import { DataSchemaError, NotReadableError, NotSupportedError } from "./errors"; import { Content } from "./content"; import Ajv from "ajv"; import { createLoggers } from "./logger"; @@ -33,7 +33,7 @@ const ajv = new Ajv({ strict: false }); export class InteractionOutput implements WoT.InteractionOutput { private content: Content; - private parsedValue: unknown; + #value: unknown; private buffer?: ArrayBuffer; private _stream?: ReadableStream; dataUsed: boolean; @@ -79,43 +79,47 @@ export class InteractionOutput implements WoT.InteractionOutput { async value(): Promise { // the value has been already read? - if (this.parsedValue !== undefined) { - return this.parsedValue as T; + if (this.#value !== undefined) { + return this.#value as T; } if (this.dataUsed) { - throw new Error("Can't read the stream once it has been already used"); + throw new NotReadableError("Can't read the stream once it has been already used"); + } + + if (this.form == null) { + throw new NotReadableError("No form defined"); + } + + if (this.schema == null || this.schema.type == null) { + throw new NotReadableError("No schema defined"); } // is content type valid? - if (!this.form || !ContentSerdes.get().isSupported(this.content.type)) { - const message = !this.form ? "Missing form" : `Content type ${this.content.type} not supported`; + if (!ContentSerdes.get().isSupported(this.content.type)) { + const message = `Content type ${this.content.type} not supported`; throw new NotSupportedError(message); } // read fully the stream - const data = await this.content.toBuffer(); + const bytes = await this.content.toBuffer(); this.dataUsed = true; - this.buffer = data; + this.buffer = bytes; // call the contentToValue - // TODO: should be fixed contentToValue MUST define schema as nullable - const value = ContentSerdes.get().contentToValue({ type: this.content.type, body: data }, this.schema ?? {}); - - // any data (schema)? - if (this.schema) { - // validate the schema - const validate = ajv.compile(this.schema); - - if (!validate(value)) { - debug(`schema = ${util.inspect(this.schema, { depth: 10, colors: true })}`); - debug(`value: ${value}`); - debug(`Errror: ${validate.errors}`); - throw new DataSchemaError("Invalid value according to DataSchema", value as WoT.DataSchemaValue); - } + const json = ContentSerdes.get().contentToValue({ type: this.content.type, body: bytes }, this.schema); + + // validate the schema + const validate = ajv.compile(this.schema); + + if (!validate(json)) { + debug(`schema = ${util.inspect(this.schema, { depth: 10, colors: true })}`); + debug(`value: ${json}`); + debug(`Errror: ${validate.errors}`); + throw new DataSchemaError("Invalid value according to DataSchema", json as WoT.DataSchemaValue); } - this.parsedValue = value; - return this.parsedValue as T; + this.#value = json; + return this.#value as T; } } diff --git a/packages/core/src/wot-impl.ts b/packages/core/src/wot-impl.ts index 00754fdb8..9b8df09a9 100644 --- a/packages/core/src/wot-impl.ts +++ b/packages/core/src/wot-impl.ts @@ -22,19 +22,16 @@ import Helpers from "./helpers"; import { createLoggers } from "./logger"; import ContentManager from "./content-serdes"; import { getLastValidationErrors, isThingDescription } from "./validation"; +import { inspect } from "util"; const { debug } = createLoggers("core", "wot-impl"); class ThingDiscoveryProcess implements WoT.ThingDiscoveryProcess { - constructor(rawThingDescriptions: WoT.DataSchemaValue, filter?: WoT.ThingFilter) { + constructor(private directory: ConsumedThing, public filter?: WoT.ThingFilter) { this.filter = filter; this.done = false; - this.rawThingDescriptions = rawThingDescriptions; } - rawThingDescriptions: WoT.DataSchemaValue; - - filter?: WoT.ThingFilter | undefined; done: boolean; error?: Error | undefined; async stop(): Promise { @@ -42,13 +39,17 @@ class ThingDiscoveryProcess implements WoT.ThingDiscoveryProcess { } async *[Symbol.asyncIterator](): AsyncIterator { - if (!(this.rawThingDescriptions instanceof Array)) { - this.error = new Error("Encountered an invalid output value."); + let rawThingDescriptions: WoT.ThingDescription[]; + try { + const thingsPropertyOutput = await this.directory.readProperty("things"); + rawThingDescriptions = (await thingsPropertyOutput.value()) as WoT.ThingDescription[]; + } catch (error) { + this.error = error instanceof Error ? error : new Error(inspect(error)); this.done = true; return; } - for (const outputValue of this.rawThingDescriptions) { + for (const outputValue of rawThingDescriptions) { if (this.done) { return; } @@ -81,10 +82,7 @@ export default class WoTImpl { const directoyThingDescription = await this.requestThingDescription(url); const consumedDirectoy = await this.consume(directoyThingDescription); - const thingsPropertyOutput = await consumedDirectoy.readProperty("things"); - const rawThingDescriptions = await thingsPropertyOutput.value(); - - return new ThingDiscoveryProcess(rawThingDescriptions, filter); + return new ThingDiscoveryProcess(consumedDirectoy, filter); } /** @inheritDoc */ diff --git a/packages/core/test/DiscoveryTest.ts b/packages/core/test/DiscoveryTest.ts index 3b41b0f5e..c8c30c776 100644 --- a/packages/core/test/DiscoveryTest.ts +++ b/packages/core/test/DiscoveryTest.ts @@ -36,6 +36,7 @@ function createDirectoryTestTd(title: string, thingsPropertyHref: string) { }, properties: { things: { + type: "array", forms: [ { href: thingsPropertyHref, From 1148fab2ca971a13f27f3c3ad4d24a5922393a58 Mon Sep 17 00:00:00 2001 From: reluc Date: Tue, 16 Jan 2024 16:13:34 +0100 Subject: [PATCH 2/4] fixup! fix(core/interaction-output): allign value function implementation with spec --- packages/core/src/interaction-output.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/interaction-output.ts b/packages/core/src/interaction-output.ts index 8d9596a06..f1d5a55e3 100644 --- a/packages/core/src/interaction-output.ts +++ b/packages/core/src/interaction-output.ts @@ -106,7 +106,6 @@ export class InteractionOutput implements WoT.InteractionOutput { this.dataUsed = true; this.buffer = bytes; - // call the contentToValue const json = ContentSerdes.get().contentToValue({ type: this.content.type, body: bytes }, this.schema); // validate the schema @@ -120,6 +119,6 @@ export class InteractionOutput implements WoT.InteractionOutput { } this.#value = json; - return this.#value as T; + return json; } } From c10cdfc7348a0c7e8579c2cc1c9ae7b4ff4a3bd2 Mon Sep 17 00:00:00 2001 From: reluc Date: Wed, 17 Jan 2024 14:39:16 +0100 Subject: [PATCH 3/4] refactor(core/interaction-output): use make all private properties use # --- packages/core/src/interaction-output.ts | 32 ++++++++++++------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/core/src/interaction-output.ts b/packages/core/src/interaction-output.ts index f1d5a55e3..a369a2161 100644 --- a/packages/core/src/interaction-output.ts +++ b/packages/core/src/interaction-output.ts @@ -32,17 +32,17 @@ const { debug } = createLoggers("core", "interaction-output"); const ajv = new Ajv({ strict: false }); export class InteractionOutput implements WoT.InteractionOutput { - private content: Content; + #content: Content; #value: unknown; - private buffer?: ArrayBuffer; - private _stream?: ReadableStream; + #buffer?: ArrayBuffer; + #stream?: ReadableStream; dataUsed: boolean; form?: WoT.Form; schema?: WoT.DataSchema; public get data(): ReadableStream { - if (this._stream) { - return this._stream; + if (this.#stream) { + return this.#stream; } if (this.dataUsed) { @@ -51,28 +51,28 @@ export class InteractionOutput implements WoT.InteractionOutput { // Once the stream is created data might be pulled unpredictably // therefore we assume that it is going to be used to be safe. this.dataUsed = true; - return (this._stream = ProtocolHelpers.toWoTStream(this.content.body) as ReadableStream); + return (this.#stream = ProtocolHelpers.toWoTStream(this.#content.body) as ReadableStream); } constructor(content: Content, form?: WoT.Form, schema?: WoT.DataSchema) { - this.content = content; + this.#content = content; this.form = form; this.schema = schema; this.dataUsed = false; } async arrayBuffer(): Promise { - if (this.buffer) { - return this.buffer; + if (this.#buffer) { + return this.#buffer; } if (this.dataUsed) { throw new Error("Can't read the stream once it has been already used"); } - const data = await this.content.toBuffer(); + const data = await this.#content.toBuffer(); this.dataUsed = true; - this.buffer = data; + this.#buffer = data; return data; } @@ -96,17 +96,17 @@ export class InteractionOutput implements WoT.InteractionOutput { } // is content type valid? - if (!ContentSerdes.get().isSupported(this.content.type)) { - const message = `Content type ${this.content.type} not supported`; + if (!ContentSerdes.get().isSupported(this.#content.type)) { + const message = `Content type ${this.#content.type} not supported`; throw new NotSupportedError(message); } // read fully the stream - const bytes = await this.content.toBuffer(); + const bytes = await this.#content.toBuffer(); this.dataUsed = true; - this.buffer = bytes; + this.#buffer = bytes; - const json = ContentSerdes.get().contentToValue({ type: this.content.type, body: bytes }, this.schema); + const json = ContentSerdes.get().contentToValue({ type: this.#content.type, body: bytes }, this.schema); // validate the schema const validate = ajv.compile(this.schema); From ac4e5ae49975512af89748d5a9843baf690fbab6 Mon Sep 17 00:00:00 2001 From: Cristiano Aguzzi Date: Wed, 17 Jan 2024 18:25:07 +0100 Subject: [PATCH 4/4] refactor(binding-opcua/test): fix typos in code tests Co-authored-by: danielpeintner --- packages/binding-opcua/test/full-opcua-thing-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/binding-opcua/test/full-opcua-thing-test.ts b/packages/binding-opcua/test/full-opcua-thing-test.ts index 12333dc78..c84a6e6b2 100644 --- a/packages/binding-opcua/test/full-opcua-thing-test.ts +++ b/packages/binding-opcua/test/full-opcua-thing-test.ts @@ -371,7 +371,7 @@ describe("Full OPCUA Thing Test", () => { const content = await thing.readProperty(propertyName, localOptions); if (forceParsing) { // In opcua binding it is possible to return a special response that contains - // reacher details than the bare value. However this make the returned value + // richer details than the bare value. However this makes the returned value // not complaint with its data schema. Therefore we have to fallback to // custom parsing. const raw = await content.arrayBuffer();