[WIP] Allow specifying root function

[steve-chavez]

As discussed in #790 (comment). With this function we could solve all the current OpenAPI issues.

This is a work in progress.

To do this first we need to be able to override the Content-Type(in case user wants to specify application/openapiv3+json or other spec). Previously we discussed this in #986 (comment).

For now content negotiation is kind of broken on RPC, since client can specify Accept: x and server can respond with Content-Type: y. Not sure if we should enforce a way to make this consistent or leave the responsibility to the user.

[steve-chavez]

Come to think of it, we can enforce content negotiation is respected.

Right now we check the content types available and respond with an error before we hit the db. For RPC we could relax this restriction and addtionally check that the requested media type Accept: application/x is available after hitting the db and getting the RPC GucHeaders. We'll check if application/x is in the GucHeaders and if isn't we rollback and respond with an error.

Would have to check matching rules for things like application/* or application/x+json.

[ruslantalpa]

(leaving aside that the changes in this pr until now are useful, setting the response headers) in what way will this PR make things better (after the changes you want to implement for the root path) compared to a user/me just creating a normal RPC, then writing a 5 line nginx config and pointing the root location to the new RPC, where by the way I can control the headers however I want, I can set caching headers (and cache the actual output), etc, etc ... and all that without any new logic and additional config parameters? While here you are talking about not trivial things like looking at request/response headers and rolling back transactions

[steve-chavez]

While here you are talking about not trivial things like looking at request/response headers and rolling back transactions

We already do something similar here, check the queryTotal and do HT.condemn. Doesn't look that complex, though for this PR that could be omitted though content negotiation would be broken.

compared to a user/me just creating a normal RPC, then writing a 5 line nginx config and pointing the root location to the new RPC, where by the way I can control the headers however I want, I can set caching headers (and cache the actual output), etc, etc ... and all that without any new logic and additional config parameters?

It'll be only one additional config option and pgrst can also control the headers.

Not every user is proficient in nginx(they could use caddy, apache, etc) and we can remove the need for the proxy plus the 5 lines of config that the user needs to add so it can start customizing it's root function. This process should be straightforward since here we are talking about contributors helping us solve our openapi issues(possibly gaining openapiv3 and other specs).

[ruslantalpa]

something similar here

That code is there because it's the only place where that can be done, it's core to what postgrest does, openapi is not

Not every user is proficient in nginx

nobody is, it's a case of reading the manual and writing a few lines.
so the users of postgrest can't handle configuring a tool (nginx) but can handle just fine SQL and databases (since this is pretty much required to use postgrest effectively)?

remove the need for the proxy

I thought we established long time ago that there needs to be a proxy in every setup

contributors helping

It's not about who does the job, it's about if this needs to be done at all.

So far what i understand is that the answer to my main question (how is it better then doing it in proxy) is "the users don't have to write the 5 lines" (which by the way can also be documented the same as the function if that is a big problem)

[steve-chavez]

it's core to what postgrest does, openapi is not

Resource metadata is core to REST, openapi happens to be the spec we have supported for years already so it should remain and prevent a major breaking change. Also other specs can be added as I mentioned.

I thought we established long time ago that there needs to be a proxy in every setup

Remove the need for the proxy for the root function.

So far what i understand is that the answer to my main question (how is it better then doing it in proxy)

It's better because you don't need the proxy plus x lines of code config(simplicity).

[ruslantalpa]

it's core

a thing that is not implemented/used in 99% of rest apis in the real world can not be "core" to rest

breaking change

both methods (new config vs proxy location) are the exact same thing in relation to breaking changes (a user needs to create a function in the db and edit a config file).

Another point is that no one is really using/caring about this more then slapping swagger-ui on top and saying "look how cool" since the output never was 100 complete/correct.

Remove the need for the proxy for the root function

in other words, the proxy is still there (because it's needed in all setups), it's just not used for the root location so there is no reduction of operational complexity here.

It's better because you don't need the proxy plus x lines of code config(simplicity).

as said above, the proxy is there (for a whole lot of reasons/features postgrest does nto implement), so you DO still needed it. A couple of haskell functions as oposed to a few nginx config lines is the oposite of simplicity.

[ruslantalpa]

I might be wrong a bit about using/caring about this, ppl might use the swagger-ui as a (semi-correct) documentation of the api (for internal consumption) but still this has no influence on how openapi should be implemented

[steve-chavez]

So far I don't see a strong reason for not implementing this. Bottom line is that we have many pending issues regarding openapi and the pr presents a solution. So, I'll proceed.

Regarding content negotiation, I'll make the pr simpler and not enforce anything for now. Just add the ability to do Accept: application/openapi+json for RPC so the user is able to get openapi or a simpler json spec.

[steve-chavez]

Closing in favor of #1317.