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

feat: add max-affected preference to prefer header #3083

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

taimoorzaeem
Copy link
Collaborator

This should fix #2887.

@taimoorzaeem
Copy link
Collaborator Author

@steve-chavez I am not sure if we need to test this with batch upserts because that would involve the POST method and we only have this feature for updates and deletes.

@steve-chavez
Copy link
Member

@taimoorzaeem Yeah, I think it's fine to not do POST/Upsert

This feature can be like the strict counterpart of Limited Update/Delete.

Comment on lines 123 to 156
context "test Prefer: max-affected" $ do
it "should fail if items deleted more than 10" $
request methodDelete "/items?id=lt.15"
[("Prefer", "handling=strict, max-affected=10")]
""
`shouldRespondWith`
[json|{"code":"PGRST124","details":"The query affects 14 rows","hint":null,"message":"Query result exceeds max-affected preference constraint"}|]
{ matchStatus = 400 }

it "should succeed if items deleted less than 10" $
request methodDelete "/items?id=lt.10"
[("Prefer", "handling=strict, max-affected=10")]
""
`shouldRespondWith`
""
{ matchStatus = 204
, matchHeaders = ["Preference-Applied" <:> "handling=strict, max-affected=10"]}

-- tests batch upserts?
it "should fail if items updated more than 0" $
request methodPatch "/tiobe_pls?name=eq.Java"
[("Prefer", "handling=strict, max-affected=0")]
[json| [{"name":"Java", "rank":19}] |]
`shouldRespondWith`
[json|{"code":"PGRST124","details":"The query affects 1 rows","hint":null,"message":"Query result exceeds max-affected preference constraint"}|]
{ matchStatus = 400 }

it "should succeed if items updated equal 1" $
request methodDelete "/tiobe_pls?name=eq.Java"
[("Prefer", "handling=strict, max-affected=1")]
[json| [{"name":"Java", "rank":19}] |]
`shouldRespondWith`
""
{ matchStatus = 204
, matchHeaders = ["Preference-Applied" <:> "handling=strict, max-affected=1"]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a test case for handling=lenient and no handling to make sure we don't err in that case.

, matchHeaders = ["Preference-Applied" <:> "handling=strict, max-affected=1"]}

context "test Prefer: max-affected with handling=lenient" $ do
it "should fail if items deleted more than 10" $
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be succeed here right?

{ matchStatus = 204
, matchHeaders = ["Preference-Applied" <:> "handling=lenient"]}

it "should fail if items updated more than 0" $
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@steve-chavez steve-chavez merged commit 0b77098 into PostgREST:main Dec 18, 2023
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Prefer: handling=strict; affected for conditional requests based on number of affected resources
2 participants