Skip to content

Support ID-based CoAP paths for exposed Things #1490

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

Open
Pranav-0440 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support ID-based CoAP paths for exposed Things #1490

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
96489f8
feat(binding-coap): support id-based CoAP paths (fixes #1458)
Pranav-0440 Feb 17, 2026
4f19e39
chore(binding-coap): address review feedback and improve coverage
Pranav-0440 Feb 18, 2026
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
63 changes: 40 additions & 23 deletions packages/binding-coap/src/coap-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export default class CoapServer implements ProtocolServer {

private readonly port: number;
private readonly address?: string;
private readonly devFriendlyUri: boolean;

private mdnsIntroducer?: MdnsIntroducer;

Expand All @@ -84,6 +85,7 @@ export default class CoapServer implements ProtocolServer {
constructor(config?: CoapServerConfig) {
this.port = config?.port ?? 5683;
this.address = config?.address;
this.devFriendlyUri = config?.devFriendlyUri ?? true;

// WoT-specific content formats
registerFormat(ContentSerdes.JSON_LD, 2100);
Expand Down Expand Up @@ -143,34 +145,44 @@ export default class CoapServer implements ProtocolServer {

public async expose(thing: ExposedThing, tdTemplate?: WoT.ExposedThingInit): Promise<void> {
const port = this.getPort();
const urlPath = this.createThingUrlPath(thing);
const paths = this.createThingUrlPaths(thing);
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

Suggested change
const paths = this.createThingUrlPaths(thing);
const urlPaths = this.createThingUrlPaths(thing);


if (port === -1) {
warn("CoapServer is assigned an invalid port, aborting expose process.");
return;
}

this.fillInBindingData(thing, port, urlPath);
for (const urlPath of paths) {
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

Suggested change
for (const urlPath of paths) {
for (const urlPath of urlPaths) {

this.fillInBindingData(thing, port, urlPath);

debug(`CoapServer on port ${port} exposes '${thing.title}' as unique '/${urlPath}'`);
debug(`CoapServer on port ${port} exposes '${thing.title}' as unique '/${urlPath}'`);

this.setUpIntroductionMethods(thing, urlPath, port);
this.setUpIntroductionMethods(thing, urlPath, port);
}
}

private createThingUrlPath(thing: ExposedThing) {
let urlPath = slugify(thing.title, { lower: true });
private createThingUrlPaths(thing: ExposedThing): string[] {
const paths: string[] = [];
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

Suggested change
const paths: string[] = [];
const urlPaths: string[] = [];

// Title-based path
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

Consider adding a comment explaining why the title-based path is created even when devFriendlyUri is false if no ID is provided, similar to the explanatory comment in the HTTP binding at http-server.ts:274. This helps future maintainers understand the fallback behavior for Things without IDs.

Suggested change
// Title-based path
// Title-based path
// Even when devFriendlyUri is false we still create a title-based path if the Thing has no ID.
// This mirrors the HTTP binding behavior and ensures that Things without IDs can still be exposed
// under a stable, discoverable path derived from their title.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

I think this comment could rather be used as the basis for a doc comment for the function. Generally speaking, though, I don't think the mirroring of the HTTP binding needs to be mentioned that often.

if (this.devFriendlyUri || thing.id == null) {
let urlPath = slugify(thing.title, { lower: true });

// avoid URL clashes
if (this.things.has(urlPath)) {
let uniqueUrlPath;
let nameClashCnt = 2;
do {
uniqueUrlPath = urlPath + "_" + nameClashCnt++;
} while (this.things.has(uniqueUrlPath));
urlPath = uniqueUrlPath;
}
if (this.things.has(urlPath)) {
let uniqueUrlPath;
let nameClashCnt = 2;
do {
uniqueUrlPath = urlPath + "_" + nameClashCnt++;
} while (this.things.has(uniqueUrlPath));
urlPath = uniqueUrlPath;
}
Comment on lines +179 to +186
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

Hmm. I was wondering whether we could end up in the following situation here:

  • Starting point: two (partial) TDs have the same title, but only the second one has an ID
  • For the second TD, a nameclash regarding the title is detected because the first TD did not have an ID and therefore an identical URL path
  • The title of the second TD is appended with a _2
  • The urlPath of the second ID is further expanded by the ID

This is certainly an edge case (I am not actually not certain at the moment whether it is possible to have a missing ID at this point or whether that one is going to be set by the Servient), but I still wanted to mention it here.


return urlPath;
paths.push(urlPath);
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

Suggested change
paths.push(urlPath);
urlPaths.push(urlPath);

}
// ID-based path
if (typeof thing.id === "string" && thing.id.length > 0) {
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The ID-based path registration doesn't check for collisions with existing paths. If a Thing's ID matches another Thing's title-based path, it will silently overwrite the existing registration. Consider adding collision detection for ID paths or at least logging a warning when an ID path overwrites an existing path, similar to how title-based paths handle collisions with the numeric suffix pattern.

Suggested change
if (typeof thing.id === "string" && thing.id.length > 0) {
if (typeof thing.id === "string" && thing.id.length > 0) {
if (this.things.has(thing.id)) {
warn(
`CoapServer path collision detected: Thing with id '${thing.id}' will overwrite existing Thing registration at path '/${thing.id}'.`
);
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

Oh, that is actually another interesting observation.

paths.push(thing.id);
}
return paths;
}

private fillInBindingData(thing: ExposedThing, port: number, urlPath: string) {
Expand Down Expand Up @@ -354,20 +366,25 @@ export default class CoapServer implements ProtocolServer {

public async destroy(thingId: string): Promise<boolean> {
debug(`CoapServer on port ${this.getPort()} destroying thingId '${thingId}'`);
for (const name of this.things.keys()) {
const exposedThing = this.things.get(name);

let deleted = false;

for (const [name, exposedThing] of this.things.entries()) {
if (exposedThing?.id === thingId) {
this.things.delete(name);
this.coreResources.delete(name);
this.mdnsIntroducer?.delete(name);

info(`CoapServer successfully destroyed '${exposedThing.title}'`);
return true;
deleted = true;
}
}

info(`CoapServer failed to destroy thing with thingId '${thingId}'`);
return false;
if (deleted) {
info(`CoapServer successfully destroyed thing with id '${thingId}'`);
} else {
info(`CoapServer failed to destroy thing with thingId '${thingId}'`);
}

return deleted;
}

private formatCoreLinkFormatResources() {
Expand Down
1 change: 1 addition & 0 deletions packages/binding-coap/src/coap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export * from "./coaps-client";
export interface CoapServerConfig {
port?: number;
address?: string;
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

Consider adding JSDoc documentation for the devFriendlyUri configuration option to explain its purpose and behavior. Similar to the HTTP binding, this option controls whether title-based paths are generated (true by default for backward compatibility). When false and a Thing has an ID, only ID-based paths are created.

Suggested change
address?: string;
address?: string;
/**
* Controls whether title-based CoAP resource paths are generated.
*
* Similar to the HTTP binding, this option is `true` by default for backward
* compatibility, so that Things are exposed using paths derived from their
* titles.
*
* When set to `false` and a Thing has an ID, only ID-based paths are created
* for that Thing.
*/

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

The reference to the HTTP binding could be removed here.

devFriendlyUri?: boolean;
}

export type CoapMethodName = "GET" | "POST" | "PUT" | "DELETE" | "FETCH" | "PATCH" | "iPATCH";
Expand Down
36 changes: 36 additions & 0 deletions packages/binding-coap/test/coap-server-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,42 @@ class CoapServerTest {

await coapServer.stop();
}
@test async "should expose Thing with id and title and be reachable from both"() {
const coapServer = new CoapServer({ port: PORT });
await coapServer.start(new Servient());

const testThing = new ExposedThing(new Servient(), {
title: "TestThing",
id: "urn:dev:wot:test-thing-1234",
properties: {
test: {
type: "string",
},
},
});

testThing.setPropertyReadHandler("test", async () => "OK");

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.test.forms = [];

await coapServer.expose(testThing);

const uriByTitle = `coap://localhost:${coapServer.getPort()}/testthing/properties/test`;
const uriById = `coap://localhost:${coapServer.getPort()}/urn:dev:wot:test-thing-1234/properties/test`;

const coapClient = new CoapClient(coapServer);

const resp1 = await coapClient.readResource(new Form(uriByTitle));
expect((await resp1.toBuffer()).toString()).to.equal('"OK"');

const resp2 = await coapClient.readResource(new Form(uriById));
expect((await resp2.toBuffer()).toString()).to.equal('"OK"');

await coapClient.stop();
await coapServer.stop();
}

Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The test validates dual-path registration but doesn't cover the case where devFriendlyUri is set to false. Consider adding a test case that verifies when devFriendlyUri is false and a Thing has an ID, only the ID-based path is registered (not the title-based path).

Suggested change
@test async "should expose Thing with id and title and be reachable only by id when devFriendlyUri is false"() {
const coapServer = new CoapServer({ port: PORT, devFriendlyUri: false });
await coapServer.start(new Servient());
const testThing = new ExposedThing(new Servient(), {
title: "TestThing",
id: "urn:dev:wot:test-thing-1234",
properties: {
test: {
type: "string",
},
},
});
testThing.setPropertyReadHandler("test", async () => "OK");
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
testThing.properties.test.forms = [];
await coapServer.expose(testThing);
const uriByTitle = `coap://localhost:${coapServer.getPort()}/testthing/properties/test`;
const uriById = `coap://localhost:${coapServer.getPort()}/urn:dev:wot:test-thing-1234/properties/test`;
const coapClient = new CoapClient(coapServer);
// ID-based path should be reachable
const respById = await coapClient.readResource(new Form(uriById));
expect((await respById.toBuffer()).toString()).to.equal('"OK"');
// Title-based path should not be registered when devFriendlyUri is false
const respByTitle = await new Promise<IncomingMessage>((resolve) => {
const req = request(uriByTitle);
req.on("response", (res) => resolve(res));
req.end();
});
expect(respByTitle.code).to.equal("4.04");
await coapClient.stop();
await coapServer.stop();
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@JKRhb JKRhb Feb 18, 2026

Choose a reason for hiding this comment

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

Seems like a valid addition at first glance.

@test async "should write property"() {
const coapServer = new CoapServer({ port: PORT });
Expand Down
Loading