w3c / wot-profile

Web of Things (WoT) Profile
http://w3c.github.io/wot-profile/
Other
16 stars 8 forks source link

Review of HTTP Webhook Profile #375

Open benfrancis opened 1 year ago

benfrancis commented 1 year ago

I have been evaluating the latest draft of the Webhook Profile with a view to potential implementation in WebThings Gateway (Producer) and WebThings Cloud (Consumer) and I think it needs some more work before it's in an implementable state. I've provided my feedback section by section below, with action items as task list items which can be turned into separate issues if necessary.

10 HTTP Webhook Profile

This section defines the HTTP Webhook Profile, including a Protocol Binding for observing properties and listening for events using WebHooks.

So far there's only a protocol binding for events.

10.1 Introduction

Suggested refinements:

10.1.1 Webhook Example

I think this example could use work. I'm not sure why the fireAlarm event would be fired when the button has been re-armed, and I'm not sure how a Consumer would know anything about sprinkler status. The cancellation payload format and unsubscribe protocol binding need updating to match the protocol binding in the specification text, i.e. DELETE instead of POST and provide the subscription ID in the URL rather than the body (a DELETE request does not usually contain a body).

10.4.1.1 subscribeevent

This involves the Web Thing initially sending an HTTP response to the Consumer with:

  • Status code set to 200
  • Content-Type header set to application/json
  • Accept header set to application/json

EXAMPLE 46

HTTP/1.1 200 OK
Content-Type: application/json
{
    subscriptionId: 1234-4544-1211
}

Although it's not mentioned in the normative text, the example shows the subscription ID being provided in the body of the HTTP response to the subscribeevent request. The problem with only including the ID in the body (as opposed to a URL) is that it requires a fixed URL design in order to know the subscription URL to which a Consumer should send an unsubscribe DELETE request.

I suggest that the response to the subscribe POST request should instead be 201 Created (since it creates a new subscription resource), and the URL of the new subscription resource should be included in a Location header. The unsubscribeevent operation can then send a DELETE request to that URL (see below). This would be consistent with what we do for asynchronous actions in the HTTP Basic Profile.

E.g.

HTTP/1.1 201 Created
Content-Type: application/json
Location: /things/lamp/events/overheated/1234-4544-1211

10.4.1.2 unsubscribeevent

10.4.1.3 subscribeallevents

E.g.

HTTP/1.1 201 CREATED
Content-Type: application/json
Location: /things/lamp/events/1234-4544-1211

10.4.1.4 unsubscribeallevents

URL set to the URL of the Event resource

DELETE /things/lamp/events looks like it's trying to delete the events endpoints altogether rather than deleting a subscription to the events. Currently this operation also doesn't use the subscription ID provided in the subscribeallevents operation, so if there are multiple subscriptions the Web Thing won't know which one to remove.

10.4.2 Event Connections


Missing Features

The payload format for events still needs to be specified, but that is being discussed in #258. My proposal is at https://github.com/w3c/wot-profile/issues/258#issuecomment-1216599450.

The observe property protocol bindings need to be specified - I have filed #376.

There are also some additional features I think may help with scalability, for which I have filed separate issues:

mlagally commented 1 year ago

Remove the sentence: "Depending on the deployment scenarios and integration requirements for existing consumers, it may be required to use specific data payload formats (e.g. Cloud Events)." (The HTTP Webhook Profile needs to define a single payload format for events in order to guarantee out-of-the-box interoperability. Compatibility with existing consumers should be a non-goal.)

I'm really concerned if we would agree on a policy of having a non-goal to be compatible with the world as it exists.