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

Prefer: handling=strict; affected for conditional requests based on number of affected resources #2887

Closed
steve-chavez opened this issue Aug 2, 2023 · 9 comments · Fixed by #3083
Assignees
Labels
difficulty: medium Haskell task involving PostgreSQL IO enhancement a feature, ready for implementation

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Aug 2, 2023

Problem

Currently checking the number of affected resources for a request is tied to the application/vnd.pgrst.object+json media type and it only checks for a value of 1.

Solution

Now that we have Prefer: handling=strict, we can err on unmet conditions. This gives us an equivalent of the Expect header and can be used independently of the media type and request method.

We can have an affected preference, which would be a count of the operation affected resources. Its syntax would be:

Prefer: affected=<lt/lte/eq/gte/gt>.<number>

Assuming items has 1000 items:

DELETE /items
Prefer: handling=strict; affected=gt.3,lte.20

400 Bad Request
{.. new PGRST code with message..}

Correcting the request with a filter:

DELETE /items?id=in.(11,23,49,80)
Prefer: handling=strict; affected=gt.3,lte.20

200 OK

If handling=lenient is used or if handling=strict is not specified, affected=.. won't have any effect.

Notes

@steve-chavez steve-chavez added the idea Needs of discussion to become an enhancement, not ready for implementation label Aug 2, 2023
@steve-chavez steve-chavez changed the title If-Affected header for conditional requests based on number of affected resources Prefer: handling=strict; affected for conditional requests based on number of affected resources Oct 3, 2023
@steve-chavez
Copy link
Member Author

steve-chavez commented Oct 3, 2023

I've repurposed this proposal to use the newly added Prefer: handling=strict. That way we don't need to add a custom header (If-Affected).

Edit: I don't see any problem this could introduce as it's namespaced and optional, so will mark it as enhancement.

@steve-chavez steve-chavez added enhancement a feature, ready for implementation difficulty: medium Haskell task involving PostgreSQL IO and removed idea Needs of discussion to become an enhancement, not ready for implementation labels Oct 3, 2023
@taimoorzaeem taimoorzaeem self-assigned this Oct 29, 2023
@taimoorzaeem
Copy link
Collaborator

@steve-chavez @laurenceisla

DELETE /items?id=in.(11,23,49,80)
Prefer: handling=strict; affected=gt.10,lte.20

200 OK

Hmm, doesn't affected=gt.10,lte.20 mean that affected resources should be greater than 10 and less than equal to 20? In this case, DELETE /items?id=in.(11,23,49,80) should affect 4 resources which doesn't conform with the above predicate so it should result in an error no?

@steve-chavez
Copy link
Member Author

@taimoorzaeem Right. My mistake, corrected the above.

@taimoorzaeem
Copy link
Collaborator

@steve-chavez
Currently we support giving multiple preferences separated by a comma.

Prefer: return=representation, handling=strict, affected=gt.1,lte.10

Internally, splitting it comma would yield

["return=representation", "handling=strict", "affected=gt.1", "lte.10"]

This split the affected preferences into two. Should we change the delimiter to ; instead as in affected=gt.1;lte.10?

@steve-chavez
Copy link
Member Author

@taimoorzaeem Right, , wasn't a good separator. ; is valid according to https://httpwg.org/specs/rfc8941.html#rfc.section.3.1.2. I'm wondering if it might be more clear to make it like the following though.

-- Example from the RFC
Example-List: abc;a=1;b=2; cde_456, (ghi;jk=4 l);q="9";r=w

-- Our syntax
Prefer: affected;gt=1;lte=10

Perhaps easier to parse too? WDYT?

@taimoorzaeem
Copy link
Collaborator

@steve-chavez I think that using the Prefer header for this feature is complicating things a lot. First of all, it makes it harder to check invalid preferences (because it requires more logic to check the validity of custom preferences). Also, with this, Preferences module is filling up with a lot of logic specifically the fromHeaders function (because all the parsing is done there). Also, it would make it harder to add new custom preferences.

I think it would be better to implement this with a custom header like If-Affected that you mentioned earlier. WDYT?

@steve-chavez
Copy link
Member Author

@taimoorzaeem I see, it makes sense that putting syntax in Prefer complicates things.

I think it would be better to implement this with a custom header like If-Affected that you mentioned earlier. WDYT?

Yes, agree.

@steve-chavez
Copy link
Member Author

steve-chavez commented Dec 1, 2023

@taimoorzaeem I've been thinking more about this. Now that we have vnd.pgrst.array, how about adding an additional parameter for it?

Really the use case is to limit the amount of rows affected, that much flexibility with operators is not really necessary. So:

Accept: application/vnd.pgrst.array?max-length=100

WDYT? Should be easier to implement too.

Caveat: this would only be tied to JSON, but that's the immediate use case.

@steve-chavez
Copy link
Member Author

Actualy it might be even easier to do:

Prefer: max-affected=100

That would not be tied to JSON.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium Haskell task involving PostgreSQL IO enhancement a feature, ready for implementation
Development

Successfully merging a pull request may close this issue.

2 participants