Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support filtering top-level resource based on embedded resource filter #1949

Merged
merged 2 commits into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
+ Enables uploading bytea to a function with `Content-Type: application/octet-stream`
+ Enables uploading raw text to a function with `Content-Type: text/plain`
- #1938, Allow escaping inside double quotes with a backslash, e.g. `?col=in.("Double\"Quote")`, `?col=in.("Back\\slash")` - @steve-chavez
- #1075, Allow filtering top-level resource based on embedded resources filters - @steve-chavez, @Iced-Sun
+ This is enabled by adding `!inner` to the embedded resource, e.g. `/projects?select=*,clients!inner(*)&clients.id=eq.12`
+ This behavior can be enabled by default with the `db-embed-default-join='inner'` config option, which saves the need for specifying `!inner` on every request. In this case, you can go back to the previous behavior per request by specifying `!left` on the embedded resource, e.g `/projects?select=*,clients!left(*)&clients.id=eq.12`

### Fixed

Expand Down Expand Up @@ -579,4 +582,4 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Fixed
- Make filter position match docs, e.g. `?order=col.asc` rather
than `?order=asc.col`.
than `?order=asc.col`.
wolfgangwalther marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions postgrest.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ test-suite spec
Feature.DeleteSpec
Feature.DisabledOpenApiSpec
Feature.EmbedDisambiguationSpec
Feature.EmbedInnerJoinSpec
Feature.ExtraSearchPathSpec
Feature.HtmlRawOutputSpec
Feature.InsertSpec
Expand Down
2 changes: 1 addition & 1 deletion src/PostgREST/App.hs
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ returnsScalar _ = False
readRequest :: Monad m => QualifiedIdentifier -> RequestContext -> Handler m ReadRequest
readRequest QualifiedIdentifier{..} (RequestContext AppConfig{..} dbStructure apiRequest _) =
liftEither $
ReqBuilder.readRequest qiSchema qiName configDbMaxRows
ReqBuilder.readRequest qiSchema qiName configDbMaxRows configDbEmbedDefaultJoin
(dbRelationships dbStructure)
apiRequest

Expand Down
12 changes: 12 additions & 0 deletions src/PostgREST/Config.hs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import PostgREST.Config.JSPath (JSPath, JSPathExp (..),
import PostgREST.Config.Proxy (Proxy (..),
isMalformedProxyUri, toURI)
import PostgREST.DbStructure.Identifiers (QualifiedIdentifier, toQi)
import PostgREST.Request.Types (JoinType (..))

import Protolude hiding (Proxy, toList, toS)
import Protolude.Conv (toS)
Expand All @@ -79,6 +80,7 @@ data AppConfig = AppConfig
, configDbTxAllowOverride :: Bool
, configDbTxRollbackAll :: Bool
, configDbUri :: Text
, configDbEmbedDefaultJoin :: JoinType
, configFilePath :: Maybe FilePath
, configJWKS :: Maybe JWKSet
, configJwtAudience :: Maybe StringOrURI
Expand Down Expand Up @@ -132,6 +134,7 @@ toText conf =
,("db-config", q . T.toLower . show . configDbConfig)
,("db-tx-end", q . showTxEnd)
,("db-uri", q . configDbUri)
,("db-embed-default-join", q . show . configDbEmbedDefaultJoin)
,("jwt-aud", toS . encode . maybe "" toJSON . configJwtAudience)
,("jwt-role-claim-key", q . T.intercalate mempty . fmap show . configJwtRoleClaimKey)
,("jwt-secret", q . toS . showJwtSecret)
Expand Down Expand Up @@ -222,6 +225,7 @@ parser optPath env dbSettings =
<*> parseTxEnd "db-tx-end" snd
<*> parseTxEnd "db-tx-end" fst
<*> reqString "db-uri"
<*> parseEmbedDefaultJoin "db-embed-default-join"
<*> pure optPath
<*> pure Nothing
<*> parseJwtAudience "jwt-aud"
Expand Down Expand Up @@ -304,6 +308,14 @@ parser optPath env dbSettings =
Just "rollback-allow-override" -> pure $ f (True, True)
Just _ -> fail "Invalid transaction termination. Check your configuration."

parseEmbedDefaultJoin :: C.Key -> C.Parser C.Config JoinType
parseEmbedDefaultJoin k =
optString k >>= \case
Nothing -> pure JTLeft
Just "left" -> pure JTLeft
Just "inner" -> pure JTInner
Just _ -> fail "Invalid db-embed-default-join. Check your configuration."

parseRoleClaimKey :: C.Key -> C.Key -> C.Parser C.Config JSPath
parseRoleClaimKey k al =
optWithAlias (optString k) (optString al) >>= \case
Expand Down
27 changes: 18 additions & 9 deletions src/PostgREST/Query/QueryBuilder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,31 @@ readRequestToQuery (Node (Select colSelects mainQi tblAlias implJoins logicFores
(joins, selects) = foldr getJoinsSelects ([],[]) forest

getJoinsSelects :: ReadRequest -> ([H.Snippet], [H.Snippet]) -> ([H.Snippet], [H.Snippet])
getJoinsSelects rr@(Node (_, (name, Just Relationship{relCardinality=card,relTable=Table{tableName=table}}, alias, _, _)) _) (j,s) =
getJoinsSelects rr@(Node (_, (name, Just Relationship{relCardinality=card,relTable=Table{tableName=table}}, alias, _, Just joinType, _)) _) (j,s) =
let subquery = readRequestToQuery rr in
case card of
M2O _ ->
let aliasOrName = fromMaybe name alias
localTableName = pgFmtIdent $ table <> "_" <> aliasOrName
sel = H.sql ("row_to_json(" <> localTableName <> ".*) AS " <> pgFmtIdent aliasOrName)
joi = " LEFT JOIN LATERAL( " <> subquery <> " ) AS " <> H.sql localTableName <> " ON TRUE " in
joi = (if joinType == JTInner then " INNER" else " LEFT")
<> " JOIN LATERAL( " <> subquery <> " ) AS " <> H.sql localTableName <> " ON TRUE " in
(joi:j,sel:s)
_ ->
let sel = "COALESCE (("
<> "SELECT json_agg(" <> H.sql (pgFmtIdent table) <> ".*) "
<> "FROM (" <> subquery <> ") " <> H.sql (pgFmtIdent table) <> " "
<> "), '[]') AS " <> H.sql (pgFmtIdent (fromMaybe name alias)) in
(j,sel:s)
getJoinsSelects (Node (_, (_, Nothing, _, _, _)) _) _ = ([], [])
_ -> case joinType of
JTInner ->
let aliasOrName = fromMaybe name alias
localTableName = pgFmtIdent $ table <> "_" <> aliasOrName
sel = H.sql $ localTableName <> "._ AS " <> pgFmtIdent aliasOrName
joi = "INNER JOIN LATERAL( SELECT json_agg(_) AS _ FROM (" <> subquery <> " ) _) AS " <>
H.sql localTableName <> " ON " <> H.sql localTableName <> "IS NOT NULL" in
(joi:j,sel:s)
JTLeft ->
let sel = "COALESCE (("
<> "SELECT json_agg(" <> H.sql (pgFmtIdent table) <> ".*) "
<> "FROM (" <> subquery <> ") " <> H.sql (pgFmtIdent table) <> " "
<> "), '[]') AS " <> H.sql (pgFmtIdent (fromMaybe name alias)) in
(j,sel:s)
getJoinsSelects _ _ = ([], [])

mutateRequestToQuery :: MutateRequest -> H.Snippet
mutateRequestToQuery (Insert mainQi iCols body onConflct putConditions returnings) =
Expand Down
4 changes: 2 additions & 2 deletions src/PostgREST/Query/SqlFragment.hs
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,11 @@ pgFmtField :: QualifiedIdentifier -> Field -> H.Snippet
pgFmtField table (c, jp) = H.sql (pgFmtColumn table c) <> pgFmtJsonPath jp

pgFmtSelectItem :: QualifiedIdentifier -> SelectItem -> H.Snippet
pgFmtSelectItem table (f@(fName, jp), Nothing, alias, _) = pgFmtField table f <> H.sql (pgFmtAs fName jp alias)
pgFmtSelectItem table (f@(fName, jp), Nothing, alias, _, _) = pgFmtField table f <> H.sql (pgFmtAs fName jp alias)
-- Ideally we'd quote the cast with "pgFmtIdent cast". However, that would invalidate common casts such as "int", "bigint", etc.
-- Try doing: `select 1::"bigint"` - it'll err, using "int8" will work though. There's some parser magic that pg does that's invalidated when quoting.
-- Not quoting should be fine, we validate the input on Parsers.
pgFmtSelectItem table (f@(fName, jp), Just cast, alias, _) = "CAST (" <> pgFmtField table f <> " AS " <> H.sql (encodeUtf8 cast) <> " )" <> H.sql (pgFmtAs fName jp alias)
pgFmtSelectItem table (f@(fName, jp), Just cast, alias, _, _) = "CAST (" <> pgFmtField table f <> " AS " <> H.sql (encodeUtf8 cast) <> " )" <> H.sql (pgFmtAs fName jp alias)

pgFmtOrderTerm :: QualifiedIdentifier -> OrderTerm -> H.Snippet
pgFmtOrderTerm qi ot =
Expand Down
36 changes: 18 additions & 18 deletions src/PostgREST/Request/DbRequestBuilder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ import Protolude hiding (from)
-- | Builds the ReadRequest tree on a number of stages.
-- | Adds filters, order, limits on its respective nodes.
-- | Adds joins conditions obtained from resource embedding.
readRequest :: Schema -> TableName -> Maybe Integer -> [Relationship] -> ApiRequest -> Either Error ReadRequest
readRequest schema rootTableName maxRows allRels apiRequest =
readRequest :: Schema -> TableName -> Maybe Integer -> JoinType -> [Relationship] -> ApiRequest -> Either Error ReadRequest
readRequest schema rootTableName maxRows defJoinType allRels apiRequest =
mapLeft ApiRequestError $
treeRestrictRange maxRows =<<
augmentRequestWithJoin schema rootRels =<<
augmentRequestWithJoin schema rootRels defJoinType =<<
(addFiltersOrdersRanges apiRequest . initReadRequest rootName =<< pRequestSelect sel)
where
sel = fromMaybe "*" $ iSelect apiRequest -- default to all columns requested (SELECT *) for a non existent ?select querystring param
Expand Down Expand Up @@ -100,16 +100,16 @@ initReadRequest rootQi =
rootDepth = 0
rootSchema = qiSchema rootQi
rootName = qiName rootQi
initial = Node (Select [] rootQi Nothing [] [] [] [] allRange, (rootName, Nothing, Nothing, Nothing, rootDepth)) []
initial = Node (Select [] rootQi Nothing [] [] [] [] allRange, (rootName, Nothing, Nothing, Nothing, Nothing, rootDepth)) []
treeEntry :: Depth -> Tree SelectItem -> ReadRequest -> ReadRequest
treeEntry depth (Node fld@((fn, _),_,alias, embedHint) fldForest) (Node (q, i) rForest) =
treeEntry depth (Node fld@((fn, _),_,alias, hint, joinType) fldForest) (Node (q, i) rForest) =
let nxtDepth = succ depth in
case fldForest of
[] -> Node (q {select=fld:select q}, i) rForest
_ -> Node (q, i) $
foldr (treeEntry nxtDepth)
(Node (Select [] (QualifiedIdentifier rootSchema fn) Nothing [] [] [] [] allRange,
(fn, Nothing, alias, embedHint, nxtDepth)) [])
(fn, Nothing, alias, hint, joinType, nxtDepth)) [])
fldForest:rForest

-- | Enforces the `max-rows` config on the result
Expand All @@ -119,26 +119,26 @@ treeRestrictRange maxRows request = pure $ nodeRestrictRange maxRows <$> request
nodeRestrictRange :: Maybe Integer -> ReadNode -> ReadNode
nodeRestrictRange m (q@Select {range_=r}, i) = (q{range_=restrictRange m r }, i)

augmentRequestWithJoin :: Schema -> [Relationship] -> ReadRequest -> Either ApiRequestError ReadRequest
augmentRequestWithJoin schema allRels request =
addRels schema allRels Nothing request
augmentRequestWithJoin :: Schema -> [Relationship] -> JoinType -> ReadRequest -> Either ApiRequestError ReadRequest
augmentRequestWithJoin schema allRels defJoinType request =
addRels schema allRels Nothing defJoinType request
>>= addJoinConditions Nothing

addRels :: Schema -> [Relationship] -> Maybe ReadRequest -> ReadRequest -> Either ApiRequestError ReadRequest
addRels schema allRels parentNode (Node (query@Select{from=tbl}, (nodeName, _, alias, hint, depth)) forest) =
addRels :: Schema -> [Relationship] -> Maybe ReadRequest -> JoinType -> ReadRequest -> Either ApiRequestError ReadRequest
addRels schema allRels parentNode defJoinType (Node (query@Select{from=tbl}, (nodeName, _, alias, hint, joinType, depth)) forest) =
case parentNode of
Just (Node (Select{from=parentNodeQi}, _) _) ->
let newFrom r = if qiName tbl == nodeName then tableQi (relForeignTable r) else tbl
newReadNode = (\r -> (query{from=newFrom r}, (nodeName, Just r, alias, Nothing, depth))) <$> rel
newReadNode = (\r -> (query{from=newFrom r}, (nodeName, Just r, alias, hint, joinType <|> Just defJoinType, depth))) <$> rel
rel = findRel schema allRels (qiName parentNodeQi) nodeName hint
in
Node <$> newReadNode <*> (updateForest . hush $ Node <$> newReadNode <*> pure forest)
_ ->
let rn = (query, (nodeName, Nothing, alias, Nothing, depth)) in
let rn = (query, (nodeName, Nothing, alias, Nothing, joinType, depth)) in
Node rn <$> updateForest (Just $ Node rn forest)
where
updateForest :: Maybe ReadRequest -> Either ApiRequestError [ReadRequest]
updateForest rq = addRels schema allRels rq `traverse` forest
updateForest rq = addRels schema allRels rq defJoinType `traverse` forest

-- Finds a relationship between an origin and a target in the request:
-- /origin?select=target(*) If more than one relationship is found then the
Expand All @@ -150,7 +150,7 @@ addRels schema allRels parentNode (Node (query@Select{from=tbl}, (nodeName, _, a
-- target = table / view / constraint / column-from-origin
-- hint = table / view / constraint / column-from-origin / column-from-target
-- (hint can take table / view values to aid in finding the junction in an m2m relationship)
findRel :: Schema -> [Relationship] -> NodeName -> NodeName -> Maybe EmbedHint -> Either ApiRequestError Relationship
findRel :: Schema -> [Relationship] -> NodeName -> NodeName -> Maybe Hint -> Either ApiRequestError Relationship
findRel schema allRels origin target hint =
case rel of
[] -> Left $ NoRelBetween origin target
Expand Down Expand Up @@ -210,7 +210,7 @@ findRel schema allRels origin target hint =

-- previousAlias is only used for the case of self joins
addJoinConditions :: Maybe Alias -> ReadRequest -> Either ApiRequestError ReadRequest
addJoinConditions previousAlias (Node node@(query@Select{from=tbl}, nodeProps@(_, rel, _, _, depth)) forest) =
addJoinConditions previousAlias (Node node@(query@Select{from=tbl}, nodeProps@(_, rel, _, _, _, depth)) forest) =
case rel of
Just r@Relationship{relCardinality=M2M Junction{junTable}} ->
let rq = augmentQuery r in
Expand Down Expand Up @@ -307,7 +307,7 @@ addProperty f (targetNodeName:remainingPath, a) (Node rn forest) =
Nothing -> Node rn forest -- the property is silenty dropped in the Request does not contain the required path
Just tn -> Node rn (addProperty f (remainingPath, a) tn:delete tn forest)
where
pathNode = find (\(Node (_,(nodeName,_,alias,_,_)) _) -> nodeName == targetNodeName || alias == Just targetNodeName) forest
pathNode = find (\(Node (_,(nodeName,_,alias,_,_, _)) _) -> nodeName == targetNodeName || alias == Just targetNodeName) forest

mutateRequest :: Schema -> TableName -> ApiRequest -> [FieldName] -> ReadRequest -> Either Error MutateRequest
mutateRequest schema tName apiRequest pkCols readReq = mapLeft ApiRequestError $
Expand Down Expand Up @@ -379,7 +379,7 @@ returningCols rr@(Node _ forest) pkCols
-- projects. So this adds the foreign key columns to ensure the embedding
-- succeeds, result would be `RETURNING name, client_id`.
fkCols = concat $ mapMaybe (\case
Node (_, (_, Just Relationship{relColumns=cols}, _, _, _)) _ -> Just cols
Node (_, (_, Just Relationship{relColumns=cols}, _, _, _, _)) _ -> Just cols
_ -> Nothing
) forest
-- However if the "client_id" is present, e.g. mutateRequest to
Expand Down
26 changes: 18 additions & 8 deletions src/PostgREST/Request/Parsers.hs
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,23 @@ pRelationSelect :: Parser SelectItem
pRelationSelect = lexeme $ try ( do
alias <- optionMaybe ( try(pFieldName <* aliasSeparator) )
fld <- pField
hint <- optionMaybe (
try ( char '!' *> pFieldName) <|>
-- deprecated, remove in next major version
try ( char '.' *> pFieldName)
)
return (fld, Nothing, alias, hint)
prm1 <- optionMaybe pEmbedParam
prm2 <- optionMaybe pEmbedParam
return (fld, Nothing, alias, embedParamHint prm1 <|> embedParamHint prm2, embedParamJoin prm1 <|> embedParamJoin prm2)
)
where
pEmbedParam :: Parser EmbedParam
pEmbedParam =
char '!' *> (
try (string "left" $> EPJoinType JTLeft) <|>
try (string "inner" $> EPJoinType JTInner) <|>
try (EPHint <$> pFieldName))
embedParamHint prm = case prm of
Just (EPHint hint) -> Just hint
_ -> Nothing
embedParamJoin prm = case prm of
Just (EPJoinType jt) -> Just jt
_ -> Nothing

pFieldSelect :: Parser SelectItem
pFieldSelect = lexeme $
Expand All @@ -173,11 +183,11 @@ pFieldSelect = lexeme $
alias <- optionMaybe ( try(pFieldName <* aliasSeparator) )
fld <- pField
cast' <- optionMaybe (string "::" *> many letter)
return (fld, toS <$> cast', alias, Nothing)
return (fld, toS <$> cast', alias, Nothing, Nothing)
)
<|> do
s <- pStar
return ((s, []), Nothing, Nothing, Nothing)
return ((s, []), Nothing, Nothing, Nothing, Nothing)

pOpExpr :: Parser SingleVal -> Parser OpExpr
pOpExpr pSVal = try ( string "not" *> pDelimiter *> (OpExpr True <$> pOperation)) <|> OpExpr False <$> pOperation
Expand Down
31 changes: 22 additions & 9 deletions src/PostgREST/Request/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
module PostgREST.Request.Types
( Alias
, Depth
, EmbedHint
, EmbedParam(..)
, EmbedPath
, Field
, Filter(..)
, Hint
, CallQuery(..)
, CallParams(..)
, CallRequest
, JoinCondition(..)
, JoinType(..)
, JsonOperand(..)
, JsonOperation(..)
, JsonPath
Expand Down Expand Up @@ -54,7 +56,7 @@ type MutateRequest = MutateQuery
type CallRequest = CallQuery

type ReadNode =
(ReadQuery, (NodeName, Maybe Relationship, Maybe Alias, Maybe EmbedHint, Depth))
(ReadQuery, (NodeName, Maybe Relationship, Maybe Alias, Maybe Hint, Maybe JoinType, Depth))

type NodeName = Text
type Depth = Integer
Expand Down Expand Up @@ -140,16 +142,27 @@ data CallParams
| OnePosParam ProcParam -- ^ Call with positional params(only one supported): func(val)

-- | The select value in `/tbl?select=alias:field::cast`
type SelectItem = (Field, Maybe Cast, Maybe Alias, Maybe EmbedHint)
type SelectItem = (Field, Maybe Cast, Maybe Alias, Maybe Hint, Maybe JoinType)

type Field = (FieldName, JsonPath)
type Cast = Text
type Alias = Text

-- | Disambiguates an embedding operation when there's multiple relationships
-- between two tables. Can be the name of a foreign key constraint, column
-- name or the junction in an m2m relationship.
type EmbedHint = Text
type Hint = Text

data EmbedParam
-- | Disambiguates an embedding operation when there's multiple relationships
-- between two tables. Can be the name of a foreign key constraint, column
-- name or the junction in an m2m relationship.
= EPHint Hint
| EPJoinType JoinType

data JoinType
= JTInner
| JTLeft
deriving Eq
instance Show JoinType where
show JTInner = "inner"
show JTLeft = "left"

-- | Path of the embedded levels, e.g "clients.projects.name=eq.." gives Path
-- ["clients", "projects"]
Expand All @@ -176,7 +189,7 @@ data JsonOperand
-- First level FieldNames(e.g get a,b from /table?select=a,b,other(c,d))
fstFieldNames :: ReadRequest -> [FieldName]
fstFieldNames (Node (sel, _) _) =
fst . (\(f, _, _, _) -> f) <$> select sel
fst . (\(f, _, _, _, _) -> f) <$> select sel


-- | Boolean logic expression tree e.g. "and(name.eq.N,or(id.eq.1,id.eq.2))" is:
Expand Down
10 changes: 0 additions & 10 deletions test/Feature/EmbedDisambiguationSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -423,16 +423,6 @@ spec =
"refereeds":[]}]
}]|] { matchHeaders = [matchContentTypeJson] }

-- TODO Remove in next major version
describe "old dot '.' symbol, deprecated" $
it "still works" $ do
get "/clients?id=eq.1&select=id,projects:projects.client_id(id,tasks(id))" `shouldRespondWith`
[json|[{"id":1,"projects":[{"id":1,"tasks":[{"id":1},{"id":2}]},{"id":2,"tasks":[{"id":3},{"id":4}]}]}]|]
{ matchHeaders = [matchContentTypeJson] }
get "/tasks?select=id,users:users.users_tasks(id)" `shouldRespondWith`
[json|[{"id":1,"users":[{"id":1},{"id":3}]},{"id":2,"users":[{"id":1}]},{"id":3,"users":[{"id":1}]},{"id":4,"users":[{"id":1}]},{"id":5,"users":[{"id":2},{"id":3}]},{"id":6,"users":[{"id":2}]},{"id":7,"users":[{"id":2}]},{"id":8,"users":[]}]|]
{ matchHeaders = [matchContentTypeJson] }

context "m2m embed when there's a junction in an internal schema" $ do
-- https://github.com/PostgREST/postgrest/issues/1736
it "works with no ambiguity when there's an exposed view of the junction" $ do
Expand Down
Loading