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

fix: report allproperties with proper type #985

Merged
merged 2 commits into from
May 11, 2023
Merged

fix: report allproperties with proper type #985

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76d1cc0
fix: report allproperties with proper type
danielpeintner May 9, 2023
d1f2e80
test(binding-http): better naming for allproperties test
relu91 May 11, 2023
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
9 changes: 6 additions & 3 deletions packages/binding-http/src/http-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,13 +758,16 @@ export default class HttpServer implements ProtocolServer {
const propMap: PropertyContentMap = await thing.handleReadAllProperties({
formIndex: 0,
});
res.setHeader("Content-Type", "application/json"); // contentType handling?
res.setHeader("Content-Type", ContentSerdes.DEFAULT); // contentType handling?
res.writeHead(200);
const recordResponse: Record<string, unknown> = {};
for (const key of propMap.keys()) {
const content: Content = propMap.get(key);
const data = await content.toBuffer();
recordResponse[key] = data.toString();
const value = ContentSerdes.get().contentToValue(
{ type: ContentSerdes.DEFAULT, body: await content.toBuffer() },
{}
);
recordResponse[key] = value;
}
res.end(JSON.stringify(recordResponse));
} catch (err) {
Expand Down
102 changes: 98 additions & 4 deletions packages/binding-http/test/http-server-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ class HttpServerTest {
resp = await (await fetch(uri + "properties/test")).text();
expect(resp).to.equal('"off"');

resp = await (await fetch(uri + "properties")).text();
expect(resp).to.equal('{"test":"\\"off\\""}');
resp = await (await fetch(uri + "properties")).json();
expect(resp).to.deep.equal({ test: "off" });

resp = await (await fetch(uri + "properties/test", { method: "PUT", body: "on" })).text();
expect(resp).to.equal("");
Expand Down Expand Up @@ -271,8 +271,8 @@ class HttpServerTest {
resp = await (await fetch(uri + "properties/test")).text();
expect(resp).to.equal("{}");

resp = await (await fetch(uri + "properties")).text();
expect(resp).to.equal('{"test":"{}"}');
resp = await (await fetch(uri + "properties")).json();
expect(resp).to.deep.equal({ test: {} });

resp = await (
await fetch(uri + "properties/test", {
Expand Down Expand Up @@ -549,6 +549,100 @@ class HttpServerTest {
return httpServer.stop();
}

@test async "should report allproperties"() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@test async "should report allproperties"() {
@test async "should report allproperties excluding no json properties"() {

or create two tests with one with only json properties and one like the current version.

Copy link
Member Author

@danielpeintner danielpeintner May 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I had to re-read it multiple times to get it right.. but I think the addition should read
"excluding non-JSON properties"
@relu91 what do you think?

see additional proposed change below

relu91 marked this conversation as resolved.
Show resolved Hide resolved
const httpServer = new HttpServer({ port: 0 });

await httpServer.start(null);

const tdTemplate: WoT.ExposedThingInit = {
title: "TestA",
properties: {
image: {
forms: [
{
contentType: "image/svg+xml",
},
],
},
testInteger: {
type: "integer",
},
testBoolean: {
type: "boolean",
},
testString: {
type: "string",
},
testObject: {
type: "object",
},
testArray: {
type: "array",
},
},
};
const testThing = new ExposedThing(null, tdTemplate);

const image = "<svg xmlns='http://www.w3.org/2000/svg'><text>FOO</text></svg>";
const integer = 123;
const boolean = true;
const string = "ABCD";
const object = { t1: "xyz", i: 77 };
const array = ["x", "y", "z"];
testThing.setPropertyReadHandler("image", (_) => Promise.resolve(image));
testThing.setPropertyReadHandler("testInteger", (_) => Promise.resolve(integer));
testThing.setPropertyReadHandler("testBoolean", (_) => Promise.resolve(boolean));
testThing.setPropertyReadHandler("testString", (_) => Promise.resolve(string));
testThing.setPropertyReadHandler("testObject", (_) => Promise.resolve(object));
testThing.setPropertyReadHandler("testArray", (_) => Promise.resolve(array));

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.image.forms = [];
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.testInteger.forms = [];
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.testBoolean.forms = [];
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.testString.forms = [];
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.testObject.forms = [];
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.testArray.forms = [];

await httpServer.expose(testThing, tdTemplate);

// check values one by one first
const responseInteger = await fetch(`http://localhost:${httpServer.getPort()}/testa/properties/testInteger`);
expect(await responseInteger.json()).to.equal(integer);
const responseBoolean = await fetch(`http://localhost:${httpServer.getPort()}/testa/properties/testBoolean`);
expect(await responseBoolean.json()).to.equal(boolean);
const responseString = await fetch(`http://localhost:${httpServer.getPort()}/testa/properties/testString`);
expect(await responseString.json()).to.equal(string);
const responseObject = await fetch(`http://localhost:${httpServer.getPort()}/testa/properties/testObject`);
expect(await responseObject.json()).to.deep.equal(object);
const responseArray = await fetch(`http://localhost:${httpServer.getPort()}/testa/properties/testArray`);
expect(await responseArray.json()).to.deep.equal(array);

// check values of readallproperties
const responseAll = await fetch(`http://localhost:${httpServer.getPort()}/testa/properties`);
expect(await responseAll.json()).to.deep.equal({
// "image": image, // Note: No support for contentTypes other than JSON -> not included
testInteger: integer,
testBoolean: boolean,
testString: string,
testObject: object,
testArray: array,
});

return httpServer.stop();
}

@test async "should support setting SVG contentType"() {
const httpServer = new HttpServer({ port: 0 });

Expand Down
31 changes: 12 additions & 19 deletions packages/core/src/exposed-thing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { InteractionOutput } from "./interaction-output";
import { Readable } from "stream";
import ProtocolHelpers from "./protocol-helpers";
import { ReadableStream as PolyfillStream } from "web-streams-polyfill/ponyfill/es2018";
import { Content, PropertyContentMap } from "./core";
import { Content, ContentSerdes, PropertyContentMap } from "./core";
import ContentManager from "./content-serdes";
import {
ActionHandlerMap,
Expand Down Expand Up @@ -422,26 +422,19 @@ export default class ExposedThing extends TD.Thing implements WoT.ExposedThing {
propertyNames: string[],
options: WoT.InteractionOptions & { formIndex: number }
): Promise<PropertyContentMap> {
// collect all single promises into array
const promises: Promise<Content>[] = [];
for (const propertyName of propertyNames) {
// Note: currently only DataSchema properties are supported
const form = this.properties[propertyName].forms.find(
(form) => form.contentType === "application/json" || !form.contentType
);
if (!form) {
continue;
}

promises.push(this.handleReadProperty(propertyName, options));
}
try {
// wait for all promises to succeed and create response
// collect all single promises
const output = new Map<string, Content>();
const results = await Promise.all(promises);

for (let i = 0; i < results.length; i++) {
output.set(propertyNames[i], results[i]);
for (const propertyName of propertyNames) {
// Note: currently only JSON DataSchema properties are supported
const form = this.properties[propertyName].forms.find(
(form) => form.contentType === ContentSerdes.DEFAULT || !form.contentType
);
if (!form) {
continue;
}
const contentResponse = await this.handleReadProperty(propertyName, options);
output.set(propertyName, contentResponse);
}
return output;
} catch (error) {
Expand Down