diff --git a/CHANGELOG.md b/CHANGELOG.md index e7820359fc1..24ccfe1fa8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #2771, Add `Server-Timing` header with JWT duration - @taimoorzaeem - #2698, Add config `jwt-cache-max-lifetime` and implement JWT caching - @taimoorzaeem - #2943, Add `handling=strict/lenient` for Prefer header - @taimoorzaeem + +### Fixed + - #2824, Fix range request with 0 rows and 0 offset return status 416 - @strengthless + - #1070, Fix PUT responding with 200 when rows are inserted - @taimoorzaeem ## [11.2.1] - 2023-10-03 diff --git a/src/PostgREST/Query.hs b/src/PostgREST/Query.hs index 79bbe5e6c81..c9334beb04b 100644 --- a/src/PostgREST/Query.hs +++ b/src/PostgREST/Query.hs @@ -188,13 +188,14 @@ openApiQuery sCache pgVer AppConfig{..} tSchema = writeQuery :: MutateReadPlan -> ApiRequest -> AppConfig -> DbHandler ResultSet writeQuery MutateReadPlan{mrReadPlan, mrMutatePlan, mrResAgg, mrMedia} 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 (QueryBuilder.readPlanToQuery mrReadPlan) (QueryBuilder.mutatePlanToQuery mrMutatePlan) isInsert + isPut mrMedia mrResAgg preferRepresentation diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index 06e4ad1f6b5..48f6c3555e2 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -87,7 +87,7 @@ 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 " <> coalesceAddPgrstInsertedF <> " AND " <> intercalateSnippet " AND " (pgFmtLogicTree (QualifiedIdentifier mempty "pgrst_body") <$> putConditions)) <> maybe mempty (\(oncDo, oncCols) -> if null oncCols then mempty @@ -98,9 +98,9 @@ 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 then mempty else "WHERE " <> coalesceSubPgrstInsertedF) ) onConflct <> " " <> - returningF mainQi returnings + returningF mainQi returnings where cols = intercalateSnippet ", " $ pgFmtIdent . cfName <$> iCols diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index f2d21efce61..b7ea05f5f48 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -24,6 +24,9 @@ module PostgREST.Query.SqlFragment , fromJsonBodyF , responseHeadersF , responseStatusF + , coalesceAddPgrstInsertedF + , coalesceSubPgrstInsertedF + , currentSettingF , returningF , singleParameter , sourceCTE @@ -439,6 +442,12 @@ responseHeadersF = currentSettingF "response.headers" responseStatusF :: SQL.Snippet responseStatusF = currentSettingF "response.status" +coalesceAddPgrstInsertedF :: SQL.Snippet +coalesceAddPgrstInsertedF = "set_config('pgrst.inserted', (coalesce(" <> currentSettingF "pgrst.inserted" <> "::int, 0) + 1)::text, true) <> '0'" + +coalesceSubPgrstInsertedF :: SQL.Snippet +coalesceSubPgrstInsertedF = "set_config('pgrst.inserted', (coalesce(" <> currentSettingF "pgrst.inserted" <> "::int, 0) - 1)::text, true) <> '-1'" + currentSettingF :: SQL.Snippet -> SQL.Snippet currentSettingF setting = -- nullif is used because of https://gist.github.com/steve-chavez/8d7033ea5655096903f3b52f8ed09a15 diff --git a/src/PostgREST/Query/Statements.hs b/src/PostgREST/Query/Statements.hs index 634568fb9c2..dfb50acb927 100644 --- a/src/PostgREST/Query/Statements.hs +++ b/src/PostgREST/Query/Statements.hs @@ -49,15 +49,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 with PUT) } | RSPlan BS.ByteString -- ^ the plan of the query -prepareWrite :: SQL.Snippet -> SQL.Snippet -> Bool -> MediaType -> ResultAggregate -> +prepareWrite :: SQL.Snippet -> SQL.Snippet -> Bool -> Bool -> MediaType -> ResultAggregate -> Maybe PreferRepresentation -> [Text] -> Bool -> SQL.Statement () ResultSet -prepareWrite selectQuery mutateQuery isInsert mt rAgg rep pKeys = +prepareWrite selectQuery mutateQuery isInsert isPut mt rAgg rep pKeys = SQL.dynamicallyParameterized (mtSnippet mt snippet) decodeIt where + checkUpsert snip = if isInsert && isPut then snip else "''" + pgrstInsertedF = checkUpsert "nullif(current_setting('pgrst.inserted', true),'')::int" snippet = "WITH " <> sourceCTE <> " AS (" <> mutateQuery <> ") " <> "SELECT " <> @@ -66,7 +70,8 @@ prepareWrite selectQuery mutateQuery isInsert mt rAgg rep pKeys = locF <> " AS header, " <> aggF Nothing rAgg <> " AS body, " <> responseHeadersF <> " AS response_headers, " <> - responseStatusF <> " AS response_status " <> + responseStatusF <> " AS response_status, " <> + pgrstInsertedF <> " AS response_inserted " <> "FROM (" <> selectF <> ") _postgrest_t" locF = @@ -86,7 +91,7 @@ prepareWrite selectQuery mutateQuery isInsert mt rAgg rep pKeys = decodeIt :: HD.Result ResultSet decodeIt = case mt of MTPlan{} -> 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 :: SQL.Snippet -> SQL.Snippet -> Bool -> MediaType -> ResultAggregate -> Bool -> SQL.Statement () ResultSet prepareRead selectQuery countQuery countTotal mt rAgg = @@ -100,7 +105,8 @@ prepareRead selectQuery countQuery countTotal mt rAgg = "pg_catalog.count(_postgrest_t) AS page_total, " <> aggF Nothing rAgg <> " 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 @@ -124,7 +130,8 @@ prepareCall rout callProcQuery selectQuery countQuery countTotal mt rAgg = "pg_catalog.count(_postgrest_t) AS page_total, " <> aggF (Just rout) rAgg <> " 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 @@ -132,7 +139,7 @@ prepareCall rout callProcQuery selectQuery countQuery countTotal mt rAgg = decodeIt :: HD.Result ResultSet decodeIt = case mt of MTPlan{} -> 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 = @@ -150,6 +157,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 = diff --git a/src/PostgREST/Response.hs b/src/PostgREST/Response.hs index 2134f566086..343cb7474c5 100644 --- a/src/PostgREST/Response.hs +++ b/src/PostgREST/Response.hs @@ -23,6 +23,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 @@ -153,11 +154,11 @@ updateResponse MutateReadPlan{mrMedia} ctxApiRequest@ApiRequest{iPreferences=Pre prefHeader = prefAppliedHeader $ Preferences Nothing preferRepresentation Nothing preferCount preferTransaction preferMissing preferHandling [] headers = catMaybes [contentRangeHeader, prefHeader] ++ serverTimingHeader serverTimingParams - let - (status, headers', body) = case preferRepresentation of - Just Full -> (HTTP.status200, headers ++ contentTypeHeaders mrMedia ctxApiRequest, LBS.fromStrict rsBody) - Just None -> (HTTP.status204, headers, mempty) - _ -> (HTTP.status204, headers, mempty) + let (status, headers', body) = + case preferRepresentation of + Just Full -> (HTTP.status200, headers ++ contentTypeHeaders mrMedia ctxApiRequest, LBS.fromStrict rsBody) + Just None -> (HTTP.status204, headers, mempty) + _ -> (HTTP.status204, headers, mempty) (ovStatus, ovHeaders) <- overrideStatusHeaders rsGucStatus rsGucHeaders status headers' @@ -174,9 +175,11 @@ singleUpsertResponse MutateReadPlan{mrMedia} ctxApiRequest@ApiRequest{iPreferenc sTHeader = serverTimingHeader serverTimingParams 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 ++ sTHeader ++ prefHeader, LBS.fromStrict rsBody) + Just Full -> (upsertStatus, cTHeader ++ sTHeader ++ prefHeader, LBS.fromStrict rsBody) Just None -> (HTTP.status204, sTHeader ++ prefHeader, mempty) _ -> (HTTP.status204, sTHeader ++ prefHeader, mempty) (ovStatus, ovHeaders) <- overrideStatusHeaders rsGucStatus rsGucHeaders status headers diff --git a/test/spec/Feature/Query/MultipleSchemaSpec.hs b/test/spec/Feature/Query/MultipleSchemaSpec.hs index 0f726a27ee4..deaa5954574 100644 --- a/test/spec/Feature/Query/MultipleSchemaSpec.hs +++ b/test/spec/Feature/Query/MultipleSchemaSpec.hs @@ -225,13 +225,11 @@ spec = it "succeeds on PUT on the v2 schema" $ request methodPut "/children?id=eq.111" [("Content-Profile", "v2"), ("Prefer", "return=representation")] - [json| [ { "id": 111, "name": "child v2-111", "parent_id": null } ]|] + [json|[{"id": 111, "name": "child v2-111", "parent_id": null}]|] `shouldRespondWith` - [json|[{ "id": 111, "name": "child v2-111", "parent_id": null }]|] - { - matchStatus = 200 - , matchHeaders = [matchContentTypeJson, "Content-Profile" <:> "v2"] - } + [json|[{"id": 111, "name": "child v2-111", "parent_id": null}]|] + { matchStatus = 201 + , 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 diff --git a/test/spec/Feature/Query/PlanSpec.hs b/test/spec/Feature/Query/PlanSpec.hs index 41083621320..221dc3382e5 100644 --- a/test/spec/Feature/Query/PlanSpec.hs +++ b/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 plan for application/vnd.pgrst.object" $ do r <- request methodDelete "/projects?id=eq.6" diff --git a/test/spec/Feature/Query/ServerTimingSpec.hs b/test/spec/Feature/Query/ServerTimingSpec.hs index 4ab8954a304..126d8a614d7 100644 --- a/test/spec/Feature/Query/ServerTimingSpec.hs +++ b/test/spec/Feature/Query/ServerTimingSpec.hs @@ -48,11 +48,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 = [ matchHeaderPresent "Server-Timing" ] } diff --git a/test/spec/Feature/Query/UpsertSpec.hs b/test/spec/Feature/Query/UpsertSpec.hs index b317db14371..41a70fb182f 100644 --- a/test/spec/Feature/Query/UpsertSpec.hs +++ b/test/spec/Feature/Query/UpsertSpec.hs @@ -282,6 +282,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 +295,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 +309,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 +322,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 +405,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 diff --git a/test/spec/Feature/RollbackSpec.hs b/test/spec/Feature/RollbackSpec.hs index 40620460691..d86176575fe 100644 --- a/test/spec/Feature/RollbackSpec.hs +++ b/test/spec/Feature/RollbackSpec.hs @@ -118,11 +118,12 @@ shouldPersistMutations reqHeaders respHeaders = do it "does persist put" $ do request methodPut "/items?id=eq.0" - reqHeaders - [json|{"id":0}|] + reqHeaders + [json|{"id":0}|] `shouldRespondWith` - [json|[{"id":0}]|] - { matchHeaders = respHeaders } + [json|[{"id":0}]|] + { 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|[]|]