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

Add TD assertion conformance #368

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Add TD assertion conformance #368

merged 3 commits into from
Apr 5, 2023

Conversation

egekorkan
Copy link
Contributor

@egekorkan egekorkan commented Mar 1, 2023

fixes #349


Preview | Diff

@egekorkan egekorkan changed the title Add td assertion conformance Add TD assertion conformance Mar 1, 2023
Copy link
Contributor

@danielpeintner danielpeintner left a comment

Choose a reason for hiding this comment

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

LGTM.
What struggles me a bit is the use of "first". It reads to me as if there would be an "order" of assertions. Maybe skip it?
Anyhow, fine for me either way...

Copy link
Member

@benfrancis benfrancis left a comment

Choose a reason for hiding this comment

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

I did a very quick review of all the mandatory assertions in the TD specification to make sure this doesn't have any unintended side effects.

There are three assertions which raised questions for me. I think probably only the first one is a problem that needs addressing.

TD 1.1 consumers MUST accept TDs satisfying the W3C WoT Thing Description 1.0 [wot-thing-description] specification.

This is problematic for Profile conformant Consumers because there was no profile member in Thing Description 1.0. If a Profile conformant Consumer also has to support Thing Description 1.0 then it also needs to support TDs where none of the assumptions implied by profiles apply. Implementation complexity therefore explodes and and interoperability goes out of the window. I suggest we need to add an assertion to exempt Profile conformant Consumers from this TD assertion.

If no response name-value pair is provided, it MUST be assumed that the content type of the response is equal to the content type assigned to the Form instance

One slight subtlety here is that in the HTTP SSE Profile we assume the text/event-stream response type from the subprotocol member of the Form being set to sse. Given this is fixed in the SSE specification, I think that's a reasonable assumption.

If the content type of an additional expected response differs from the content type of the form, the Form instance MUST include an entry in the array associated with the name additionalResponses that includes a value for the name contentType. If the data schema of an additional expected response differs from the output data schema of the interaction, the Form instance MUST include an entry in the array associated with the name additionalResponses that includes a value for the name schema.

WoT Profile has an assertion which says that if a body is included in an error response, it MUST conform with the Problem Details Format [RFC7807]. If applied strictly, the above assertions mean that every affordance which may provide an error response must provide additionalResponses objects for every possible error, and if that error response provides a body then it must also denote the application/problem+json content type. Hopefully the data schema would be implied by the content type otherwise every error response would also have to include a data schema for the problem details format. This all seems a bit excessive to me, but I guess it's up to implementations how many possible error states they want to describe.

@egekorkan
Copy link
Contributor Author

@benfrancis I think I agree with your points and that we can add the exception for the first one. The other two, I do not know yet if they need additional assertive text in the profile.

index.html Outdated Show resolved Hide resolved
@egekorkan
Copy link
Contributor Author

@mlagally I have added the exception.

@benfrancis , if we add text proposed at #299 (comment) , we do not need any text about this in the profile.

@mlagally
Copy link
Contributor

mlagally commented Apr 5, 2023

Profile call on April 5th: Agree to merge - Please continue discussion on issue #299 separately.

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

Successfully merging this pull request may close these issues.

Are all TD 1.1 assertions valid in all profiles?
4 participants