Skip to content

Commit

Permalink
refactor: remove Table from Relationship
Browse files Browse the repository at this point in the history
Just having the QualifiedIdentifier gets us closer to having
Relationship as a Table attribute since it avoids a cyclic dependency

* remove unnecessary findTable
* modify RootSpec test
  • Loading branch information
steve-chavez committed Apr 18, 2022
1 parent cdcc175 commit 7f1507b
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 65 deletions.
5 changes: 2 additions & 3 deletions src/PostgREST/App.hs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ import PostgREST.Config (AppConfig (..),
import PostgREST.Config.PgVersion (PgVersion (..))
import PostgREST.ContentType (ContentType (..))
import PostgREST.DbStructure (DbStructure (..),
findIfView, findTable,
tablePKCols)
findIfView, tablePKCols)
import PostgREST.DbStructure.Identifiers (FieldName,
QualifiedIdentifier (..),
Schema)
Expand Down Expand Up @@ -417,7 +416,7 @@ handleDelete identifier context@(RequestContext _ ctxDbStructure ApiRequest{..}

handleInfo :: Monad m => QualifiedIdentifier -> RequestContext -> Handler m Wai.Response
handleInfo identifier RequestContext{..} =
case findTable identifier $ dbTables ctxDbStructure of
case M.lookup identifier $ dbTables ctxDbStructure of
Just table ->
return $ Wai.responseLBS HTTP.status200 [allOrigins, allowH table] mempty
Nothing ->
Expand Down
45 changes: 20 additions & 25 deletions src/PostgREST/DbStructure.hs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ module PostgREST.DbStructure
, accessibleTables
, accessibleProcs
, findIfView
, findTable
, schemaDescription
, tableCols
, tablePKCols
Expand Down Expand Up @@ -57,7 +56,8 @@ import PostgREST.DbStructure.Relationship (Cardinality (..),
Junction (..),
PrimaryKey (..),
Relationship (..))
import PostgREST.DbStructure.Table (Column (..), Table (..), TablesMap)
import PostgREST.DbStructure.Table (Column (..), Table (..),
TablesMap)

import Protolude
import Protolude.Unsafe (unsafeHead)
Expand All @@ -80,11 +80,8 @@ tableCols dbs tSchema tName = filter (\Column{colTable=Table{tableSchema=s, tabl
tablePKCols :: DbStructure -> Schema -> TableName -> [Text]
tablePKCols dbs tSchema tName = pkName <$> filter (\pk -> tSchema == (tableSchema . pkTable) pk && tName == (tableName . pkTable) pk) (dbPrimaryKeys dbs)

findTable :: QualifiedIdentifier -> TablesMap -> Maybe Table
findTable identifier tbls = M.lookup identifier tbls

findIfView :: QualifiedIdentifier -> TablesMap -> Bool
findIfView identifier tbls = maybe False tableIsView $ findTable identifier tbls
findIfView identifier tbls = maybe False tableIsView $ M.lookup identifier tbls

-- | The source table column a view column refers to
type SourceColumn = (Column, ViewColumn)
Expand All @@ -100,7 +97,7 @@ queryDbStructure schemas extraSearchPath prepared = do
tabs <- SQL.statement mempty $ allTables pgVer prepared
cols <- SQL.statement schemas $ allColumns tabs prepared
srcCols <- SQL.statement (schemas, extraSearchPath) $ pfkSourceColumns cols prepared
m2oRels <- SQL.statement mempty $ allM2ORels tabs cols prepared
m2oRels <- SQL.statement mempty $ allM2ORels cols prepared
keys <- SQL.statement mempty $ allPrimaryKeys tabs prepared
procs <- SQL.statement schemas $ allProcs pgVer prepared

Expand All @@ -121,15 +118,15 @@ removeInternal schemas dbStruct =
DbStructure {
dbTables = M.filterWithKey (\(QualifiedIdentifier sch _) _ -> sch `elem` schemas) $ dbTables dbStruct
, dbColumns = filter (\x -> tableSchema (colTable x) `elem` schemas) (dbColumns dbStruct)
, dbRelationships = filter (\x -> tableSchema (relTable x) `elem` schemas &&
tableSchema (relForeignTable x) `elem` schemas &&
, dbRelationships = filter (\x -> qiSchema (relTable x) `elem` schemas &&
qiSchema (relForeignTable x) `elem` schemas &&
not (hasInternalJunction x)) $ dbRelationships dbStruct
, dbPrimaryKeys = filter (\x -> tableSchema (pkTable x) `elem` schemas) $ dbPrimaryKeys dbStruct
, dbProcs = dbProcs dbStruct -- procs are only obtained from the exposed schemas, no need to filter them.
}
where
hasInternalJunction rel = case relCardinality rel of
M2M Junction{junTable} -> tableSchema junTable `notElem` schemas
M2M Junction{junTable} -> qiSchema junTable `notElem` schemas
_ -> False

decodeTables :: HD.Result TablesMap
Expand Down Expand Up @@ -160,9 +157,9 @@ decodeColumns tables =
<*> nullableColumn HD.text
<*> nullableColumn HD.text

decodeRels :: TablesMap -> [Column] -> HD.Result [Relationship]
decodeRels tables cols =
mapMaybe (relFromRow tables cols) <$> HD.rowList relRow
decodeRels :: [Column] -> HD.Result [Relationship]
decodeRels cols =
mapMaybe (relFromRow cols) <$> HD.rowList relRow
where
relRow = (,,,,,,)
<$> column HD.text
Expand Down Expand Up @@ -378,8 +375,8 @@ addViewM2ORels allSrcCols = concatMap (\rel@Relationship{..} -> rel :
filter (\(c, _) -> c `elem` relCols) allSrcCols
relSrcCols = srcColsGroupedByView relColumns
relFSrcCols = srcColsGroupedByView relForeignColumns
getView :: [SourceColumn] -> Table
getView = colTable . snd . unsafeHead
getView :: [SourceColumn] -> QualifiedIdentifier
getView = (\t -> QualifiedIdentifier (tableSchema t) (tableName t)) . colTable . snd . unsafeHead
srcCols `allSrcColsOf` cols = S.fromList (fst <$> srcCols) == S.fromList cols
-- Relationship is dependent on the order of relColumns and relFColumns to get the join conditions right in the generated query.
-- So we need to change the order of the SourceColumns to match the relColumns
Expand Down Expand Up @@ -612,13 +609,13 @@ columnFromRow :: TablesMap ->
columnFromRow tabs (s, t, n, desc, nul, typ, l, d, e) = buildColumn <$> table
where
buildColumn tbl = Column tbl n desc nul typ l d (parseEnum e)
table = findTable (QualifiedIdentifier s t) tabs
table = M.lookup (QualifiedIdentifier s t) tabs
parseEnum :: Maybe Text -> [Text]
parseEnum = maybe [] (split (==','))

allM2ORels :: TablesMap -> [Column] -> Bool -> SQL.Statement () [Relationship]
allM2ORels tabs cols =
SQL.Statement sql HE.noParams (decodeRels tabs cols)
allM2ORels :: [Column] -> Bool -> SQL.Statement () [Relationship]
allM2ORels allCols =
SQL.Statement sql HE.noParams (decodeRels allCols)
where
sql = [q|
SELECT ns1.nspname AS table_schema,
Expand All @@ -643,13 +640,11 @@ allM2ORels tabs cols =
WHERE confrelid != 0
ORDER BY (conrelid, column_info.nums) |]

relFromRow :: TablesMap -> [Column] -> (Text, Text, Text, [Text], Text, Text, [Text]) -> Maybe Relationship
relFromRow allTabs allCols (rs, rt, cn, rcs, frs, frt, frcs) =
Relationship <$> table <*> cols <*> tableF <*> colsF <*> pure (M2O cn)
relFromRow :: [Column] -> (Text, Text, Text, [Text], Text, Text, [Text]) -> Maybe Relationship
relFromRow allCols (rs, rt, cn, rcs, frs, frt, frcs) =
Relationship (QualifiedIdentifier rs rt) <$> cols <*> pure (QualifiedIdentifier frs frt) <*> colsF <*> pure (M2O cn)
where
findCol s t c = find (\col -> tableSchema (colTable col) == s && tableName (colTable col) == t && colName col == c) allCols
table = findTable (QualifiedIdentifier rs rt) allTabs
tableF = findTable (QualifiedIdentifier frs frt) allTabs
cols = mapM (findCol rs rt) rcs
colsF = mapM (findCol frs frt) frcs

Expand Down Expand Up @@ -731,7 +726,7 @@ allPrimaryKeys tabs =

pkFromRow :: TablesMap -> (Schema, Text, Text) -> Maybe PrimaryKey
pkFromRow tabs (s, t, n) = PrimaryKey <$> table <*> pure n
where table = findTable (QualifiedIdentifier s t) tabs
where table = M.lookup (QualifiedIdentifier s t) tabs

-- returns all the primary and foreign key columns which are referenced in views
pfkSourceColumns :: [Column] -> Bool -> SQL.Statement ([Schema], [Schema]) [SourceColumn]
Expand Down
9 changes: 5 additions & 4 deletions src/PostgREST/DbStructure/Relationship.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ module PostgREST.DbStructure.Relationship

import qualified Data.Aeson as JSON

import PostgREST.DbStructure.Table (Column (..), Table (..))
import PostgREST.DbStructure.Identifiers (QualifiedIdentifier)
import PostgREST.DbStructure.Table (Column (..), Table)

import Protolude

Expand All @@ -23,9 +24,9 @@ import Protolude
--
-- TODO merge relColumns and relForeignColumns to a tuple or Data.Bimap
data Relationship = Relationship
{ relTable :: Table
{ relTable :: QualifiedIdentifier
, relColumns :: [Column]
, relForeignTable :: Table
, relForeignTable :: QualifiedIdentifier
, relForeignColumns :: [Column]
, relCardinality :: Cardinality
}
Expand All @@ -44,7 +45,7 @@ type FKConstraint = Text

-- | Junction table on an M2M relationship
data Junction = Junction
{ junTable :: Table
{ junTable :: QualifiedIdentifier
, junConstraint1 :: FKConstraint
, junColumns1 :: [Column]
, junConstraint2 :: FKConstraint
Expand Down
6 changes: 1 addition & 5 deletions src/PostgREST/DbStructure/Table.hs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
module PostgREST.DbStructure.Table
( Column(..)
, Table(..)
, tableQi
, TablesMap
) where

import qualified Data.Aeson as JSON
import qualified Data.Aeson as JSON
import qualified Data.HashMap.Strict as M

import PostgREST.DbStructure.Identifiers (FieldName,
Expand All @@ -34,9 +33,6 @@ data Table = Table
instance Eq Table where
Table{tableSchema=s1,tableName=n1} == Table{tableSchema=s2,tableName=n2} = s1 == s2 && n1 == n2

tableQi :: Table -> QualifiedIdentifier
tableQi Table{tableSchema=s, tableName=n} = QualifiedIdentifier s n

data Column = Column
{ colTable :: Table
, colName :: FieldName
Expand Down
15 changes: 8 additions & 7 deletions src/PostgREST/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ import qualified PostgREST.ContentType as ContentType
import PostgREST.Request.Types (ApiRequestError (..),
QPError (..))

import PostgREST.DbStructure.Identifiers (QualifiedIdentifier (..))
import PostgREST.DbStructure.Proc (ProcDescription (..),
ProcParam (..))
import PostgREST.DbStructure.Relationship (Cardinality (..),
Junction (..),
Relationship (..))
import PostgREST.DbStructure.Table (Column (..), Table (..))
import PostgREST.DbStructure.Table (Column (..))

import Protolude

Expand Down Expand Up @@ -153,28 +154,28 @@ compressedRel Relationship{..} =
fmtEls els = "(" <> T.intercalate ", " els <> ")"
in
JSON.object $
("embedding" .= (tableName relTable <> " with " <> tableName relForeignTable :: Text))
("embedding" .= (qiName relTable <> " with " <> qiName relForeignTable :: Text))
: case relCardinality of
M2M Junction{..} -> [
"cardinality" .= ("many-to-many" :: Text)
, "relationship" .= (tableName junTable <> " using " <> junConstraint1 <> fmtEls (colName <$> junColumns1) <> " and " <> junConstraint2 <> fmtEls (colName <$> junColumns2))
, "relationship" .= (qiName junTable <> " using " <> junConstraint1 <> fmtEls (colName <$> junColumns1) <> " and " <> junConstraint2 <> fmtEls (colName <$> junColumns2))
]
M2O cons -> [
"cardinality" .= ("many-to-one" :: Text)
, "relationship" .= (cons <> " using " <> tableName relTable <> fmtEls (colName <$> relColumns) <> " and " <> tableName relForeignTable <> fmtEls (colName <$> relForeignColumns))
, "relationship" .= (cons <> " using " <> qiName relTable <> fmtEls (colName <$> relColumns) <> " and " <> qiName relForeignTable <> fmtEls (colName <$> relForeignColumns))
]
O2M cons -> [
"cardinality" .= ("one-to-many" :: Text)
, "relationship" .= (cons <> " using " <> tableName relTable <> fmtEls (colName <$> relColumns) <> " and " <> tableName relForeignTable <> fmtEls (colName <$> relForeignColumns))
, "relationship" .= (cons <> " using " <> qiName relTable <> fmtEls (colName <$> relColumns) <> " and " <> qiName relForeignTable <> fmtEls (colName <$> relForeignColumns))
]

relHint :: [Relationship] -> Text
relHint rels = T.intercalate ", " (hintList <$> rels)
where
hintList Relationship{..} =
let buildHint rel = "'" <> tableName relForeignTable <> "!" <> rel <> "'" in
let buildHint rel = "'" <> qiName relForeignTable <> "!" <> rel <> "'" in
case relCardinality of
M2M Junction{..} -> buildHint (tableName junTable)
M2M Junction{..} -> buildHint (qiName junTable)
M2O cons -> buildHint cons
O2M cons -> buildHint cons

Expand Down
6 changes: 4 additions & 2 deletions src/PostgREST/OpenAPI.hs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ import PostgREST.Config (AppConfig (..), Proxy (..),
isMalformedProxyUri, toURI)
import PostgREST.DbStructure (DbStructure (..),
tableCols, tablePKCols)
import PostgREST.DbStructure.Identifiers (QualifiedIdentifier (..))
import PostgREST.DbStructure.Proc (ProcDescription (..),
ProcParam (..))
import PostgREST.DbStructure.Relationship (Cardinality (..),
PrimaryKey (..),
Relationship (..))
import PostgREST.DbStructure.Table (Column (..), Table (..), TablesMap)
import PostgREST.DbStructure.Table (Column (..), Table (..),
TablesMap)
import PostgREST.Version (docsVersion, prettyVersion)

import PostgREST.ContentType
Expand Down Expand Up @@ -103,7 +105,7 @@ makeProperty rels pks c = (colName c, Inline s)
_ -> False
) rels
fCol = colName <$> (headMay . relForeignColumns =<< rel)
fTbl = tableName . relForeignTable <$> rel
fTbl = qiName . relForeignTable <$> rel
fTblCol = (,) <$> fTbl <*> fCol
in
(\(a, b) -> T.intercalate "" ["This is a Foreign Key to `", a, ".", b, "`.<fk table='", a, "' column='", b, "'/>"]) <$> fTblCol
Expand Down
3 changes: 1 addition & 2 deletions src/PostgREST/Query/QueryBuilder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import PostgREST.DbStructure.Identifiers (QualifiedIdentifier (..))
import PostgREST.DbStructure.Proc (ProcParam (..))
import PostgREST.DbStructure.Relationship (Cardinality (..),
Relationship (..))
import PostgREST.DbStructure.Table (Table (..))
import PostgREST.Request.Preferences (PreferResolution (..))

import PostgREST.Query.SqlFragment
Expand All @@ -51,7 +50,7 @@ readRequestToQuery (Node (Select colSelects mainQi tblAlias implJoins logicFores
(joins, selects) = foldr getJoinsSelects ([],[]) forest

getJoinsSelects :: ReadRequest -> ([SQL.Snippet], [SQL.Snippet]) -> ([SQL.Snippet], [SQL.Snippet])
getJoinsSelects rr@(Node (_, (name, Just Relationship{relCardinality=card,relTable=Table{tableName=table}}, alias, _, joinType, _)) _) (joins,selects) =
getJoinsSelects rr@(Node (_, (name, Just Relationship{relCardinality=card,relTable=QualifiedIdentifier{qiName=table}}, alias, _, joinType, _)) _) (joins,selects) =
let subquery = readRequestToQuery rr in
case card of
M2O _ ->
Expand Down
25 changes: 12 additions & 13 deletions src/PostgREST/Request/DbRequestBuilder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ import PostgREST.DbStructure.Proc (ProcDescription (..),
import PostgREST.DbStructure.Relationship (Cardinality (..),
Junction (..),
Relationship (..))
import PostgREST.DbStructure.Table (Column (..), Table (..),
tableQi)
import PostgREST.DbStructure.Table (Column (..))
import PostgREST.Error (Error (..))
import PostgREST.Query.SqlFragment (sourceCTEName)
import PostgREST.RangeQuery (NonnegRange, allRange,
Expand Down Expand Up @@ -88,7 +87,7 @@ rootWithRels schema rootTableName allRels action = case action of
-- findRel can find relationships with sourceCTEName.
toSourceRel :: Relationship -> Maybe Relationship
toSourceRel r@Relationship{relTable=t}
| rootTableName == tableName t = Just $ r {relTable=t {tableName=_sourceCTEName}}
| rootTableName == qiName t = Just $ r {relTable=t {qiName=_sourceCTEName}}
| otherwise = Nothing

-- Build the initial tree with a Depth attribute so when a self join occurs we
Expand Down Expand Up @@ -131,7 +130,7 @@ addRels :: Schema -> [Relationship] -> Maybe ReadRequest -> ReadRequest -> Eithe
addRels schema allRels parentNode (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
let newFrom r = if qiName tbl == nodeName then relForeignTable r else tbl
newReadNode = (\r -> (query{from=newFrom r}, (nodeName, Just r, alias, hint, joinType, depth))) <$> rel
rel = findRel schema allRels (qiName parentNodeQi) nodeName hint
in
Expand Down Expand Up @@ -175,25 +174,25 @@ findRel schema allRels origin target hint =
M2O cons -> tar == Just cons
_ -> False
matchJunction hint_ card = case card of
M2M Junction{junTable} -> hint_ == Just (tableName junTable)
M2M Junction{junTable} -> hint_ == Just (qiName junTable)
_ -> False
rel = filter (
\Relationship{..} ->
-- Both relationship ends need to be on the exposed schema
schema == tableSchema relTable && schema == tableSchema relForeignTable &&
schema == qiSchema relTable && schema == qiSchema relForeignTable &&
(
-- /projects?select=clients(*)
origin == tableName relTable && -- projects
target == tableName relForeignTable || -- clients
origin == qiName relTable && -- projects
target == qiName relForeignTable || -- clients

-- /projects?select=projects_client_id_fkey(*)
(
origin == tableName relTable && -- projects
origin == qiName relTable && -- projects
matchConstraint (Just target) relCardinality -- projects_client_id_fkey
) ||
-- /projects?select=client_id(*)
(
origin == tableName relTable && -- projects
origin == qiName relTable && -- projects
matchFKSingleCol (Just target) relColumns -- client_id
)
) && (
Expand All @@ -217,7 +216,7 @@ addJoinConditions previousAlias (Node node@(query@Select{from=tbl}, nodeProps@(_
case rel of
Just r@Relationship{relCardinality=M2M Junction{junTable}} ->
let rq = augmentQuery r in
Node (rq{implicitJoins=tableQi junTable:implicitJoins rq}, nodeProps) <$> updatedForest
Node (rq{implicitJoins=junTable:implicitJoins rq}, nodeProps) <$> updatedForest
Just r -> Node (augmentQuery r, nodeProps) <$> updatedForest
Nothing -> Node node <$> updatedForest
where
Expand All @@ -235,9 +234,9 @@ addJoinConditions previousAlias (Node node@(query@Select{from=tbl}, nodeProps@(_

-- previousAlias and newAlias are used in the case of self joins
getJoinConditions :: Maybe Alias -> Maybe Alias -> Relationship -> [JoinCondition]
getJoinConditions previousAlias newAlias (Relationship Table{tableSchema=tSchema, tableName=tN} cols Table{tableName=ftN} fCols card) =
getJoinConditions previousAlias newAlias (Relationship QualifiedIdentifier{qiSchema=tSchema, qiName=tN} cols QualifiedIdentifier{qiName=ftN} fCols card) =
case card of
M2M (Junction Table{tableName=jtn} _ jc1 _ jc2) ->
M2M (Junction QualifiedIdentifier{qiName=jtn} _ jc1 _ jc2) ->
zipWith (toJoinCondition tN jtn) cols jc1 ++ zipWith (toJoinCondition ftN jtn) fCols jc2
_ ->
zipWith (toJoinCondition tN ftN) cols fCols
Expand Down
5 changes: 1 addition & 4 deletions test/spec/Feature/OpenApi/RootSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ spec =
request methodGet "/"
[("Accept", "application/json")] "" `shouldRespondWith`
[json| {
"tableName": "orders_view", "tableSchema": "test",
"tableDeletable": true, "tableUpdatable": true,
"tableIsView":true, "tableInsertable": true,
"tableDescription": null
"qiSchema":"test","qiName":"orders_view"
} |]
{ matchHeaders = [matchContentTypeJson] }

0 comments on commit 7f1507b

Please sign in to comment.