-
Notifications
You must be signed in to change notification settings - Fork 384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optional body parameters #3237
Comments
yeah I don't think it is intentional either. |
It seems like element-android does not provide a body for |
Other REST - or REST-like - APIs very often allow for completely omitting empty bodies, so this could easily become confusing for developers. |
On the other hand, this is something that's easily noticed and fixed whilst developing clients (if it gets rejected), but it's more annoying leaving the lack of clarity open to the point that different homeserver implementations might act differently. |
examples would be useful, otherwise it's hard to evaluate the truth of this assertion. Presumably you'd advocate opening up other endpoints to allow omitting bodies too, for consistency? |
yes, it's clear that the spec should be making a stand in one direction or the other. |
I'll have a look through some other APIs too, unfortunately the three I have in mind right now aren't public so I don't know if I'm allowed to share them as examples.
This is basically the core of my concern, doesn't actually matter overly much if it's allowed or not, as long it is clear to the developers what is to be expected - and that the actual implementation doesn't differ between servers.
And yeah, if the endpoints don't require arguments then - if empty bodies should be allowed - they should also allow omitting the empty object entirely. |
I'd instead say that it's better to have the body required in all instances — it's my opinion that there should be only one way to do things. If there's only one way to do things, clients will very quickly switch to doing it in only one way. If clients only do it in one way, then servers will definitely support doing it that way (otherwise they notice quickly that clients don't work!!). If there are two ways to do things, then we have to test that you can do it both ways, and that all homeservers can do it both ways (even though common clients may actually only do it in one of those two ways). You could say that we could make the spec mandate that empty bodies are ALWAYS omitted and then that would satisfy my opinion above. That's true, but it probably means having extra special-case code in clients and servers when, actually, emitting empty bodies was probably easier to do as it didn't need a special code path. (naturally, this is just my opinion, nothing more or less :) |
Another datapoint here: synapse currently requires a body for all of the endpoints mentioned above except |
There are 5 operations in data/api/client-server that don't require their body parameter (missing
required: true
):Is this intentional? If yes, how can requests be made without body (for example, send
null
asapplication/json
body)? And what is the effect of omitting the body? Except for searchUserDirectory, there aren't required parameters, so an empty body can be sent instead without obvious differences. About searchUserDirectory: Is search without search term supported by omitting the body completely? If so, why is the search term a required parameter of the body? postReceipt adds an extra field in the body, which would not work if the body is null.I think this isn't intentional. Would you accept a PR that sets
required: true
in these bodies?The text was updated successfully, but these errors were encountered: