PostgREST / postgrest

REST API for any Postgres database
https://postgrest.org
MIT License
23.47k stars 1.03k forks source link

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

Closed steve-chavez closed 11 months ago

steve-chavez commented 1 year ago

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 commented 1 year ago

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.

taimoorzaeem commented 1 year ago

@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 commented 1 year ago

@taimoorzaeem Right. My mistake, corrected the above.

taimoorzaeem commented 1 year ago

@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 commented 12 months ago

@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 commented 12 months ago

@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 commented 12 months ago

@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 commented 11 months ago

@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 commented 11 months ago

Actualy it might be even easier to do:

Prefer: max-affected=100

That would not be tied to JSON.