Skip to content

Commit f026d44

Browse files
Auke Booijhasura-bot
Auke Booij
authored andcommitted
Role-invariant schema constructors
We build the GraphQL schema by combining building blocks such as `tableSelectionSet` and `columnParser`. These building blocks individually build `{InputFields,Field,}Parser` objects. Those object specify the valid GraphQL schema. Since the GraphQL schema is role-dependent, at some point we need to know what fragment of the GraphQL schema a specific role is allowed to access, and this is stored in `{Sel,Upd,Ins,Del}PermInfo` objects. We have passed around these permission objects as function arguments to the schema building blocks since we first started dealing with permissions during the PDV refactor - see 5168b99 in #4111. This means that, for instance, `tableSelectionSet` has as its type: ```haskell tableSelectionSet :: forall b r m n. MonadBuildSchema b r m n => SourceName -> TableInfo b -> SelPermInfo b -> m (Parser 'Output n (AnnotatedFields b)) ``` There are three reasons to change this. 1. We often pass a `Maybe (xPermInfo b)` instead of a proper `xPermInfo b`, and it's not clear what the intended semantics of this is. Some potential improvements on the data types involved are discussed in issue hasura/graphql-engine-mono#3125. 2. In most cases we also already pass a `TableInfo b`, and together with the `MonadRole` that is usually also in scope, this means that we could look up the required permissions regardless: so passing the permissions explicitly undermines the "single source of truth" principle. Breaking this principle also makes the code more difficult to read. 3. We are working towards role-based parsers (see hasura/graphql-engine-mono#2711), where the `{InputFields,Field,}Parser` objects are constructed in a role-invariant way, so that we have a single object that can be used for all roles. In particular, this means that the schema building blocks _need_ to be constructed in a role-invariant way. While this PR doesn't accomplish that, it does reduce the amount of role-specific arguments being passed, thus fixing hasura/graphql-engine-mono#3068. Concretely, this PR simply drops the `xPermInfo b` argument from almost all schema building blocks. Instead these objects are looked up from the `TableInfo b` as-needed. The resulting code is considerably simpler and shorter. One way to interpret this change is as follows. Before this PR, we figured out permissions at the top-level in `Hasura.GraphQL.Schema`, passing down the obtained `xPermInfo` objects as required. After this PR, we have a bottom-up approach where the schema building blocks themselves decide whether they want to be included for a particular role. So this moves some permission logic out of `Hasura.GraphQL.Schema`, which is very complex. PR-URL: hasura/graphql-engine-mono#3608 GitOrigin-RevId: 51a744f34ec7d57bc8077667ae7f9cb9c4f6c962
1 parent d1ba271 commit f026d44

File tree

23 files changed

+636
-732
lines changed

23 files changed

+636
-732
lines changed

server/src-lib/Hasura/Backends/BigQuery/Instances/Schema.hs

+10-20
Original file line numberDiff line numberDiff line change
@@ -68,22 +68,19 @@ bqBuildTableRelayQueryFields ::
6868
TableInfo 'BigQuery ->
6969
G.Name ->
7070
NESeq (ColumnInfo 'BigQuery) ->
71-
SelPermInfo 'BigQuery ->
7271
m [a]
73-
bqBuildTableRelayQueryFields _sourceName _tableName _tableInfo _gqlName _pkeyColumns _selPerms =
72+
bqBuildTableRelayQueryFields _sourceName _tableName _tableInfo _gqlName _pkeyColumns =
7473
pure []
7574

7675
bqBuildTableInsertMutationFields ::
7776
MonadBuildSchema 'BigQuery r m n =>
77+
Scenario ->
7878
SourceName ->
7979
TableName 'BigQuery ->
8080
TableInfo 'BigQuery ->
8181
G.Name ->
82-
InsPermInfo 'BigQuery ->
83-
Maybe (SelPermInfo 'BigQuery) ->
84-
Maybe (UpdPermInfo 'BigQuery) ->
8582
m [a]
86-
bqBuildTableInsertMutationFields _sourceName _tableName _tableInfo _gqlName _insPerms _selPerms _updPerms =
83+
bqBuildTableInsertMutationFields _scenario _sourceName _tableName _tableInfo _gqlName =
8784
pure []
8885

8986
bqBuildTableUpdateMutationFields ::
@@ -92,10 +89,8 @@ bqBuildTableUpdateMutationFields ::
9289
TableName 'BigQuery ->
9390
TableInfo 'BigQuery ->
9491
G.Name ->
95-
UpdPermInfo 'BigQuery ->
96-
Maybe (SelPermInfo 'BigQuery) ->
9792
m [a]
98-
bqBuildTableUpdateMutationFields _sourceName _tableName _tableInfo _gqlName _updPerns _selPerms =
93+
bqBuildTableUpdateMutationFields _sourceName _tableName _tableInfo _gqlName =
9994
pure []
10095

10196
bqBuildTableDeleteMutationFields ::
@@ -104,10 +99,8 @@ bqBuildTableDeleteMutationFields ::
10499
TableName 'BigQuery ->
105100
TableInfo 'BigQuery ->
106101
G.Name ->
107-
DelPermInfo 'BigQuery ->
108-
Maybe (SelPermInfo 'BigQuery) ->
109102
m [a]
110-
bqBuildTableDeleteMutationFields _sourceName _tableName _tableInfo _gqlName _delPerns _selPerms =
103+
bqBuildTableDeleteMutationFields _sourceName _tableName _tableInfo _gqlName =
111104
pure []
112105

113106
bqBuildFunctionQueryFields ::
@@ -116,9 +109,8 @@ bqBuildFunctionQueryFields ::
116109
FunctionName 'BigQuery ->
117110
FunctionInfo 'BigQuery ->
118111
TableName 'BigQuery ->
119-
SelPermInfo 'BigQuery ->
120112
m [a]
121-
bqBuildFunctionQueryFields _ _ _ _ _ =
113+
bqBuildFunctionQueryFields _ _ _ _ =
122114
pure []
123115

124116
bqBuildFunctionRelayQueryFields ::
@@ -128,9 +120,8 @@ bqBuildFunctionRelayQueryFields ::
128120
FunctionInfo 'BigQuery ->
129121
TableName 'BigQuery ->
130122
NESeq (ColumnInfo 'BigQuery) ->
131-
SelPermInfo 'BigQuery ->
132123
m [a]
133-
bqBuildFunctionRelayQueryFields _sourceName _functionName _functionInfo _tableName _pkeyColumns _selPerms =
124+
bqBuildFunctionRelayQueryFields _sourceName _functionName _functionInfo _tableName _pkeyColumns =
134125
pure []
135126

136127
bqBuildFunctionMutationFields ::
@@ -139,9 +130,8 @@ bqBuildFunctionMutationFields ::
139130
FunctionName 'BigQuery ->
140131
FunctionInfo 'BigQuery ->
141132
TableName 'BigQuery ->
142-
SelPermInfo 'BigQuery ->
143133
m [a]
144-
bqBuildFunctionMutationFields _ _ _ _ _ =
134+
bqBuildFunctionMutationFields _ _ _ _ =
145135
pure []
146136

147137
----------------------------------------------------------------
@@ -391,9 +381,9 @@ bqComputedField ::
391381
SourceName ->
392382
ComputedFieldInfo 'BigQuery ->
393383
TableName 'BigQuery ->
394-
SelPermInfo 'BigQuery ->
384+
TableInfo 'BigQuery ->
395385
m (Maybe (FieldParser n (AnnotatedField 'BigQuery)))
396-
bqComputedField _sourceName _fieldInfo _table _selectPemissions = pure Nothing
386+
bqComputedField _sourceName _fieldInfo _table _tableInfo = pure Nothing
397387

398388
-- | Remote join field parser.
399389
-- Currently unsupported: returns Nothing for now.

server/src-lib/Hasura/Backends/MSSQL/Instances/Schema.hs

+32-32
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import Hasura.GraphQL.Schema.BoolExp
2525
import Hasura.GraphQL.Schema.Build qualified as GSB
2626
import Hasura.GraphQL.Schema.Common
2727
import Hasura.GraphQL.Schema.Select
28+
import Hasura.GraphQL.Schema.Table
2829
import Hasura.GraphQL.Schema.Update qualified as SU
2930
import Hasura.Prelude
3031
import Hasura.RQL.IR
@@ -82,21 +83,18 @@ msBuildTableRelayQueryFields ::
8283
TableInfo 'MSSQL ->
8384
G.Name ->
8485
NESeq (ColumnInfo 'MSSQL) ->
85-
SelPermInfo 'MSSQL ->
8686
m [a]
87-
msBuildTableRelayQueryFields _sourceName _tableName _tableInfo _gqlName _pkeyColumns _selPerms =
87+
msBuildTableRelayQueryFields _sourceName _tableName _tableInfo _gqlName _pkeyColumns =
8888
pure []
8989

9090
backendInsertParser ::
9191
forall m r n.
9292
MonadBuildSchema 'MSSQL r m n =>
9393
SourceName ->
9494
TableInfo 'MSSQL ->
95-
Maybe (SelPermInfo 'MSSQL) ->
96-
Maybe (UpdPermInfo 'MSSQL) ->
9795
m (InputFieldsParser n (BackendInsert (UnpreparedValue 'MSSQL)))
98-
backendInsertParser sourceName tableInfo selectPerms updatePerms = do
99-
ifMatched <- ifMatchedFieldParser sourceName tableInfo selectPerms updatePerms
96+
backendInsertParser sourceName tableInfo = do
97+
ifMatched <- ifMatchedFieldParser sourceName tableInfo
10098
let _biIdentityColumns = _tciExtraTableMetadata $ _tiCoreInfo tableInfo
10199
pure $ do
102100
_biIfMatched <- ifMatched
@@ -108,21 +106,27 @@ msBuildTableUpdateMutationFields ::
108106
TableName 'MSSQL ->
109107
TableInfo 'MSSQL ->
110108
G.Name ->
111-
UpdPermInfo 'MSSQL ->
112-
Maybe (SelPermInfo 'MSSQL) ->
113109
m [FieldParser n (AnnotatedUpdateG 'MSSQL (RemoteRelationshipField UnpreparedValue) (UnpreparedValue 'MSSQL))]
114-
msBuildTableUpdateMutationFields =
115-
GSB.buildTableUpdateMutationFields
116-
( \ti updPerms ->
117-
fmap BackendUpdate
118-
<$> SU.buildUpdateOperators
119-
(UpdateSet <$> SU.presetColumns updPerms)
120-
[ UpdateSet <$> SU.setOp,
121-
UpdateInc <$> SU.incOp
122-
]
123-
ti
124-
updPerms
125-
)
110+
msBuildTableUpdateMutationFields sourceName tableName tableInfo gqlName = do
111+
fieldParsers <- runMaybeT do
112+
tablePerms <- MaybeT $ tablePermissions tableInfo
113+
updatePerms <- hoistMaybe $ _permUpd tablePerms
114+
let mkBackendUpdate backendUpdateTableInfo =
115+
(fmap . fmap) BackendUpdate $
116+
SU.buildUpdateOperators
117+
(UpdateSet <$> SU.presetColumns updatePerms)
118+
[ UpdateSet <$> SU.setOp,
119+
UpdateInc <$> SU.incOp
120+
]
121+
backendUpdateTableInfo
122+
lift $
123+
GSB.buildTableUpdateMutationFields
124+
mkBackendUpdate
125+
sourceName
126+
tableName
127+
tableInfo
128+
gqlName
129+
pure . fold @Maybe @[_] $ fieldParsers
126130

127131
msBuildTableDeleteMutationFields ::
128132
MonadBuildSchema 'MSSQL r m n =>
@@ -142,9 +146,8 @@ msBuildFunctionQueryFields ::
142146
FunctionName 'MSSQL ->
143147
FunctionInfo 'MSSQL ->
144148
TableName 'MSSQL ->
145-
SelPermInfo 'MSSQL ->
146149
m [a]
147-
msBuildFunctionQueryFields _ _ _ _ _ =
150+
msBuildFunctionQueryFields _ _ _ _ =
148151
pure []
149152

150153
msBuildFunctionRelayQueryFields ::
@@ -154,9 +157,8 @@ msBuildFunctionRelayQueryFields ::
154157
FunctionInfo 'MSSQL ->
155158
TableName 'MSSQL ->
156159
NESeq (ColumnInfo 'MSSQL) ->
157-
SelPermInfo 'MSSQL ->
158160
m [a]
159-
msBuildFunctionRelayQueryFields _sourceName _functionName _functionInfo _tableName _pkeyColumns _selPerms =
161+
msBuildFunctionRelayQueryFields _sourceName _functionName _functionInfo _tableName _pkeyColumns =
160162
pure []
161163

162164
msBuildFunctionMutationFields ::
@@ -165,9 +167,8 @@ msBuildFunctionMutationFields ::
165167
FunctionName 'MSSQL ->
166168
FunctionInfo 'MSSQL ->
167169
TableName 'MSSQL ->
168-
SelPermInfo 'MSSQL ->
169170
m [a]
170-
msBuildFunctionMutationFields _ _ _ _ _ =
171+
msBuildFunctionMutationFields _ _ _ _ =
171172
pure []
172173

173174
----------------------------------------------------------------
@@ -179,11 +180,10 @@ msTableArgs ::
179180
MonadBuildSchema 'MSSQL r m n =>
180181
SourceName ->
181182
TableInfo 'MSSQL ->
182-
SelPermInfo 'MSSQL ->
183183
m (InputFieldsParser n (IR.SelectArgsG 'MSSQL (UnpreparedValue 'MSSQL)))
184-
msTableArgs sourceName tableInfo selectPermissions = do
185-
whereParser <- tableWhereArg sourceName tableInfo selectPermissions
186-
orderByParser <- tableOrderByArg sourceName tableInfo selectPermissions
184+
msTableArgs sourceName tableInfo = do
185+
whereParser <- tableWhereArg sourceName tableInfo
186+
orderByParser <- tableOrderByArg sourceName tableInfo
187187
pure do
188188
whereArg <- whereParser
189189
orderByArg <- orderByParser
@@ -432,9 +432,9 @@ msComputedField ::
432432
SourceName ->
433433
ComputedFieldInfo 'MSSQL ->
434434
TableName 'MSSQL ->
435-
SelPermInfo 'MSSQL ->
435+
TableInfo 'MSSQL ->
436436
m (Maybe (FieldParser n (AnnotatedField 'MSSQL)))
437-
msComputedField _sourceName _fieldInfo _table _selectPemissions = pure Nothing
437+
msComputedField _sourceName _fieldInfo _table _tableInfo = pure Nothing
438438

439439
-- | Remote join field parser.
440440
-- Currently unsupported: returns Nothing for now.

server/src-lib/Hasura/Backends/MSSQL/Schema/IfMatched.hs

+30-43
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,9 @@ ifMatchedFieldParser ::
5151
MonadBuildSchema 'MSSQL r m n =>
5252
SourceName ->
5353
TableInfo 'MSSQL ->
54-
Maybe (SelPermInfo 'MSSQL) ->
55-
Maybe (UpdPermInfo 'MSSQL) ->
5654
m (InputFieldsParser n (Maybe (IfMatched (UnpreparedValue 'MSSQL))))
57-
ifMatchedFieldParser sourceName tableInfo selectPerms updatePerms = do
58-
maybeObject <- ifMatchedObjectParser sourceName tableInfo selectPerms updatePerms
55+
ifMatchedFieldParser sourceName tableInfo = do
56+
maybeObject <- ifMatchedObjectParser sourceName tableInfo
5957
return $ withJust maybeObject $ P.fieldOptional $$(G.litName "if_matched") (Just "upsert condition")
6058

6159
-- | Parse a @tablename_if_matched@ object.
@@ -64,45 +62,35 @@ ifMatchedObjectParser ::
6462
(MonadBuildSchema 'MSSQL r m n) =>
6563
SourceName ->
6664
TableInfo 'MSSQL ->
67-
Maybe (SelPermInfo 'MSSQL) ->
68-
Maybe (UpdPermInfo 'MSSQL) ->
6965
m (Maybe (Parser 'Input n (IfMatched (UnpreparedValue 'MSSQL))))
70-
ifMatchedObjectParser sourceName tableInfo maybeSelectPerms maybeUpdatePerms = runMaybeT do
66+
ifMatchedObjectParser sourceName tableInfo = runMaybeT do
7167
-- Short-circuit if we don't have sufficient permissions.
72-
selectPerms <- hoistMaybe maybeSelectPerms
73-
updatePerms <- hoistMaybe maybeUpdatePerms
74-
matchColumnsEnum <- MaybeT $ tableInsertMatchColumnsEnum sourceName tableInfo selectPerms
75-
updateColumnsEnum <- lift $ updateColumnsPlaceholderParser tableInfo updatePerms
68+
updatePerms <- MaybeT $ (_permUpd =<<) <$> tablePermissions tableInfo
69+
matchColumnsEnum <- MaybeT $ tableInsertMatchColumnsEnum sourceName tableInfo
70+
lift do
71+
updateColumnsEnum <- updateColumnsPlaceholderParser tableInfo
72+
tableGQLName <- getTableGQLName tableInfo
73+
objectName <- P.mkTypename $ tableGQLName <> $$(G.litName "_if_matched")
74+
let _imColumnPresets = partialSQLExpToUnpreparedValue <$> upiSet updatePerms
75+
updateFilter = fmap partialSQLExpToUnpreparedValue <$> upiFilter updatePerms
76+
objectDesc = G.Description $ "upsert condition type for table " <>> tableInfoName tableInfo
77+
matchColumnsName = $$(G.litName "match_columns")
78+
updateColumnsName = $$(G.litName "update_columns")
79+
whereName = $$(G.litName "where")
80+
whereExpParser <- boolExp sourceName tableInfo
81+
pure $
82+
P.object objectName (Just objectDesc) do
83+
_imConditions <-
84+
(\whereExp -> BoolAnd $ updateFilter : maybeToList whereExp)
85+
<$> P.fieldOptional whereName Nothing whereExpParser
86+
_imMatchColumns <-
87+
P.fieldWithDefault matchColumnsName Nothing (G.VList []) (P.list matchColumnsEnum)
88+
_imUpdateColumns <-
89+
P.fieldWithDefault updateColumnsName Nothing (G.VList []) (P.list updateColumnsEnum) `P.bindFields` \cs ->
90+
-- this can only happen if the placeholder was used
91+
sequenceA cs `onNothing` parseError "erroneous column name"
7692

77-
-- The style of the above code gives me some cognitive dissonance: We could
78-
-- push the @hoistMaybe@ checks further away to callers, but not the enum
79-
-- parsers, and as a result we end up with @MaybeT@ is a sort of local maximum
80-
-- of legible expression.
81-
-- See https://github.com/hasura/graphql-engine-mono/issues/3125.
82-
83-
tableGQLName <- getTableGQLName tableInfo
84-
objectName <- P.mkTypename $ tableGQLName <> $$(G.litName "_if_matched")
85-
let _imColumnPresets = partialSQLExpToUnpreparedValue <$> upiSet updatePerms
86-
updateFilter = fmap partialSQLExpToUnpreparedValue <$> upiFilter updatePerms
87-
objectDesc = G.Description $ "upsert condition type for table " <>> tableInfoName tableInfo
88-
matchColumnsName = $$(G.litName "match_columns")
89-
updateColumnsName = $$(G.litName "update_columns")
90-
whereName = $$(G.litName "where")
91-
92-
whereExpParser <- lift $ boolExp sourceName tableInfo (Just selectPerms)
93-
pure $
94-
P.object objectName (Just objectDesc) $ do
95-
_imConditions <-
96-
(\whereExp -> BoolAnd $ updateFilter : maybeToList whereExp)
97-
<$> P.fieldOptional whereName Nothing whereExpParser
98-
_imMatchColumns <-
99-
P.fieldWithDefault matchColumnsName Nothing (G.VList []) (P.list matchColumnsEnum)
100-
_imUpdateColumns <-
101-
P.fieldWithDefault updateColumnsName Nothing (G.VList []) (P.list updateColumnsEnum) `P.bindFields` \cs ->
102-
-- this can only happen if the placeholder was used
103-
sequenceA cs `onNothing` parseError "erroneous column name"
104-
105-
pure $ IfMatched {..}
93+
pure $ IfMatched {..}
10694

10795
-- | Table insert_match_columns enum
10896
--
@@ -117,11 +105,10 @@ tableInsertMatchColumnsEnum ::
117105
(MonadSchema n m, MonadRole r m, MonadTableInfo r m, Has P.MkTypename r) =>
118106
SourceName ->
119107
TableInfo 'MSSQL ->
120-
SelPermInfo 'MSSQL ->
121108
m (Maybe (Parser 'Both n (Column 'MSSQL)))
122-
tableInsertMatchColumnsEnum sourceName tableInfo selectPermissions = do
109+
tableInsertMatchColumnsEnum sourceName tableInfo = do
123110
tableGQLName <- getTableGQLName @'MSSQL tableInfo
124-
columns <- tableSelectColumns sourceName tableInfo selectPermissions
111+
columns <- tableSelectColumns sourceName tableInfo
125112
enumName <- P.mkTypename $ tableGQLName <> $$(G.litName "_insert_match_column")
126113
let description =
127114
Just $

0 commit comments

Comments
 (0)