Page MenuHomePhabricator

REST API ResponseFactory
Closed, ResolvedPublic

Description

When planning the REST API, I initially thought that a ResponseFactory with a single method, returning an empty response, would be sufficient. Handlers could modify this response object to achieve any goal. The problem with this, immediately apparently when I started writing code, was that handler authors shouldn't be asked to format every response from scratch. There needs to be a policy framework which makes it easier to generate standard or preferred kinds of responses. ResponseFactory itself could be this policy framework, providing various methods for creating pre-filled responses.

Writing ResponseFactory thus requires us to answer several of the questions I asked on T221177:

  • What should standard responses look like? For example, standard 404s and parameter validation errors. Should they be JSON? Localised?
  • What sorts of responses will endpoints typically need?
  • Is ResponseFactory responsible for localisation?
  • Do iteration/index routes require any special handling?

Event Timeline

tstarling reassigned this task from tstarling to Tgr.May 15 2019, 6:28 AM
tstarling updated the task description. (Show Details)
tstarling added a subscriber: tstarling.
Tgr added a comment.May 27 2019, 2:27 PM

What should standard responses look like? For example, standard 404s and parameter validation errors. Should they be JSON? Localised?

I don't see any reason not to use JSON - it's standard, easy to parse, easy to extend, reasonably human-readable.
Localisation is a tougher question. On one hand we want Wikimedia software to be inclusive to non-native-English-speakers, on the other hand we want most errors to be cacheable, and we probably don't want to split the cache by language.
Also, what language to localize in? The action API has a separate uselang and errorlang; the REST API would only have Accept-Language which is the equivalent of uselang. That's the language of the user of the app that issues the API calls, not necessarily the language of the API developer. For example as a developer I might support a multilingual userbase but wouldn't want the errors to be in a random sampling of the supported languages.

I think the least bad option is to keep human-readable messages out of the response and use error codes instead, and include a link to the documentation (which is easy to localize) and have that list the error codes (to the extent possible - some will come from non-API layers of MediaWiki and will be unexpected), and have an API endpoint for localizing an error code (which I imagine will be a serialized Message or Status), and advertise that prominently.

What sorts of responses will endpoints typically need?

Status codes:

  • obvious ones: 200 OK, 204 No Content 301 Moved Permanently, 302 Found, 304 Not Modified, 400 Bad Request, 401 Unauthorized, 403 Forbidden, 404 Not Found, 500 Internal Server Error, 503 Service Unavailable
  • obvious in theory, not sure about the practical benefits/drawbacks wrt. substituting it with a more common code: 201 Created, 405 Method Not Allowed (if Router is capable of emitting that), 406 Not Acceptable (for API version negotiation), 417 Expectation Failed (at the very least we should have expect logged in / out, like the action API does)
  • has a clear use case but probably too fringe and better handled via a generic createFromStatusCode($code,$message) method: 202 Accepted (chunked uploads, endpoints that just put something in the job queue), 409 Conflict (edit conflicts), 410 Gone (deleted content), 415 Unsupported Media Type (when storing revision content), 504 Gateway Timeout

Also probably a convenience method for common content types (text, JSON) for successful responses.
Also we might want something like the action API's ApiResponse for JSON responses, that makes it easy for other handlers/extensions to add extra fields (e.g. for debugging) via some hook mechanism.

Is ResponseFactory responsible for localisation?

Seems like a reasonable enough place for now (although if we want to librarize it, we might have to rethink that once i18n itself is librarized).

Do iteration/index routes require any special handling?

There should probably be a helper if we want consistent structure for continuation. I don't think ResponseFactory has anything to do with that. (In theory we could get fancy and use Content-Range header, HTTP 206 and so on, but it would make developers' job more confusing with no real benefit).

By index route, do you also mean iteration/continuation? Or something like https://en.wikipedia.org/api/rest_v1/page/ ? That would have to be handled by the Router (personally I do not see much value in it).

Tgr added a comment.May 27 2019, 2:33 PM

Various things try to output cookies or headers on arbitrary requests (e.g. SessionManager backends or ChronologyProtector). Should ResponseFactory copy those into the Response? Should the Router do it after the end of the call? or should those just be output via a completely different mechanism? (Works OK for cookies, for headers it would be a spec violation if they conflict.)
Also, not clear how/where sessions should be handled.

Change 512775 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] [WIP] Expand ResponseFactory

https://gerrit.wikimedia.org/r/512775

Change 514207 had a related patch set uploaded (by Tim Starling; owner: Gergő Tisza):
[mediawiki/core@master] Expand ResponseFactory

https://gerrit.wikimedia.org/r/514207

Change 512775 abandoned by Gergő Tisza:
[WIP] Expand ResponseFactory

Reason:
Ie185b2bd4369 contains most of this except JsonResponse, which was problematic in many ways. I still feel we'll need some kind of extensible JSON response at some point but it's definitely not something the core functionality of the API should be blocked on (and trying to hack it on top of the ResponseInterface was a bad idea). So probably something to think about later once (if) we have actual use cases.

https://gerrit.wikimedia.org/r/512775

Change 514207 merged by jenkins-bot:
[mediawiki/core@master] Expand ResponseFactory

https://gerrit.wikimedia.org/r/514207

Tgr closed this task as Resolved.Jun 14 2019, 8:17 PM