From 893b7a71264847deca6eaafa7550f5d4f0fd0c88 Mon Sep 17 00:00:00 2001 From: Diogo Biazus Date: Wed, 10 Feb 2016 22:45:52 -0500 Subject: [PATCH 1/3] Applies range headers to /rpc calls using LIMIT/OFFSET. --- src/PostgREST/App.hs | 27 ++++++++++++++------------- src/PostgREST/Main.hs | 2 -- src/PostgREST/QueryBuilder.hs | 6 +++--- test/Feature/QuerySpec.hs | 5 ++++- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 000d66544..812379c83 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -71,13 +71,10 @@ app dbStructure conf reqBody req = case readSqlParts of Left e -> return $ responseLBS status400 [jsonH] $ cs e Right (q, cq) -> do - let range = restrictRange (configMaxRows conf) $ iRange apiRequest - singular = iPreferSingular apiRequest + let singular = iPreferSingular apiRequest stm = createReadStatement q cq range singular (iPreferCount apiRequest) (contentType == TextCSV) - if range == emptyRange - then return $ errResponse status416 "HTTP Range error" - else do + respondToRange $ do row <- H.query () stm let (tableTotal, queryTotal, _ , body) = row if singular @@ -165,14 +162,14 @@ app dbStructure conf reqBody req = then do let p = V.head payload jwtSecret = configJwtSecret conf - - bodyJson <- H.query () (callProc qi p) - returnJWT <- H.query qi doesProcReturnJWT - return $ responseLBS status200 [jsonH] - (let body = fromMaybe emptyArray bodyJson in - if returnJWT - then "{\"token\":\"" <> cs (tokenJWT jwtSecret body) <> "\"}" - else cs $ encode body) + respondToRange $ do + bodyJson <- H.query () (callProc qi p range) + returnJWT <- H.query qi doesProcReturnJWT + return $ responseLBS status200 [jsonH] + (let body = fromMaybe emptyArray bodyJson in + if returnJWT + then "{\"token\":\"" <> cs (tokenJWT jwtSecret body) <> "\"}" + else cs $ encode body) else return notFound (ActionRead, TargetRoot, Nothing) -> do @@ -196,6 +193,7 @@ app dbStructure conf reqBody req = allOrigins = ("Access-Control-Allow-Origin", "*") :: Header schema = cs $ configSchema conf apiRequest = userApiRequest schema req reqBody + range = restrictRange (configMaxRows conf) $ iRange apiRequest readDbRequest = DbRead <$> buildReadRequest (dbRelations dbStructure) apiRequest mutateDbRequest = DbMutate <$> buildMutateRequest apiRequest selectQuery = requestToQuery schema <$> readDbRequest @@ -203,6 +201,9 @@ app dbStructure conf reqBody req = mutateQuery = requestToQuery schema <$> mutateDbRequest readSqlParts = (,) <$> selectQuery <*> countQuery mutateSqlParts = (,) <$> selectQuery <*> mutateQuery + respondToRange response = if range == emptyRange + then return $ errResponse status416 "HTTP Range error" + else response rangeStatus :: Integer -> Integer -> Maybe Integer -> Status rangeStatus _ _ Nothing = status200 diff --git a/src/PostgREST/Main.hs b/src/PostgREST/Main.hs index ae840249d..c3aa95563 100644 --- a/src/PostgREST/Main.hs +++ b/src/PostgREST/Main.hs @@ -30,7 +30,6 @@ import System.IO (BufferMode (..), hSetBuffering, stderr, stdin, stdout) import Web.JWT (secret) - #ifndef mingw32_HOST_OS import System.Posix.Signals import Control.Concurrent (myThreadId) @@ -64,7 +63,6 @@ main = do Prelude.putStrLn $ "Listening on port " ++ (show $ configPort conf :: String) - pool <- P.acquire (configPool conf, 10, pgSettings) #ifndef mingw32_HOST_OS diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index 046040909..f551196df 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -203,8 +203,8 @@ addJoinConditions schema (Node (query, (n, r)) forest) = updatedForest = mapM (addJoinConditions schema) forest addCond query' con = query'{flt_=con ++ flt_ query'} -callProc :: QualifiedIdentifier -> JSON.Object -> H.Query () (Maybe JSON.Value) -callProc qi params = +callProc :: QualifiedIdentifier -> JSON.Object -> NonnegRange -> H.Query () (Maybe JSON.Value) +callProc qi params range = H.statement sql HE.unit decodeObj True where sql = [qc| SELECT array_to_json( @@ -213,7 +213,7 @@ callProc qi params = from ({_callSql}) t |] _args = intercalate "," $ map _assignment (HM.toList params) _assignment (n,v) = pgFmtIdent n <> ":=" <> insertableValue v - _callSql = [qc| select * from {fromQi qi}({_args}) |] :: BS.ByteString + _callSql = [qc| select * from {fromQi qi}({_args}) {limitF range} |] :: BS.ByteString decodeObj = HD.maybeRow (HD.value HD.json) operators :: [(Text, SqlFragment)] diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index 6ae4194e7..b70109a04 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -370,7 +370,10 @@ spec = do [json| [{"data": {"id": 1, "foo": {"bar": "baz"}}}] |] describe "remote procedure call" $ do - context "a proc that returns a set" $ + context "a proc that returns a set" $ do + it "returns paginated results" $ + request methodPost "/rpc/getitemrange" (rangeHdrs (ByteRangeFromTo 0 0)) [json| { "min": 2, "max": 4 } |] `shouldRespondWith` + [json| [ {"id": 3} ] |] it "returns proper json" $ post "/rpc/getitemrange" [json| { "min": 2, "max": 4 } |] `shouldRespondWith` [json| [ {"id": 3}, {"id":4} ] |] From 4496a9501433650c647ac76e81f0c0ff13cc335d Mon Sep 17 00:00:00 2001 From: Diogo Biazus Date: Wed, 10 Feb 2016 22:48:12 -0500 Subject: [PATCH 2/3] Updates changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f901f0fa..714f3de23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Throw an error for OPTIONS on nonexistent tables - @calebmer - Remove deadlock on simultaneous contentious updates - @ruslantalpa, @begriffs +### Added +- Applies range headers to RPC calls - @diogob + ## [0.3.0.3] - 2016-01-08 ### Fixed From 9d0f3573c6038ca31916da8a0f08c70861159382 Mon Sep 17 00:00:00 2001 From: Diogo Biazus Date: Fri, 12 Feb 2016 10:28:33 -0500 Subject: [PATCH 3/3] Implements query counting in proc call and adds Content-Rage to response headers in /rpc calls. --- src/PostgREST/App.hs | 23 ++++---- src/PostgREST/QueryBuilder.hs | 22 +++++--- test/Feature/QuerySpec.hs | 11 +++- test/Feature/RangeSpec.hs | 99 ++++++++++++++++++++++++++++++++++- 4 files changed, 135 insertions(+), 20 deletions(-) diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 812379c83..445e6bcdd 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -73,7 +73,7 @@ app dbStructure conf reqBody req = Right (q, cq) -> do let singular = iPreferSingular apiRequest stm = createReadStatement q cq range singular - (iPreferCount apiRequest) (contentType == TextCSV) + shouldCount (contentType == TextCSV) respondToRange $ do row <- H.query () stm let (tableTotal, queryTotal, _ , body) = row @@ -82,10 +82,7 @@ app dbStructure conf reqBody req = then responseLBS status404 [] "" else responseLBS status200 [contentTypeH] (cs body) else do - let frm = rangeOffset range - to = frm + toInteger queryTotal - 1 - contentRange = contentRangeH frm to (toInteger <$> tableTotal) - status = rangeStatus frm to (toInteger <$> tableTotal) + let (status, contentRange) = rangeHeader queryTotal tableTotal canonical = urlEncodeVars -- should this be moved to the dbStructure (location)? . sortBy (comparing fst) . map (join (***) cs) @@ -163,11 +160,13 @@ app dbStructure conf reqBody req = let p = V.head payload jwtSecret = configJwtSecret conf respondToRange $ do - bodyJson <- H.query () (callProc qi p range) + row <- H.query () (callProc qi p range shouldCount) returnJWT <- H.query qi doesProcReturnJWT - return $ responseLBS status200 [jsonH] - (let body = fromMaybe emptyArray bodyJson in - if returnJWT + let (tableTotal, queryTotal, body) = fromMaybe (Just 0, 0, emptyArray) row + (status, contentRange) = rangeHeader queryTotal tableTotal + in + return $ responseLBS status [jsonH, contentRange] + (if returnJWT then "{\"token\":\"" <> cs (tokenJWT jwtSecret body) <> "\"}" else cs $ encode body) else return notFound @@ -193,6 +192,7 @@ app dbStructure conf reqBody req = allOrigins = ("Access-Control-Allow-Origin", "*") :: Header schema = cs $ configSchema conf apiRequest = userApiRequest schema req reqBody + shouldCount = iPreferCount apiRequest range = restrictRange (configMaxRows conf) $ iRange apiRequest readDbRequest = DbRead <$> buildReadRequest (dbRelations dbStructure) apiRequest mutateDbRequest = DbMutate <$> buildMutateRequest apiRequest @@ -204,6 +204,11 @@ app dbStructure conf reqBody req = respondToRange response = if range == emptyRange then return $ errResponse status416 "HTTP Range error" else response + rangeHeader queryTotal tableTotal = let frm = rangeOffset range + to = frm + toInteger queryTotal - 1 + contentRange = contentRangeH frm to (toInteger <$> tableTotal) + status = rangeStatus frm to (toInteger <$> tableTotal) + in (status, contentRange) rangeStatus :: Integer -> Integer -> Maybe Integer -> Status rangeStatus _ _ Nothing = status200 diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index f551196df..732e749b1 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -203,18 +203,26 @@ addJoinConditions schema (Node (query, (n, r)) forest) = updatedForest = mapM (addJoinConditions schema) forest addCond query' con = query'{flt_=con ++ flt_ query'} -callProc :: QualifiedIdentifier -> JSON.Object -> NonnegRange -> H.Query () (Maybe JSON.Value) -callProc qi params range = - H.statement sql HE.unit decodeObj True +type ProcResults = (Maybe Int64, Int64, JSON.Value) +callProc :: QualifiedIdentifier -> JSON.Object -> NonnegRange -> Bool -> H.Query () (Maybe ProcResults) +callProc qi params range countTotal = + H.statement sql HE.unit decodeProc True where - sql = [qc| SELECT array_to_json( + sql = [qc| SELECT + {countQuery} as countTotal, + {countResult} as countResult, + array_to_json( coalesce(array_agg(row_to_json(t)), '\{}') )::character varying - from ({_callSql}) t |] + from (select * {_callSql} {limitF range}) t |] _args = intercalate "," $ map _assignment (HM.toList params) _assignment (n,v) = pgFmtIdent n <> ":=" <> insertableValue v - _callSql = [qc| select * from {fromQi qi}({_args}) {limitF range} |] :: BS.ByteString - decodeObj = HD.maybeRow (HD.value HD.json) + _callSql = [qc| from {fromQi qi}({_args}) |] :: BS.ByteString + countQuery = if countTotal then [qc| (select pg_catalog.count(1) {_callSql} c) |] else "null::bigint" :: BS.ByteString + countResult = "pg_catalog.count(t)" :: BS.ByteString + decodeProc = HD.maybeRow procRow + procRow = (,,) <$> HD.nullableValue HD.int8 <*> HD.value HD.int8 + <*> HD.value HD.json operators :: [(Text, SqlFragment)] operators = [ diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index b70109a04..f374e53cd 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -372,8 +372,15 @@ spec = do describe "remote procedure call" $ do context "a proc that returns a set" $ do it "returns paginated results" $ - request methodPost "/rpc/getitemrange" (rangeHdrs (ByteRangeFromTo 0 0)) [json| { "min": 2, "max": 4 } |] `shouldRespondWith` - [json| [ {"id": 3} ] |] + request methodPost "/rpc/getitemrange" + (rangeHdrs (ByteRangeFromTo 0 0)) [json| { "min": 2, "max": 4 } |] + `shouldRespondWith` ResponseMatcher { + matchBody = Just [json| [{"id":3}] |] + , matchStatus = 206 + , matchHeaders = ["Content-Range" <:> "0-0/2"] + } + + it "returns proper json" $ post "/rpc/getitemrange" [json| { "min": 2, "max": 4 } |] `shouldRespondWith` [json| [ {"id": 3}, {"id":4} ] |] diff --git a/test/Feature/RangeSpec.hs b/test/Feature/RangeSpec.hs index 1b20c8d5a..8a9e75e75 100644 --- a/test/Feature/RangeSpec.hs +++ b/test/Feature/RangeSpec.hs @@ -6,14 +6,109 @@ import Test.Hspec.Wai.JSON import Network.HTTP.Types import Network.Wai.Test (SResponse(simpleHeaders,simpleStatus)) +import qualified Data.ByteString.Lazy as BL + import SpecHelper import Network.Wai (Application) +defaultRange :: BL.ByteString +defaultRange = [json| { "min": 0, "max": 15 } |] + +emptyRange :: BL.ByteString +emptyRange = [json| { "min": 2, "max": 2 } |] + spec :: SpecWith Application -spec = +spec = do + describe "POST /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 + context "when I don't want the count" $ do + it "returns range Content-Range with */* for empty range" $ + request methodPost "/rpc/getitemrange" + [("Prefer", "count=none")] emptyRange + `shouldRespondWith` ResponseMatcher { + matchBody = Just [json| [] |] + , matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "*/*"] + } + + it "returns range Content-Range with range/*" $ + request methodPost "/rpc/getitemrange" + [("Prefer", "count=none")] defaultRange + `shouldRespondWith` ResponseMatcher { + matchBody = Just [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}] |] + , matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "0-14/*"] + } + + 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 + liftIO $ do + simpleHeaders r `shouldSatisfy` + matchHeader "Content-Range" "0-1/15" + simpleStatus r `shouldBe` partialContent206 + + it "understands open-ended ranges" $ + request methodPost "/rpc/getitemrange" + (rangeHdrs $ ByteRangeFrom 0) defaultRange + `shouldRespondWith` 200 + + it "returns an empty body when there are no results" $ + request methodPost "/rpc/getitemrange" + (rangeHdrs $ ByteRangeFromTo 0 1) emptyRange + `shouldRespondWith` ResponseMatcher { + matchBody = Just "[]" + , matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "*/0"] + } + + it "allows one-item requests" $ do + r <- request methodPost "/rpc/getitemrange" + (rangeHdrs $ ByteRangeFromTo 0 0) defaultRange + liftIO $ do + simpleHeaders r `shouldSatisfy` + matchHeader "Content-Range" "0-0/15" + simpleStatus r `shouldBe` partialContent206 + + it "handles ranges beyond collection length via truncation" $ do + r <- request methodPost "/rpc/getitemrange" + (rangeHdrs $ ByteRangeFromTo 10 100) defaultRange + liftIO $ do + simpleHeaders r `shouldSatisfy` + matchHeader "Content-Range" "10-14/15" + simpleStatus r `shouldBe` partialContent206 + + context "of invalid range" $ do + it "fails with 416 for offside range" $ + request methodPost "/rpc/getitemrange" + (rangeHdrs $ ByteRangeFromTo 1 0) emptyRange + `shouldRespondWith` 416 + + it "refuses a range with nonzero start when there are no items" $ + request methodPost "/rpc/getitemrange" + (rangeHdrs $ ByteRangeFromTo 1 2) emptyRange + `shouldRespondWith` ResponseMatcher { + matchBody = Nothing + , matchStatus = 416 + , matchHeaders = ["Content-Range" <:> "*/0"] + } + + it "refuses a range requesting start past last item" $ + request methodPost "/rpc/getitemrange" + (rangeHdrs $ ByteRangeFromTo 100 199) defaultRange + `shouldRespondWith` ResponseMatcher { + matchBody = Nothing + , matchStatus = 416 + , matchHeaders = ["Content-Range" <:> "*/15"] + } describe "GET /items" $ do - context "without range headers" $ do context "with response under server size limit" $ it "returns whole range with status 200" $