Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Optionally respond to application/vnd.pgrst.object+json with application/json #2809

Closed
dbaynard opened this issue May 31, 2023 · 6 comments · Fixed by PostgREST/postgrest-docs#639

Comments

@dbaynard
Copy link
Contributor

Environment

  • PostgreSQL version: 15.2
  • PostgREST version: 10.1.1 — though I did check release notes for 10.2 and 11 and saw no mention of this.
  • Operating system: macOS

Description of issue feature request

I think I would like to configure postgrest to reply to Accept: application/vnd.pgrst.object+json requests with Content-Type: application/json. If application/vnd.pgrst.array+json is added, following #2676, I would like the option to vary the Content-Type for that, too.

If this is of interest, I'd be happy to work on it. I'd imagine something like a Prefers: content-type=application/json header might work, as might a configuration option (to apply this to all such responses), but I'm not familiar with the postgrest code.

I did search the issue tracker, and the closest things I found were #2188, and (via that) #1582.

I'm working with a frontend client that checks a response is json by checking the start of the Content-Type is application/json. This seems like a bug on this client's part — it ought to check that the start is application/ and there's a +json before either the end or a ;. Alas, that isn't the case, and I've had a horrible time debugging (it was compounded by another bug where if it doesn't match the header, it returns the Response object rather than the result of calling the.text() method… don't ask). The right solution for my issue is to fix the frontend code (for various reasons I can't do that now, but it's fine, I'm altering the headers in the reverse proxy).

Anyway, thanks, team, for all your work on this.

@steve-chavez
Copy link
Member

steve-chavez commented May 31, 2023

Hm, I think the Prefer approach is too ad hoc. #1582 looks it might solve your issue in a generic way.

I'm working with a frontend client that checks a response is json by checking the start of the Content-Type is application/json. This seems like a bug on this client's part —

Q: Why is the client using vnd.pgrst.object+json?

If you're only using it to validate that the response contains 1 row and you can convert the json to an object client-side, then #2164 might be a much easier solution to implement.

Edit: Sorry, missed the following

The right solution for my issue is to fix the frontend code (for various reasons I can't do that now, but it's fine, I'm altering the headers in the reverse proxy).

Yeah, then #1582 is the way to go.


Just came to my attention that vnd.pgrst.object+json single row enforcement can be implemented in an aggregate function - by using a finalfunc that does the json length checking.

@dbaynard
Copy link
Contributor Author

Thanks. Feel free to close and if #1582 doesn't support this I can reopen.

Q: Why is the client using vnd.pgrst.object+json?

To clarify: the frontend expects single objects for every endpoint, so I changed Accept to a/vpo+jbefore requests hit postgrest. I didn't realize I'd need to change the response back.

I was thinking: if you're sending json, call it application/json. Hence raising the issue, rather than just fixing the reverse proxy.

@dbaynard
Copy link
Contributor Author

Actually, would you welcome a docs PR that points out that Accept: application/vnd.pgrst.object+json results in a Content-Type that starts similarly, a the point the Accept: application/vnd.pgrst.object+json concept is introduced?

@steve-chavez
Copy link
Member

I was thinking: if you're sending json, call it application/json. Hence raising the issue, rather than just fixing the reverse proxy.

That seems sensible. Unfortunately at this stage it would a breaking change since there are many client libraries out there.

Actually, would you welcome a docs PR that points out that Accept: application/vnd.pgrst.object+json results in a Content-Type that starts similarly,

Sure. Please go ahead.

@wolfgangwalther
Copy link
Member

I was thinking: if you're sending json, call it application/json. Hence raising the issue, rather than just fixing the reverse proxy.

No. Even though our response might be valid application/json we must properly respond to Accept: application/vnd.pgrst.object+json. And that proper response is Content-Type: application/vnd.pgrst.object+json. If we don't - we can't accept this request.

@dbaynard
Copy link
Contributor Author

dbaynard commented Jun 9, 2023

I agree that's the right approach; my impression was that Accept is a preference, but from further reading that doesn't appear to be the case. The right fix in my case is then to do what I've done, and convert the header in the reverse proxy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants