From 1aed55be685a36625dff21d7825cab4809ee7de8 Mon Sep 17 00:00:00 2001 From: Laurence Isla Date: Mon, 3 Apr 2023 10:40:25 -0500 Subject: [PATCH] Ignore the Range header when the method is different than GET fix: bug when using Range header on PATCH/DELETE - Fix the "message": "syntax error at or near \"RETURNING\"" error - Fix doing a limited update/delete when an order query parameter was present breaking: The Range header is now only considered on GET requests and is ignored for any other method - Other methods should use the `limit/offset` query parameters for sub-ranges - PUT requests no longer return an error when this header is present --- CHANGELOG.md | 9 ++ src/PostgREST/ApiRequest.hs | 6 +- src/PostgREST/ApiRequest/Types.hs | 2 +- src/PostgREST/Error.hs | 6 +- test/spec/Feature/Query/DeleteSpec.hs | 54 ++++++- test/spec/Feature/Query/PgSafeUpdateSpec.hs | 8 +- test/spec/Feature/Query/RangeSpec.hs | 57 +++----- test/spec/Feature/Query/RpcSpec.hs | 153 ++++++++++++++------ test/spec/Feature/Query/UpdateSpec.hs | 76 +++++++++- test/spec/Feature/Query/UpsertSpec.hs | 23 +-- test/spec/SpecHelper.hs | 6 +- 11 files changed, 291 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9fc86d10..dc69b23a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,15 @@ This project adheres to [Semantic Versioning](http://semver.org/). + `PGRST003`: Timed out when acquiring connection to db - #2667, Fix `db-pool-acquisition-timeout` not logging to stderr when the timeout is reached - @steve-chavez - #1652, Fix function call with arguments not inlining - @steve-chavez + - #2705, Fix bug when using the `Range` header on `PATCH/DELETE` - @laurenceisla + + Fix the`"message": "syntax error at or near \"RETURNING\""` error + + Fix doing a limited update/delete when an `order` query parameter was present + +### Changed + + - #2705, The `Range` header is now only considered on `GET` requests and is ignored for any other method - @laurenceisla + + Other methods should use the `limit/offset` query parameters for sub-ranges + + `PUT` requests no longer return an error when this header is present (using `limit/offset` still triggers the error) ## [10.1.2] - 2023-02-01 diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index 5da127cc8..2532cd02f 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -237,10 +237,12 @@ getRanges :: ByteString -> QueryParams -> RequestHeaders -> Either ApiRequestErr getRanges method QueryParams{qsOrder,qsRanges} hdrs | isInvalidRange = Left $ InvalidRange (if rangeIsEmpty headerRange then LowerGTUpper else NegativeLimit) | method `elem` ["PATCH", "DELETE"] && not (null qsRanges) && null qsOrder = Left LimitNoOrderError - | method == "PUT" && topLevelRange /= allRange = Left PutRangeNotAllowedError + | method == "PUT" && topLevelRange /= allRange = Left PutLimitNotAllowedError | otherwise = Right (topLevelRange, ranges) where - headerRange = rangeRequested hdrs + -- According to the RFC (https://www.rfc-editor.org/rfc/rfc9110.html#name-range), + -- the Range header must be ignored for all methods other than GET + headerRange = if method == "GET" then rangeRequested hdrs else allRange limitRange = fromMaybe allRange (HM.lookup "limit" qsRanges) headerAndLimitRange = rangeIntersection headerRange limitRange -- Bypass all the ranges and send only the limit zero range (0 <= x <= -1) if diff --git a/src/PostgREST/ApiRequest/Types.hs b/src/PostgREST/ApiRequest/Types.hs index df373dacc..e7acc0712 100644 --- a/src/PostgREST/ApiRequest/Types.hs +++ b/src/PostgREST/ApiRequest/Types.hs @@ -76,7 +76,7 @@ data ApiRequestError | NoRelBetween Text Text (Maybe Text) Text RelationshipsMap | NoRpc Text Text [Text] Bool MediaType Bool [QualifiedIdentifier] [ProcDescription] | NotEmbedded Text - | PutRangeNotAllowedError + | PutLimitNotAllowedError | QueryParamError QPError | RelatedOrderNotToOne Text Text | SpreadNotToOne Text Text diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 6682f8980..a02629b54 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -72,7 +72,7 @@ instance PgrstError ApiRequestError where status NoRelBetween{} = HTTP.status400 status NoRpc{} = HTTP.status404 status NotEmbedded{} = HTTP.status400 - status PutRangeNotAllowedError = HTTP.status400 + status PutLimitNotAllowedError = HTTP.status400 status QueryParamError{} = HTTP.status400 status RelatedOrderNotToOne{} = HTTP.status400 status SpreadNotToOne{} = HTTP.status400 @@ -142,9 +142,9 @@ instance JSON.ToJSON ApiRequestError where "details" .= JSON.Null, "hint" .= JSON.Null] - toJSON PutRangeNotAllowedError = JSON.object [ + toJSON PutLimitNotAllowedError = JSON.object [ "code" .= ApiRequestErrorCode14, - "message" .= ("Range header and limit/offset querystring parameters are not allowed for PUT" :: Text), + "message" .= ("limit/offset querystring parameters are not allowed for PUT" :: Text), "details" .= JSON.Null, "hint" .= JSON.Null] diff --git a/test/spec/Feature/Query/DeleteSpec.hs b/test/spec/Feature/Query/DeleteSpec.hs index 772c7d490..698c7ea46 100644 --- a/test/spec/Feature/Query/DeleteSpec.hs +++ b/test/spec/Feature/Query/DeleteSpec.hs @@ -154,7 +154,7 @@ spec = it "works with the limit and offset query params" $ baseTable "limited_delete_items" "id" tblDataBefore `mutatesWith` - requestMutation methodDelete "/limited_delete_items?order=id&limit=1&offset=1" mempty + requestMutation methodDelete "/limited_delete_items?order=id&limit=1&offset=1" mempty mempty `shouldMutateInto` [json|[ { "id": 1, "name": "item-1" } @@ -164,7 +164,7 @@ spec = it "works with the limit query param plus a filter" $ baseTable "limited_delete_items" "id" tblDataBefore `mutatesWith` - requestMutation methodDelete "/limited_delete_items?order=id&limit=1&id=gt.1" mempty + requestMutation methodDelete "/limited_delete_items?order=id&limit=1&id=gt.1" mempty mempty `shouldMutateInto` [json|[ { "id": 1, "name": "item-1" } @@ -200,7 +200,7 @@ spec = it "works with views with an explicit order by unique col" $ baseTable "limited_delete_items_view" "id" tblDataBefore `mutatesWith` - requestMutation methodDelete "/limited_delete_items_view?order=id&limit=1&offset=1" mempty + requestMutation methodDelete "/limited_delete_items_view?order=id&limit=1&offset=1" mempty mempty `shouldMutateInto` [json|[ { "id": 1, "name": "item-1" } @@ -210,7 +210,7 @@ spec = it "works with views with an explicit order by composite pk" $ baseTable "limited_delete_items_cpk_view" "id" tblDataBefore `mutatesWith` - requestMutation methodDelete "/limited_delete_items_cpk_view?order=id,name&limit=1&offset=1" mempty + requestMutation methodDelete "/limited_delete_items_cpk_view?order=id,name&limit=1&offset=1" mempty mempty `shouldMutateInto` [json|[ { "id": 1, "name": "item-1" } @@ -220,9 +220,53 @@ spec = it "works on a table without a pk by ordering by 'ctid'" $ baseTable "limited_delete_items_no_pk" "id" tblDataBefore `mutatesWith` - requestMutation methodDelete "/limited_delete_items_no_pk?order=ctid&limit=1&offset=1" mempty + requestMutation methodDelete "/limited_delete_items_no_pk?order=ctid&limit=1&offset=1" mempty mempty `shouldMutateInto` [json|[ { "id": 1, "name": "item-1" } , { "id": 3, "name": "item-3" } ]|] + + it "ignores the Range header" $ do + baseTable "limited_delete_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodDelete "/limited_delete_items" + (rangeHdrs (ByteRangeFromTo 0 0)) mempty + `shouldMutateInto` + [json|[]|] + + baseTable "limited_delete_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodDelete "/limited_delete_items?id=gte.2" + (rangeHdrs (ByteRangeFromTo 0 0)) mempty + `shouldMutateInto` + [json|[ { "id": 1, "name": "item-1" } ]|] + + it "ignores the Range header and does not do a limited delete" $ + baseTable "limited_delete_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodDelete "/limited_delete_items?order=id" + (rangeHdrs (ByteRangeFromTo 0 0)) mempty + `shouldMutateInto` + [json|[]|] + + it "ignores the Range header and does not throw an invalid range error" $ + baseTable "limited_delete_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodDelete "/limited_delete_items?order=id&limit=1&offset=1" + (rangeHdrs (ByteRangeFromTo 0 0)) mempty + `shouldMutateInto` + [json|[ + { "id": 1, "name": "item-1" } + , { "id": 3, "name": "item-3" } + ]|] + + it "ignores the Range header but not the limit and offset params" $ + baseTable "limited_delete_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodDelete "/limited_delete_items?order=id&limit=2&offset=1" + (rangeHdrs (ByteRangeFromTo 1 1)) mempty + `shouldMutateInto` + [json|[ + { "id": 1, "name": "item-1" } + ]|] diff --git a/test/spec/Feature/Query/PgSafeUpdateSpec.hs b/test/spec/Feature/Query/PgSafeUpdateSpec.hs index 452133a6f..7415c1a79 100644 --- a/test/spec/Feature/Query/PgSafeUpdateSpec.hs +++ b/test/spec/Feature/Query/PgSafeUpdateSpec.hs @@ -38,7 +38,7 @@ spec = it "allows full table update if a filter is present" $ baseTable "safe_update_items" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/safe_update_items?id=gt.0" [json| {"name": "updated-item"} |] + requestMutation methodPatch "/safe_update_items?id=gt.0" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ { "id": 1, "name": "updated-item", "observation": null } @@ -61,7 +61,7 @@ spec = it "allows full table delete if a filter is present" $ baseTable "safe_delete_items" "id" tblDataBefore `mutatesWith` - requestMutation methodDelete "/safe_delete_items?id=gt.0" mempty + requestMutation methodDelete "/safe_delete_items?id=gt.0" mempty mempty `shouldMutateInto` [json|[]|] @@ -72,7 +72,7 @@ disabledSpec = it "works if no condition is present" $ baseTable "unsafe_update_items" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/unsafe_update_items" [json| {"name": "updated-item"} |] + requestMutation methodPatch "/unsafe_update_items" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ { "id": 1, "name": "updated-item", "observation": null } @@ -84,6 +84,6 @@ disabledSpec = it "works if no condition is present" $ baseTable "unsafe_delete_items" "id" tblDataBefore `mutatesWith` - requestMutation methodDelete "/unsafe_delete_items" mempty + requestMutation methodDelete "/unsafe_delete_items" mempty mempty `shouldMutateInto` [json|[]|] diff --git a/test/spec/Feature/Query/RangeSpec.hs b/test/spec/Feature/Query/RangeSpec.hs index c2481b155..5fa2dcf7c 100644 --- a/test/spec/Feature/Query/RangeSpec.hs +++ b/test/spec/Feature/Query/RangeSpec.hs @@ -1,7 +1,5 @@ module Feature.Query.RangeSpec where -import qualified Data.ByteString.Lazy as BL - import Network.Wai (Application) import Network.Wai.Test (SResponse (simpleHeaders, simpleStatus)) @@ -13,36 +11,29 @@ import Test.Hspec.Wai.JSON import Protolude hiding (get) import SpecHelper -defaultRange :: BL.ByteString -defaultRange = [json| { "min": 0, "max": 15 } |] - -emptyRange :: BL.ByteString -emptyRange = [json| { "min": 2, "max": 2 } |] - spec :: SpecWith ((), Application) spec = do - describe "POST /rpc/getitemrange" $ do + describe "GET /rpc/getitemrange" $ do context "without range headers" $ do context "with response under server size limit" $ it "returns whole range with status 200" $ - post "/rpc/getitemrange" defaultRange `shouldRespondWith` 200 + get "/rpc/getitemrange?min=0&max=15" `shouldRespondWith` 200 context "when I don't want the count" $ do it "returns range Content-Range with */* for empty range" $ - request methodPost "/rpc/getitemrange" [] emptyRange + get "/rpc/getitemrange?min=2&max=2" `shouldRespondWith` [json| [] |] {matchHeaders = ["Content-Range" <:> "*/*"]} it "returns range Content-Range with range/*" $ - post "/rpc/getitemrange?order=id" - defaultRange + get "/rpc/getitemrange?order=id&min=0&max=15" `shouldRespondWith` [json| [{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":6},{"id":7},{"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},{"id":14},{"id":15}] |] { matchHeaders = ["Content-Range" <:> "0-14/*"] } context "of invalid range" $ do it "refuses a range with nonzero start when there are no items" $ - request methodPost "/rpc/getitemrange?offset=1" - [("Prefer", "count=exact")] emptyRange + request methodGet "/rpc/getitemrange?offset=1&min=2&max=2" + [("Prefer", "count=exact")] mempty `shouldRespondWith` [json| { "message":"Requested range not satisfiable", @@ -55,8 +46,8 @@ spec = do } it "refuses a range requesting start past last item" $ - request methodPost "/rpc/getitemrange?offset=100" - [("Prefer", "count=exact")] defaultRange + request methodGet "/rpc/getitemrange?offset=100&min=0&max=15" + [("Prefer", "count=exact")] mempty `shouldRespondWith` [json| { "message":"Requested range not satisfiable", @@ -71,37 +62,37 @@ spec = do context "with range headers" $ do context "of acceptable range" $ do it "succeeds with partial content" $ do - r <- request methodPost "/rpc/getitemrange" - (rangeHdrs $ ByteRangeFromTo 0 1) defaultRange + r <- request methodGet "/rpc/getitemrange?min=0&max=15" + (rangeHdrs $ ByteRangeFromTo 0 1) mempty liftIO $ do simpleHeaders r `shouldSatisfy` matchHeader "Content-Range" "0-1/*" simpleStatus r `shouldBe` ok200 it "understands open-ended ranges" $ - request methodPost "/rpc/getitemrange" - (rangeHdrs $ ByteRangeFrom 0) defaultRange + request methodGet "/rpc/getitemrange?min=0&max=15" + (rangeHdrs $ ByteRangeFrom 0) mempty `shouldRespondWith` 200 it "returns an empty body when there are no results" $ - request methodPost "/rpc/getitemrange" - (rangeHdrs $ ByteRangeFromTo 0 1) emptyRange + request methodGet "/rpc/getitemrange?min=2&max=2" + (rangeHdrs $ ByteRangeFromTo 0 1) mempty `shouldRespondWith` "[]" { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "*/*"] } it "allows one-item requests" $ do - r <- request methodPost "/rpc/getitemrange" - (rangeHdrs $ ByteRangeFromTo 0 0) defaultRange + r <- request methodGet "/rpc/getitemrange?min=0&max=15" + (rangeHdrs $ ByteRangeFromTo 0 0) mempty liftIO $ do simpleHeaders r `shouldSatisfy` matchHeader "Content-Range" "0-0/*" simpleStatus r `shouldBe` ok200 it "handles ranges beyond collection length via truncation" $ do - r <- request methodPost "/rpc/getitemrange" - (rangeHdrs $ ByteRangeFromTo 10 100) defaultRange + r <- request methodGet "/rpc/getitemrange?min=0&max=15" + (rangeHdrs $ ByteRangeFromTo 10 100) mempty liftIO $ do simpleHeaders r `shouldSatisfy` matchHeader "Content-Range" "10-14/*" @@ -109,8 +100,8 @@ spec = do context "of invalid range" $ do it "fails with 416 for offside range" $ - request methodPost "/rpc/getitemrange" - (rangeHdrs $ ByteRangeFromTo 1 0) emptyRange + request methodGet "/rpc/getitemrange?min=2&max=2" + (rangeHdrs $ ByteRangeFromTo 1 0) mempty `shouldRespondWith` [json| { "message":"Requested range not satisfiable", @@ -121,8 +112,8 @@ spec = do { matchStatus = 416 } it "refuses a range with nonzero start when there are no items" $ - request methodPost "/rpc/getitemrange" - (rangeHdrsWithCount $ ByteRangeFromTo 1 2) emptyRange + request methodGet "/rpc/getitemrange?min=2&max=2" + (rangeHdrsWithCount $ ByteRangeFromTo 1 2) mempty `shouldRespondWith` [json| { "message":"Requested range not satisfiable", @@ -135,8 +126,8 @@ spec = do } it "refuses a range requesting start past last item" $ - request methodPost "/rpc/getitemrange" - (rangeHdrsWithCount $ ByteRangeFromTo 100 199) defaultRange + request methodGet "/rpc/getitemrange?min=0&max=15" + (rangeHdrsWithCount $ ByteRangeFromTo 100 199) mempty `shouldRespondWith` [json| { "message":"Requested range not satisfiable", diff --git a/test/spec/Feature/Query/RpcSpec.hs b/test/spec/Feature/Query/RpcSpec.hs index 3b95e5db2..54c65baf1 100644 --- a/test/spec/Feature/Query/RpcSpec.hs +++ b/test/spec/Feature/Query/RpcSpec.hs @@ -24,50 +24,65 @@ spec :: PgVersion -> SpecWith ((), Application) spec actualPgVersion = describe "remote procedure call" $ do context "a proc that returns a set" $ do - it "returns paginated results" $ do - request methodPost "/rpc/getitemrange" - (rangeHdrs (ByteRangeFromTo 0 0)) [json| { "min": 2, "max": 4 } |] - `shouldRespondWith` [json| [{"id":3}] |] - { matchStatus = 200 - , matchHeaders = ["Content-Range" <:> "0-0/*"] - } - request methodGet "/rpc/getitemrange?min=2&max=4" - (rangeHdrs (ByteRangeFromTo 0 0)) "" - `shouldRespondWith` [json| [{"id":3}] |] - { matchStatus = 200 - , matchHeaders = ["Content-Range" <:> "0-0/*"] - } - request methodHead "/rpc/getitemrange?min=2&max=4" - (rangeHdrs (ByteRangeFromTo 0 0)) "" - `shouldRespondWith` - "" - { matchStatus = 200 - , matchHeaders = [ matchContentTypeJson - , "Content-Range" <:> "0-0/*" ] - } + context "returns paginated results" $ do + it "using the Range header" $ + request methodGet "/rpc/getitemrange?min=2&max=4" + (rangeHdrs (ByteRangeFromTo 1 1)) mempty + `shouldRespondWith` [json| [{"id":4}] |] + { matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "1-1/*"] + } - it "includes total count if requested" $ do - request methodPost "/rpc/getitemrange" - (rangeHdrsWithCount (ByteRangeFromTo 0 0)) - [json| { "min": 2, "max": 4 } |] - `shouldRespondWith` [json| [{"id":3}] |] - { matchStatus = 206 -- it now knows the response is partial - , matchHeaders = ["Content-Range" <:> "0-0/2"] - } - request methodGet "/rpc/getitemrange?min=2&max=4" - (rangeHdrsWithCount (ByteRangeFromTo 0 0)) "" - `shouldRespondWith` [json| [{"id":3}] |] - { matchStatus = 206 - , matchHeaders = ["Content-Range" <:> "0-0/2"] - } - request methodHead "/rpc/getitemrange?min=2&max=4" - (rangeHdrsWithCount (ByteRangeFromTo 0 0)) "" - `shouldRespondWith` - "" - { matchStatus = 206 - , matchHeaders = [ matchContentTypeJson - , "Content-Range" <:> "0-0/2" ] - } + it "using limit and offset" $ do + post "/rpc/getitemrange?limit=1&offset=1" [json| { "min": 2, "max": 4 } |] + `shouldRespondWith` [json| [{"id":4}] |] + { matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "1-1/*"] + } + get "/rpc/getitemrange?min=2&max=4&limit=1&offset=1" + `shouldRespondWith` [json| [{"id":4}] |] + { matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "1-1/*"] + } + request methodHead "/rpc/getitemrange?min=2&max=4&limit=1&offset=1" mempty mempty + `shouldRespondWith` + "" + { matchStatus = 200 + , matchHeaders = [ matchContentTypeJson + , "Content-Range" <:> "1-1/*" ] + } + + context "includes total count if requested" $ do + it "using the Range header" $ + request methodGet "/rpc/getitemrange?min=2&max=4" + (rangeHdrsWithCount (ByteRangeFromTo 1 1)) "" + `shouldRespondWith` [json| [{"id":4}] |] + { matchStatus = 206 -- it now knows the response is partial + , matchHeaders = ["Content-Range" <:> "1-1/2"] + } + + it "using limit and offset" $ do + request methodPost "/rpc/getitemrange?limit=1&offset=1" + [("Prefer", "count=exact")] + [json| { "min": 2, "max": 4 } |] + `shouldRespondWith` [json| [{"id":4}] |] + { matchStatus = 206 -- it now knows the response is partial + , matchHeaders = ["Content-Range" <:> "1-1/2"] + } + request methodGet "/rpc/getitemrange?min=2&max=4&limit=1&offset=1" + [("Prefer", "count=exact")] mempty + `shouldRespondWith` [json| [{"id":4}] |] + { matchStatus = 206 + , matchHeaders = ["Content-Range" <:> "1-1/2"] + } + request methodHead "/rpc/getitemrange?min=2&max=4&limit=1&offset=1" + [("Prefer", "count=exact")] mempty + `shouldRespondWith` + "" + { matchStatus = 206 + , matchHeaders = [ matchContentTypeJson + , "Content-Range" <:> "1-1/2" ] + } it "includes exact count if requested" $ do request methodHead "/rpc/getallprojects" @@ -113,6 +128,58 @@ spec actualPgVersion = , matchHeaders = ["Content-Type" <:> "text/csv; charset=utf-8"] } + context "ignores Range header when method is different than GET" $ do + it "without limit and offset" $ do + request methodPost "/rpc/getitemrange" + (rangeHdrsWithCount (ByteRangeFromTo 1 1)) + [json| { "min": 2, "max": 4 } |] + `shouldRespondWith` [json| [{"id": 3}, {"id": 4}] |] + { matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "0-1/2"] + } + request methodHead "/rpc/getitemrange?min=2&max=4" + (rangeHdrsWithCount (ByteRangeFromTo 1 1)) "" + `shouldRespondWith` + "" + { matchStatus = 200 + , matchHeaders = [ matchContentTypeJson + , "Content-Range" <:> "0-1/2" ] + } + + it "with limit and offset" $ do + request methodPost "/rpc/getitemrange?limit=2&offset=1" + (rangeHdrsWithCount (ByteRangeFromTo 1 1)) + [json| { "min": 2, "max": 5 } |] + `shouldRespondWith` [json| [{"id": 4}, {"id": 5}] |] + { matchStatus = 206 + , matchHeaders = ["Content-Range" <:> "1-2/3"] + } + request methodHead "/rpc/getitemrange?min=2&max=5&limit=2&offset=1" + (rangeHdrsWithCount (ByteRangeFromTo 1 1)) "" + `shouldRespondWith` + "" + { matchStatus = 206 + , matchHeaders = [ matchContentTypeJson + , "Content-Range" <:> "1-2/3" ] + } + + it "does not throw an invalid range error" $ do + request methodPost "/rpc/getitemrange?limit=2&offset=1" + (rangeHdrsWithCount (ByteRangeFromTo 0 0)) + [json| { "min": 2, "max": 5 } |] + `shouldRespondWith` [json| [{"id": 4}, {"id": 5}] |] + { matchStatus = 206 + , matchHeaders = ["Content-Range" <:> "1-2/3"] + } + request methodHead "/rpc/getitemrange?min=2&max=5&limit=2&offset=1" + (rangeHdrsWithCount (ByteRangeFromTo 0 0)) "" + `shouldRespondWith` + "" + { matchStatus = 206 + , matchHeaders = [ matchContentTypeJson + , "Content-Range" <:> "1-2/3" ] + } + context "unknown function" $ do it "returns 404" $ post "/rpc/fakefunc" [json| {} |] `shouldRespondWith` 404 diff --git a/test/spec/Feature/Query/UpdateSpec.hs b/test/spec/Feature/Query/UpdateSpec.hs index 205a36029..1ce7c47d4 100644 --- a/test/spec/Feature/Query/UpdateSpec.hs +++ b/test/spec/Feature/Query/UpdateSpec.hs @@ -500,7 +500,7 @@ spec = do it "works with the limit query param" $ baseTable "limited_update_items" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/limited_update_items?order=id&limit=2" + requestMutation methodPatch "/limited_update_items?order=id&limit=2" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ @@ -512,7 +512,7 @@ spec = do it "works with the limit query param plus a filter" $ baseTable "limited_update_items" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/limited_update_items?order=id&limit=1&id=gt.2" + requestMutation methodPatch "/limited_update_items?order=id&limit=1&id=gt.2" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ @@ -524,7 +524,7 @@ spec = do it "works with the limit and offset query params" $ baseTable "limited_update_items" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/limited_update_items?order=id&limit=1&offset=1" + requestMutation methodPatch "/limited_update_items?order=id&limit=1&offset=1" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ @@ -562,7 +562,7 @@ spec = do it "works with views with an explicit order by unique col" $ baseTable "limited_update_items_view" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/limited_update_items_view?order=id&limit=1&offset=1" + requestMutation methodPatch "/limited_update_items_view?order=id&limit=1&offset=1" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ @@ -574,7 +574,7 @@ spec = do it "works with views with an explicit order by composite pk" $ baseTable "limited_update_items_cpk_view" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/limited_update_items_cpk_view?order=id,name&limit=1&offset=1" + requestMutation methodPatch "/limited_update_items_cpk_view?order=id,name&limit=1&offset=1" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ @@ -586,7 +586,7 @@ spec = do it "works on a table without a pk by ordering by 'ctid'" $ baseTable "limited_update_items_no_pk" "id" tblDataBefore `mutatesWith` - requestMutation methodPatch "/limited_update_items_no_pk?order=ctid&limit=1" + requestMutation methodPatch "/limited_update_items_no_pk?order=ctid&limit=1" mempty [json| {"name": "updated-item"} |] `shouldMutateInto` [json|[ @@ -594,3 +594,67 @@ spec = do , { "id": 2, "name": "item-2" } , { "id": 3, "name": "item-3" } ]|] + + it "ignores the Range header" $ do + baseTable "limited_update_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodPatch "/limited_update_items" + (rangeHdrs (ByteRangeFromTo 0 0)) + [json| {"name": "updated-item"} |] + `shouldMutateInto` + [json|[ + { "id": 1, "name": "updated-item" } + , { "id": 2, "name": "updated-item" } + , { "id": 3, "name": "updated-item" } + ]|] + + baseTable "limited_update_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodPatch "/limited_update_items?id=gte.2" + (rangeHdrs (ByteRangeFromTo 0 0)) + [json| {"name": "updated-item"} |] + `shouldMutateInto` + [json|[ + { "id": 1, "name": "item-1" } + , { "id": 2, "name": "updated-item" } + , { "id": 3, "name": "updated-item" } + ]|] + + it "ignores the Range header and does not do a limited update" $ + baseTable "limited_update_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodPatch "/limited_update_items?order=id" + (rangeHdrs (ByteRangeFromTo 0 0)) + [json| {"name": "updated-item"} |] + `shouldMutateInto` + [json|[ + { "id": 1, "name": "updated-item" } + , { "id": 2, "name": "updated-item" } + , { "id": 3, "name": "updated-item" } + ]|] + + it "ignores the Range header and does not throw an invalid range error" $ + baseTable "limited_update_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodPatch "/limited_update_items?order=id&limit=1&offset=1" + (rangeHdrs (ByteRangeFromTo 0 0)) + [json| {"name": "updated-item"} |] + `shouldMutateInto` + [json|[ + { "id": 1, "name": "item-1" } + , { "id": 2, "name": "updated-item" } + , { "id": 3, "name": "item-3" } + ]|] + + it "ignores the Range header but not the limit and offset params" $ + baseTable "limited_update_items" "id" tblDataBefore + `mutatesWith` + requestMutation methodPatch "/limited_update_items?order=id&limit=2&offset=1" + (rangeHdrs (ByteRangeFromTo 1 1)) + [json| {"name": "updated-item"} |] + `shouldMutateInto` + [json|[ + { "id": 1, "name": "item-1" } + , { "id": 2, "name": "updated-item" } + , { "id": 3, "name": "updated-item" } + ]|] diff --git a/test/spec/Feature/Query/UpsertSpec.hs b/test/spec/Feature/Query/UpsertSpec.hs index 590be3a7e..4aa396198 100644 --- a/test/spec/Feature/Query/UpsertSpec.hs +++ b/test/spec/Feature/Query/UpsertSpec.hs @@ -195,25 +195,18 @@ spec actualPgVersion = context "with PUT" $ do context "Restrictions" $ do - it "fails if Range is specified" $ - request methodPut "/tiobe_pls?name=eq.Javascript" [("Range", "0-5")] - [json| [ { "name": "Javascript", "rank": 1 } ]|] - `shouldRespondWith` - [json|{"message":"Range header and limit/offset querystring parameters are not allowed for PUT","code":"PGRST114","details":null,"hint":null}|] - { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } - it "fails if limit is specified" $ put "/tiobe_pls?name=eq.Javascript&limit=1" [json| [ { "name": "Javascript", "rank": 1 } ]|] `shouldRespondWith` - [json|{"message":"Range header and limit/offset querystring parameters are not allowed for PUT","code":"PGRST114","details":null,"hint":null}|] + [json|{"message":"limit/offset querystring parameters are not allowed for PUT","code":"PGRST114","details":null,"hint":null}|] { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } it "fails if offset is specified" $ put "/tiobe_pls?name=eq.Javascript&offset=1" [json| [ { "name": "Javascript", "rank": 1 } ]|] `shouldRespondWith` - [json|{"message":"Range header and limit/offset querystring parameters are not allowed for PUT","code":"PGRST114","details":null,"hint":null}|] + [json|{"message":"limit/offset querystring parameters are not allowed for PUT","code":"PGRST114","details":null,"hint":null}|] { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } it "rejects every other filter than pk cols eq's" $ do @@ -382,6 +375,18 @@ spec actualPgVersion = `shouldRespondWith` [json|[ { "id": 1 } ]|] + it "ignores the Range header" $ do + -- assert that the next request will indeed be an update + get "/tiobe_pls?name=eq.Java" + `shouldRespondWith` + [json|[ { "name": "Java", "rank": 1 } ]|] + + request methodPut "/tiobe_pls?name=eq.Java" + [("Prefer", "return=representation"), ("Range", "1-1")] + [json| [ { "name": "Java", "rank": 5 } ]|] + `shouldRespondWith` + [json| [ { "name": "Java", "rank": 5 } ]|] + -- TODO: move this to SingularSpec? it "works with return=representation and vnd.pgrst.object+json" $ request methodPut "/tiobe_pls?name=eq.Ruby" diff --git a/test/spec/SpecHelper.hs b/test/spec/SpecHelper.hs index 6ba079038..3ea2a083e 100644 --- a/test/spec/SpecHelper.hs +++ b/test/spec/SpecHelper.hs @@ -280,9 +280,9 @@ baseTable :: ByteString -> ByteString -> Value -> BaseTable baseTable = BaseTable -- | The mutation (update/delete) that will be applied to the base table -requestMutation :: Method -> ByteString -> BL.ByteString -> WaiExpectation () -requestMutation method path body = - request method path [("Prefer", "tx=commit")] body `shouldRespondWith` 204 +requestMutation :: Method -> ByteString -> [Header] -> BL.ByteString -> WaiExpectation () +requestMutation method path headers body = + request method path (("Prefer", "tx=commit") : headers) body `shouldRespondWith` 204 data BaseTable = BaseTable ByteString ByteString Value data MutationCheck = MutationCheck BaseTable (WaiExpectation ())