fix: HTTP status responses for upserts

* PUT returns 201 instead of 200 when rows are inserted
* POST with "Prefer: resolution=merge-duplicates" returns 200 instead of 201 when no rows are inserted

CHANGELOG.md

@@ -20,6 +20,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
  - #3015, Fix unnecessary count() on RPC returning single - @steve-chavez
+ - #1070, Fix HTTP status responses for upserts - @taimoorzaeem
+   + `PUT` returns `201` instead of `200` when rows are inserted
+   + `POST` with `Prefer: resolution=merge-duplicates` returns `200` instead of `201` when no rows are inserted
 
 ### Changed
 

src/PostgREST/ApiRequest/Preferences.hs

@@ -176,6 +176,7 @@ class ToHeaderValue a where
 data PreferResolution
   = MergeDuplicates
   | IgnoreDuplicates
+  deriving Eq
 
 instance ToHeaderValue PreferResolution where
   toHeaderValue MergeDuplicates  = "resolution=merge-duplicates"

src/PostgREST/Query.hs

@@ -190,7 +190,7 @@ openApiQuery sCache pgVer AppConfig{..} tSchema =
 writeQuery :: MutateReadPlan -> ApiRequest -> AppConfig  -> DbHandler ResultSet
 writeQuery MutateReadPlan{..} ApiRequest{iPreferences=Preferences{..}} conf =
   let
-    (isInsert, pkCols) = case mrMutatePlan of {Insert{insPkCols} -> (True, insPkCols); _ -> (False, mempty);}
+    (isPut, isInsert, pkCols) = case mrMutatePlan of {Insert{where_,insPkCols} -> ((not . null) where_, True, insPkCols); _ -> (False,False, mempty);}
   in
   lift . SQL.statement mempty $
     Statements.prepareWrite
@@ -198,9 +198,11 @@ writeQuery MutateReadPlan{..} ApiRequest{iPreferences=Preferences{..}} conf =
       (QueryBuilder.readPlanToQuery mrReadPlan)
       (QueryBuilder.mutatePlanToQuery mrMutatePlan)
       isInsert
+      isPut
       mrMedia
       mrHandler
       preferRepresentation
+      preferResolution
       pkCols
       (configDbPreparedStatements conf)
 

src/PostgREST/Query/QueryBuilder.hs

@@ -87,7 +87,8 @@ mutatePlanToQuery (Insert mainQi iCols body onConflct putConditions returnings _
   "INSERT INTO " <> fromQi mainQi <> (if null iCols then " " else "(" <> cols <> ") ") <>
   fromJsonBodyF body iCols True False applyDefaults <>
   -- Only used for PUT
-  (if null putConditions then mempty else "WHERE " <> intercalateSnippet " AND " (pgFmtLogicTree (QualifiedIdentifier mempty "pgrst_body") <$> putConditions)) <>
+  (if null putConditions then mempty else "WHERE " <> addConfigPgrstInserted True <> " AND " <> intercalateSnippet " AND " (pgFmtLogicTree (QualifiedIdentifier mempty "pgrst_body") <$> putConditions)) <>
+  (if null putConditions && mergeDups then "WHERE " <> addConfigPgrstInserted True else mempty) <>
   maybe mempty (\(oncDo, oncCols) ->
     if null oncCols then
       mempty
@@ -98,11 +99,12 @@ mutatePlanToQuery (Insert mainQi iCols body onConflct putConditions returnings _
       MergeDuplicates  ->
         if null iCols
            then "DO NOTHING"
-           else "DO UPDATE SET " <> intercalateSnippet ", " ((pgFmtIdent . cfName) <> const " = EXCLUDED." <> (pgFmtIdent . cfName) <$> iCols)
+           else "DO UPDATE SET " <> intercalateSnippet ", " ((pgFmtIdent . cfName) <> const " = EXCLUDED." <> (pgFmtIdent . cfName) <$> iCols) <> (if null putConditions && not mergeDups then mempty else "WHERE " <> addConfigPgrstInserted False)
     ) onConflct <> " " <>
     returningF mainQi returnings
   where
     cols = intercalateSnippet ", " $ pgFmtIdent . cfName <$> iCols
+    mergeDups = case onConflct of {Just (MergeDuplicates,_) -> True; _ -> False;}
 
 -- An update without a limit is always filtered with a WHERE
 mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings applyDefaults)

src/PostgREST/Query/SqlFragment.hs

@@ -24,6 +24,8 @@ module PostgREST.Query.SqlFragment
   , fromJsonBodyF
   , responseHeadersF
   , responseStatusF
+  , addConfigPgrstInserted
+  , currentSettingF
   , returningF
   , singleParameter
   , sourceCTE
@@ -433,6 +435,11 @@ responseHeadersF = currentSettingF "response.headers"
 responseStatusF :: SQL.Snippet
 responseStatusF = currentSettingF "response.status"
 
+addConfigPgrstInserted :: Bool -> SQL.Snippet
+addConfigPgrstInserted add =
+  let (symbol, num) =  if add then ("+", "0") else ("-", "-1") in
+  "set_config('pgrst.inserted', (coalesce(" <> currentSettingF "pgrst.inserted" <> "::int, 0) " <> symbol <> " 1)::text, true) <> '" <> num <> "'"
+
 currentSettingF :: SQL.Snippet -> SQL.Snippet
 currentSettingF setting =
   -- nullif is used because of https://gist.github.com/steve-chavez/8d7033ea5655096903f3b52f8ed09a15

src/PostgREST/Query/Statements.hs

@@ -50,15 +50,19 @@ data ResultSet
   -- ^ the HTTP headers to be added to the response
   , rsGucStatus  :: Maybe Text
   -- ^ the HTTP status to be added to the response
+  , rsInserted   :: Maybe Int64
+  -- ^ the number of rows inserted (Only used for upserts)
   }
   | RSPlan BS.ByteString -- ^ the plan of the query
 
 
-prepareWrite :: QualifiedIdentifier -> SQL.Snippet -> SQL.Snippet -> Bool -> MediaType -> MediaHandler ->
+prepareWrite :: QualifiedIdentifier -> SQL.Snippet -> SQL.Snippet -> Bool -> Bool -> MediaType -> MediaHandler ->
-                Maybe PreferRepresentation -> [Text] -> Bool -> SQL.Statement () ResultSet
+                Maybe PreferRepresentation -> Maybe PreferResolution -> [Text] -> Bool -> SQL.Statement () ResultSet
-prepareWrite qi selectQuery mutateQuery isInsert mt handler rep pKeys =
+prepareWrite qi selectQuery mutateQuery isInsert isPut mt handler rep resolution pKeys =
   SQL.dynamicallyParameterized (mtSnippet mt snippet) decodeIt
  where
+  checkUpsert snip = if isInsert && (isPut || resolution == Just MergeDuplicates) then snip else "''"
+  pgrstInsertedF = checkUpsert "nullif(current_setting('pgrst.inserted', true),'')::int"
   snippet =
     "WITH " <> sourceCTE <> " AS (" <> mutateQuery <> ") " <>
     "SELECT " <>
@@ -67,7 +71,8 @@ prepareWrite qi selectQuery mutateQuery isInsert mt handler rep pKeys =
       locF <> " AS header, " <>
       handlerF Nothing qi handler <> " AS body, " <>
       responseHeadersF <> " AS response_headers, " <>
-      responseStatusF  <> " AS response_status " <>
+      responseStatusF  <> " AS response_status, " <>
+      pgrstInsertedF <> " AS response_inserted " <>
     "FROM (" <> selectF <> ") _postgrest_t"
 
   locF =
@@ -87,7 +92,7 @@ prepareWrite qi selectQuery mutateQuery isInsert mt handler rep pKeys =
   decodeIt :: HD.Result ResultSet
   decodeIt = case mt of
     MTVndPlan{} -> planRow
-    _        -> fromMaybe (RSStandard Nothing 0 mempty mempty Nothing Nothing) <$> HD.rowMaybe (standardRow False)
+    _           -> fromMaybe (RSStandard Nothing 0 mempty mempty Nothing Nothing Nothing) <$> HD.rowMaybe (standardRow False)
 
 prepareRead :: QualifiedIdentifier -> SQL.Snippet -> SQL.Snippet -> Bool -> MediaType -> MediaHandler -> Bool -> SQL.Statement () ResultSet
 prepareRead qi selectQuery countQuery countTotal mt handler =
@@ -101,7 +106,8 @@ prepareRead qi selectQuery countQuery countTotal mt handler =
       "pg_catalog.count(_postgrest_t) AS page_total, " <>
       handlerF Nothing qi handler <> " AS body, " <>
       responseHeadersF <> " AS response_headers, " <>
-      responseStatusF <> " AS response_status " <>
+      responseStatusF <> " AS response_status, " <>
+      "''" <> " AS response_inserted " <>
     "FROM ( SELECT * FROM " <> sourceCTE <> " ) _postgrest_t"
 
   (countCTEF, countResultF) = countF countQuery countTotal
@@ -127,7 +133,8 @@ prepareCall qi rout callProcQuery selectQuery countQuery countTotal mt handler =
           else "pg_catalog.count(_postgrest_t)") <> " AS page_total, " <>
         handlerF (Just rout) qi handler <> " AS body, " <>
         responseHeadersF <> " AS response_headers, " <>
-        responseStatusF <> " AS response_status " <>
+        responseStatusF <> " AS response_status, " <>
+        "''" <> " AS response_inserted " <>
       "FROM (" <> selectQuery <> ") _postgrest_t"
 
     (countCTEF, countResultF) = countF countQuery countTotal
@@ -135,7 +142,7 @@ prepareCall qi rout callProcQuery selectQuery countQuery countTotal mt handler =
     decodeIt :: HD.Result ResultSet
     decodeIt = case mt of
       MTVndPlan{} -> planRow
-      _        -> fromMaybe (RSStandard (Just 0) 0 mempty mempty Nothing Nothing) <$> HD.rowMaybe (standardRow True)
+      _           -> fromMaybe (RSStandard (Just 0) 0 mempty mempty Nothing Nothing Nothing) <$> HD.rowMaybe (standardRow True)
 
 preparePlanRows :: SQL.Snippet -> Bool -> SQL.Statement () (Maybe Int64)
 preparePlanRows countQuery =
@@ -153,6 +160,7 @@ standardRow noLocation =
              <*> (if noLocation then pure mempty else fmap splitKeyValue <$> arrayColumn HD.bytea) <*> column HD.bytea
              <*> nullableColumn HD.bytea
              <*> nullableColumn HD.text
+             <*> nullableColumn HD.int8
   where
     splitKeyValue :: ByteString -> (ByteString, ByteString)
     splitKeyValue kv =

src/PostgREST/Response.hs

@@ -22,6 +22,7 @@ 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           Data.Maybe                (fromJust)
 import           Data.Text.Read            (decimal)
 import qualified Network.HTTP.Types.Header as HTTP
 import qualified Network.HTTP.Types.Status as HTTP
@@ -35,6 +36,7 @@ import qualified PostgREST.Response.OpenAPI as OpenAPI
 import PostgREST.ApiRequest              (ApiRequest (..),
                                           InvokeMethod (..))
 import PostgREST.ApiRequest.Preferences  (PreferRepresentation (..),
+                                          PreferResolution (..),
                                           Preferences (..),
                                           prefAppliedHeader,
                                           shouldCount)
@@ -119,8 +121,13 @@ createResponse QualifiedIdentifier{..} MutateReadPlan{mrMutatePlan, mrMedia} ctx
               if shouldCount preferCount then Just rsQueryTotal else Nothing
           , prefHeader ]
 
-    let status = HTTP.status201
+    let isInsertIfGTZero i =
-    let (headers', bod) = case preferRepresentation of
+          if i <= 0 && preferResolution == Just MergeDuplicates then
+            HTTP.status200
+          else
+            HTTP.status201
+        status = maybe HTTP.status200 isInsertIfGTZero rsInserted
+        (headers', bod) = case preferRepresentation of
           Just Full -> (headers ++ contentTypeHeaders mrMedia ctxApiRequest, LBS.fromStrict rsBody)
           Just None -> (headers, mempty)
           Just HeadersOnly -> (headers, mempty)
@@ -142,8 +149,8 @@ updateResponse MutateReadPlan{mrMedia} ctxApiRequest@ApiRequest{iPreferences=Pre
       prefHeader = prefAppliedHeader $ Preferences Nothing preferRepresentation Nothing preferCount preferTransaction preferMissing preferHandling []
       headers = catMaybes [contentRangeHeader, prefHeader]
 
-    let
+    let (status, headers', body) =
-      (status, headers', body) = case preferRepresentation of
+          case preferRepresentation of
             Just Full -> (HTTP.status200, headers ++ contentTypeHeaders mrMedia ctxApiRequest, LBS.fromStrict rsBody)
             Just None -> (HTTP.status204, headers, mempty)
             _         -> (HTTP.status204, headers, mempty)
@@ -162,9 +169,11 @@ singleUpsertResponse MutateReadPlan{mrMedia} ctxApiRequest@ApiRequest{iPreferenc
       prefHeader = maybeToList . prefAppliedHeader $ Preferences Nothing preferRepresentation Nothing preferCount preferTransaction Nothing preferHandling []
       cTHeader = contentTypeHeaders mrMedia ctxApiRequest
 
-    let (status, headers, body) =
+    let isInsertIfGTZero i = if i > 0 then HTTP.status201 else HTTP.status200
+        upsertStatus       = isInsertIfGTZero $ fromJust rsInserted
+        (status, headers, body) =
           case preferRepresentation of
-            Just Full -> (HTTP.status200, cTHeader ++ prefHeader, LBS.fromStrict rsBody)
+            Just Full -> (upsertStatus, cTHeader ++ prefHeader, LBS.fromStrict rsBody)
             Just None -> (HTTP.status204,  prefHeader, mempty)
             _ -> (HTTP.status204, prefHeader, mempty)
     (ovStatus, ovHeaders) <- overrideStatusHeaders rsGucStatus rsGucHeaders status headers

test/spec/Feature/Query/MultipleSchemaSpec.hs

@@ -228,10 +228,8 @@ spec =
           [json|[{"id": 111, "name": "child v2-111", "parent_id": null}]|]
           `shouldRespondWith`
           [json|[{"id": 111, "name": "child v2-111", "parent_id": null}]|]
-          {
+          { matchStatus  = 201
-            matchStatus = 200
+          , matchHeaders = [matchContentTypeJson, "Content-Profile" <:> "v2"]}
-          , matchHeaders = [matchContentTypeJson, "Content-Profile" <:> "v2"]
-          }
 
     context "OpenAPI output" $ do
       it "succeeds in reading table definition from default schema v1 if no schema is selected via header" $ do

test/spec/Feature/Query/PlanSpec.hs

@@ -192,7 +192,7 @@ spec actualPgVersion = do
       liftIO $ do
         resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.plan+json; for=\"application/json\"; charset=utf-8")
         resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
-        totalCost `shouldBe` 1.29
+        totalCost `shouldBe` 3.55
 
     it "outputs the total cost for 2 upserts" $ do
       r <- request methodPost "/tiobe_pls"
@@ -206,7 +206,7 @@ spec actualPgVersion = do
       liftIO $ do
         resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.plan+json; for=\"application/json\"; charset=utf-8")
         resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
-        totalCost `shouldBe` 3.27
+        totalCost `shouldBe` 5.53
 
     it "outputs the total cost for an upsert with 10 rows" $ do
       r <- request methodPost "/tiobe_pls"
@@ -220,7 +220,7 @@ spec actualPgVersion = do
       liftIO $ do
         resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.plan+json; for=\"application/json\"; charset=utf-8")
         resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
-        totalCost `shouldBe` 3.27
+        totalCost `shouldBe` 5.53
 
     it "outputs the total cost for an upsert with 100 rows" $ do
       r <- request methodPost "/tiobe_pls"
@@ -234,7 +234,7 @@ spec actualPgVersion = do
       liftIO $ do
         resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.plan+json; for=\"application/json\"; charset=utf-8")
         resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
-        totalCost `shouldBe` 3.27
+        totalCost `shouldBe` 5.53
 
     it "outputs the total cost for an upsert with 1000 rows" $ do
       r <- request methodPost "/tiobe_pls"
@@ -248,7 +248,7 @@ spec actualPgVersion = do
       liftIO $ do
         resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.plan+json; for=\"application/json\"; charset=utf-8")
         resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
-        totalCost `shouldBe` 3.27
+        totalCost `shouldBe` 5.53
 
     it "outputs the plan for application/vnd.pgrst.object" $ do
       r <- request methodDelete "/projects?id=eq.6"

test/spec/Feature/Query/ServerTimingSpec.hs

@@ -45,11 +45,11 @@ spec =
             }
 
       it "works with put request" $
-        request methodPut "/tiobe_pls?name=eq.Go"
+        request methodPut "/tiobe_pls?name=eq.Python"
             [("Prefer", "return=representation")]
-            [json| [ { "name": "Go", "rank": 19 } ]|]
+            [json| [ { "name": "Python", "rank": 19 } ]|]
           `shouldRespondWith`
-            [json| [ { "name": "Go", "rank": 19 } ]|]
+            [json| [ { "name": "Python", "rank": 19 } ]|]
             { matchStatus  = 200
             , matchHeaders = map matchServerTimingHasTiming ["jwt", "plan", "query", "render"]
             }

test/spec/Feature/Query/UpsertSpec.hs

@@ -32,6 +32,21 @@ spec actualPgVersion =
             , matchHeaders = ["Preference-Applied" <:> "resolution=merge-duplicates, return=representation", matchContentTypeJson]
             }
 
+        it "UPDATEs rows on pk conflict" $
+          request methodPost "/tiobe_pls" [("Prefer", "return=representation"), ("Prefer", "resolution=merge-duplicates")]
+            [json| [
+              { "name": "Python", "rank": 6 },
+              { "name": "Java", "rank": 2 },
+              { "name": "C", "rank": 1 }
+            ]|] `shouldRespondWith` [json| [
+              { "name": "Python", "rank": 6 },
+              { "name": "Java", "rank": 2 },
+              { "name": "C", "rank": 1 }
+            ]|]
+            { matchStatus = 200
+            , matchHeaders = ["Preference-Applied" <:> "resolution=merge-duplicates, return=representation", matchContentTypeJson]
+            }
+
         it "INSERTs and UPDATEs row on composite pk conflict" $
           request methodPost "/employees" [("Prefer", "return=representation"), ("Prefer", "resolution=merge-duplicates")]
             [json| [
@@ -62,7 +77,7 @@ spec actualPgVersion =
         it "succeeds when the payload has no elements" $
           request methodPost "/articles" [("Prefer", "return=representation"), ("Prefer", "resolution=merge-duplicates")]
             [json|[]|] `shouldRespondWith`
-            [json|[]|] { matchStatus = 201
+            [json|[]|] { matchStatus = 200 -- nothing was inserted, so it should be 200
                        , matchHeaders = [matchContentTypeJson] }
 
         it "INSERTs and UPDATEs rows on single unique key conflict" $
@@ -282,6 +297,7 @@ spec actualPgVersion =
               [json| [ { "name": "Go", "rank": 19 } ]|]
             `shouldRespondWith`
               [json| [ { "name": "Go", "rank": 19 } ]|]
+              { matchStatus = 201 }
 
         it "succeeds on table with composite pk" $ do
           -- assert that the next request will indeed be an insert
@@ -294,6 +310,7 @@ spec actualPgVersion =
               [json| [ { "first_name": "Susan", "last_name": "Heidt", "salary": "48000", "company": "GEX", "occupation": "Railroad engineer" } ]|]
             `shouldRespondWith`
               [json| [ { "first_name": "Susan", "last_name": "Heidt", "salary": "$48,000.00", "company": "GEX", "occupation": "Railroad engineer" } ]|]
+              { matchStatus = 201 }
 
         when (actualPgVersion >= pgVersion110) $
           it "succeeds on a partitioned table with composite pk" $ do
@@ -307,6 +324,7 @@ spec actualPgVersion =
                 [json| [ { "name": "Supra", "year": 2021 } ]|]
               `shouldRespondWith`
                 [json| [ { "name": "Supra", "year": 2021, "car_brand_name": null } ]|]
+              { matchStatus = 201 }
 
         it "succeeds if the table has only PK cols and no other cols" $ do
           -- assert that the next request will indeed be an insert
@@ -319,6 +337,7 @@ spec actualPgVersion =
               [json|[ { "id": 10 } ]|]
             `shouldRespondWith`
               [json|[ { "id": 10 } ]|]
+              { matchStatus = 201 }
 
       context "Updating row" $ do
         it "succeeds on table with single pk col" $ do
@@ -401,7 +420,11 @@ spec actualPgVersion =
         request methodPut "/tiobe_pls?name=eq.Ruby"
           [("Prefer", "return=representation"), ("Accept", "application/vnd.pgrst.object+json")]
           [json| [ { "name": "Ruby", "rank": 11 } ]|]
-          `shouldRespondWith` [json|{ "name": "Ruby", "rank": 11 }|] { matchHeaders = [matchContentTypeSingular] }
+          `shouldRespondWith`
+          [json|{ "name": "Ruby", "rank": 11 }|]
+          { matchStatus  = 201
+          , matchHeaders = [matchContentTypeSingular] }
+
 
     context "with a camel case pk column" $ do
       it "works with POST and merge-duplicates" $ do

test/spec/Feature/RollbackSpec.hs

@@ -122,7 +122,8 @@ shouldPersistMutations reqHeaders respHeaders = do
       [json|{"id":0}|]
       `shouldRespondWith`
       [json|[{"id":0}]|]
-        { matchHeaders = respHeaders }
+      { matchStatus  = 201
+      , matchHeaders = respHeaders }
     get "/items?id=eq.0"
       `shouldRespondWith`
         [json|[{"id":0}]|]
@@ -175,7 +176,8 @@ shouldNotPersistMutations reqHeaders respHeaders = do
         [json|{"id":0}|]
       `shouldRespondWith`
         [json|[{"id":0}]|]
-        { matchHeaders = respHeaders }
+        { matchStatus  = 201
+        , matchHeaders = respHeaders }
     get "/items?id=eq.0"
       `shouldRespondWith`
         [json|[]|]