diff --git a/CHANGELOG.md b/CHANGELOG.md index ba9ef68217..08fa9b0c14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,10 +19,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #2458, Fix a regression with the location header when inserting into views with PKs from multiple tables - @wolfgangwalther - #2356, Fix a regression in openapi output with mode follow-privileges - @wolfgangwalther - #2283, Fix infinite recursion when loading schema cache with self-referencing view - @wolfgangwalther + - #2343, Return status code 200 for PATCH requests which don't affect any rows - @wolfgangwalther ### Changed - #2444, Removed `db-pool-timeout` option, because this was removed upstream in hasql-pool. - @robx + - #2343, PATCH requests that don't affect any rows no longer return 404 - @wolfgangwalther ### Deprecated diff --git a/src/PostgREST/Response.hs b/src/PostgREST/Response.hs index 790fcc788a..9f710b660d 100644 --- a/src/PostgREST/Response.hs +++ b/src/PostgREST/Response.hs @@ -18,7 +18,6 @@ import qualified Data.Aeson as JSON import qualified Data.ByteString.Char8 as BS import qualified Data.ByteString.Lazy as LBS import qualified Data.HashMap.Strict as HM -import qualified Data.Set as S import Data.Text.Read (decimal) import qualified Network.HTTP.Types.Header as HTTP import qualified Network.HTTP.Types.Status as HTTP @@ -121,21 +120,17 @@ updateResponse ctxApiRequest@ApiRequest{..} resultSet = case resultSet of RSStandard{..} -> do let response = gucResponse rsGucStatus rsGucHeaders - fullRepr = iPreferRepresentation == Full - updateIsNoOp = S.null iColumns - status - | rsQueryTotal == 0 && not updateIsNoOp = HTTP.status404 - | fullRepr = HTTP.status200 - | otherwise = HTTP.status204 contentRangeHeader = RangeQuery.contentRangeH 0 (rsQueryTotal - 1) $ if shouldCount iPreferCount then Just rsQueryTotal else Nothing headers = [contentRangeHeader] - if fullRepr then - response status (headers ++ contentTypeHeaders ctxApiRequest) (LBS.fromStrict rsBody) + if iPreferRepresentation == Full then + response HTTP.status200 + (headers ++ contentTypeHeaders ctxApiRequest) + (LBS.fromStrict rsBody) else - response status headers mempty + response HTTP.status204 headers mempty RSPlan plan -> Wai.responseLBS HTTP.status200 (contentTypeHeaders ctxApiRequest) $ LBS.fromStrict plan diff --git a/test/spec/Feature/Query/UpdateSpec.hs b/test/spec/Feature/Query/UpdateSpec.hs index 247a542efe..23d0b63886 100644 --- a/test/spec/Feature/Query/UpdateSpec.hs +++ b/test/spec/Feature/Query/UpdateSpec.hs @@ -28,12 +28,12 @@ spec = do `shouldRespondWith` 404 context "on an empty table" $ - it "indicates no records found to update by returning 404" $ + it "succeeds with status code 204" $ request methodPatch "/empty_table" [] [json| { "extra":20 } |] `shouldRespondWith` "" - { matchStatus = 404, + { matchStatus = 204, matchHeaders = [matchHeaderAbsent hContentType] } @@ -71,14 +71,14 @@ spec = do [("Prefer", "return=representation")] [json| { "id":999999 } |] `shouldRespondWith` "[]" { - matchStatus = 404, + matchStatus = 200, matchHeaders = [] } - it "gives a 404 when no rows updated" $ + it "returns status code 200 when no rows updated" $ request methodPatch "/items?id=eq.99999999" [] [json| { "id": 42 } |] - `shouldRespondWith` 404 + `shouldRespondWith` 204 it "returns updated object as array when return=rep" $ request methodPatch "/items?id=eq.2" @@ -137,13 +137,13 @@ spec = do matchHeaders = [matchContentTypeJson, "Content-Range" <:> "0-0/*"] } - it "indicates no records updated by returning 404" $ + it "returns empty array when no rows updated and return=rep" $ request methodPatch "/items?always_true=eq.false" [("Prefer", "return=representation")] [json| { id: 100 } |] `shouldRespondWith` "[]" - { matchStatus = 404, + { matchStatus = 200, matchHeaders = [] } @@ -304,9 +304,9 @@ spec = do `shouldRespondWith` [json|[{"id": 1, "body": "Some real content", "owner": "postgrest_test_anonymous"}]|] - it "ignores json keys and gives 404 if no record updated" $ + it "ignores json keys and gives 200 if no record updated" $ request methodPatch "/articles?id=eq.2001&columns=body" [("Prefer", "return=representation")] - [json| {"body": "Some real content", "smth": "here", "other": "stuff", "fake_id": 13} |] `shouldRespondWith` 404 + [json| {"body": "Some real content", "smth": "here", "other": "stuff", "fake_id": 13} |] `shouldRespondWith` 200 context "tables with self reference foreign keys" $ do it "embeds children after update" $