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.

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

ProtocolClient Interface does not allow to specify the affordance name, if the information is not contained in the Form #1342

Open
RobWin opened this issue Dec 19, 2024 · 10 comments
Labels
core Issues with the core library enhancement New feature or request

Comments

@RobWin
Copy link

RobWin commented Dec 19, 2024

The implementation of the ProtocolClient methods in node-wot assume that all the information needed to read a property, invoke an action or subscribe to an event is contained in a Form. See https://github.com/eclipse-thingweb/node-wot/blob/master/packages/core/src/protocol-interfaces.ts#L48
When implementing a websocket binding based on the Webthingprotocol, we saw that this is not the case for all protocols.
We had a long discussion on Discord about it. And in the Webthingprotocol spec the form href will not contain a property name or action name, so we would need to overload ProtocolClient methods with additional versions which also take the names as an input parameter.

@RobWin RobWin changed the title ProtocolClient Interface does not allow to specify the propertyName, actionName or eventName, if the information is not contained in the Form ProtocolClient Interface does not allow to specify the affordance name, if the information is not contained in the Form Dec 19, 2024
@danielpeintner
Copy link
Member

It seems that the readResource() / writeResource() / invokeResource() / invokeResource() / unlinkResource() method calls could be extended to hand in the interaction name.

Having said that, I wonder what is the best way forward with the following aspects in mind:

  • most bindings don't need it..
  • failure cases: property names might be different.. What should happen?
  • backward compatibility (adding and optional argument or require this new argument)
    • if optional, failing if needed?
    • more aspects?
  • is the name of an interaction enough, or might we need to add more (maybe in the future?) --> adding an (optional) object instead?

Any opinions?

@danielpeintner danielpeintner added enhancement New feature or request core Issues with the core library labels Dec 19, 2024
@RobWin
Copy link
Author

RobWin commented Dec 19, 2024

In kotlin-wot I modified ConsumedThing.readProperty to call client.readResource(affordanceName, form).
In the ProtocolClient I added a new overloaded method with a default implementation.

  suspend fun readResource(resourceName: String, form: Form): Content {
      return readResource(form)
  }

Thus I didn't have to modify existing bindings, but can override this new method in the WebsocketProtocolClient.

But Typescript cannot have default implementations in interfaces or?

But I agree a single Resource object as an input parameter would be more flexible to extend, but it would introduce a lot of optional fields. But that would be fine, since the ProtocolClient implementation could use the fields it requires.
The Resource object could contain a name, form and in the future additional information. Maybe tracing context metadata which needs to be added to a protocol request?
Right now the protocol client does not allow a ConsumedThing implementation to attach additional tracing context to a protocol communication.
I actually prefer to have an object as an input parameter instead of a list of individial parameters 👍

@JKRhb
Copy link
Member

JKRhb commented Dec 19, 2024

In dart_wot, I actually had a similar problem and solved it (for now) using an additional AugmentedForm class, that contains references to both the related interaction affordance and Thing Description. Having a best practice here across implementations would definitely be nice.

@RobWin
Copy link
Author

RobWin commented Dec 19, 2024

Indeed, we need an object (or AugmentedForm), because the thingId is also required in the WebthingProtocol websocket messages.

@danielpeintner
Copy link
Member

@relu91 do you have additional opinions?

I kind of like the proposal from @JKRhb to hand over an augmented form.
I do see the rationale for the "interaction affordance" part. I wonder why you needed the full TD? @JKRhb do you need parts outside the interaction? In node-wot it shouldn't be needed (I think) since the href etc. is converted to absolute hrefs... or do I miss anything?

@relu91
Copy link
Member

relu91 commented Jan 10, 2025

TL;DR: Forms are the source of truth for protocol bindings. Extending ProtocolInterface violates this principle, so we won’t pursue that path. Instead, the WebThing protocol should have its own binding defining the property name, and verbosity could be addressed through preprocessing or lazy processing at runtime.

Apologies for the delayed response! Here's a summary of today's discussion during the committers' call:

In node-wot (and WoT), we emphasize a clear separation between protocol-specific logic (protocol realm) and application-dependent concerns. During the call, we agreed that forms should serve as the definitive source of truth for protocol bindings. They must include all the necessary information to construct requests. Extending the ProtocolInterface would violate this architectural principle and potentially introduce further complications down the line. For this reason, we decided not to pursue that direction.

We’re still interested in hearing from @JKRhb about the specific need for passing additional context information to the protocol interface.

Regarding the WebThing protocol, it should have its own dedicated protocol binding (which we can incubate) to define how property names or other affordance identifiers should be expressed. This approach would also allow affordance names to differ from protocol-level identifiers. To address verbosity, we discussed implementing a default fill preprocessing step at the consume time in node-wot (or other runtimes). This step would let protocol implementations augment forms with any required context, if needed. Lazy processing could also be explored as an alternative but we have to keep an eye on the getThingDescription method which should return the correct td with all the filled in defaults.

@RobWin
Copy link
Author

RobWin commented Jan 10, 2025

I'm not sure I understand you correctly. There are protocols where the current form schema and href might not be sufficient to describe everything needed for communication.

Do you mean that a protocol binding can extend the form by adding protocol-specific extensions at runtime, but not in the Thing Description itself, correct?
But the protocol binding is implementing the ProtocolClient and the data to augument the form is not available in the binding implementation, but only in the Serviant code. Or do I missunderstand something,

@relu91
Copy link
Member

relu91 commented Jan 10, 2025

Let's try with a possible example. In theory, we should have a protocol binding document for Webthing protocol. Let's assume that in there they define that to describe the configuration in forms you must use wtv:affordanceName. Therefore, all valid forms that uses the webthing protocol binding should look like:

{
  "href": "https://websocket.endpoint"
  "wtv:affordanceName": "propertyA"
}

If people are ok with this we don't have to do anything as the Form instance contains all the information needed. However, one of the most common complains for ThingDescriptions is that it is too verbose. Therefore, the protocol binding document could say that wtv:affordanceName is optional and its default is equal to the name of the affordance where the form is defined.

To support this default filling we can preprocess the TD and ask the protocol binding implementation (which knows the defaults rules) to fill the missing fields. I know that in the end the result is that the protocol binding would need to access application level information, but at least we separate the concerns and make clear that the form is the only source when processing a WoT operation request.

@danielpeintner
Copy link
Member

danielpeintner commented Jan 10, 2025

To make it a bit more practical ... which might help to understand what we propose to do for node-wot (and maybe other implementation can do the same?).

Our assumption is that handing in a form for a read request should be sufficient (just like we do today in https://github.com/eclipse-thingweb/node-wot/blob/d534535d68d2583a04e5d930a9aeba28e94cef14/packages/core/src/protocol-interfaces.ts#L50C18-L50C28).

If a given protocol needs additional information (such as the name or any other information) it can be added in a protocol-specific pre-processing step. This step is protocol dependent and might even be a void operation in most of the cases.

@RobWin let's get back to the example you raised about the need to know the interaction name.

We plan to extend ProtocolClient with something like the following

export interface ProtocolClient {
    /** update the form with additional information (if needed) */
    enhanceForm(form: Form, ...): Promise<void>;
}

node-wot will go over the forms and pass it to the according protocol implementation and ask each binding to add additional information (if needed). As said, in most of the cases it will be a no-op. In the case where the name is needed, you can add this information to the form.

A subsequent readResource(form: Form) will hand in the form you enhanced before. So it is up to each binding to do a proper job.

I hope this explains a bit our current thinking.
Note: at the moment it is just an idea, and we don't know yet how the enhancement process might look like exactly.

Does this make sense?

EDIT: As a side-node. We do already something similar with relative hrefs. At the moment, node-wot updates each form from a relative URL to an absolute URL. Otherwise, we would need to hand in base every time and every time a resource is read, one needs to create the absolute href etc.

@JKRhb
Copy link
Member

JKRhb commented Jan 11, 2025

We’re still interested in hearing from @JKRhb about the specific need for passing additional context information to the protocol interface.

I now finally found the time to revisit my implementation and the reason I added the AugmentedForm class and I think the main context information that motivated was actually the TD's ID which I intended to use in callbacks for passing in credentials to the binding implementation. However, I noticed that this information is actually not really necessary or that it is more useful to map the credentials to host names/base URLs.

A more important aspect in practice, though, is the exposure of securityDefinitions to the binding implementations, which makes it unnecessary to have a setSecurity method on the protocol interfaces (as it is currently the case in node-wot), making the interaction methods act solely on the fields of the form that has been passed. Other aspects that resolved by the AugmentedForm are the href, security, and uriVariables fields, but this could also be done via a constructor on the original Form class that accepts another Form and a ThingDescription. In the case of the security definitions, however, an extension of the original Form class is currently necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues with the core library enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants