jimmy/postgrest
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Location header improvements(#1475)

Browse Source 
* Create location header only on primary key columns, closes https://github.com/PostgREST/postgrest/issues/1461

* Return location header when pk columns not selected, fixes https://github.com/PostgREST/postgrest/issues/1162
This commit is contained in:
Wolfgang Walther
2020-07-08 14:36:40 -05:00
committed by GitHub
parent 55b4f4fbe7
commit 343e41c51d
5 changed files with 43 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+2
View File
@@ -14,10 +14,12 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- #1530, Fix how the PostgREST version is shown in the help text when the `.git` directory is not available - @monacoremo
- #1094, Fix expired JWTs starting an empty transaction on the db - @steve-chavez
- #1475, Fix location header for POST request with select= without PK (#1162) - @wolfgangwalther
### Changed
- #1522, #1528, #1535, Docker images are now built from scratch based on a the static PostgREST executable (#1494) and with Nix instead of a `Dockerfile`. This reduces the compressed image size from over 30mb to about 4mb - @monacoremo
- #1475, Location header for POST request is only included when PK is available on the table (#1461) - @wolfgangwalther
## [7.0.1] - 2020-05-18
src/PostgREST/App.hs
+1 -1
View File
@@ -361,7 +361,7 @@ app dbStructure proc cols conf apiRequest =
let
readReq = readRequest s t maxRows (dbRelations dbStructure) apiRequest
returnings :: ReadRequest -> Either Response [FieldName]
returnings rr = Right (returningCols rr)
returnings rr = Right (returningCols rr [])
in
(,,,) <$>
(readRequestToQuery <$> readReq) <*>
src/PostgREST/DbRequestBuilder.hs
+8 -4
View File
@@ -316,7 +316,7 @@ mutateRequest schema tName apiRequest cols pkCols readReq = mapLeft errorRespons
returnings =
if iPreferRepresentation apiRequest == None
then []
else returningCols readReq
else returningCols readReq pkCols
filters = map snd <$> mapM pRequestFilter mutateFilters
logic = map snd <$> mapM pRequestLogicTree logicFilters
combinedLogic = foldr addFilterToLogicForest <$> logic <*> filters
@@ -324,8 +324,11 @@ mutateRequest schema tName apiRequest cols pkCols readReq = mapLeft errorRespons
(mutateFilters, logicFilters) = join (***) onlyRoot (iFilters apiRequest, iLogic apiRequest)
onlyRoot = filter (not . ( "." `isInfixOf` ) . fst)
returningCols :: ReadRequest -> [FieldName]
returningCols rr@(Node _ forest) = returnings
returningCols :: ReadRequest -> [FieldName] -> [FieldName]
returningCols rr@(Node _ forest) pkCols
-- if * is part of the select, we must not add pk or fk columns manually - otherwise those would be selected and output twice
| "*" `elem` fldNames = ["*"]
| otherwise = returnings
where
fldNames = fstFieldNames rr
-- Without fkCols, when a mutateRequest to /projects?select=name,clients(name) occurs, the RETURNING SQL part would be
@@ -343,7 +346,8 @@ returningCols rr@(Node _ forest) = returnings
-- However if the "client_id" is present, e.g. mutateRequest to /projects?select=client_id,name,clients(name)
-- we would get `RETURNING client_id, name, client_id` and then we would produce the "column reference \"client_id\" is ambiguous"
-- error from PostgreSQL. So we deduplicate with Set:
returnings = S.toList . S.fromList $ fldNames ++ (colName <$> fkCols)
-- We are adding the primary key columns as well to make sure, that a proper location header can always be built for INSERT/POST
returnings = S.toList . S.fromList $ fldNames ++ (colName <$> fkCols) ++ pkCols
-- Traditional filters(e.g. id=eq.1) are added as root nodes of the LogicTree
-- they are later concatenated with AND in the QueryBuilder
src/PostgREST/Private/QueryFragment.hs
+4 -2
View File
@@ -87,10 +87,12 @@ asBinaryF fieldName = "coalesce(string_agg(_postgrest_t." <> pgFmtIdent fieldNam
locationF :: [Text] -> SqlFragment
locationF pKeys = [qc|(
WITH data AS (SELECT row_to_json(_) AS row FROM {sourceCTEName} AS _ LIMIT 1)
SELECT array_agg(json_data.key || '=' || coalesce('eq.' || json_data.value, 'is.null'))
SELECT array_agg(json_data.key || '=eq.' || json_data.value)
FROM data CROSS JOIN json_each_text(data.row) AS json_data
{("WHERE json_data.key IN ('" <> intercalate "','" pKeys <> "')") `emptyOnFalse` null pKeys}
WHERE json_data.key IN ('{fmtPKeys}')
)|]
where
fmtPKeys = intercalate "','" pKeys
fromQi :: QualifiedIdentifier -> SqlFragment
fromQi t = (if s == "" then "" else pgFmtIdent s <> ".") <> pgFmtIdent n
test/Feature/InsertSpec.hs
+28 -15
View File
@@ -104,6 +104,25 @@ spec actualPgVersion = do
, "Content-Range" <:> "*/*" ]
}
it "should not throw and return location header when selecting without PK" $
request methodPost "/projects?select=name,client_id" [("Prefer", "return=representation")]
[str|{"id":10,"name":"New Project","client_id":2}|] `shouldRespondWith`
[str|[{"name":"New Project","client_id":2}]|]
{ matchStatus = 201
, matchHeaders = [ matchContentTypeJson
, "Location" <:> "/projects?id=eq.10"
, "Content-Range" <:> "*/*" ]
}
context "requesting no representation" $
it "should not throw and return location header when selecting without PK" $
request methodPost "/projects?select=name,client_id" []
[str|{"id":11,"name":"New Project","client_id":2}|] `shouldRespondWith` ""
{ matchStatus = 201
, matchHeaders = [ "Location" <:> "/projects?id=eq.11"
, "Content-Range" <:> "*/*" ]
}
context "from an html form" $
it "accepts disparate json types" $ do
p <- request methodPost "/menagerie"
@@ -139,11 +158,11 @@ spec actualPgVersion = do
}
context "into a table with no pk" $ do
it "succeeds with 201 and a link including all fields" $ do
it "succeeds with 201 but no location header" $ do
p <- post "/no_pk" [json| { "a":"foo", "b":"bar" } |]
liftIO $ do
simpleBody p `shouldBe` ""
simpleHeaders p `shouldSatisfy` matchHeader hLocation "/no_pk\\?a=eq.foo&b=eq.bar"
lookup hLocation (simpleHeaders p) `shouldBe` Nothing
simpleStatus p `shouldBe` created201
it "returns full details of inserted record if asked" $ do
@@ -152,7 +171,7 @@ spec actualPgVersion = do
[json| { "a":"bar", "b":"baz" } |]
liftIO $ do
simpleBody p `shouldBe` [json| [{ "a":"bar", "b":"baz" }] |]
simpleHeaders p `shouldSatisfy` matchHeader hLocation "/no_pk\\?a=eq.bar&b=eq.baz"
lookup hLocation (simpleHeaders p) `shouldBe` Nothing
simpleStatus p `shouldBe` created201
it "returns empty array when no items inserted, and return=rep" $ do
@@ -169,7 +188,7 @@ spec actualPgVersion = do
[json| { "a":null, "b":"foo" } |]
liftIO $ do
simpleBody p `shouldBe` [json| [{ "a":null, "b":"foo" }] |]
simpleHeaders p `shouldSatisfy` matchHeader hLocation "/no_pk\\?a=is.null&b=eq.foo"
lookup hLocation (simpleHeaders p) `shouldBe` Nothing
simpleStatus p `shouldBe` created201
context "with compound pk supplied" $
@@ -238,24 +257,20 @@ spec actualPgVersion = do
context "jsonb" $ do
it "serializes nested object" $ do
let inserted = [json| { "data": { "foo":"bar" } } |]
location = "/json?data=eq.%7B%22foo%22%3A%22bar%22%7D"
request methodPost "/json"
[("Prefer", "return=representation")]
inserted
`shouldRespondWith` [str|[{"data":{"foo":"bar"}}]|]
{ matchStatus = 201
, matchHeaders = ["Location" <:> location]
}
it "serializes nested array" $ do
let inserted = [json| { "data": [1,2,3] } |]
location = "/json?data=eq.%5B1%2C2%2C3%5D"
request methodPost "/json"
[("Prefer", "return=representation")]
inserted
`shouldRespondWith` [str|[{"data":[1,2,3]}]|]
{ matchStatus = 201
, matchHeaders = ["Location" <:> location]
}
context "empty objects" $ do
@@ -358,8 +373,7 @@ spec actualPgVersion = do
"a,b\nbar,baz"
`shouldRespondWith` "a,b\nbar,baz"
{ matchStatus = 201
, matchHeaders = ["Content-Type" <:> "text/csv; charset=utf-8",
"Location" <:> "/no_pk?a=eq.bar&b=eq.baz"]
, matchHeaders = ["Content-Type" <:> "text/csv; charset=utf-8"]
}
it "can post nulls" $
@@ -368,8 +382,7 @@ spec actualPgVersion = do
"a,b\nNULL,foo"
`shouldRespondWith` "a,b\n,foo"
{ matchStatus = 201
, matchHeaders = ["Content-Type" <:> "text/csv; charset=utf-8",
"Location" <:> "/no_pk?a=is.null&b=eq.foo"]
, matchHeaders = ["Content-Type" <:> "text/csv; charset=utf-8"]
}
it "only returns the requested column header with its associated data" $
@@ -393,8 +406,8 @@ spec actualPgVersion = do
context "with unicode values" $
it "succeeds and returns usable location header" $ do
let payload = [json| { "a":"圍棋", "b":"￥" } |]
p <- request methodPost "/no_pk"
let payload = [json| { "k":"圍棋", "extra":"￥" } |]
p <- request methodPost "/simple_pk?select=extra,k"
[("Prefer", "return=representation")]
payload
liftIO $ do
@@ -402,7 +415,7 @@ spec actualPgVersion = do
simpleStatus p `shouldBe` created201
let Just location = lookup hLocation $ simpleHeaders p
r <- get location
r <- get (location <> "&select=extra,k")
liftIO $ simpleBody r `shouldBe` "["<>payload<>"]"
describe "Patching record" $ do