Page MenuHomePhabricator

[SPIKE] Determine a strategy for documenting header parameters
Closed, ResolvedPublic

Description

@BPirkle to fill in more details

Event Timeline

HCoplin-WMF triaged this task as Medium priority.
HCoplin-WMF raised the priority of this task from Medium to High.
HCoplin-WMF lowered the priority of this task from High to Medium.Jan 9 2025, 4:00 PM

Incase you haven't already started looking at this @BPirkle For T412668 this is how I'm envisioning the headers added to the openapi specs according to OpenApi v3.x documentation:

Current request parameters block:

"paths" : {

"/v1/page/{title}/history": {
     "get": {
              "parameters": [
                            "name": "title",
                            "description": "Wiki page title",
                            "in": "path",
                            "schema": {
                                      "type": "string"
                              },
                              "required": true

Proposed Paramers block with request headers

"paths" : {

"/v1/page/{title}/history": {
     "get": {
              "parameters": [
                      {
                            "name": "title",
                            "description": "Wiki page title",
                            "in": "path",
                            "schema": {
                                      "type": "string"
                              },
                              "required": true
                     },
                     {
                            "name": "If-Match",
                            "description": "Transforms the HTTP request into one that is conditional based on a set of HTTP ETag headers matching.",
                            "in": "header",
                            "schema": {
                                      "type": "string"
                              },
                              "required": false
                     }

Proposed Paramers block with request headers as reusable component - Response components object already available at ResponseFactory::getResponseComponents() Will have to have similar setup for requests

"paths" : {

"/v1/page/{title}/history": {
     "get": {
              "parameters": [
                      {
                            "name": "title",
                            "description": "Wiki page title",
                            "in": "path",
                            "schema": {
                                      "type": "string"
                              },
                              "required": true
                     },
                    "$ref": "#/components/parameters/IfMatchHeader"

Current responses block:

"paths" : {

"/v1/page/{title}/history": {
     "get": {
           "responses": {
                    "200": {
                           "description": "OK",
                            "content": {}
                     },
                    "default": {
                             "$ref": "#/components/responses/GenericErrorResponse"
                    }
          },

Proposed Responses block with response headers

"paths" : {

"/v1/page/{title}/history": {
     "get": {
           "responses": {
                    "200": {
                           "description": "OK",
                            "content": {},
                            "headers": {
                                    "Last-Modified": {
                                        {
                                            "description": "Date and time the resource was last changed"
                                             "schema":
                                                     {
                                                         type: string
                                                    },
                    "default": {
                             "$ref": "#/components/responses/GenericErrorResponse"
                    }
          },

Proposed Responses block with response headers as reusable component - Response components object already available at ResponseFactory::getResponseComponents()

"paths" : {

"/v1/page/{title}/history": {
     "get": {
           "responses": {
                    "200": {
                           "description": "OK",
                            "content": {},
                            "headers": {
                                "$ref": "#/components/responses/LastModifiedHeader"
                    },
                    "default": {
                             "$ref": "#/components/responses/GenericErrorResponse"
                    }
          },

How did you test/confirm all that was correct? Some of it doesn't look like valid json. For example:

"responses": {
                    "200": {
                           "description": "OK",
                            "content": {},
                            "headers": {
                                    "Last-Modified": {
                                        {
                                            "description": "Date and time the resource was last changed"
                                             "schema":
                                                     {
                                                         type: string
                                                    },
                    "default": {
                             "$ref": "#/components/responses/GenericErrorResponse"
                    }
          },

There's no comma after the description value, the Last-Modified block isn't closed, type and string and missing quotes, etc.

I took a try at cleaning this up, by taking the following steps:

  1. load the "MediaWiki REST API" module in the REST Sandbox
  2. click the link to the spec
  3. copy the (really big) spec into a scratch file in PHP Storm
  4. strip out pretty much everything except the required elements, the /v1/page/{title}/history, and the reusable error components at the bottom (which are referenced by the history endpoint spec)
  5. strip out a bunch of request/response parameters from within the history endpoint to make it even smaller
  6. paste this into a swagger validator/viewer (I used https://editor.swagger.io/) to make sure I hadn't gunked anything up with all my manual fiddling
  7. fix a couple of things I'd broken. This gave me a minimal valid Open API spec to work with.
  8. add header request and response entries, pasting them into the viewer and fixing any errors

I ended up with a fairly minimal spec that passes validation and includes request and response headers, defined both inline and in reusable components. See this gist:
https://gist.github.com/bpirkle/4e9e64efe9c2c83982bccac9c11865c4

I'm not claiming that's the perfect way to construct these within our specs, but it at least passes validation. Take a look at that and see if it matches up with what you had in mind.

What'S the plan for generating these? With other parameters, the code that consumes the parameters also declares them, and we then generate the spec from that declaration. Is the idea to declare header params in getParamSettings? Or do we add getHeaderSettings?

Also, for a generic header like "If-Match" which is supported by the framework, how do we determine if the respective endpoint actuall supports it? E.g. "If-Match" only works if the Handler overrides getETag() to return something other than null. But that cannot be tested wouthout asking the Handler for a specific resources. So the handler would need to somehow indicate whether or not it supports etags in general.

One way to achioeve that would be to move conditionals into middleware, and let the middleware contribute to the spec generation. I think that would be the best solution, but it would reqire quite a bite of refactoring.

@BPirkle For the example json I did write them by hand...So technically any Json errors are "typos"..The idea was just to place the fields where I think they should be not really generate a valid json object for phabricator

I ended up with a fairly minimal spec that passes validation and includes request and response headers, defined both inline and in reusable components. See this gist:
https://gist.github.com/bpirkle/4e9e64efe9c2c83982bccac9c11865c4

I'm not claiming that's the perfect way to construct these within our specs, but it at least passes validation. Take a look at that and see if it matches up with what you had in mind.

Having something like that end up in the spec seems fine. There is some issue around how to normalize the case (lowercase? only first and post-hyphen letter uppercase). It might be slightly confusing if these will be parameters in the openapi sense but not in the getParamSettings() sense. Not a big deal though. Can header name parameters have the same name as other parameters? If no, perhaps they could go in getParamSettings()/getValidatedParams(), keyed with the normalized case. Otherwise, they're could be some getHeaderSettings()/getValidatedHeaders() methods. Either way, would getParamSettings/getHeaderSettings have to call super or know to add the relevant ones from some ConditionalHeaderUtil instance method?

As Daniel mentioned, the most interesting part is how the header specs get added. Methods like Handler::getETag and Handler::getLastModified can either always return null or sometimes return null. Code-wise, there is no way to "stand back" and easily see which one is the case. If we add boolean methods to convey the answer it will be unenforced. It would be kind of nice if Handlers could just declare the conditional header handling class, which would have a method for getting header params. If a chain of such classes could be used, then it would basically be middleware...

Also, note that getLastModified() being non null lets ConditionalHeaderUtil handle If-Unmodified-Since, and, setting getEtag() also lets If-None-Match get handled. Given that, bundling these parameters into schema components is useful.

Change #1225632 had a related patch set uploaded (by Atieno; author: Atieno):

[mediawiki/core@master] OpenApiSpec: Support for request headers in the REST OpenAPI spec

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

Atieno renamed this task from [SPIKE] Determine a strategy for documenting header parameters to [SPIKE] Determine a strategy for documenting header parameters.Jan 13 2026, 4:12 PM
Atieno claimed this task.

Request Headers Implementation
Based off This PoC that adds Accept-Language request header to the /v1/page/{title}/html the header definition is in the derived Handler's getParamSettings. Since we have defined a new param source header this source has to be declared in the KNOWN_PARAM_SOURCES in Validator and has to have a matching case in ParamValidatorCallbacks. For the ParamValidatorCallbacks to give us all the headers for case 'header' we have define getHeaderLines() in the RequestBase and RequestInterface that returns the corresponding HeaderContainer's getHeaderLines()

2 Minor edits to Request Headers Implementation

Instead of getParamSettings we have headers defined in getHeaderParamSettings.
$request->getHeaderLines is not part of psr-7 so we will be using the available getHeaders that returns an array and imploding it to a comma separated string inside ParamCalidatorCallbacks::getValue

Response Headers Implementation

  • The Base framework defines all the response headers specs - a class that has all the specs defined.
  • Implements in Handler base class an overridable getResponseHeaderSpec
  • Derived classes will override getResponseHeaderSpec to call the class defined above requesting the specs for the response headers that it returns.

Note: This SPIKE does not cover conditiona headers. That will come in a follow-up SPIKE

Change #1235744 had a related patch set uploaded (by Atieno; author: Atieno):

[mediawiki/core@master] OpenApiSpec: Support for request headers in the REST OpenAPI spec

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

Change #1236275 had a related patch set uploaded (by Atieno; author: Atieno):

[mediawiki/core@master] OpenApiSpec: Add support for response headers in the REST OpenAPI spec

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

Change #1236290 had a related patch set uploaded (by Atieno; author: Atieno):

[mediawiki/core@master] OpenApiSpec: Add response headers in the REST OpenAPI spec for remaining endpoints

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

Change #1225632 merged by jenkins-bot:

[mediawiki/core@master] OpenApiSpec: Support for request headers in the REST OpenAPI spec

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

Change #1235744 merged by jenkins-bot:

[mediawiki/core@master] OpenApiSpec: Add request headers in the REST OpenAPI spec for remaining endpoints

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

Change #1236290 abandoned by Atieno:

[mediawiki/core@master] OpenApiSpec: Add response headers in the REST OpenAPI spec for remaining endpoints

Reason:

This changes merged into the other patch https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1236275

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

Change #1236275 merged by jenkins-bot:

[mediawiki/core@master] OpenApiSpec: Add support for response headers in the REST OpenAPI spec

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