Page MenuHomePhabricator

Confirm Reading Lists endpoint changes with callers
Open, In Progress, LowPublic

Description

tl;dr: after the new Reading Lists endpoints are complete, discuss API changes with callers to ensure everything is satisfactory.

I've subscribed some folks from the mobile apps teams to this task. But we're not quite ready for the conversations yet, because the endpoints are still being finalized. Shouldn't be too long now - days or at worst weeks rather than months.

Some changes to endpoint behavior are inevitable as part of reimplementing the Reading Lists endpoints. For example, some error messages include the word "Hyperswitch", which is a RESTBase-related technology that we will no longer be using. These error messages will therefor contain different text in the new endpoints. Some headers may also be sent differently by MediaWiki than by RESTBase.

Additionally, there may be intentional changes to the contract, such as adding parameters previously supported by the Action API endpoints that RESTBase forwarded to but not exposed by RESTBase (limit, etc.) Callers should be made aware of these parameters, in case they wish to make use of them.

Completion criteria:

  • Changes have been documented in a way conducive to a conversation with the. mobile apps teams
  • IOS app team has confirmed they are comfortable with the changes (and/or any necessary code adjustments have been made)
  • Android app has confirmed they are comfortable with the changes (and/or any necessary code adjustments have been made)

Event Timeline

Status:

Initial implementation of new handlers have been merged and is available on production. Nothing is routing to them, so there is no change to callers at this time. WMF Mobile apps are still hitting the RESTBase endpoints, with no changes. This will be the case until we proactively reroute calls to the new endpoints via the API Gateway. We do not plan to do this until callers have had the opportunity to review changes, and any necessary adjustments are made.

The new endpoints are at "v0" paths in the MW REST API, which essentially means "no promises". In other words, we can still make any changes we desire to these endpoints without violating anything we have promised to other callers. And with that said, we're not aware of callers we need to support other than the WMF mobile apps, and we have also not announced the new endpoints in any way (other than phabricator tasks, gerrit changes, and their presence in the MW REST API Open API spec).

I've closed the phab tasks for the initial implementations of the new endpoints. Any subsequent changes can be handled under new tasks.

Open API spec for all new REST endpoints is available here: https://meta.wikimedia.beta.wmflabs.org/w/rest.php/

For comparison, the RESTBase spec for the existing endpoints is available here: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/services/restbase/+/d544e9da48b9c79afb2338b7bab6960a94db6c61/v1/lists.yaml

I'm not aware of a great way to directly compare the two specs. The MW REST one includes all endpoints provided by MW REST, not just the Reading Lists endpoints. (We have an open phab task for filtering, but that is not being actively worked on at this time.) Also, the RESTBase one is very ... well ... RESTBase-ish while the MW REST one is generated from the endpoint parameter validation code and is a little underwhelming, particularly on things like response formats.

Instead, I will summarize contract changes in this comment. If it would be more helpful, I can present this in a different format or in a synchronous meeting. Just let me know.

General contract changes:

requests:

  • endpoints that require csrf tokens will now accept them by either the name "csrf_token" (per RESTBase spec) or "token" (as preferred by the MediaWiki REST API infrastructure). We would prefer to see callers use the name "token", because that would allow us to remove some special-case code. But that change can be made over time after the switchover, at caller's convenience.

response:

  • endpoints will return 403 (vs 400) in some error situations (ex. bad token).
  • error response format changed to match MW REST API conventions. See related code here.

Example error responses:

RESTBase:

{
  "type": "https://mediawiki.org/wiki/HyperSwitch/errors/bad_request",
  "title": "badtoken",
  "method": "post",
  "detail": "Invalid CSRF token.",
  "uri": "/en.wikipedia.org/v1/data/lists/setup"
}

MW REST:

{
    "errorKey": "rest-badtoken",
    "messageTranslations": {
        "en": "The <var>token</var> parameter is required unless using a CSRF-safe authentication method."
    },
    "httpCode": 403,
    "httpReason": "Forbidden"
}

Another example (this one for data/lists/?sort=invalidSort):

RESTBase:

{"type":"https://mediawiki.org/wiki/HyperSwitch/errors/bad_request","title":"Invalid parameters","method":"get","detail":"data.query.sort should be equal to one of the allowed values: [name, updated]","uri":"/en.wikipedia.org/v1/data/lists/"}

MW REST:

{
    "error":"parameter-validation-failed",
    "name":"sort",
    "value":"invalidSort",
    "failureCode":"badvalue",
    "failureData":[],
    "errorKey":"badvalue",
    "messageTranslations":{
        "en":"Unrecognized value for parameter \"sort\": invalidSort."
    },
    "httpCode":400,
    "httpReason":"Bad Request"
}

If these changes are problematic to callers, we could look into matching most closely the existing error responses. Some things seem infeasible to match (for example, it would be nonsensical to include the term "HyperSwitch" anywhere). But we could get a lot closer to the existing responses if need be.

GET /lists endpoint

Added the following parameters that were previously exposed by the underlying Action API endpoint, but not by RESTBase:

namesourcerequiredtypeexampledefaultpossible valuesdescription
dirquerynostringdescendingascendingascending, descendingsort direction
limitquerynointeger5101..10Maximum number of values to return
  • GET /lists/{id} endpoint **

This handler did not existing in the set of endpoints exposed by RESTBase, but mirrors functionality that was available through the Action API. It returns a single list, specified by id. Although unnecessary for callers (they haven't needed it before now, and nobody asked for it), it was trivial to add, and lack of this endpoint was glaring from a CRUD perspective. It will be available if callers choose to use it.

GET /lists/pages endpoint

Added the following parameter that was previously exposed by the underlying Action API endpoint, but not by RESTBase:

namesourcerequiredtypeexampledefaultpossible valuesdescription
limitquerynointeger5101..10Maximum number of values to return
  • GET /lists/changes/since endpoint **

Added the following parameters that were previously exposed by the underlying Action API endpoint, but not by RESTBase:

namesourcerequiredtypeexampledefaultpossible valuesdescription
sortquerynostringupdatednamename, updatedsort type
dirquerynostringdescendingascendingascending, descendingsort direction
limitquerynointeger5101..10Maximum number of values to return
  • GET /lists/{id}/entries endpoint **

Added the following parameters that were previously exposed by the underlying Action API endpoint, but not by RESTBase:

namesourcerequiredtypeexampledefaultpossible valuesdescription
dirquerynostringdescendingascendingascending, descendingsort direction
limitquerynointeger5101..100Maximum number of values to return
FJoseph-WMF changed the task status from Open to In Progress.Fri, Apr 5, 2:43 PM