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

Consider adding DataSchema to response and additionalResponses #1053

Open
mmccool opened this issue Feb 17, 2021 · 21 comments
Open

Consider adding DataSchema to response and additionalResponses #1053

mmccool opened this issue Feb 17, 2021 · 21 comments
Assignees
Labels
Defer to TD 2.0 Needed by other TF An issue or UC from another TF to fullfill a requirement in their spec or gap PR needed

Comments

@mmccool
Copy link
Contributor

mmccool commented Feb 17, 2021

When there are additionalResponses, and maybe also when there is just a "response", the output DataSchema may not be the same as the input DataSchema. For example, even when getting a property, an error response may not contain the property but an error code and message instead.

However, right now ExpectedResponse only has a ContentType. This is OK if the response is unstructured (eg JPEG image), but it the response is JSON or XML a DataSchema is needed.

I propose adding an optional "data" field with type "DataSchema" to both "response" and "additionalResponses"

@mmccool
Copy link
Contributor Author

mmccool commented Feb 17, 2021

One problem: actions already have an "input" and "output" data schema, so adding another data schema inside "response" would be redundant and confusing. We can just avoid using it for properties since if the response is not the same as the input then we can just call it an action. So we could do it only in additionalResponses, but then I would have to create a subclass of ExpectedResponse in the ontology, etc. to model this.

@sebastiankb
Copy link
Contributor

from today's TD call: @mjkoster can support here since they had the same issue in OneDM

@mjkoster
Copy link
Contributor

mjkoster commented Feb 17, 2021

It seems like the processing exception (error report) case is the most important to allow additional response types, where "the usual programming is interrupted" and a different payload is sent, for example in the case of Modbus errors. The important distinction is that there is special header information like a response code that tells the consumer to look for the special data payload.

In the case of diverse payloads that occur "normally", the anyOf construct might be more suitable.

In other cases, we want to model optionality in the TM but will choose one for the instance and TD.

@farshidtz
Copy link
Member

One problem: actions already have an "input" and "output" data schema, so adding another data schema inside "response" would be redundant and confusing.

Also, events already have a data field for output data schema, equivalent to actions' output.

Seems like all interactions need a schema for the default response and this could go into Interaction Affordance, deprecating event data and action output.

@mjkoster
Copy link
Contributor

mjkoster commented Mar 7, 2021

I'd like to consider the logical structures here, and try to drive to a concrete proposal.

There are input and output schemas for actions, and data schemas for properties. Events have a notification schema. These schemas are all part of the application data model for interactions. Only the formatting is expected to change when the protocol binding changes (array vs. object for multiple items, key names, contentType...).

Subscription schemas and cancellation schemas for events are arguably part of the protocol binding, because they would be different for different protocols. I recommend that we try to work subscription and cancellation into these schema extensions for forms, also bc that's where the ops for them are defined.

As for request and response, these are also protocol artifacts. NB for some protocols, the action output data will be packaged in the response that goes with the request that sends the input data. For other protocols, the action output data will not be in the response. Likewise, sometimes the response will contain a location from which to get the output data or status data for long running actions. We may also need a separate getStatus op and schema for these. The response may contain different information in the case of errors, as we note elsewhere.

I recommend we not try to collapse the action output and event notification schemas into responses, but we do need to provide a way to indicate that a response contains action output data or event notification data for those protocols where they are packed into responses. Putting the contentType and location, plus any other header information, is also a logical choice for response definitions.

It might be a good practice to specify response/ExpectedResponse only for the cases that return application data specified in the dataschema, output, or notification, and always use additionalResponses for errors and exceptions. I think MMC has proposed along these lines. Then responses would never contain schemas when there is a dataschema, or output or notification schema in the interaction definition.

Then, additionalResponses could always be used for error cases or where alternate control schemas are needed. A protocol may have an observe or subscribe response that isn't part of the application data model, so we could use additionalResponses to indicate that case. For example, the new CoAP pubsub protocol passes control documents before any data are available.

Finally, we could extend requests in forms to accommodate subscription and cancellation, and other cases like aborting a long-running action; maybe additionalRequests would be an appropriate vocabulary term here, since they would need to have bespoke payload schemas like additionalResponses do.

To recall another discussion, if there are cases where there are multiple schemas as normal data model options, these will be specified in the interaction definition using an optionality construct like "anyOf" or sdfChoice type pattern.

We could sketch up some examples if any of this is not well enough defined.

PS. This is a good place to mention that my original protocol binding strawman from Santa Clara 2017 had requests and responses with all of the on-the-air schemas in the binding section.

@egekorkan
Copy link
Contributor

Subscription schemas and cancellation schemas for events are arguably part of the protocol binding, because they would be different for different protocols. I recommend that we try to work subscription and cancellation into these schema extensions for forms, also bc that's where the ops for them are defined.

There is a related issue in Scripting: w3c/wot-scripting-api#208

I also agree that we can deprecate these two fields since even the TDs provided that use them are actually using WebSub and putting the payload descriptions actually does not help with the implementation unless there is a mention of WebSub somewhere (in subprotocols) since the consumer needs to keep track of some parts of the payload which are not related to the application logic (where scripting API lives) but should be actually handled by the implementation.

@relu91
Copy link
Member

relu91 commented Mar 8, 2021

I too tend to agree that what should be handled transparently by the runtime should not be exposed by the TD. However, about the subscription payload, I had a use case in mind. TDDs you can have events about added or removed Thing Descriptions. Therefore one client can subscribe to one or the other to obtain useful information about what is currently available in its environment. Translating that to a TD is pretty straightforward:

{
    /*other TD fields */
    "events": {
        "added":{
            "title": "Thing Description added",
            "description": "Fired every time a new TD is added to the TDD"
        },
        "removed":{
            "title": "Thing Description removed",
            "description": "Fired every time a new TD is removed from the TDD",
        }
    }
}

Now consider that I can use JSONPath to filter the notifications and be notified only for things with @type : "sosa:Sensor". How could I describe this use case without the subscription field? I think this is strong evidence that there might be subscriptions that depend on applciation data rather than just protocol configuration and prelude.

Not sure if we can say the same about cancellation.

concerning this issue:
I think the current approach of the TD is to keep protocol and logical interaction levels separated. So what it is really wired is that with additionalResponses we are mixing protocols and DataSchemas, concepts that are kept in different abstract categories so far (e.g. we have a response and we have output).

Since we are gradually introducing JSONPointers in TDs it might worth exploring the idea of having additionalSchemas and point to one named schema from additionalResponses or simply pointing to a portion of the output schema without introducing a new field.

@egekorkan
Copy link
Contributor

PS. This is a good place to mention that my original protocol binding strawman from Santa Clara 2017 had requests and responses with all of the on-the-air schemas in the binding section.

@mjkoster could you share it in this issue?

@sebastiankb
Copy link
Contributor

@mjkoster

I recommend we not try to collapse the action output and event notification schemas into responses, but we do need to provide a way to indicate that a response contains action output data or event notification data for those protocols where they are packed into responses. Putting the contentType and location, plus any other header information, is also a logical choice for response definitions.

+1

I think, a good way is to simple define multiple schemas with anyOf and a JSON Pointer points to a schema entry. Maybe a more clean way can be that we use a new data schema field like additionalSchemas at interaction level (similar we have for uriVariable):

    "properties": {
        "foo": {
            "type": "string",            // for default response
            "additionalSchemas": {
                "anyOf": [
                    {"type": "string"},  // for additional response I
                    {"type": "number"}   // for additional response II
                ]
            }
        },
        "forms": [
            {

                "href": "https://mylamp.example.com/status",
                // for default response
                "contentType" : "text/plain;charset=UTF-8",
                
                // for additional response I and II
                 "additionalResponses" : [{
                    "contentType" : "application/json",
                    "schemaPointer" : "$/properties/foo/additionalSchemas[0]"
                },
                {
                    "contentType" : "application/json",
                    "schemaPointer" : "$/properties/foo/additionalSchemas[1]"
                }
                ]
            }
        ]
    }

@egekorkan
Copy link
Contributor

I like the above proposal. Just that I don't think we need to use anyOf in additionalSchemas, the value of additionalSchemas can simply be an array or an object. In the object case, we can predefine some possible values.

Also while we are at it, we can solve this issue (I cannot find the exact one) where a platform or protocol can contain predefined schema constructs that are reproduced in every interaction. Since that needed use of JSON Pointers.

@sebastiankb
Copy link
Contributor

sebastiankb commented Mar 10, 2021

From today's TD call:

  • we discussed this example
  • the $ have to be removed since this is related to JSON Schema
  • the pointer approach should be similar as applied for security
  • the additionalSchemas term should be introduced at the interaction level
  • improved version can be that the additionalSchema is a map:
    "properties": {
        "foo": {
            "type": "string",                           // for default response
            "additionalSchemas": {
                "additionalRes_I":  {"type": "string"},   // for additional response I
                "additionalRes_II"  {"type": "number"}    // for additional response II
            }
        },
        "forms": [
            {

                "href": "https://mylamp.example.com/status",
                // for default response
                "contentType" : "text/plain;charset=UTF-8",
                
                // for additional response I and II
                 "additionalResponses" : [{
                    "contentType" : "application/json",
                    "schema" : "additionalRes_I"
                },
                {
                    "contentType" : "application/json",
                    "schema" : "additionalRes_II"
                }
                ]
            }
        ]
    }

@mmccool
Copy link
Contributor Author

mmccool commented Apr 5, 2021

So I am blocked on making a PR for this. I wanted to cut-and-paste the mechanism used for securityDefinitions BUT somehow that got broken! When it has been fixed I can proceed.

Again, I want to introduce a Map, perhaps "schemaDefinitions", that define some named data schemas that can be used in additional Responses.

One question in general though: is this ever useful in other places?

@benfrancis
Copy link
Member

schemaDefinitions appears to have landed in 100f0de. Is this feature now considered complete?

I have added an example using schemaDefinitions to describe asynchronous action requests in #302 (comment). This example supports the idea of separating the output schema of an action from the response schema of a Form, since the initial response to the invokeaction request doesn't include the output of the action. It therefore also suggests that the schema member might be useful for an ExpectedResponse as well as an AdditionalResponse, which isn't currently included in the specification.

@egekorkan
Copy link
Contributor

@benfrancis schemaDefinitions is not currently complete but it will be very soon. I am preparing a PR for it to be used anywhere

@egekorkan egekorkan added the Propose closing Problem will be closed shortly if there is no veto. label Oct 14, 2021
@benfrancis
Copy link
Member

Just to follow up on this, on several calls we have discussed adding the schema member from AdditionalResponse to ExpectedResponse as well. As far as I can see this hasn't been done yet.

This would be needed in order to describe the actions protocol binding from the Core Profile in a Thing Description for example, see #302 (comment)

@benfrancis
Copy link
Member

I'm re-opening this and marking as deferred until TD 2.0, since #1341 was deferred rather than being merged.

Please close again if I mis-understood.

I think something like this is still needed, e.g. for defining a schema for a response separate from an action output, see w3c/wot-profile#259.

@danielpeintner
Copy link
Contributor

I'm re-opening this and marking as deferred until TD 2.0, since #1341 was deferred rather than being merged.

Note: I am re-opening the issue since only the label was added...

@sebastiankb
Copy link
Contributor

from today's TD call, decided to close.
This topic will be revisited when we need to talk about a standardized query action approach in TD 2.0.

@benfrancis
Copy link
Member

@sebastiankb Just a suggestion, but is it not better to keep unresolved issues which have been deferred to a future release open and labelled as 2.0, rather than closing them?

@egekorkan
Copy link
Contributor

egekorkan commented Nov 24, 2022

For this specific one, it is actually not really necessary to add this to the response field due to the fact that the output that is actually expected for an operation should be in the affordance level and not form level.

Edit: You can still reopen it though

@benfrancis benfrancis reopened this Nov 28, 2022
@benfrancis benfrancis removed the Propose closing Problem will be closed shortly if there is no veto. label Nov 28, 2022
@benfrancis
Copy link
Member

For this specific one, it is actually not really necessary to add this to the response field due to the fact that the output that is actually expected for an operation should be in the affordance level and not form level.

The open issue which I would suggest is yet to be resolved is the distinction between the "output" of an action and the response to a request to invoke an action, which are not necessarily the same thing. See also: #1665

@danielpeintner danielpeintner added the Needed by other TF An issue or UC from another TF to fullfill a requirement in their spec or gap label Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defer to TD 2.0 Needed by other TF An issue or UC from another TF to fullfill a requirement in their spec or gap PR needed
Projects
None yet
Development

No branches or pull requests

8 participants