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

W3C compliant actions protocol binding #2871

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

benfrancis
Copy link
Member

@benfrancis benfrancis commented Sep 7, 2021

This is an implementation to make the invoke action part of the gateway's API W3C compliant and is a partial fix to #2807.

  • Remove object wrapper from invoke action API requests (to make action inputs directly match the action input data schema in Thing Descriptions)
  • Update the front end to consume the new API
  • Fix integration tests

My current thinking is to leave the rest of the API intact until the proposed action protocol binding for WoT Profile is finalised, so that we don't have to implement it twice. The first step is to simply make the invokeaction operation API match the Thing Description.

One thing I'm not sure about is what to do with the /actions endpoint where no action name is specified in the URL, but one option is to leave it as it is and just remove that endpoint from Thing Descriptions until the Thing Description specification has a way to describe it (see w3c/wot-thing-description#1200).

@benfrancis benfrancis force-pushed the actions-protocol-binding branch from e41dab5 to 7bcd592 Compare September 22, 2021 16:28
@benfrancis benfrancis marked this pull request as ready for review September 22, 2021 16:32
@codecov-commenter
Copy link

Codecov Report

Merging #2871 (e79ce4a) into master (7a4c641) will increase coverage by 0.09%.
The diff coverage is 42.85%.

❗ Current head e79ce4a differs from pull request most recent head 7bcd592. Consider uploading reports for the commit 7bcd592 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2871      +/-   ##
==========================================
+ Coverage   65.30%   65.40%   +0.09%     
==========================================
  Files         124      124              
  Lines        7987     7974      -13     
  Branches     1320     1317       -3     
==========================================
- Hits         5216     5215       -1     
+ Misses       2729     2717      -12     
  Partials       42       42              
Impacted Files Coverage Δ
src/models/thing.ts 68.93% <0.00%> (+0.11%) ⬆️
src/controllers/actions_controller.ts 89.36% <40.00%> (+4.17%) ⬆️
src/app.ts 56.32% <100.00%> (ø)
src/plugin/plugin.ts 60.66% <0.00%> (+0.47%) ⬆️
src/controllers/groups_controller.ts 27.73% <0.00%> (+0.72%) ⬆️
src/models/things.ts 76.68% <0.00%> (+1.22%) ⬆️
src/controllers/new_things_controller.ts 46.15% <0.00%> (+2.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a4c641...7bcd592. Read the comment docs.

@benfrancis
Copy link
Member Author

benfrancis commented Sep 22, 2021

I've fixed the integration tests and this is now ready for review.

What this PR does:

  • Removes the object wrapper from invokeaction operations on Action endpoints (e.g. /things/mything/actions/foo) so that the payload directly matches the input schema for the action affordance in the Thing Description

What this PR does not do:

  • Change the API for querying or cancelling an action
  • Change the API for invoking an action via an Actions endpoint (e.g. /things/mything/actions/)

My thinking is that in this first phase we will just make the invokeaction and queryallactions operations W3C compliant in order to be able to provide a W3C compliant Thing Description. The API for the other operations will still exist, it just won't be described by the Thing Description.

In a later phase we can fully implement the new actions protocol binding from the WoT Profile specification and also expose queryaction and cancelaction operations in Thing Descriptions.

Note that we will need to add the new queryallactions operation to Forms in #2811.

@benfrancis
Copy link
Member Author

@relu91 FYI I think we need to land this before landing relu91#2 and #2811 because they are starting to conflict with one another.

Copy link
Collaborator

@relu91 relu91 left a comment

Choose a reason for hiding this comment

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

No objections! looks good.

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.

3 participants