thephpleague / openapi-psr7-validator

It validates PSR-7 messages (HTTP request/response) against OpenAPI specifications
MIT License
523 stars 92 forks source link

Deprecation HTTP Header #100

Open philsturgeon opened 3 years ago

philsturgeon commented 3 years ago

User story.

As an API developer, I don't need to mark an endpoint as deprecated in OpenAPI and in my API code by emitting a HTTP Deprecated header.

Is your feature request related to a problem?

Currently setting deprecated: true only updates documentation, but what if it also emitted headers from the API? This falls slightly outside of validation, but talking to an endpoint which might vanish soon seems like something people should know about, and this package is about helping developers avoid rewriting stuff in code that's already handled nicely in OpenAPI...

Describe the solution you'd like

When a request comes in to an operation has deprecated: true, emit a Deprecated HTTP header.

Deprecated: true

Seeing as OpenAPI does not offer any other information, like a date or replacement information, we cannot do anything more than this, but this is enough to let developers know that something is up (if they're looking for it, but that is a whole other story).

scaytrase commented 3 years ago

This lib has nothing to do with creating respone object in order to emit anything. I think the most close thing to do is to trigger E_USER_DEPRECATED error, but I don't really think it's a good idea, since it's not something you can easilty process as a validation result.

Library should have no side effects, since there can be other use cases not only validating incoming requests but for example processing delayed requests in a batch and emiting any headers here would be invalid

philsturgeon commented 3 years ago

Hey @scaytrase, I'm familiar with this package, and a whole bunch of other HTTP middleware packages. I started the PHP league, and suggested this package to come here. 👋

E_USER_DEPRECATED would spit out deprecation warnings in the PHP logs of the server this is being called on, which is not what we're looking for here. I'm suggesting emitting Deprecation HTTP Headers so that the consumer hitting the endpoint knows the endpoint they just hit is deprecated.

As an example, GitHub do this:

curl -I https://api.github.com/teams/123

... snip ...

deprecation: Sat, 01 Feb 2020 00:00:00 GMT
sunset: Mon, 01 Feb 2021 00:00:00 GMT
link: <https://developer.github.com/changes/2020-01-21-moving-the-team-api-endpoints/>; rel="deprecation"; type="text/html", <https://api.github.com/organizations/0/team/0>; rel="alternate"

Most HTTP middlewares in most frameworks and stacks read request information and build up extra response information, e.g.: a caching middleware will add Expires or Cache-Control so that people don't need to do that in their controllers every time.

An OpenAPI validation middleware does the following:

If the request is bad, you get an error.

If the request is good, you might get the response.

If the response is bad, you get an error.

If the response is good, ahh look, JSON.

We built similar logic for the Prism Validation Proxy, so you can have it validate the request and response if you like, but you can have it return a header with information about the problems instead of blowing up, because in production you probably don't want HTTP traffic failing just because there was a mismatch in your prod code and the OpenAPI describing it, that probably could have been caught sooner.

Many validators support errors and warnings in this way, with some things being "blow up" worthy, and some things being "mention it in case they've got client-side code logging that", and deprecation seems like something else worth adding as a warning about the validity of the request/response. Being able to know that "this was a valid request but watch out it's not going to be here much longer" seems pretty relevant to an OpenAPI Validation middleware.

The alternative is openapi-deprecated which is a boringly specific package to make, which is going to have to inspect the OpenAPI file, resolve refs, find the right operation, etc. just like this package is already doing.

I think it would be excellent to bring the deprecation status into consideration for if an endpoint is valid or not. It's not worthy of an error, but its worthy of warning callers!

scaytrase commented 3 years ago

Most HTTP middlewares in most frameworks and stacks read request information and build up extra response information, e.g.: a caching middleware will add Expires or Cache-Control so that people don't need to do that in their controllers every time.

Middleware of any kind is good place to do this kind of logic, yes. But not the validator itself. We decided to not to put here any framework specific tools here https://github.com/thephpleague/openapi-psr7-validator/pull/70#issuecomment-663498352

Although while I was writing response, I remembered that we already have PSR-15 middleware and it's OK to put such logic there. But in general I'd rather move all these unrelated logic out of package.

So we can make $operationAddress here https://github.com/thephpleague/openapi-psr7-validator/blob/master/src/PSR15/ValidationMiddleware.php#L41 be aware of deprecation and put header to the given response without and side effects to the user.

philsturgeon commented 3 years ago

You got it, this is exactly what I'm getting at! Doing it in that middleware looks ideal.