Enforce singular behavior despite Prefer: return (#1417)

CHANGELOG.md

@@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Added
 
+- #1417, `Accept: application/vnd.pgrst.object+json` behavior is now enforced for POST/PATCH/DELETE regardless of `Prefer: return=representation/minimal` - @dwagin
 - #1415, Add support for user defined socket permission via `server-unix-socket-mode` config option - @Dansvidania
 - #1383, Add support for HEAD request - @steve-chavez
 - #1378, Add support for `Prefer: count=planned` and `Prefer: count=estimated` on GET /table - @steve-chavez

src/PostgREST/App.hs

@@ -175,7 +175,6 @@ app dbStructure proc cols conf apiRequest =
                     ]
               if contentType == CTSingularJSON
                  && queryTotal /= 1
-                 && iPreferRepresentation apiRequest == Full
                 then do
                   HT.condemn
                   return . errorResponseFor . singularityError $ queryTotal
@@ -193,28 +192,23 @@ app dbStructure proc cols conf apiRequest =
                     (iPreferRepresentation apiRequest) []
               row <- H.statement (toS $ pjRaw pJson) stm
               let (_, queryTotal, _, body) = row
-
                   updateIsNoOp = S.null cols
                   contentRangeHeader = contentRangeH 0 (queryTotal - 1) $
                         if shouldCount then Just queryTotal else Nothing
-                  minimalHeaders     = [contentRangeHeader]
+                  headers | iPreferRepresentation apiRequest == Full = [toHeader contentType, contentRangeHeader]
-                  fullHeaders        = toHeader contentType : minimalHeaders
+                          | otherwise                                = [contentRangeHeader]
-
                   status | queryTotal == 0 && not updateIsNoOp      = status404
                          | iPreferRepresentation apiRequest == Full = status200
                          | otherwise                                = status204
-
+              if contentType == CTSingularJSON
-              case (contentType, iPreferRepresentation apiRequest) of
+                 && queryTotal /= 1
-                (CTSingularJSON, Full)
+                then do
-                      | queryTotal == 1 -> return $ responseLBS status fullHeaders (toS body)
+                  HT.condemn
-                      | otherwise       -> HT.condemn >> (return . errorResponseFor . singularityError) queryTotal
+                  return . errorResponseFor . singularityError $ queryTotal
-
+                else
-                (_, Full) ->
+                  return $ if iPreferRepresentation apiRequest == Full
-                  return $ responseLBS status fullHeaders (toS body)
+                    then responseLBS status headers (toS body)
-
+                    else responseLBS status headers mempty
-                (_, _) ->
-                  return $ responseLBS status minimalHeaders mempty
-
 
         (ActionSingleUpsert, TargetIdent (QualifiedIdentifier tSchema tName), Just ProcessedJSON{pjRaw, pjType, pjKeys}) ->
           case mutateSqlParts tSchema tName of
@@ -257,18 +251,17 @@ app dbStructure proc cols conf apiRequest =
                     (iPreferRepresentation apiRequest) []
               row <- H.statement mempty stm
               let (_, queryTotal, _, body) = row
-                  r = contentRangeH 1 0 $
+                  contentRangeHeader = contentRangeH 1 0 $
                         if shouldCount then Just queryTotal else Nothing
               if contentType == CTSingularJSON
                  && queryTotal /= 1
-                 && iPreferRepresentation apiRequest == Full
                 then do
                   HT.condemn
                   return . errorResponseFor . singularityError $ queryTotal
                 else
                   return $ if iPreferRepresentation apiRequest == Full
-                    then responseLBS status200 [toHeader contentType, r] (toS body)
+                    then responseLBS status200 [toHeader contentType, contentRangeHeader] (toS body)
-                    else responseLBS status204 [r] ""
+                    else responseLBS status204 [contentRangeHeader] ""
 
         (ActionInfo, TargetIdent (QualifiedIdentifier tSchema tTable), Nothing) ->
           let mTable = find (\t -> tableName t == tTable && tableSchema t == tSchema) (dbTables dbStructure) in

test/Feature/SingularSpec.hs

@@ -56,7 +56,7 @@ spec =
         _ <- 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]
+                [singular]
                 [json| { address: "zzz" } |]
         liftIO $ do
           simpleStatus p `shouldBe` notAcceptable406
@@ -65,7 +65,29 @@ spec =
         -- the rows should not be updated, either
         get "/addresses?id=eq.98" `shouldRespondWith` [str|[{"id":98,"address":"xxx"}]|]
 
+      it "raises an error for multiple rows with return=rep" $ do
+        _ <- post "/addresses" [json| { id: 100, address: "xxx" } |]
+        _ <- post "/addresses" [json| { id: 101, address: "yyy" } |]
+        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
+
+        -- the rows should not be updated, either
+        get "/addresses?id=eq.100" `shouldRespondWith` [str|[{"id":100,"address":"xxx"}]|]
+
       it "raises an error for zero rows" $
+        request methodPatch "/items?id=gt.0&id=lt.0"
+                [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"]
+                  }
+
+      it "raises an error for zero rows with return=rep" $
         request methodPatch "/items?id=gt.0&id=lt.0"
                 [("Prefer", "return=representation"), singular] [json|{"id":1}|]
           `shouldRespondWith`
@@ -79,20 +101,20 @@ spec =
         p <- request methodPost
           "/addresses"
           [("Prefer", "return=representation"), singular]
-          [json| [ { id: 100, address: "xxx" } ] |]
+          [json| [ { id: 102, address: "xxx" } ] |]
-        liftIO $ simpleBody p `shouldBe` [str|{"id":100,"address":"xxx"}|]
+        liftIO $ simpleBody p `shouldBe` [str|{"id":102,"address":"xxx"}|]
 
       it "works for one row even with return=minimal" $ do
         request methodPost "/addresses"
           [("Prefer", "return=minimal"), singular]
-          [json| [ { id: 101, address: "xxx" } ] |]
+          [json| [ { id: 103, address: "xxx" } ] |]
           `shouldRespondWith` ""
           { matchStatus  = 201
           , matchHeaders = ["Content-Range" <:> "*/*"]
           }
         -- and the element should exist
-        get "/addresses?id=eq.101"
+        get "/addresses?id=eq.103"
-          `shouldRespondWith` [str|[{"id":101,"address":"xxx"}]|]
+          `shouldRespondWith` [str|[{"id":103,"address":"xxx"}]|]
           { matchStatus  = 200
           , matchHeaders = []
           }
@@ -100,23 +122,47 @@ spec =
       it "raises an error when attempting to create multiple entities" $ do
         p <- request methodPost
           "/addresses"
-          [("Prefer", "return=representation"), singular]
+          [singular]
           [json| [ { id: 200, address: "xxx" }, { id: 201, address: "yyy" } ] |]
         liftIO $ simpleStatus p `shouldBe` notAcceptable406
 
         -- the rows should not exist, either
         get "/addresses?id=eq.200" `shouldRespondWith` "[]"
 
-      it "return=minimal allows request to create multiple elements" $
+      it "raises an error when attempting to create multiple entities with return=rep" $ do
+        p <- request methodPost
+          "/addresses"
+          [("Prefer", "return=representation"), singular]
+          [json| [ { id: 202, address: "xxx" }, { id: 203, address: "yyy" } ] |]
+        liftIO $ simpleStatus p `shouldBe` notAcceptable406
+
+        -- the rows should not exist, either
+        get "/addresses?id=eq.202" `shouldRespondWith` "[]"
+
+      it "raises an error regardless of return=minimal" $ do
         request methodPost "/addresses"
                 [("Prefer", "return=minimal"), singular]
-          [json| [ { id: 200, address: "xxx" }, { id: 201, address: "yyy" } ] |]
+                [json| [ { id: 204, address: "xxx" }, { id: 205, address: "yyy" } ] |]
-          `shouldRespondWith` ""
+          `shouldRespondWith`
-          { matchStatus  = 201
+                  [str|{"details":"Results contain 2 rows, application/vnd.pgrst.object+json requires 1 row","message":"JSON object requested, multiple (or no) rows returned"}|]
-          , matchHeaders = ["Content-Range" <:> "*/*"]
+                  { matchStatus  = 406
+                  , matchHeaders = ["Content-Type" <:> "application/vnd.pgrst.object+json; charset=utf-8"]
                   }
 
+        -- the rows should not exist, either
+        get "/addresses?id=eq.204" `shouldRespondWith` "[]"
+
       it "raises an error when creating zero entities" $
+        request methodPost "/addresses"
+                [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"]
+                  }
+
+      it "raises an error when creating zero entities with return=rep" $
         request methodPost "/addresses"
                 [("Prefer", "return=representation"), singular]
                 [json| [ ] |]
@@ -134,18 +180,39 @@ spec =
         liftIO $ simpleBody p `shouldBe` [str|{"id":11}|]
 
       it "raises an error when attempting to delete multiple entities" $ do
-        let firstItems = "/items?id=gt.0&id=lt.11"
+        let firstItems = "/items?id=gt.0&id=lt.6"
+        request methodDelete firstItems
+          [singular] ""
+          `shouldRespondWith` 406
+
+        get firstItems
+          `shouldRespondWith` [json| [{"id":1},{"id":2},{"id":3},{"id":4},{"id":5}] |]
+          { matchStatus  = 200
+          , matchHeaders = ["Content-Range" <:> "0-4/*"]
+          }
+
+      it "raises an error when attempting to delete multiple entities with return=rep" $ do
+        let firstItems = "/items?id=gt.5&id=lt.11"
         request methodDelete firstItems
           [("Prefer", "return=representation"), singular] ""
           `shouldRespondWith` 406
 
         get firstItems
-          `shouldRespondWith` [json| [{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":6},{"id":7},{"id":8},{"id":9},{"id":10}] |]
+          `shouldRespondWith` [json| [{"id":6},{"id":7},{"id":8},{"id":9},{"id":10}] |]
           { matchStatus  = 200
-          , matchHeaders = ["Content-Range" <:> "0-9/*"]
+          , matchHeaders = ["Content-Range" <:> "0-4/*"]
           }
 
       it "raises an error when deleting zero entities" $
+        request methodDelete "/items?id=lt.0"
+                [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"]
+                }
+
+      it "raises an error when deleting zero entities with return=rep" $
         request methodDelete "/items?id=lt.0"
                 [("Prefer", "return=representation"), singular] ""
           `shouldRespondWith`