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

Design questions around the exploreDirectory method #1221

Open
JKRhb opened this issue Jan 19, 2024 · 6 comments
Open

Design questions around the exploreDirectory method #1221

JKRhb opened this issue Jan 19, 2024 · 6 comments

Comments

@JKRhb
Copy link
Member

JKRhb commented Jan 19, 2024

Recently, I've been dealing a lot with the exploreDirectory method from the Scripting API and its potential implementation. Although we've made some progress both in node-wot and dart_wot regarding the support of the method, I've been noticing at least two challenges for our current architecture that we need to deal with to have proper support in place.

Problem 1: No type in TDD things property

The first problem relates to our current approach of first consuming the Directory TD and then using the readProperty method to access the things property defined in the standardized API. While this has been working so far, the fix for the value() method in #1217 broke the implementation since calling the value() method with a data schema that has an undefined type field is supposed to throw and the things property does not have a type defined. While this is probably also an oversight in the Discovery specification, this does raise the question of whether we should loosen the requirements a bit, both in node-wot and the Scripting API (as also mentioned by @danielpeintner in #1216).

Problem 2: Passing of information from HTTP headers upstream

Another problem that has several implications relates to the fact that pagination information from TDDs is supposed to be transmitted via HTTP headers by default, namely the indication of the offset and limit that should be used for retrieving the next page of TDs. By default (i.e., when using the array format), the directory includes this information in a Link header with a URL that allows accessing the next page (an example would be /things?offset=10&limit=10).

Currently, we would need to control this behavior at the (HTTP) binding level, as there is no standardized way of mapping this header information to a DataSchema at the moment. I think this is a good example of one of the shortcomings mentioned in w3c/wot#1165. A potential alternative could be using the collection format by the way, where the information in question is moved from the HTTP headers to the response body. However, it seems to me as if a directory does not have to implement this format (if I am not mistaken), therefore we don't have the guarantee that this variant will work with every TDD.

The problem with passing the information upstream made me wonder whether we should create a dedicated method in the ProtocolClient interface to handle the exploreDirectory method. However, this would probably lead to a lot of binding implementations only having a method stub that just throws a "Not implemented" error :/

These are just a couple of observations I've made so far which can probably be the basis for further discussions, both on an implementation and use-case/specification level.

@danielpeintner
Copy link
Member

whether we should loosen the requirements a bit, both in node-wot and the Scripting API (as also mentioned by @danielpeintner in #1216).

@relu91 would you be opposed to such a change in the spec?

The problem with passing the information upstream made me wonder whether we should create a dedicated method in the ProtocolClient interface to handle the exploreDirectory method.

I would try to avoid doing that.
Do we think w3c/wot#1165 could lead to a solution, also for the Scripting API?

@relu91
Copy link
Member

relu91 commented Jan 19, 2024

@relu91 would you be opposed to such a change in the spec?

Yes as I mentioned the value function is behaving correctly. However, as I said in #1217 supporting TDD without schema is a TODO, it is just a matter of aligning the implementation with Example 3. I can provide a PR for this. I would say that updating the official TDD schema for things property is something we should do.

Do we think w3c/wot#1165 could lead to a solution, also for the Scripting API?

I hope so, when the times comes we have to remember to take this use-case into account.

@JKRhb
Copy link
Member Author

JKRhb commented Jan 19, 2024

However, as I said in #1217 supporting TDD without schema is a TODO, it is just a matter of aligning the implementation with Example 3.

Oh, sorry, you are right, I somehow did not take into account that you have already mentioned that 🙈 However, I am wondering whether requiring the type in a schema actually makes that much sense as this generally does not cover cases where we have multiple potential output values. For example, a DataSchema like

{
  "oneOf": [
    {
      "type": "string"
    },
    {
      "type": "number"
    }
  ]
}

could also not be supported at the moment. Do we really want users to implement their own parsing logic for cases like this one? A potential alternative could also be to add support for "subschemas" such as this one to the Scripting API if we want to keep certain restrictions in place.

Do we think w3c/wot#1165 could lead to a solution, also for the Scripting API?

I hope so, when the times comes we have to remember to take this use-case into account.

Maybe a workaround for the time being could be adding an (internal) field like additionalMetadata to the Content class where we could include data like certain HTTP headers. This could be replaced later with the approach worked out in the context of w3c/wot#1165. This would not be the nicest solution, but it clarifies what kind of metadata needs to be passed "upstream".

Edit: Thank you for opening w3c/wot-discovery#538, @relu91!

@relu91
Copy link
Member

relu91 commented Jan 19, 2024

For example, a DataSchema like

{
  "oneOf": [
    {
      "type": "string"
    },
    {
      "type": "number"
    }
  ]
}

could also not be supported at the moment. Do we really want users to implement their own parsing logic for cases like this one?

Why this is not supported? 🤔

@JKRhb
Copy link
Member Author

JKRhb commented Jan 19, 2024

For example, a DataSchema like

{
  "oneOf": [
    {
      "type": "string"
    },
    {
      "type": "number"
    }
  ]
}

could also not be supported at the moment. Do we really want users to implement their own parsing logic for cases like this one?

Why this is not supported? 🤔

If the type field in a schema is not defined, then the current algorithm demands that an error be thrown, which also corresponds with the updated implementation:

if (this.schema == null || this.schema.type == null) {
throw new NotReadableError("No schema defined");
}

@relu91
Copy link
Member

relu91 commented Jan 19, 2024

Ah gotcha, I see the loophole, but this is just to the fact the we failed to express that the only requirement is that the DataSchema exists and is valid. This is something we have to fix in the ScriptingAPI doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants