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

refactor: handle "non"-validating output for async actions #1279

Merged
merged 6 commits into from
Jun 7, 2024
+72 −9
Merged

refactor: handle "non"-validating output for async actions #1279

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
066f725
test: add testcase for async action
danielpeintner Apr 29, 2024
5f8a5c0
refactor: allow to specify whether we want to have DataSchema validation
danielpeintner Apr 29, 2024
54e887a
docs: remove now useless comment
danielpeintner Apr 29, 2024
4708fe7
refactor: align flag using ignoreValidation everywhere
danielpeintner Apr 30, 2024
8a05b90
refactor: introduce anonymous InteractionOutput options parameter
danielpeintner May 9, 2024
f14ae6b
refactor: make ignoreValidation required
danielpeintner May 9, 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
14 changes: 8 additions & 6 deletions packages/core/src/consumed-thing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ export default class ConsumedThing extends TD.Thing implements IConsumedThing {

const content = await client.readResource(form);
try {
return this.handleInteractionOutput(content, form, tp);
return this.handleInteractionOutput(content, form, tp, false);
} catch (e) {
const error = e instanceof Error ? e : new Error(JSON.stringify(e));
throw new Error(`Error while processing property for ${tp.title}. ${error.message}`);
Expand All @@ -568,7 +568,8 @@ export default class ConsumedThing extends TD.Thing implements IConsumedThing {
private handleInteractionOutput(
content: Content,
form: TD.Form,
outputDataSchema: WoT.DataSchema | undefined
outputDataSchema: WoT.DataSchema | undefined,
ignoreValidation: boolean | undefined
): InteractionOutput {
// infer media type from form if not in response metadata
content.type ??= form.contentType ?? "application/json";
Expand All @@ -583,7 +584,7 @@ export default class ConsumedThing extends TD.Thing implements IConsumedThing {
);
}
}
return new InteractionOutput(content, form, outputDataSchema);
return new InteractionOutput(content, form, outputDataSchema, ignoreValidation);
}

async _readProperties(propertyNames: string[]): Promise<WoT.PropertyReadMap> {
Expand Down Expand Up @@ -703,7 +704,8 @@ export default class ConsumedThing extends TD.Thing implements IConsumedThing {

const content = await client.invokeResource(form, input);
try {
return this.handleInteractionOutput(content, form, ta.output);
const ignoreValidation = ta.synchronous === undefined ? true : !ta.synchronous;
return this.handleInteractionOutput(content, form, ta.output, ignoreValidation);
} catch (e) {
const error = e instanceof Error ? e : new Error(JSON.stringify(e));
throw new Error(`Error while processing action for ${ta.title}. ${error.message}`);
Expand Down Expand Up @@ -746,7 +748,7 @@ export default class ConsumedThing extends TD.Thing implements IConsumedThing {
// next
(content) => {
try {
listener(this.handleInteractionOutput(content, form, tp));
listener(this.handleInteractionOutput(content, form, tp, false));
} catch (e) {
const error = e instanceof Error ? e : new Error(JSON.stringify(e));
warn(`Error while processing observe property for ${tp.title}. ${error.message}`);
Expand Down Expand Up @@ -802,7 +804,7 @@ export default class ConsumedThing extends TD.Thing implements IConsumedThing {
formWithoutURITemplates,
(content) => {
try {
listener(this.handleInteractionOutput(content, form, te.data));
listener(this.handleInteractionOutput(content, form, te.data, false));
} catch (e) {
const error = e instanceof Error ? e : new Error(JSON.stringify(e));
warn(`Error while processing event for ${te.title}. ${error.message}`);
Expand Down
8 changes: 5 additions & 3 deletions packages/core/src/interaction-output.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export class InteractionOutput implements WoT.InteractionOutput {
dataUsed: boolean;
form?: WoT.Form;
schema?: WoT.DataSchema;
ignoreValidation: boolean; // by default set to false

public get data(): ReadableStream {
if (this.#stream) {
Expand All @@ -57,10 +58,11 @@ export class InteractionOutput implements WoT.InteractionOutput {
return (this.#stream = ProtocolHelpers.toWoTStream(this.#content.body) as ReadableStream);
}

constructor(content: Content, form?: WoT.Form, schema?: WoT.DataSchema) {
constructor(content: Content, form?: WoT.Form, schema?: WoT.DataSchema, ignoreValidation?: boolean) {
danielpeintner marked this conversation as resolved.
Show resolved Hide resolved
this.#content = content;
this.form = form;
this.schema = schema;
this.ignoreValidation = ignoreValidation ?? false;
this.dataUsed = false;
}

Expand Down Expand Up @@ -122,14 +124,14 @@ export class InteractionOutput implements WoT.InteractionOutput {
// validate the schema
const validate = ajv.compile<T>(this.schema);

if (!validate(json)) {
if (!this.ignoreValidation && !validate(json)) {
Copy link
Member

Choose a reason for hiding this comment

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

I would skip schema compilation since in the case of ingnoreValidation === true is completely useless.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mhh, in the case of ignoreValidation === true the part after && is no longer taken into account since the if cannot turn into true anymore?

Or did I misunderstand the comment?

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.#value = json;
return json;
return json as T;
}
}
34 changes: 34 additions & 0 deletions packages/core/test/ClientTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,18 @@ const myThingDesc = {
},
],
},
anAsyncAction: {
input: { type: "integer" },
output: { type: "integer" },
synchronous: false,
forms: [
{
href: "testdata://host/athing/actions/anasyncaction",
mediaType: "application/json",
response: { contentType: "application/json" },
},
],
},
},
events: {
anEvent: {
Expand Down Expand Up @@ -510,6 +522,28 @@ class WoTClientTest {
}
}

@test async "call an async action"() {
// should not throw Error: Invalid value according to DataSchema
WoTClientTest.clientFactory.setTrap(async (form: Form, content: Content) => {
const valueData = await content.toBuffer();
expect(valueData.toString()).to.equal("23");
return new Content("application/json", Readable.from(Buffer.from(JSON.stringify({ status: "pending" }))));
});
const td = (await WoTClientTest.WoTHelpers.fetch("td://foo")) as ThingDescription;

const thing = await WoTClientTest.WoT.consume(td);

expect(thing).to.have.property("title").that.equals("aThing");
expect(thing).to.have.property("actions").that.has.property("anAction");

// deal with ActionStatus object
const result = await thing.invokeAction("anAsyncAction", 23);
// eslint-disable-next-line no-unused-expressions
expect(result).not.to.be.null;
const value = await result?.value();
expect(value).to.have.property("status");
}

@test async "subscribe to event"() {
WoTClientTest.clientFactory.setTrap(() => {
return new Content("application/json", Readable.from(Buffer.from("triggered")));
Expand Down
25 changes: 25 additions & 0 deletions packages/core/test/InteractionOutputTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { expect, use } from "chai";
import { Readable } from "stream";
import { InteractionOutput } from "../src/interaction-output";
import { Content } from "..";
import { fail } from "assert";

use(promised);
const delay = (ms: number) => {
Expand Down Expand Up @@ -106,6 +107,30 @@ class InteractionOutputTests {
expect(result).be.true;
}

@test async "should fail returning unexpected value with no validation"() {
const stream = Readable.from(Buffer.from("not boolean", "utf-8"));
const content = new Content("application/json", stream);

const out = new InteractionOutput(content, {}, { type: "boolean" }); // ignoreValidation false by default
try {
const result = await out.value();
expect(result).be.true;
fail("Wrongly allows invalid value");
} catch {
// expected to throw
}
}

@test async "should accept returning unexpected value with no validation"() {
// type boolean should not throw since we set ignoreValidation to true
const stream = Readable.from(Buffer.from("not boolean", "utf-8"));
const content = new Content("application/json", stream);

const out = new InteractionOutput(content, {}, { type: "boolean" }, true);
const result = await out.value();
expect(result).to.eql("not boolean");
}

@test async "should data be used after arrayBuffer"() {
const stream = Readable.from(Buffer.from("true", "utf-8"));
const content = new Content("application/json", stream);
Expand Down
Loading