Combine all queries into their own section #1443
Conversation
turt2live
added
the
blocked
There isn't a whole lot to this section that needed work. The section overall lost the table schema in favour of having the endpoints close by. The directory query is improved in matrix-org#1443
There isn't a whole lot to this section that needed work. The section overall lost the table schema in favour of having the endpoints close by. The directory query is improved in matrix-org#1443
turt2live
force-pushed
the
travis/s2s/query
branch
from
f88173e
to
1873f2a
Compare
This removes the Directory and Profile sections, instead opting to document them as Queries. The behaviour of profile queries is based on Synapse's behaviour. A few issues have been opened to improve the behaviour: * matrix-org#1434 * matrix-org#1435 * https://github.com/matrix-org/matrix-doc/issues/1436 * https://github.com/matrix-org/matrix-doc/issues/1437 This fixes matrix-org#1404
This is a mix of Synapse and Dendrite behaviour, mostly Dendrite. Synapse returns `null` for field values that aren't set, however Dendrite just doesn't return them and instead opts for an empty object. Further, synapse is lacking in error codes in this area. Dendrite does some additional validation on this API which introduces more errors for bad requests, instead of defaulting to empty objects/200 OK responses. Likewise, Dendrite returns a 404 when the user is not found while Synapse returns 200 OK/empty object.
turt2live
force-pushed
the
travis/s2s/query
branch
from
f1c943a
to
d914c40
Compare
There isn't a whole lot to this section that needed work. The section overall lost the table schema in favour of having the endpoints close by. The directory query is improved in matrix-org#1443
turt2live
removed
the
blocked
specification/server_server_api.rst
Outdated
|
|
||
| Directory | ||
| --------- | ||
| Queries are a way to retrieve information from a homeserver abotu a resource, |
api/server-server/query.yaml
Outdated
| description: |- | ||
| Performs a query to get the mapped room ID and list of resident homeservers in | ||
| the room for a given room alias. Homeservers should only query room aliases | ||
| that belong to the target server (idenfified by the DNS Name in the alias). |
There was a problem hiding this comment.
I should re-learn how to type.
api/server-server/query.yaml
Outdated
| Performs a query to get the mapped room ID and list of resident homeservers in | ||
| the room for a given room alias. Homeservers should only query room aliases | ||
| that belong to the target server (idenfified by the DNS Name in the alias). | ||
| The target server may not appear in the resident servers list. |
There was a problem hiding this comment.
this is a bit ambiguous. how about "The target server may be absent from the resident servers list."
There was a problem hiding this comment.
In fact I question the value of this, given it's not entirely clear what "the resident servers list" refers to, and we already have "This list may or may not include the server answering the query" below.
There was a problem hiding this comment.
I think the statement later on should be okay, meaning this one can just be removed.
api/server-server/query.yaml
Outdated
| servers: | ||
| type: array | ||
| description: |- | ||
| An array of server names that are likely to hold then given room. This |
api/server-server/query.yaml
Outdated
| that belong to the target server (idenfified by the DNS Name in the alias). | ||
| The target server may not appear in the resident servers list. | ||
|
|
||
| Servers may wish to cache the response to this query to prevent requesting the |
api/server-server/query.yaml
Outdated
| $ref: "../client-server/definitions/errors/error.yaml" | ||
| examples: | ||
| application/json: { | ||
| "errcode": "M_UNKNOWN", |
There was a problem hiding this comment.
I'm really not keen on speccing M_UNKNOWN here.
There was a problem hiding this comment.
I guess it's only an example, but still.
api/server-server/query.yaml
Outdated
| } | ||
| 400: | ||
| description: |- | ||
| The room alias is not hosted on the server. This can happen if the directory |
There was a problem hiding this comment.
this implies that this is the only reason we would return a 400.
I'm not sure I'd bother speccing this as it currently stands - asking example.org for a matrix.org alias is just an invalid request like any other. Typically we don't spell out such things.
There was a problem hiding this comment.
The only reason I've specified the 400 for this case is because it felt like a gotcha that can be easily hit. I suppose the text in the description asking for the caller to use the right server is a good enough hint though...
api/server-server/query.yaml
Outdated
| for a given user. Homeservers should only query profiles for users that belong | ||
| to the target server (identified by the DNS Name in the user ID). | ||
|
|
||
| Servers may wish to cache the response to this query to prevent requesting the |
api/server-server/query.yaml
Outdated
| } | ||
| 400: | ||
| description: |- | ||
| The request was missing parameters or had invalid values for the parameters. This |
There was a problem hiding this comment.
can we make it explicit that the list of reasons is not exhaustive?
There was a problem hiding this comment.
Like the directory query, I think this can actually just disappear - the gotcha is covered in the description.
|
Thanks for reviewing this. Clearly it's suffered the result of my train of thought where parts get documented ahead of others, and things didn't get shuffled around like they normally do before opening the PR. Sorry about that :( Will address the concerns, and double check the other PRs to avoid excess review time. |
Rendered: See "docs" commit status.
This removes the Directory and Profile sections, instead opting to document them as Queries.
The behaviour of profile queries is based on Synapse's behaviour. A few issues have been opened to improve the behaviour:
M_NOT_FOUND, notM_UNKNOWN#1435This fixes #1404