Refactoring TDD Thing Description

w3c / wot-discovery

Repository for WoT discovery discussion

https://w3c.github.io/wot-discovery/

Other

20 stars 17 forks source link

Refactoring TDD Thing Description #133

Closed relu91 closed 3 years ago

relu91 commented 3 years ago

It seems that the current specification of the TDD Thing Description relies on this @farshidtz's assumption (which was actually taken from the TD spec):

Action: An Interaction Affordance that allows to invoke a function of the Thing, which manipulates state (e.g., toggling a lamp >>on or off) or triggers a process on the Thing (e.g., dim a lamp over time). Property: An Interaction Affordance that exposes state of the Thing. This state can then be retrieved (read) and optionally updated (write).

Action is for manipulating state of the Thing, in this case the Thing is the directory and its resources are being manipulated. I don't know why at the same time setting a property is allowed by the TD spec.

However, the current architecture document gives a less strict definition for Web Things Actions affordances:

An Action is an Interaction Affordance that allows to invoke a function of the Thing. An Action MAY manipulate state that is not directly exposed (cf. Properties), manipulate multiple Properties at a time, or manipulate Properties based on internal logic (e.g., toggle). Invoking an Action MAY also trigger a process on the Thing that manipulates state (including physical state through actuators) over time.

Notice that it's using MAY and not MUST. What feels odd in the current TD specification is the fact that the search operations are modeled as properties with uriVariables. I wonder if we can evaluate the following refactoring, where they are modeled as actions:

{
  // other TD properties
 "properties": {
     // here we should heavly use pagination to return the list of TDs
    // not sure if this was somehow removed from the spec because it's reduant. 
     "thingDescriptions": {
            "description": "Retrieve Thing Description collection",
            "forms": [
                {
                    "href": "/td",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 200,
                        "contentType": "application/td+json"
                    },
                    "additionalResponses": [
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "read"
                }
            ]
        }
 },
 "actions": {
        /* other actions */
        "retrieveTD": {
            "description": "Retrieve a Thing Description",
            "uriVariables": {
                "id": {
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                }
            },
            "forms": [
                {
                    "href": "/td/{id}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 200,
                        "contentType": "application/td+json"
                    },
                    "additionalResponses": [
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "read"
                }
            ]
        },
        "searchJSONPath": {
            "description": "JSONPath syntactic search",
            "uriVariables": {
                "query": {
                    "title": "A valid JSONPath expression",
                    "type": "string"
                }
            },
            "forms": [
                {
                    "href": "/search/jsonpath?query={query}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "JSONPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                }
            ]
        },
        "searchXPath": {
            "description": "XPath syntactic search",
            "uriVariables": {
                "query": {
                    "title": "A valid XPath expression",
                    "type": "string"
                }
            },
            "forms": [
                {
                    "href": "/search/xpath?query={query}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "JSONPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                }
            ]
        },
        "searchSPARQL": {
            "description": "SPARQL semantic search",
            "uriVariables": {
                "query": {
                    "title": "A valid SPARQL 1.1. query",
                    "type": "string"
                }
            },
            "forms": [
                {
                    "href": "/search/sparql?query={query}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "JSONPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/search/sparql",
                    "htv:methodName": "POST",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "JSONPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                }
            ]
        }
    },
}

In general, it seems that in the current design we are blending design decisions from the REST layer which is just a binding and it could follow a different interaction model than WoT.

As a side note, I still find the usage of URI variables in actions weird as described here.

farshidtz commented 3 years ago

// here we should heavly use pagination to return the list of TDs // not sure if this was somehow removed from the spec because it's reduant.

There is a PR to discuss and add this: https://github.com/w3c/wot-discovery/pull/130

In your proposal, why retrieving all TDs is still a property but retrieving one TD is an action?

Regarding uriVariables, they are used because the information is passed in the URL and not the body. The values are primitive types, not object or array.

relu91 commented 3 years ago

// here we should heavly use pagination to return the list of TDs // not sure if this was somehow removed from the spec because it's reduant.

There is a PR to discuss and add this: #130

Thank you for linking the PR. I think we could go ahead with that because it is consistent with the previous assumptions. Then maybe if we reach a consensus here do a simple refactor.

In your proposal, why retrieving all TDs is still a property but retrieving one TD is an action?

I see that it might have been confusing introducing that property, but let me try to explain. My rationale was that the Thing Description Directory contains a list or a collection of TDs. In principle obtaining this list would not require any logical processing: just take the list and return it. On the other hand, retrieving a single TD requires, again logically, scanning through the list and find the correct document.

Anyhow, I could live with having a retriveAllTD as an action. I would focus on the refactor itself rather than this detail.

Regarding uriVariables, they are used because the information is passed in the URL and not the body. The values are primitive types, not object or array.

Thanks for the clarification. I am aware of the reasons behind that design choice in this TD, but as I pointed in the post above (I know it's not a short comment 🤣 and it might need improvements) I commented here why we might think again about their roles in actions.

benfrancis commented 3 years ago

@relu91 wrote:

In general, it seems that in the current design we are blending design decisions from the REST layer which is just a binding and it could follow a different interaction model than WoT.

Is there going to be a version of the Directory Service API which isn't a REST API? My understanding from the current draft was that this was a concrete prescriptive API which always uses HTTP, not an abstract API which would have multiple bindings.

I do think it's challenging to describe this API using a Thing Description because TDs don't deal well with collections of resources. But FWIW my understanding of actions is that they do always cause a change in state. The wording used in Mozilla's draft specification (which incidentally also defines an API for listing things but not described by a TD), was:

"Actions are used when the setting of a property alone is not sufficient to affect a required change in state. This may be used to describe a function which takes a period of time to complete, manipulates multiple properties, or has a different outcome based on provided arguments or current state. "

In a REST API reading, listing and searching resources maps well onto GET requests since they are operations which do not cause a change in state and are idempotent. I would expect these types of operations to map onto properties, not actions.

relu91 commented 3 years ago

Is there going to be a version of the Directory Service API which isn't a REST API? My understanding from the current draft was that this was a concrete prescriptive API which always uses HTTP, not an abstract API which would have multiple bindings.

I sensed during different calls that the direction was "let's start from HTTP than we can later move on with other protocol bindings", but I might be wrong. I think @farshidtz is more aligned with your statement. However, I would like to underline that if we follow an abstract designing moving to other protocols would be free. Which are the benefits of pursuing the current design? I'd like to quote @k-toumura (complete comment) :

I think the advantage of Thing Description is that it can handle protocol binding and interaction affordance separately.

I do think it's challenging to describe this API using a Thing Description because TDs don't deal well with collections of resources. But FWIW my understanding of actions is that they do always cause a change in state. The wording used in Mozilla's draft specification (which incidentally also defines an API for listing things but not described by a TD), was:

"Actions are used when the setting of a property alone is not sufficient to affect a required change in state. This may be used to describe a function which takes a period of time to complete, manipulates multiple properties, or has a different outcome based on provided arguments or current state. "

In a REST API reading, listing and searching resources maps well onto GET requests since they are operations which do not cause a change in state and are idempotent. I would expect these types of operations to map onto properties, not actions.

Not sure if I am following, but the definition that you quoted says that an Action can also be used as a function that has a different outcome based on the provided arguments (i.e., a filter). Also, the current definition in WoT Architecture does not mandate that an Action Affordance MUST/SHOULD always change the state or using it, as quoted in the first post it now uses MAY. I have some doubts, don't you think that calling a Property with a verb is strange? (e.g., retrieve, searchJSON) I think it hints at some bad design...

What does stop us to use a GET operation in Action?

benfrancis commented 3 years ago

@relu91 wrote:

I think the advantage of Thing Description is that it can handle protocol binding and interaction affordance separately.

That is true. But the directory API is a bit of a strange example because it's not actually a device with physical properties, the TD is (somewhat awkwardly) being used to describe an HTTP REST API of a software web service describing a collection of resources (more like OpenAPI).

Not sure if I am following, but the definition that you quoted says that an Action can also be used as a function that has a different outcome based on the provided arguments.

Yes, where the "outcome" is a change in state.

Also, the current definition in WoT Architecture does not mandate that an Action Affordance MUST/SHOULD always change the state or using it, as quoted in the first post it now uses MAY.

That is true, and I agree with the comments in https://github.com/w3c/wot-thing-description/issues/1020 that the use of properties vs. actions is poorly defined in the current TD specification.

I have some doubts, don't you think that calling a Property with a verb is strange? (e.g., retrieve, searchJSON) I think it hints at some bad design...

Yes that's another matter and I agree. But https://google.com/search?q=foo with a GET request is a very common pattern for searching on the web and...

What does stop us to use a GET operation in Action?

If we agree that actions always change state, then a GET request is not appropriate because GET requests should never change state and should be idempotent. But it seems that's a separate debate.

I'm planning on doing an in-depth review of the discovery specification today so will get back to you with further comments.

relu91 commented 3 years ago

That is true. But the directory API is a bit of a strange example because it's not actually a device with physical properties, the TD is (somewhat awkwardly) being used to describe an HTTP REST API of a software web service describing a collection of resources (more like OpenAPI).

BTW we could have a more REST-oriented description using OpenAPI and a TD at the same time. In my understanding having a well-defined TD (which should be more a TM but we still don't have a stable description for that) is that consumers who already have the WoT stack would find more natural to interact with a TDD described by a TD (i.e., use the same libraries and utilities as if they were interacting with another WebThing).

farshidtz commented 3 years ago

I sensed during different calls that the direction was "let's start from HTTP than we can later move on with other protocol bindings", but I might be wrong.

We currently describe the HTTP API but don't say that HTTP should be the only protocol.

it now uses MAY. I have some doubts, don't you think that calling a Property with a verb is strange? (e.g., retrieve, searchJSON) I think it hints at some bad design...

Is the name of an interaction really the basis to choose the interaction type? If so, changing "retrieveTD" to "TD" solved that problem! We should choose based on the interaction specifications. Every operation needs processing, only some change the state. Some operations always have input and output (e.g. SPARQL search with POST method) and could be modeled better as actions. But properties may also have different input and outputs schemas (see https://github.com/w3c/wot-thing-description/issues/1053).

Since the updated specification allows the use of action for an interaction that doesn't change the state, I have no problem using actions for ALL RESTful interactions on the TDD.

BTW we could have a more REST-oriented description using OpenAPI and a TD at the same time. In my understanding having a well-defined TD (which should be more a TM but we still don't have a stable description for that) is that consumers who already have the WoT stack would find more natural to interact with a TDD described by a TD (i.e., use the same libraries and utilities as if they were interacting with another WebThing).

See https://github.com/w3c/wot-discovery/issues/82

relu91 commented 3 years ago

Is the name of an interaction really the basis to choose the interaction type? If so, changing "retrieveTD" to "TD" solved that problem!

Well since a TD is not read only by machines but also humans naming is important as it conveys purpose and meaning. About renaming again it is about the meaning, retrievedTD is Action whereas TDs is a Property.

We should choose based on the interaction specifications. Every operation needs processing, only some change the state. Some operations always have input and output (e.g. SPARQL search with POST method) and could be modeled better as actions. But properties may also have different input and outputs schemas (see w3c/wot-thing-description#1053).

Since the updated specification allows the use of action for an interaction that doesn't change the state, I have no problem using actions for ALL RESTful interactions on the TDD.

Totally agree that we should follow the spec and I think we are on the same page then!

benfrancis commented 3 years ago

OK, having read the specification in more depth I have an alternative proposal to simplify the Directory Service API and its Thing Description.

This includes:

properties
- things - A single property¹ which represents the list of Thing Descriptions and can accept a GET request with optional URL variables to filter that list, either down to a single TD via its ID or a subset of TDs using a JSON Path, XPath or SPARQL query
actions²
- createThing - accepts a POST or PUT request (depending on whether the ID is known) to add a Thing Description to the list
- updateThing - accepts a PUT or PATCH request (depending on whether a full or partial TD is provided) to update a Thing Description
- deleteThing - accpets a DELETE request to delete a Thing Description by its ID
events
- thingCreated - fired when a Thing Description is created
- thingUpdated - fired when a Thing Description is updated
- thingDeleted - fired when a Thing Description is deleted

(See additional notes at the end).

{
  "@context": [
    "https://www.w3.org/2019/wot/td/v1",
    "https://www.w3.org/2021/wot/tdd/v1"
  ],
  "@type": "Directory",
  "title": "My Directory",
  "properties": {
    "things": {
      "@type": "ThingListProperty",
      "title": "Things",
      "description": "The collection of things the directory serves",
      "forms": [
        {
          "href": "/things/",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "href": "/things/{id}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "href": "/things/?jq={jq}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "href": "/things/?xq={xq}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "href": "/things/?sq={sq}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        }
      ],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        },
        "jq": {
          "@type": "JSONPathQuery",
          "type": "string",
          "title": "JSON Path search query"
        },
        "xq": {
          "@type": "XPathQuery",
          "type": "string",
          "title": "XPath search query"
        },
        "sq": {
          "@type": "SPARQLQuery",
          "type": "string",
          "title": "SPARQL search query"
        }
      }
    }
  },
  "actions": {
    "createThing": {
      "@type": "CreateThingAction",
      "forms": [
        {
          "href": "/things/",
          "htv:methodName": "POST",
          "contentType": "application/td+json"
        },
        {
          "href": "/things/{id}",
          "htv:methodName": "PUT",
          "contentType": "application/td+json"
        },
      ],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        }
      },
      "input": {
        "@type": "Thing",
        "type": "object",
      },
    },
    "updateThing": {
      "@type": "UpdateThingAction",
      "forms": [
        {
          "href": "/things/{id}",
          "htv:methodName": "PUT",
          "contentType": "application/td+json"
        },
        {
          "href": "/things/{id}",
          "htv:methodName": "PATCH",
          "contentType": "application/td+json"
        }
      ],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        }
      },
      "input": {
        "@type": "Thing",
        "type": "object",
      },
    },
    "deleteThing": {
      "@type": "DeleteThingAction",
      "forms": [{
        "href": "/things/{id}",
        "htv:methodName": "DELETE",
      }],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        }
      },
    }
  },
  "events": {
    "thingCreated": {
      "@type": "ThingCreatedEvent",
      "data": {
        "type": "object"
      },
      "forms": [{
        "href": "/events/thingCreated",
        "contentType": "application/json"
      }],
    },
    "thingUpdated": {
      "@type:": "ThingUpdatedEvent",
      "data": {
        "type": "object"
      },
      "forms": [{
        "href": "/events/thingUpdated",
        "contentType": "application/json"
      }],
    },
    "thingDeleted": {
      "@type": "ThingDeletedEvent",
      "data": {
        "type": "object"
      },
      "forms": [{
        "href": "/events/thingDeleted",
        "contentType": "application/json"
      }],
    }
  }
}

Notes:

As TDs don't have a good way of modelling collections, I've concluded that a single property is the most effective way of modelling this.
None of the actions (create, update and delete) could be performed by simply writing the whole things property, so I think it's reasonable to make them into actions.
I've made thingCreated, thingUpdated and thingDeleted separate events to mirror the actions which represent the different modifications which consumers may need to be notified of.
Note that I've added additional semantic annotations to denote the types of properties, actions and events so that the names and URLs of interaction affordances do not need to be standardised and can be left up to the author. (I also tweaked the context URI to be something cleaner).
My intention is that the full details about the expected headers, payloads and response codes of requests and responses would be described in prose the body of the specification, but the masochists amongst us are welcome to try to describe those declaratively in forms ;)
I've not included any security metadata because given the specification says that clients already have to be authenticated in order to access the directory the security metadata is useless. That's a general problem with TDs and I'm going to file a separate issue about that.

benfrancis commented 3 years ago

@relu91: Could you live with the above? The property is a noun and the actions are verbs and it's compliant with the description of properties vs. actions in the TD spec (though I expect the debate will continue in https://github.com/w3c/wot-thing-description/issues/1020). The query parameters on the GET requests are really just to filter the response.

relu91 commented 3 years ago

Thank you for researching and providing a concrete full refactoring of the TD. Your proposal seems reasonable and well designed, what I like is the rationale that you put in your first 4 notes:

As TDs don't have a good way of modelling collections, I've concluded that a single property is the most effective way of modelling this.

None of the actions (create, update and delete) could be performed by simply writing the whole things property, so I think it's reasonable to make them into actions.

I've made thingCreated, thingUpdated and thingDeleted separate events to mirror the actions which represent the different modifications which consumers may need to be notified of.

Note that I've added additional semantic annotations to denote the types of properties, actions and events so that the names and URLs of interaction affordances do not need to be standardised and can be left up to the author. (I also tweaked the context URI to be something cleaner).

+1 especially for point 4. So yeah it is an improvement that I would approve 👍 . What concern me is: how I client can choose the right form to issue query the TDD? I think the TD spec misses to explain how to select a form using the uriVariables as hint. Meanwhile, we could define semantic tags (to be added in forms instances) to indicate which form can be used for a particular query type.

Aside from that, sorry for stressing this again, but what is wrong with this TD?

{
    "@context": [
        "https://www.w3.org/2019/wot/td/v1",
        "https://www.w3.org/2021/wot/tdd/v1"
    ],
    "@type": "Directory",
    "title": "My Directory",
    "properties": {
        "things": {
            "@type": "ThingListProperty",
            "title": "Things",
            "description": "The collection of things the directory serves",
            "forms": [
                {
                    "href": "/things/",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                }
            ]
        }
    },
    "actions": {
        "search": {
            "@type": "SearchThingsAction",
            "title": "search",
            "description": "Search among the list of Thing Descriptions",
            "forms":[
                {
                    "href": "/things/{id}",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                },
                {
                    "href": "/things/?jq={jq}",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                },
                {
                    "href": "/things/?xq={xq}",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                },
                {
                    "href": "/things/?sq={sq}",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                }
            ],
            "uriVariables": {
                "id": {
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "uri"
                },
                "jq": {
                    "@type": "JSONPathQuery",
                    "type": "string",
                    "title": "JSON Path search query"
                },
                "xq": {
                    "@type": "XPathQuery",
                    "type": "string",
                    "title": "XPath search query"
                },
                "sq": {
                    "@type": "SPARQLQuery",
                    "type": "string",
                    "title": "SPARQL search query"
                }
            }

        }
        /*Same fields of above*/
    }
}

Notice, we still have the how to select the right form problem. A simple workaround here (which is not possible modeling search operations as filters of the property things) is to spawn different actions:

{
    "@context": [
        "https://www.w3.org/2019/wot/td/v1",
        "https://www.w3.org/2021/wot/tdd/v1"
    ],
    "@type": "Directory",
    "title": "My Directory",
    "properties": {
        "things": {
            "@type": "ThingListProperty",
            "title": "Things",
            "description": "The collection of things the directory serves",
            "forms": [
                {
                    "href": "/things/",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                }
            ]
        }
    },
    "actions": {
        "find":{
            "@type": "FindTDAction",
            "title": "search",
            "description": "Search among the list of Thing Descriptions",
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                }
            ]
        },
        "searchJsonPath":{
            "@type": "SearchJsonPathThingsAction",
            "title": "Search using JSON Path",
            "description": "Query the list of Thing Descriptions using JSON Path",
            "forms":[{
                    "href": "/things/?jq={jq}",
                    "htv:methodName": "GET",
                    "contentType": "application/json"
                }]
        }
        /*etc. */
    }
}

As a bonus feature detection for nonsematic consumers would be simply td.actions.searchSPARQL !== undefined, instead of looping inside forms of things property.

benfrancis commented 3 years ago

how I client can choose the right form to issue query the TDD? I think the TD spec misses to explain how to select a form using the uriVariables as hint.

I agree this isn't clear from the WoT Thing Description specification and would need specifying in the WoT Discovery specification. But what exactly should be standardised? The names of interaction affordances and variable names of uriVariables?

My suggestion would be to standardize a capability schema for a directory with:

Semantic annotation at the top level to denote that the Thing Description describes a directory, e.g. "@type": "Directory"
Semantic annotations for each interaction affordance to indicate what kind of operation can be carried out, e.g. "@type": "CreateThingAction"
Semantic annotations at the data schema level, e.g. "@type": "Thing" to denote that the createThingAction accepts a Thing Description as an input
Semantic annotations for URI variables, e.g. "@type": "JSONPathQuery" tells a consumer that a given URI variable is a JSON Path query and "@type": "ThingID" annotates which URI variable can be used to identify an individual Thing Description to retrieve.

This allows all the names of interaction affordances, URIs and URI variables to be chosen by the developer, but should give consumers enough hints about how to consume the API.

Meanwhile, we could define semantic tags (to be added in forms instances) to indicate which form can be used for a particular query type.

Yes, that might be useful too.

The only other tool I can think of which might be useful here is adding a new set of ops for forms which describe directories. E.g. "op": "creatething", "op": "updatething", "op": "deletething", but that doesn't solve all of the other cases above.

but what is wrong with this TD?

Only that it breaks the rule that actions should only be used to modify state (and only where setting a property alone is not sufficient). That debate really needs resolving in https://github.com/w3c/wot-thing-description/issues/1020

A simple workaround here (which is not possible modeling search operations as filters of the property things) is to spawn different actions:

Yes, though that only really works if search operations are actions (see above) and makes for a messier TD in my opinion.

benfrancis commented 3 years ago

The only other tool I can think of which might be useful here is adding a new set of ops for forms which describe directories. E.g. "op": "creatething", "op": "updatething", "op": "deletething", but that doesn't solve all of the other cases above.

I've provided an example of this below, which works quite well for properties and actions, but as you can see doesn't provide enough information on its own for a client to know how to to carry out all operations (e.g. distinguishing between different types of readmultiplethings, creating with PUT vs. POST, updating with PUT vs. PATCH and distinguishing between different types of events.) I think semantic annotations would still be needed to provide consumers with all of this information.

{
  "@context": [
    "https://www.w3.org/2019/wot/td/v1",
    "https://www.w3.org/2021/wot/tdd/v1"
  ],
  "@type": "Directory",
  "title": "My Directory",
  "properties": {
    "things": {
      "@type": "ThingListProperty",
      "title": "Things",
      "description": "The collection of things the directory serves",
      "forms": [
        {
          "op": "readallthings",
          "href": "/things/",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "op": "readthing",
          "href": "/things/{id}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "op": "readmultiplethings",
          "href": "/things/?jq={jq}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "op": "readmultiplethings",
          "href": "/things/?xq={xq}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "op": "readmultiplethings",
          "href": "/things/?sq={sq}",
          "htv:methodName": "GET",
          "contentType": "application/json"
        }
      ],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        },
        "jq": {
          "@type": "JSONPathQuery",
          "type": "string",
          "title": "JSON Path search query"
        },
        "xq": {
          "@type": "XPathQuery",
          "type": "string",
          "title": "XPath search query"
        },
        "sq": {
          "@type": "SPARQLQuery",
          "type": "string",
          "title": "SPARQL search query"
        }
      }
    }
  },
  "actions": {
    "createThing": {
      "@type": "CreateThingAction",
      "forms": [
        {
          "op": "creatething",
          "href": "/things/",
          "htv:methodName": "POST",
          "contentType": "application/td+json"
        },
        {
          "op": "creatething",
          "href": "/things/{id}",
          "htv:methodName": "PUT",
          "contentType": "application/td+json"
        },
      ],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        }
      },
      "input": {
        "@type": "Thing",
        "type": "object",
      },
    },
    "updateThing": {
      "@type": "UpdateThingAction",
      "forms": [
        {
          "op": "updatething",
          "href": "/things/{id}",
          "htv:methodName": "PUT",
          "contentType": "application/td+json"
        },
        {
          "op": "updatething",
          "href": "/things/{id}",
          "htv:methodName": "PATCH",
          "contentType": "application/td+json"
        }
      ],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        }
      },
      "input": {
        "@type": "Thing",
        "type": "object",
      },
    },
    "deleteThing": {
      "@type": "DeleteThingAction",
      "forms": [{
        "op": "deletething",
        "href": "/things/{id}",
        "htv:methodName": "DELETE",
      }],
      "uriVariables": {
        "id": {
          "@type": "ThingID",
          "title": "Thing Description ID",
          "type": "string",
          "format": "uri"
        }
      },
    }
  },
  "events": {
    "thingCreated": {
      "@type": "ThingCreatedEvent",
      "data": {
        "type": "object"
      },
      "forms": [{
        "href": "/events/thingCreated",
        "contentType": "application/json"
      }],
    },
    "thingUpdated": {
      "@type:": "ThingUpdatedEvent",
      "data": {
        "type": "object"
      },
      "forms": [{
        "href": "/events/thingUpdated",
        "contentType": "application/json"
      }],
    },
    "thingDeleted": {
      "@type": "ThingDeletedEvent",
      "data": {
        "type": "object"
      },
      "forms": [{
        "href": "/events/thingDeleted",
        "contentType": "application/json"
      }],
    }
  }
}

relu91 commented 3 years ago

My suggestion would be to standardize a capability schema for a directory with:

Semantic annotation at the top level to denote that the Thing Description describes a directory, e.g. "@type": "Directory" Semantic annotations for each interaction affordance to indicate what kind of operation can be carried out, e.g. "@type": "CreateThingAction" Semantic annotations at the data schema level, e.g. "@type": "Thing" to denote that the createThingAction accepts a Thing Description as an input Semantic annotations for URI variables, e.g. "@type": "JSONPathQuery" tells a consumer that a given URI variable is a JSON Path query and "@type": "ThingID" annotates which URI variable can be used to identify an individual Thing Description to retrieve.

Just want to say that even if we enforce the whole TD / Thing Model, it might be beneficial to specify semantic types as you are suggesting.

Only that it breaks the rule that actions should only be used to modify state (and only where setting a property alone is not sufficient). That debate really needs resolving in w3c/wot-thing-description#1020

A simple workaround here (which is not possible modeling search operations as filters of the property things) is to spawn different actions:

Yes, though that only really works if search operations are actions (see above) and makes for a messier TD in my opinion.

that's right, even if I am not really sure about the current definition of Action:

An Action is an Interaction Affordance that allows to invoke a function of the Thing. An Action MAY manipulate state that is not directly exposed (cf. Properties), manipulate multiple Properties at a time, or manipulate Properties based on internal logic (e.g., toggle). Invoking an Action MAY also trigger a process on the Thing that manipulates state (including physical state through actuators) over time.

To me, it just says that everything is optional (it's all a big MAY) and the only thing that is stated is that it is something to invoke a function, but I agree let's move the discussion onto w3c/wot-thing-description#1020.

Not sure about the new operation types, they would add quite a complexity to the protocol binding implementation. I would prefer to resolve the debate on actions/properties and spawn different affordances.

Anyhow, given the current spec, I think we reached a good proposal here, which is an improvement with the current solution.

benfrancis commented 3 years ago

@farshidtz What do you think of the proposal in https://github.com/w3c/wot-discovery/issues/133#issuecomment-797710015 ?

I am happy to create a PR if it helps.

farshidtz commented 3 years ago

My comments will be based on the current TD available here.

Some general comments:

The type should be DirectoryDescription (see the exploration draft or context)
The content types are not consistent with the current draft.
I agree that the resource name should be plural in a RESTful API (in your case things). thingDescriptions was too long and tds could be read as TDS instead of TDs. Making the path case sensitive and forcing TDs is a bad practice. Now, calling it things is confusing. The resource is not a Thing, it is a Thing Description. But I'm fine with calling it a thing, since TD's spec also refers to it a td:Thing. But it good to make it consistent with the name of the directory. Note that we had long discussions why the directory should be called Thing Description Directory (TDD) and not a Thing Directory (TD) to avoid having a confusing acronym.
I like having filtering and listing on the same endpoint. You can actually see this in my original design (TD, OpenAPI). But we separated them because of SPARQL which standardizes the query argument and additionally requires POST method (see sparql11-protocol/#query-operation). POST /td was already reserved for creating a TD. So the decision was to serve search queries on dedicate endpoints.
Apart from search, I'm fine with merging retrieval of 1 / many / all TDs into one property.
Merging update and partialUpdate is a probably okay. We kept them together in the text, but had them separate in the TD to simplify referencing. The URIs are identical, but the HTTP method and contentType (PATCH uses application/merge-patch+json) can be used to distinguish.
In general, merging the properties (1/many/all TDs) and actions (update/partialUpdate TD) make no functional difference to how the directory works. We should opt for something that is more readable and simpler to consume programmatically.

I've made thingCreated, thingUpdated and thingDeleted separate events to mirror the actions which represent the different modifications which consumers may need to be notified of.

The events were merged into one form to allow subscription to one, multiple, or all events. Also, other query arguments for subscribing to a particular TD or toggling differences are missing. We could try to simplify current data schema by reducing to a single schema for all types.

I also tweaked the context URI to be something cleaner

The first context URI is the current URI used in TD 1.1. The second context URI is intentionally set to the draft URL. The URL should be a valid one, pointing to the context file to allow resolution of the type from DirectoryDescription to discovery:DirectoryDescription. This can be changed only after we have a w3.org URL.

My intention is that the full details about the expected headers, payloads and response codes of requests and responses would be described in prose the body of the specification, but the masochists amongst us are welcome to try to describe those declaratively in forms

I think those are important to consumers. The TD should be complete if we don't want to provide any OpenAPI spec.

I've not included any security metadata because given the specification says that clients already have to be authenticated in order to access the directory the security metadata is useless. That's a general problem with TDs and I'm going to file a separate issue about that.

The clients should already be authenticated in order to access the Directory Description (if not public). The security metadata is meant for accessing the interactions.

benfrancis commented 3 years ago

@farshidtz thank you for your detailed reply!

* The type should be DirectoryDescription (see the [exploration draft](https://w3c.github.io/wot-discovery/#exploration-mech) or [context](https://w3c.github.io/wot-discovery/context/discovery-context.jsonld))

Why does a description of a thing have a @type of Thing, but a description of a directory has a @type of DirectoryDescription? Surely it should either be ThingDescription & DirectoryDescription or Thing & Directory for consistency?

* The content types are not consistent with the current draft.

OK, the ones in the draft you linked to make sense.

* I agree that the resource name should be plural in a RESTful API (in your case `things`). thingDescriptions was too long and tds could be read as TDS instead of TDs. Making the path case sensitive and forcing TDs is a bad practice.  Now, calling it things is confusing. The resource is not a Thing, it is a Thing Description. But I'm fine with calling it a thing, since TD's spec also refers to it a td:Thing. But it good to make it consistent with the name of the directory.

I agree tds/TDS/TDs are not great choices. As you say, the Thing Description specification already refers to a Thing Description as a "Thing". (As far as the Web of Things is concerned, the Thing Description resource is the thing, or at least its top level resource, like an index.html).

I don't see any confusion in the idea that a Directory has a list of things as a property. But I also think the property name and URI should be up to the developer (see #144).

The important point is that there's a single resource which represents a list of things, which can be filtered in different ways.

Note that we had long discussions why the directory should be called Thing Description Directory (TDD) and not a Thing Directory (~TD~) to avoid having a confusing acronym.

That's really a separate issue.

* I like having filtering and listing on the same endpoint. You can actually see this in my original design ([TD](https://github.com/w3c/wot-discovery/blob/main/prior-work/fraunhofer/directory-td.jsonld#L93), [OpenAPI](https://github.com/linksmart/thing-directory/blob/v1.0.0-beta.21/apidoc/openapi-spec.yml#L30)). But we separated them because of SPARQL which standardizes the query argument and additionally requires POST method (see [sparql11-protocol/#query-operation](https://www.w3.org/TR/sparql11-protocol/#query-operation)). POST /td was already reserved for creating a TD. So the decision was to serve search queries on dedicate endpoints.

I see. Fortunately it is still possible to differentiate between a POST to create a thing and a POST to query things using contentType (application/td+json vs. application/x-www-form-urlencoded or application/sparql-query).

* Apart from search, I'm fine with merging retrieval of 1 / many / all TDs into one property.

With the above distinction, I think search can be combined too. Note that "search" is not a good name for a property because it is a verb (the same applies to "retrieveTD"), property names should really be nouns.

* Merging update and partialUpdate is a probably okay. We kept them together in the text, but had them separate in the TD to simplify referencing. The URIs are identical, but the HTTP method and contentType (PATCH uses application/merge-patch+json) can be used to distinguish.

I agree.

In general, merging the properties (1/many/all TDs) and actions (update/partialUpdate TD) make no functional difference to how the directory works. We should opt for something that is more readable and simpler to consume programmatically.

I agree, though obviously "readable" and "simple" is subjective, hence my suggestion of more flexibility for developers (#144).

The events were merged into one form to allow subscription to one, multiple, or all events. Also, other query arguments for subscribing to a particular TD or toggling differences are missing. We could try to simplify current data schema by reducing to a single schema for all types.

This feels really strange, because you could make the same argument for any Thing Description. Should all Thing Descriptions only define a single event? I think this is an argument for a subscribeallevents operation like readallproperties (we actually have this in WebThings, where we have top level events and actions resources like the top level properties resource), rather than munging all the events into one giant event called "registration".

The first context URI is the current URI used in TD 1.1. The second context URI is intentionally set to the draft URL. The URL should be a valid one, pointing to the context file to allow resolution of the type from DirectoryDescription to discovery:DirectoryDescription. This can be changed only after we have a w3.org URL.

OK, makes sense.

I think those are important to consumers. The TD should be complete if we don't want to provide any OpenAPI spec.

OK, fair enough.

The clients should already be authenticated in order to access the Directory Description (if not public). The security metadata is meant for accessing the interactions.

Let's continue to discuss this in #135.

Other questions:

How does the server differentiate between a PUT to create a thing and a PUT to update a thing? They currently have the same URI and contentType but different response codes.
Why is the format of an id an "iri-reference" rather than a "uri"?
Is there any reason a system-generated ID has to be a UUID (version 4)? WebThings uses HTTP URIs and the TD spec says "anyURI".
What's the Last-Event-ID header in events for?

I have drafted an updated proposed TD based on the one you linked to. It's very large though so at this point it might be easier to discuss the finer points in a pull request. What branch should I send a pull request to?

Thanks.

{
    "@context": [
        "http://www.w3.org/ns/td",
        "https://w3c.github.io/wot-discovery/context/discovery-context.jsonld"
    ],
    "@type": "Directory",
    "title": "Example Thing Description Directory (TDD)",
    "version": {
        "instance": "1.0.0-alpha"
    },
    "securityDefinitions": {
        "oauth2_code": {
            "scheme": "oauth2",
            "flow": "code",
            "authorization": "https://auth.example.com/authorization",
            "token": "https://auth.example.com/token",
            "scopes": [
                "write",
                "read",
                "search"
            ]
        },
        "oauth2_client": {
            "scheme": "oauth2",
            "flow": "client",
            "token": "https://auth.example.com/token",
            "scopes": [
                "write",
                "read",
                "search"
            ]
        },
        "oauth2_device": {
            "scheme": "oauth2",
            "flow": "device",
            "authorization": "https://auth.example.com/device_authorization",
            "token": "https://auth.example.com/token",
            "scopes": [
                "write",
                "read",
                "search"
            ]
        },
        "combo_sc": {
            "scheme": "combo",
            "oneOf": [
                "oauth2_code",
                "oauth2_client",
                "oauth2_device"
            ]
        }
    },
    "security": "combo_sc",
    "base": "https://tdd.example.com",
    "actions": {
        "createThing": {
            "@type": "CreateThingAction",
            "description": "Create a Thing Description",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "PUT",
                    "contentType": "application/td+json",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 201
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "write"
                },
                {
                    "href": "/things",
                    "htv:methodName": "POST",
                    "contentType": "application/td+json",
                    "response": {
                        "description": "Success response",
                        "htv:headers": [
                            {
                                "description": "System-generated UUID (version 4) URN",
                                "htv:fieldName": "Location",
                                "htv:fieldValue": ""
                            }
                        ],
                        "htv:statusCodeValue": 201
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "write"
                }
            ]
        },
        "updateThing": {
            "description": "Update a Thing Description",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "PUT",
                    "contentType": "application/td+json",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 204
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "write"
                },
                {
                    "href": "/things/{id}",
                    "htv:methodName": "PATCH",
                    "contentType": "application/merge-patch+json",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 204
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        },
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "write"
                }
            ]
        },
        "deleteThing": {
            "description": "Delete a Thing Description",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "DELETE",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 204
                    },
                    "additionalResponses": [
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "write"
                }
            ]
        }
    },
    "properties": {
        "things": {
           "@type": "ThingListProperty",
            "description": "Collection of Thing Descriptions in the directory",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                },
                "jquery": {
                    "@type": "JSONPathQuery",
                    "title": "A valid JSONPath expression",
                    "type": "string"
                },
                "xquery": {
                    "@type": "XPathQuery",
                    "title": "A valid XPath expression",
                    "type": "string"
                },
                "query": {
                    "@type": "SPARQLQuery",
                    "title": "A valid SPARQL 1.1. query",
                    "type": "string"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 200,
                        "contentType": "application/td+json"
                    },
                    "additionalResponses": [
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "read"
                },
                {
                    "href": "/things/?jquery={jquery}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "JSONPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/things/?xquery={xquery}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "XPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/things?query={query}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "SPARQL query not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/things",
                    "htv:methodName": "POST",
                    "contentType:": "application/x-www-form-urlencoded",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "SPARQL query not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/things",
                    "htv:methodName": "POST",
                    "contentType:": "application/sparql-query",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "JSONPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                }

            ]
        },
    },
    "events": {
        "thingCreated": {
          "@type": "ThingCreatedEvent",
          "title": "Thing created",
          "data": {
              "type": "object",
              "description": "The schema of created TD event data including the created TD",
              "properties": {
                  "td_id": {
                      "type": "string",
                      "format": "iri-reference",
                      "description": "Identifier of TD in directory"
                  },
                  "td": {
                      "type": "object",
                      "description": "The created TD in a create event"
                  }
              }
          },
          "forms": [
              {
                  "op": "subscribeevent",
                  "href": "/events/thingcreated",
                  "subprotocol": "sse",
                  "contentType": "text/event-stream",
                  "htv:headers": [
                      {
                          "description": "ID of the last event for reconnection",
                          "htv:fieldName": "Last-Event-ID",
                          "htv:fieldValue": ""
                      }
                  ],
                  "scopes": "notifications"
              }
          ]
        },
        "thingUpdated": {
          "@type:": "ThingUpdatedEvent",
          "title": "Thing updated",
          "data": {
              "type": "object",
              "description": "The schema of updated TD event data including the TD updates",
              "properties": {
                  "td_id": {
                      "type": "string",
                      "format": "iri-reference",
                      "description": "Identifier of TD in directory"
                  },
                  "td_updates": {
                      "type": "object",
                      "description": "The partial TD composed of modified TD parts in an update event"
                  }
              }
          },
          "forms": [
              {
                  "op": "subscribeevent",
                  "href": "/events/thingupdated",
                  "subprotocol": "sse",
                  "contentType": "text/event-stream",
                  "htv:headers": [
                      {
                          "description": "ID of the last event for reconnection",
                          "htv:fieldName": "Last-Event-ID",
                          "htv:fieldValue": ""
                      }
                  ],
                  "scopes": "notifications"
              }
          ]
        },
        "thingDeleted": {
          "@type": "ThingDeletedEvent",
          "title": "Thing deleted",
          "data": {
              "type": "object",
              "description": "The schema of deleted TD event data",
              "properties": {
                  "td_id": {
                      "type": "string",
                      "format": "iri-reference",
                      "description": "Identifier of TD in directory"
                  }
              }
          },
          "forms": [
              {
                  "op": "subscribeevent",
                  "href": "/events/thingupdated",
                  "subprotocol": "sse",
                  "contentType": "text/event-stream",
                  "htv:headers": [
                      {
                          "description": "ID of the last event for reconnection",
                          "htv:fieldName": "Last-Event-ID",
                          "htv:fieldValue": ""
                      }
                  ],
                  "scopes": "notifications"
              }
          ]
        }
    }
}

farshidtz commented 3 years ago

Why does a description of a thing have a @type of Thing, but a description of a directory has a @type of DirectoryDescription? Surely it should either be ThingDescription & DirectoryDescription or Thing & Directory for consistency?

DirectoryDescription and LinkDescription were added together. See #54. Please raise your concerns in #43 and discuss it with the semantics members (you can mention AndreaCimminoArriaga). Keep in mind that DirectoryDescription is an alias; you can set it to anything you want and map it to discovery:DirectoryDescription in context.

I see. Fortunately it is still possible to differentiate between a POST to create a thing and a POST to query things using contentType (application/td+json vs. application/x-www-form-urlencoded or application/sparql-query).

This is a reasonable approach, keeping application/td+json as default. We should remember to disallow other pagination arguments (if any) when a query is given to avoid double pagination (jsonpath, xpath, sparql have their own mechanisms).

The events were merged into one form to allow subscription to one, multiple, or all events. Also, other query arguments for subscribing to a particular TD or toggling differences are missing. We could try to simplify current data schema by reducing to a single schema for all types.

This feels really strange, because you could make the same argument for any Thing Description. Should all Thing Descriptions only define a single event? I think this is an argument for a subscribeallevents operation like readallproperties (we actually have this in WebThings, where we have top level events and actions resources like the top level properties resource), rather than munging all the events into one giant event called "registration".

This is about a directory, serving a large number of clients. Separating the events forces each client who wants all events to maintain three persistent connections with the server. This is fine in HTTP/2 because these connections gets multiplexed on a single socket. But clients still need to subscribe three times. With that said, I don't have have a strong objection against separating them. We should discuss this further though, so let's try to do an incremental refactoring.

How does the server differentiate between a PUT to create a thing and a PUT to update a thing? They currently have the same URI and contentType but different response codes.

The server should have the means to check this internally, e.g. by checking if it already exists or checking the DB response.

Why is the format of an id an "iri-reference" rather than a "uri"?

You should ask this on the TD issue tracker. TD IDs are IRIs as far as the JSON Schema states.

Is there any reason a system-generated ID has to be a UUID (version 4)? WebThings uses HTTP URIs and the TD spec says "anyURI".

It doesn't have to be. It is recommended. From the spec: "The identifier SHOULD be a Version 4 UUID URN [RFC4122]."

What's the Last-Event-ID header in events for?

It is from SSE; please read https://w3c.github.io/wot-discovery/#exploration-directory-api-notification

Some minor things about the TD: I think jsonpath and xpath are better than jquery (jQuery?) and xquery. Also, without the trailing slash is between /things and ?.

farshidtz commented 3 years ago

I see. Fortunately it is still possible to differentiate between a POST to create a thing and a POST to query things using contentType (application/td+json vs. application/x-www-form-urlencoded or application/sparql-query).

This is a reasonable approach, keeping application/td+json as default. We should remember to disallow other pagination arguments (if any) when a query is given to avoid double pagination (jsonpath, xpath, sparql have their own mechanisms).

On a second thought, while that is is technically valid in HTTP, it has some problems:

It is not RESTful: POST /things implies that a thing is being submitted. But here, a sparql query is being submitted. The current GET /search/jsonpath|xpath|sparql suffered from a similar issue by implying a jsonpath|xpath|sparql is being retrieved.
Using POST /things for both submission of a (1) TD and (2) sparql query makes tracing, logging, and authorization harder:
- Tracing and log monitoring tools will not be able to easily identify the root cause behind long running POST /things requests. Processing request headers for this purpose in not common.
- Simple authorization is usually done on the HTTP method and path. API middlewares or API gateways will have to look at the request header and support such enforcement capability to do simple authorization.

One alternative would be to just decouple sparql's POST endpoint and leave the rest on /things:

GET /things{?jsonpath,xpath,query} // I think we can merge these into a single form
POST /sparql

benfrancis commented 3 years ago

This is about a directory, serving a large number of clients. Separating the events forces each client who wants all events to maintain three persistent connections with the server. This is fine in HTTP/2 because these connections gets multiplexed on a single socket. But clients still need to subscribe three times. With that said, I don't have have a strong objection against separating them. We should discuss this further though, so let's try to do an incremental refactoring.

We might need to test this, but presumably if there was a subscribeallevents operation there could be a single /events endpoint via which a client could subscribe to all events on a single connection.

Some minor things about the TD: I think jsonpath and xpath are better than jquery (jQuery?) and xquery. Also, without the trailing slash is between /things and ?.

Agreed, both JQuery and XQuery have other meanings so would be confusing.

One alternative would be to just decouple sparql's POST endpoint and leave the rest on /things

It's not as neat, but I agree the POST on /things isn't perfect either and we are constrained by what is fixed by the SPARQL specification, so something like this may indeed be necessary as a workaround.

Although I'm not an expert in any of these technologies, it does make some sense in my head to handle JSONPath and XPath separately from SPARQL given that the former two provide expressions to select nodes from the JSON tree, whereas SPARQL is more of a fully fledged database query language like SQL and XQuery (which builds on XPath). Others may see the distinction differently though. I'm therefore not sure whether the GET /things?query should also become GET /sparql?query.

Updated TD below. I'll try to split this into multiple pull requests to review when I get the chance.

{
    "@context": [
        "http://www.w3.org/ns/td",
        "https://w3c.github.io/wot-discovery/context/discovery-context.jsonld"
    ],
    "@type": "Directory",
    "title": "Example Thing Description Directory (TDD)",
    "version": {
        "instance": "1.0.0-alpha"
    },
    "securityDefinitions": {
        "oauth2_code": {
            "scheme": "oauth2",
            "flow": "code",
            "authorization": "https://auth.example.com/authorization",
            "token": "https://auth.example.com/token",
            "scopes": [
                "write",
                "read",
                "search"
            ]
        },
        "oauth2_client": {
            "scheme": "oauth2",
            "flow": "client",
            "token": "https://auth.example.com/token",
            "scopes": [
                "write",
                "read",
                "search"
            ]
        },
        "oauth2_device": {
            "scheme": "oauth2",
            "flow": "device",
            "authorization": "https://auth.example.com/device_authorization",
            "token": "https://auth.example.com/token",
            "scopes": [
                "write",
                "read",
                "search"
            ]
        },
        "combo_sc": {
            "scheme": "combo",
            "oneOf": [
                "oauth2_code",
                "oauth2_client",
                "oauth2_device"
            ]
        }
    },
    "security": "combo_sc",
    "base": "https://tdd.example.com",
    "actions": {
        "createThing": {
            "@type": "CreateThingAction",
            "description": "Create a Thing Description",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "PUT",
                    "contentType": "application/td+json",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 201
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "write"
                },
                {
                    "href": "/things",
                    "htv:methodName": "POST",
                    "contentType": "application/td+json",
                    "response": {
                        "description": "Success response",
                        "htv:headers": [
                            {
                                "description": "System-generated URI",
                                "htv:fieldName": "Location",
                                "htv:fieldValue": ""
                            }
                        ],
                        "htv:statusCodeValue": 201
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "write"
                }
            ]
        },
        "updateThing": {
            "description": "Update a Thing Description",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "PUT",
                    "contentType": "application/td+json",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 204
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "write"
                },
                {
                    "href": "/things/{id}",
                    "htv:methodName": "PATCH",
                    "contentType": "application/merge-patch+json",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 204
                    },
                    "additionalResponses": [
                        {
                            "description": "Invalid serialization or TD",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        },
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "write"
                }
            ]
        },
        "deleteThing": {
            "description": "Delete a Thing Description",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "DELETE",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 204
                    },
                    "additionalResponses": [
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "write"
                }
            ]
        }
    },
    "properties": {
        "things": {
           "@type": "ThingListProperty",
            "description": "Collection of Thing Descriptions in the directory",
            "uriVariables": {
                "id": {
                    "@type": "ThingID",
                    "title": "Thing Description ID",
                    "type": "string",
                    "format": "iri-reference"
                },
                "jsonpath": {
                    "@type": "JSONPathExpression",
                    "title": "A valid JSONPath expression",
                    "type": "string"
                },
                "xpath": {
                    "@type": "XPathExpression",
                    "title": "A valid XPath expression",
                    "type": "string"
                },
                "query": {
                    "@type": "SPARQLQuery",
                    "title": "A valid SPARQL 1.1. query",
                    "type": "string"
                }
            },
            "forms": [
                {
                    "href": "/things/{id}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 200,
                        "contentType": "application/td+json"
                    },
                    "additionalResponses": [
                        {
                            "description": "TD with the given id not found",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 404
                        }
                    ],
                    "scopes": "read"
                },
                {
                    "href": "/things?jsonpath={jsonpath}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "JSONPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/things?xpath={xpath}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "XPath expression not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/things?query={query}",
                    "htv:methodName": "GET",
                    "response": {
                        "description": "Success response",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "SPARQL query not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                }
            ]
        },
        "sparql": {
            "@type": "SPARQLProperty",
            "description": "Endpoint for querying the directory with SPARQL",
            "forms": [
                {
                    "href": "/sparql",
                    "htv:methodName": "POST",
                    "contentType:": "application/x-www-form-urlencoded",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "SPARQL query not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                },
                {
                    "href": "/sparql",
                    "htv:methodName": "POST",
                    "contentType:": "application/sparql-query",
                    "response": {
                        "description": "Success response",
                        "contentType": "application/json",
                        "htv:statusCodeValue": 200
                    },
                    "additionalResponses": [
                        {
                            "description": "SPARQL query not provided or contains syntax errors",
                            "contentType": "application/problem+json",
                            "htv:statusCodeValue": 400
                        }
                    ],
                    "scopes": "search"
                }
            ]
        }
    },
    "events": {
        "thingCreated": {
          "@type": "ThingCreatedEvent",
          "title": "Thing created",
          "data": {
              "type": "object",
              "description": "The schema of created TD event data including the created TD",
              "properties": {
                  "td_id": {
                      "type": "string",
                      "format": "iri-reference",
                      "description": "Identifier of TD in directory"
                  },
                  "td": {
                      "type": "object",
                      "description": "The created TD in a create event"
                  }
              }
          },
          "forms": [
              {
                  "op": "subscribeevent",
                  "href": "/events/thingcreated",
                  "subprotocol": "sse",
                  "contentType": "text/event-stream",
                  "htv:headers": [
                      {
                          "description": "ID of the last event for reconnection",
                          "htv:fieldName": "Last-Event-ID",
                          "htv:fieldValue": ""
                      }
                  ],
                  "scopes": "notifications"
              }
          ]
        },
        "thingUpdated": {
          "@type:": "ThingUpdatedEvent",
          "title": "Thing updated",
          "data": {
              "type": "object",
              "description": "The schema of updated TD event data including the TD updates",
              "properties": {
                  "td_id": {
                      "type": "string",
                      "format": "iri-reference",
                      "description": "Identifier of TD in directory"
                  },
                  "td_updates": {
                      "type": "object",
                      "description": "The partial TD composed of modified TD parts in an update event"
                  }
              }
          },
          "forms": [
              {
                  "op": "subscribeevent",
                  "href": "/events/thingupdated",
                  "subprotocol": "sse",
                  "contentType": "text/event-stream",
                  "htv:headers": [
                      {
                          "description": "ID of the last event for reconnection",
                          "htv:fieldName": "Last-Event-ID",
                          "htv:fieldValue": ""
                      }
                  ],
                  "scopes": "notifications"
              }
          ]
        },
        "thingDeleted": {
          "@type": "ThingDeletedEvent",
          "title": "Thing deleted",
          "data": {
              "type": "object",
              "description": "The schema of deleted TD event data",
              "properties": {
                  "td_id": {
                      "type": "string",
                      "format": "iri-reference",
                      "description": "Identifier of TD in directory"
                  }
              }
          },
          "forms": [
              {
                  "op": "subscribeevent",
                  "href": "/events/thingupdated",
                  "subprotocol": "sse",
                  "contentType": "text/event-stream",
                  "htv:headers": [
                      {
                          "description": "ID of the last event for reconnection",
                          "htv:fieldName": "Last-Event-ID",
                          "htv:fieldValue": ""
                      }
                  ],
                  "scopes": "notifications"
              }
          ]
        }
    }
}

farshidtz commented 3 years ago

It's not as neat, but I agree the POST on /things isn't perfect either and we are constrained by what is fixed by the SPARQL specification, so something like this may indeed be necessary as a workaround.

Although I'm not an expert in any of these technologies, it does make some sense in my head to handle JSONPath and XPath separately from SPARQL given that the former two provide expressions to select nodes from the JSON tree, whereas SPARQL is more of a fully fledged database query language like SQL and XQuery (which builds on XPath). Others may see the distinction differently though. I'm therefore not sure whether the GET /things?query should also become GET /sparql?query.

I also vote for keeping SPARQL separate and add the following reasons:

Having all the three SPARQL interactions under the same path simplifies the API and makes development, logging, and access control easier.
SPARQL also defines other query arguments which we have missed:

query (exactly 1) default-graph-uri (0 or more) named-graph-uri (0 or more)

I don't know what the other two mean, but we need to consider them. So far, they don't collide with other query arguments on the /things endpoint. But we don't know what other query arguments future SPARQL spec and extensions would need to add to that. The safest and most extensible option is to keep it separate.

Updated TD below. I'll try to split this into multiple pull requests to review when I get the chance.

Apart from the events, I consider all changes as refactoring because they involve no functional changes to the directory. PRs would be very useful.

For events, we can discuss the functional changes in #28 and #42: that is not being able to subscribe to multiple or all events in a single operation. I personally think we should design the API based on requirements (derived from use cases) and not based on what is possible or nicer in a TD.

benfrancis commented 3 years ago

I personally think we should design the API based on requirements (derived from use cases) and not based on what is possible or nicer in a TD.

I agree with starting from use cases & requirements, but if the W3C is going to provide a normative Thing Description/Thing Model in a specification then it should demonstrate best practices. Combining all events into a single event so that they can all be subscribed to in a single operation seems like a workaround for a missing feature in Thing Description, which suggests there is an additional unfulfilled requirement for that specification. I have filed https://github.com/w3c/wot-thing-description/issues/1082 for that.

farshidtz commented 3 years ago

@benfrancis are you able to join the discovery call now? We are going to discuss this in a couple of minutes.

benfrancis commented 3 years ago

On my way.

farshidtz commented 3 years ago

Notes from the discovery call:

things vs tds vs td (current resource name) in Directory's REST API
- td is not plural
- tds is shorter than things
- tds may be read as T D S
Adding filtering on top of listing. Keeping sparql separate.
- sparql as property is weird. We can start by having sparql separate and decide later about the name and whether it should be an action called sparqlSearch.

relu91 commented 3 years ago

Just a comment that wasn't captured in the call today. With the current things property and the uriVariables, the consumer does not really have clue to which form to use when filtering the collection. The fact that each form can only accept one specific URI variable complicates a little bit the situation. Please fill in if I missed something.

I think this is a good playground where we can stress the current TD spec and see its strengths and limits. I would introduce what we can in the current discovery spec and continue the discussion of the open questions on the TD spec. I am opening an issue about the problem above.

farshidtz commented 3 years ago

For events, I still think registration as the event name is fine. It is the event triggered upon registration in a directory. Similar to "overheating" event on a machine. It can be changed to thingRegistration if it helps.

But I can live with breaking the event:

registration: /events{?type,td_id,include_changes}

into:

thingCreated: /events?type=created_td{&td_id,include_changes}
thingUpdated: /events?type=updated_td{&td_id,include_changes}
thingDeleted: /events?type=deleted_td{&td_id}

The API remains the same and would implicitly allow subscription to multiple: /events?type=created_td&type=deleted_td or all: /events. Optionally adding a fourth event to describe multiple/all:

thingRegistration: /events{?type,td_id,include_changes} 😌 -> back to the original?

benfrancis commented 3 years ago

For events, I still think registration as the event name is fine. It is the event triggered upon registration in a directory.

The issue isn't that the event is called "registration", it's that it actually represents three different types of events: created, updated and deleted. Arguably only the first of those is actually a "registration".

thingCreated: /events?type=created_td{&td_id,include_changes}
thingUpdated: /events?type=updated_td{&td_id,include_changes}
thingDeleted: /events?type=deleted_td{&td_id}

Why do you prefer this to:

thingCreated: /events/thingcreated
thingUpdate: /events/thingupdated
thingDeleted: /events/thingdeleted

(Again, I don't think URIs and URI variables should be fixed in the specification, which would allow either of these examples to be valid.)

thingRegistration: /events{?type,td_id,include_changes} relieved -> back to the original?

  {
    ...
    "forms": [{
        "op": "subscribeallevents",
        "href": "/events",
    }
  }

https://github.com/w3c/wot-thing-description/issues/1082

farshidtz commented 3 years ago

thingCreated: /events?type=created_td{&td_id,include_changes}
thingUpdated: /events?type=updated_td{&td_id,include_changes}
thingDeleted: /events?type=deleted_td{&td_id}
Why do you prefer this to:

thingCreated: /events/thingcreated

thingUpdate: /events/thingupdated

thingDeleted: /events/thingdeleted

?

Please note my example:

The API remains the same and would implicitly allow subscription to multiple: /events?type=created_td&type=deleted_td or all: /events.

From the current spec:

The server MUST support event filtering based on the event types passed as one or more type query parameters. For example, in response to query ?type=created_td&type=deleted_td, the server must only deliver events of types created_td and deleted_td. At the absence of any type query parameter, the server must deliver all types of events.

benfrancis commented 3 years ago

I have submitted four pull requests to apply the changes proposed in this issue:

Combine properties into a single things property #158
Split registration event into multiple events #159
Rename actions to match properties and events #160
Rename DirectoryDescription to ThingDirectory #161

Hopefully this is granular enough to continue this discussion and start to nail down some of the details.

If any of these changes are accepted then follow-up PRs will be needed to update the text of the specification to match the machine-readable description of the API in the example Thing Description.

Note: Included in these PRs are a collection of semantic annotations which help to identify various interaction affordances, URI variables and data schemas without needing to hard code affordance names, URIs and URI variables in the specification. This is a separate issue being discussed in #144 but the proposed changes don't make as much sense without those annotations.

benfrancis commented 3 years ago

P.S. If you'd like to see what the Thing Directory Description would look like with all of the above four changes combined, you can see the combined result here and a rendered version here.

mmccool commented 3 years ago

Original issue has been dealt with by a set of PRs (@farshidtz to add links afterwards).

farshidtz commented 3 years ago

List of merged PRs related to this issue:

Update exploration figures and types #181
Refactor Directory Service API TD (td->thing, split anonymous create action) #188
Refactor Directory Service API TD (retrieve one and search as actions, listing as things property) #191
Refactor Directory Service API TD (split events) #196
Notification API: pass event type in HTTP path instead of query #198