diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 9176f7318..accf76bf3 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -162,8 +162,8 @@ app dbStructure proc conf apiRequest = , if iPreferRepresentation apiRequest == Full then Just $ toHeader contentType else Nothing - , Just . contentRangeH 1 0 $ - toInteger <$> if shouldCount then Just queryTotal else Nothing + , Just $ contentRangeH 1 0 $ + if shouldCount then Just queryTotal else Nothing , if null pkCols then Nothing else (\x -> ("Preference-Applied", show x)) <$> iPreferResolution apiRequest @@ -188,21 +188,28 @@ app dbStructure proc conf apiRequest = (iPreferRepresentation apiRequest) [] row <- H.statement (toS $ pjRaw pJson) stm let (_, queryTotal, _, body) = extractQueryResult row - if contentType == CTSingularJSON - && queryTotal /= 1 - && iPreferRepresentation apiRequest == Full - then do - HT.condemn - return $ singularityError (toInteger queryTotal) - else do - let r = contentRangeH 0 (toInteger $ queryTotal-1) - (toInteger <$> if shouldCount then Just queryTotal else Nothing) - s = if iPreferRepresentation apiRequest == Full - then status200 - else status204 - return $ if iPreferRepresentation apiRequest == Full - then responseLBS s [toHeader contentType, r] (toS body) - else responseLBS s [r] "" + + updateIsNoOp = S.null $ pjKeys pJson + contentRangeHeader = contentRangeH 0 (queryTotal - 1) $ + if shouldCount then Just queryTotal else Nothing + minimalHeaders = [contentRangeHeader] + fullHeaders = toHeader contentType : minimalHeaders + + status | queryTotal == 0 && not updateIsNoOp = status404 + | iPreferRepresentation apiRequest == Full = status200 + | otherwise = status204 + + case (contentType, iPreferRepresentation apiRequest) of + (CTSingularJSON, Full) + | queryTotal == 1 -> return $ responseLBS status fullHeaders (toS body) + | otherwise -> HT.condemn >> return (singularityError queryTotal) + + (_, Full) -> + return $ responseLBS status fullHeaders (toS body) + + (_, _) -> + return $ responseLBS status minimalHeaders mempty + (ActionSingleUpsert, TargetIdent (QualifiedIdentifier tSchema tName), Just ProcessedJSON{pjRaw, pjType, pjKeys}) -> case mutateSqlParts tSchema tName of @@ -246,7 +253,7 @@ app dbStructure proc conf apiRequest = row <- H.statement mempty stm let (_, queryTotal, _, body) = extractQueryResult row r = contentRangeH 1 0 $ - toInteger <$> if shouldCount then Just queryTotal else Nothing + if shouldCount then Just queryTotal else Nothing if contentType == CTSingularJSON && queryTotal /= 1 && iPreferRepresentation apiRequest == Full @@ -332,9 +339,10 @@ app dbStructure proc conf apiRequest = selectQuery = requestToQuery schema False <$> readDbRequest countQuery = requestToCountQuery schema <$> readDbRequest readSqlParts = (,) <$> selectQuery <*> countQuery + mutationDbRequest s t = mutateRequest apiRequest t (tablePKCols dbStructure s t) =<< fldNames mutateSqlParts s t = (,) <$> selectQuery - <*> (requestToQuery schema False . DbMutate <$> (mutateRequest apiRequest t (tablePKCols dbStructure s t) =<< fldNames)) + <*> (requestToQuery schema False . DbMutate <$> mutationDbRequest s t) responseContentTypeOrError :: [ContentType] -> Action -> Either Response ContentType responseContentTypeOrError accepts action = serves contentTypesForRequest accepts @@ -381,7 +389,7 @@ rangeStatus lower upper (Just total) | (1 + upper - lower) < total = status206 | otherwise = status200 -contentRangeH :: Integer -> Integer -> Maybe Integer -> Header +contentRangeH :: (Integral a, Show a) => a -> a -> Maybe a -> Header contentRangeH lower upper total = ("Content-Range", headerValue) where diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 7aee67c5f..b27e4ae6d 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -64,7 +64,7 @@ pgError authed e = else [jsonType] in responseLBS status hdrs (encodeError e) -singularityError :: Integer -> Response +singularityError :: (Integral a, Show a) => a -> Response singularityError numRows = responseLBS HT.status406 [toHeader CTSingularJSON] diff --git a/test/Feature/InsertSpec.hs b/test/Feature/InsertSpec.hs index 639c7894d..3fd0bb50b 100644 --- a/test/Feature/InsertSpec.hs +++ b/test/Feature/InsertSpec.hs @@ -385,18 +385,18 @@ spec actualPgVersion = do describe "Patching record" $ do context "to unknown uri" $ - it "gives a 404" $ + it "indicates no table found by returning 404" $ request methodPatch "/fake" [] [json| { "real": false } |] `shouldRespondWith` 404 context "on an empty table" $ - it "indicates no records found to update" $ + it "indicates no records found to update by returning 404" $ request methodPatch "/empty_table" [] [json| { "extra":20 } |] `shouldRespondWith` "" - { matchStatus = 204, - matchHeaders = ["Content-Range" <:> "*/*"] + { matchStatus = 404, + matchHeaders = [] } context "in a nonempty table" $ do @@ -423,10 +423,15 @@ spec actualPgVersion = do [("Prefer", "return=representation")] [json| { "id":999999 } |] `shouldRespondWith` "[]" { - matchStatus = 200, - matchHeaders = ["Content-Range" <:> "*/*"] + matchStatus = 404, + matchHeaders = [] } + it "gives a 404 when no rows updated" $ + request methodPatch "/items?id=eq.99999999" [] + [json| { "id": 42 } |] + `shouldRespondWith` 404 + it "returns updated object as array when return=rep" $ request methodPatch "/items?id=eq.2" [("Prefer", "return=representation")] [json| { "id":2 } |] @@ -454,15 +459,26 @@ spec actualPgVersion = do [json| [{ a: "keepme", b: null }] |] { matchHeaders = [matchContentTypeJson] } - it "can update based on a computed column" $ - request methodPatch - "/items?always_true=eq.false" - [("Prefer", "return=representation")] - [json| { id: 100 } |] - `shouldRespondWith` "[]" - { matchStatus = 200, - matchHeaders = ["Content-Range" <:> "*/*"] - } + context "filtering by a computed column" $ do + it "is successful" $ + request methodPatch + "/items?is_first=eq.true" + [("Prefer", "return=representation")] + [json| { id: 100 } |] + `shouldRespondWith` [json| [{ id: 100 }] |] + { matchStatus = 200, + matchHeaders = [matchContentTypeJson, "Content-Range" <:> "0-0/*"] + } + + it "indicates no records updated by returning 404" $ + request methodPatch + "/items?always_true=eq.false" + [("Prefer", "return=representation")] + [json| { id: 100 } |] + `shouldRespondWith` "[]" + { matchStatus = 404, + matchHeaders = [] + } it "can provide a representation" $ do _ <- post "/items" diff --git a/test/Feature/SingularSpec.hs b/test/Feature/SingularSpec.hs index c0fd97bfe..c389ba026 100644 --- a/test/Feature/SingularSpec.hs +++ b/test/Feature/SingularSpec.hs @@ -43,7 +43,6 @@ spec = { matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"] } context "when updating rows" $ do - it "works for one row" $ do _ <- post "/addresses" [json| { id: 97, address: "A Street" } |] request methodPatch @@ -56,10 +55,9 @@ spec = it "raises an error for multiple rows" $ do _ <- post "/addresses" [json| { id: 98, address: "xxx" } |] _ <- post "/addresses" [json| { id: 99, address: "yyy" } |] - p <- request methodPatch - "/addresses?id=gt.0" - [("Prefer", "return=representation"), singular] - [json| { address: "zzz" } |] + p <- request methodPatch "/addresses?id=gt.0" + [("Prefer", "return=representation"), singular] + [json| { address: "zzz" } |] liftIO $ do simpleStatus p `shouldBe` notAcceptable406 isErrorFormat (simpleBody p) `shouldBe` True @@ -67,15 +65,16 @@ spec = -- the rows should not be updated, either get "/addresses?id=eq.98" `shouldRespondWith` [str|[{"id":98,"address":"xxx"}]|] - it "raises an error for zero rows" $ do - p <- request methodPatch "/items?id=gt.0&id=lt.0" - [("Prefer", "return=representation"), singular] [json|{"id":1}|] - liftIO $ do - simpleStatus p `shouldBe` notAcceptable406 - isErrorFormat (simpleBody p) `shouldBe` True + it "raises an error for zero rows" $ + request methodPatch "/items?id=gt.0&id=lt.0" + [("Prefer", "return=representation"), singular] [json|{"id":1}|] + `shouldRespondWith` + [str|{"details":"Results contain 0 rows, application/vnd.pgrst.object+json requires 1 row","message":"JSON object requested, multiple (or no) rows returned"}|] + { matchStatus = 406 + , matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"] + } context "when creating rows" $ do - it "works for one row" $ do p <- request methodPost "/addresses" @@ -117,17 +116,17 @@ spec = , matchHeaders = ["Content-Range" <:> "*/*"] } - it "raises an error when creating zero entities" $ do - p <- request methodPost - "/addresses" - [("Prefer", "return=representation"), singular] - [json| [ ] |] - liftIO $ do - simpleStatus p `shouldBe` notAcceptable406 - isErrorFormat (simpleBody p) `shouldBe` True + it "raises an error when creating zero entities" $ + request methodPost "/addresses" + [("Prefer", "return=representation"), singular] + [json| [ ] |] + `shouldRespondWith` + [str|{"details":"Results contain 0 rows, application/vnd.pgrst.object+json requires 1 row","message":"JSON object requested, multiple (or no) rows returned"}|] + { matchStatus = 406 + , matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"] + } context "when deleting rows" $ do - it "works for one row" $ do p <- request methodDelete "/items?id=eq.11" @@ -146,21 +145,24 @@ spec = , matchHeaders = ["Content-Range" <:> "0-9/*"] } - it "raises an error when deleting zero entities" $ do - p <- request methodDelete "/items?id=lt.0" - [("Prefer", "return=representation"), singular] "" - liftIO $ do - simpleStatus p `shouldBe` notAcceptable406 - isErrorFormat (simpleBody p) `shouldBe` True + it "raises an error when deleting zero entities" $ + request methodDelete "/items?id=lt.0" + [("Prefer", "return=representation"), singular] "" + `shouldRespondWith` + [str|{"details":"Results contain 0 rows, application/vnd.pgrst.object+json requires 1 row","message":"JSON object requested, multiple (or no) rows returned"}|] + { matchStatus = 406 + , matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"] + } context "when calling a stored proc" $ do - - it "fails for zero rows" $ do - p <- request methodPost "/rpc/getproject" - [singular] [json|{ "id": 9999999}|] - liftIO $ do - simpleStatus p `shouldBe` notAcceptable406 - isErrorFormat (simpleBody p) `shouldBe` True + it "fails for zero rows" $ + request methodPost "/rpc/getproject" + [singular] [json|{ "id": 9999999}|] + `shouldRespondWith` + [str|{"details":"Results contain 0 rows, application/vnd.pgrst.object+json requires 1 row","message":"JSON object requested, multiple (or no) rows returned"}|] + { matchStatus = 406 + , matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"] + } -- this one may be controversial, should vnd.pgrst.object include -- the likes of 2 and "hello?" @@ -174,20 +176,26 @@ spec = [singular] [json|{ "id": 1}|] `shouldRespondWith` [str|{"id":1,"name":"Windows 7","client_id":1}|] - it "fails for multiple rows" $ do - p <- request methodPost "/rpc/getallprojects" [singular] "{}" - liftIO $ do - simpleStatus p `shouldBe` notAcceptable406 - isErrorFormat (simpleBody p) `shouldBe` True + it "fails for multiple rows" $ + request methodPost "/rpc/getallprojects" + [singular] "{}" + `shouldRespondWith` + [str|{"details":"Results contain 5 rows, application/vnd.pgrst.object+json requires 1 row","message":"JSON object requested, multiple (or no) rows returned"}|] + { matchStatus = 406 + , matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"] + } it "executes the proc exactly once per request" $ do request methodPost "/rpc/getproject?select=id,name" [] [json| {"id": 1} |] `shouldRespondWith` [str|[{"id":1,"name":"Windows 7"}]|] - p <- request methodPost "/rpc/setprojects" [singular] - [json| {"id_l": 1, "id_h": 2, "name": "changed"} |] - liftIO $ do - simpleStatus p `shouldBe` notAcceptable406 - isErrorFormat (simpleBody p) `shouldBe` True + + request methodPost "/rpc/setprojects" [singular] + [json| {"id_l": 1, "id_h": 2, "name": "changed"} |] + `shouldRespondWith` + [str|{"details":"Results contain 2 rows, application/vnd.pgrst.object+json requires 1 row","message":"JSON object requested, multiple (or no) rows returned"}|] + { matchStatus = 406 + , matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"] + } -- should not actually have executed the function request methodPost "/rpc/getproject?select=id,name" [] [json| {"id": 1} |] diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index ff62b1e4d..3d043c8f9 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -116,6 +116,11 @@ CREATE FUNCTION always_true(test.items) RETURNS boolean LANGUAGE sql STABLE AS $$ SELECT true $$; +CREATE FUNCTION is_first(test.items) RETURNS boolean + LANGUAGE sql STABLE + AS $$ SELECT $1.id = 1 $$; + + CREATE FUNCTION anti_id(test.items) RETURNS bigint LANGUAGE sql STABLE AS $_$ SELECT $1.id * -1 $_$;