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

Allow array for response in From class? #617

Closed
mkovatsc opened this issue May 4, 2019 · 14 comments
Closed

Allow array for response in From class? #617

mkovatsc opened this issue May 4, 2019 · 14 comments
Assignees
Labels
Forms Topics related to the forms container PR available Propose closing Problem will be closed shortly if there is no veto.

Comments

@mkovatsc
Copy link
Contributor

mkovatsc commented May 4, 2019

Originally, I expected that all possible responses can be described through the response(s) vocabulary term.

A use case is where the input of an Action might select the representation format (e.g., RAW vs JPEG vs PNG), and hence multiple responses are possible.

Furthermore, it might be an extension point to also describe expected error responses out-of-band in the future (i.e., to retrofit hypermedia behavior to devices that would only respond with an error code).

@sebastiankb
Copy link
Contributor

Theoretically, it is possible to set up different form instances with different combinations of input contentType and response contentType.
I know, this may lead to complex TDs.
I do not have a hard opinion on that.

@mkovatsc
Copy link
Contributor Author

mkovatsc commented May 7, 2019

I would change the Form class definition, so that the Type for response says "ExpectedResponse" or array of "ExpectedResponse".

Otherwise, it only supports a subset of the targeted use cases.

@mkovatsc
Copy link
Contributor Author

mkovatsc commented May 7, 2019

A test is easily done through the takeSnapshot Action, where input can also specify the output format / media type.

@mkovatsc mkovatsc added Needs discussion more discussion is needed before getting to a solution Defer to next TD spec version This topic is not covered in this charter, maybe included for the next TD version. and removed Needs discussion more discussion is needed before getting to a solution labels May 7, 2019
@mkovatsc
Copy link
Contributor Author

mkovatsc commented May 8, 2019

Giving multiple possible responses would on the other hand rely on content metadata. Good protocols have that, MQTT does not...

Let's defer.

@sebastiankb sebastiankb added the Forms Topics related to the forms container label Jan 23, 2020
@farshidtz
Copy link
Member

Array of responses in Form may also be used to describe various response codes.

Describing a PUT method (based on https://tools.ietf.org/html/rfc7231#section-4.3.4):

{
    "href": "/resources/{id}",
    "htv:methodName": "PUT",
    "contentType": "application/json",
    "response": [
        {
            "description": "Created new resource",
            "htv:statusCodeValue": 201
        },
        {
            "description": "Updated existing resource",
            "htv:statusCodeValue": 204
        }
    ]
}

Response errors (relevant to #303):

{
    "href": "/resources/{id}",
    "htv:methodName": "GET",
    "response": [
        {
            "description": "Success response",
            "contentType": "application/json",
            "htv:statusCodeValue": 200
        },
        {
            "description": "Resource not found",
            "htv:statusCodeValue": 404
        }
    ]
}

Headers on success response (based on https://tools.ietf.org/html/rfc7231#section-4.3.3):

{
    "href": "/resources",
    "htv:methodName": "POST",
    "contentType": "application/json",
    "response": [
        {
            "description": "Success response",
            "htv:headers": [
                {
                    "description": "Resource identifier",
                    "htv:fieldName": "Location"
                }
            ],
            "htv:statusCodeValue": 201
        },
        {
            "description": "Invalid input",
            "htv:statusCodeValue": 400
        }
    ]
}

NOTE: the above examples may not be valid TD forms.

@egekorkan
Copy link
Contributor

From @farshidtz comments above, this issue in the scripting api is also relevant: w3c/wot-scripting-api#200

@mmccool
Copy link
Contributor

mmccool commented Aug 17, 2020

Adding an array in responses would break backward compatibility, and also if multiple responses are possible the TD NEEDS a way to distinguish the various response (eg a header field). So I'm thinking something like using the current "response" field as the "default" response, and a NEW field, which has an array of responses, each of which include a header to distinguish then. This could be called something like "alternateResponses" although we could argue about better names (eg just "alternates") if you would like...

@takuki
Copy link
Contributor

takuki commented Sep 1, 2020

Adding an array in responses would break backward compatibility, and also if multiple responses are possible the TD NEEDS a way to distinguish the various response (eg a header field). So I'm thinking something like using the current "response" field as the "default" response, and a NEW field, which has an array of responses, each of which include a header to distinguish then. This could be called something like "alternateResponses" although we could argue about better names (eg just "alternates") if you would like...

If the Thing with an alternateResponses field does not know the consumer can handle v1.1, the consumer would be surprised to receive a response that is different from what it expects.

I think although it does not break interoperability when parsing a TD, but it may break interoperability at run-time this way.

@sebastiankb
Copy link
Contributor

From today's TD call:

  • as @takuki mentioned supporting response as an array we will have backwards compatible issue
  • we should clarify with the Discovery TF (@farshidtz @mmccool) how essential this feature is and may decide to do an exception

@egekorkan
Copy link
Contributor

I wasn't there in the call but what's the relation to the Discovery TF?

@sebastiankb
Copy link
Contributor

I wasn't there in the call but what's the relation to the Discovery TF?

w3c/wot-discovery#54

@sebastiankb sebastiankb removed the Defer to next TD spec version This topic is not covered in this charter, maybe included for the next TD version. label Jan 13, 2021
@sebastiankb
Copy link
Contributor

sebastiankb commented Jan 13, 2021

From today's TD call:

  • we introduce a new term additionalResponses as array in forms
  • this keeps backwards compatibility to 1.1
  • we have to evaluate the definition about different data schema that can be assigned to different responses
  • first idea is to use anyOf at data schema level to provide different schema definitions
  • we should also compare the discussion with SDFchoice
  • within the response entries there can be a JSON pointer to a schema definition
  • there needs a default assumption for the pointer
  • @mmccool will discuss some use cases during the discovery TF
  • @mmccool will also provide a PR to integrate this new feature if there is consensus

@mjkoster
Copy link
Contributor

Entry point for the discussion on "anyOf" in SDF: ietf-wg-asdf/SDF#8

We should write up a FAQ entry for this

@sebastiankb sebastiankb added PR available Propose closing Problem will be closed shortly if there is no veto. and removed PR needed labels Mar 25, 2021
@sebastiankb
Copy link
Contributor

issue is addressed by the latest draft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Forms Topics related to the forms container PR available Propose closing Problem will be closed shortly if there is no veto.
Projects
None yet
Development

No branches or pull requests

7 participants