diff --git a/server/src-lib/Hasura/GraphQL/Context.hs b/server/src-lib/Hasura/GraphQL/Context.hs index 3a4896baff5..c2299aa8242 100644 --- a/server/src-lib/Hasura/GraphQL/Context.hs +++ b/server/src-lib/Hasura/GraphQL/Context.hs @@ -10,11 +10,13 @@ import Language.Haskell.TH.Syntax (Lift) import qualified Data.HashMap.Strict as Map import qualified Data.HashSet as Set +import qualified Data.Text as T import qualified Language.GraphQL.Draft.Syntax as G import Hasura.GraphQL.Resolve.Types import Hasura.GraphQL.Validate.Types import Hasura.RQL.Types.Permission +import Hasura.Server.Utils (duplicates) -- | A /GraphQL context/, aka the final output of GraphQL schema generation. Used to both validate -- incoming queries and respond to introspection queries. @@ -86,7 +88,27 @@ data TableCustomRootFields , _tcrfUpdate :: !(Maybe G.Name) , _tcrfDelete :: !(Maybe G.Name) } deriving (Show, Eq, Lift) -$(deriveJSON (aesonDrop 5 snakeCase) ''TableCustomRootFields) +$(deriveToJSON (aesonDrop 5 snakeCase) ''TableCustomRootFields) + +instance FromJSON TableCustomRootFields where + parseJSON = withObject "Object" $ \obj -> do + select <- obj .:? "select" + selectByPk <- obj .:? "select_by_pk" + selectAggregate <- obj .:? "select_aggregate" + insert <- obj .:? "insert" + update <- obj .:? "update" + delete <- obj .:? "delete" + + let duplicateRootFields = duplicates $ + catMaybes [ select, selectByPk, selectAggregate + , insert, update, delete + ] + when (not $ null duplicateRootFields) $ fail $ T.unpack $ + "the following custom root field names are duplicated: " + <> showNames duplicateRootFields + + pure $ TableCustomRootFields select selectByPk selectAggregate + insert update delete emptyCustomRootFields :: TableCustomRootFields emptyCustomRootFields = diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index 8a21604983f..23dfae67d5a 100644 --- a/server/src-lib/Hasura/GraphQL/Schema.hs +++ b/server/src-lib/Hasura/GraphQL/Schema.hs @@ -333,8 +333,8 @@ getRootFldsRole' getRootFldsRole' tn primCols constraints fields funcs insM selM updM delM viM tableConfig = RootFields - { rootQueryFields = makeFieldMap - $ funcQueries + { rootQueryFields = makeFieldMap $ + funcQueries <> funcAggQueries <> catMaybes [ getSelDet <$> selM @@ -348,10 +348,10 @@ getRootFldsRole' tn primCols constraints fields funcs insM ] } where + makeFieldMap = mapFromL (_fiName . snd) customRootFields = _tcCustomRootFields tableConfig colGNameMap = mkPGColGNameMap $ getValidCols fields - makeFieldMap = mapFromL (_fiName . snd) allCols = getCols fields funcQueries = maybe [] getFuncQueryFlds selM funcAggQueries = maybe [] getFuncAggQueryFlds selM @@ -697,27 +697,44 @@ noFilter :: AnnBoolExpPartialSQL noFilter = annBoolExpTrue mkGCtxMap - :: (MonadError QErr m) + :: forall m. (MonadError QErr m) => TableCache PGColumnInfo -> FunctionCache -> m GCtxMap mkGCtxMap tableCache functionCache = do typesMapL <- mapM (mkGCtxMapTable tableCache functionCache) $ filter tableFltr $ Map.elems tableCache - -- since root field names are customisable, we need to check for - -- duplicate root field names across all tables - duplicateRootFlds <- (duplicates . concat) <$> forM typesMapL getRootFlds - unless (null duplicateRootFlds) $ - throw400 Unexpected $ "following root fields are duplicated: " - <> showNames duplicateRootFlds - let typesMap = foldr (Map.unionWith mappend) Map.empty typesMapL + typesMap <- combineTypes typesMapL return $ flip Map.map typesMap $ \(ty, flds, insCtxMap) -> mkGCtx ty flds insCtxMap where tableFltr ti = not (isSystemDefined $ _tiSystemDefined ti) && isValidObjectName (_tiName ti) - getRootFlds roleMap = do - (_, RootFields query mutation, _) <- onNothing - (Map.lookup adminRole roleMap) $ throw500 "admin schema not found" - return $ Map.keys query <> Map.keys mutation + combineTypes + :: [Map.HashMap RoleName (TyAgg, RootFields, InsCtxMap)] + -> m (Map.HashMap RoleName (TyAgg, RootFields, InsCtxMap)) + combineTypes maps = do + let listMap = foldr (Map.unionWith (++) . Map.map pure) Map.empty maps + flip Map.traverseWithKey listMap $ \_ typeList -> do + let tyAgg = mconcat $ map (^. _1) typeList + insCtx = mconcat $ map (^. _3) typeList + rootFields <- combineRootFields $ map (^. _2) typeList + pure (tyAgg, rootFields, insCtx) + + combineRootFields :: [RootFields] -> m RootFields + combineRootFields rootFields = do + let duplicateQueryFields = duplicates $ + concatMap (Map.keys . rootQueryFields) rootFields + duplicateMutationFields = duplicates $ + concatMap (Map.keys . rootMutationFields) rootFields + + when (not $ null duplicateQueryFields) $ + throw400 Unexpected $ "following query root fields are duplicated: " + <> showNames duplicateQueryFields + + when (not $ null duplicateMutationFields) $ + throw400 Unexpected $ "following mutation root fields are duplicated: " + <> showNames duplicateMutationFields + + pure $ mconcat rootFields -- | build GraphQL schema from postgres tables and functions buildGCtxMapPG @@ -784,7 +801,7 @@ data RootFields instance Semigroup RootFields where RootFields a1 b1 <> RootFields a2 b2 - = RootFields (Map.union a1 a2) (Map.union b1 b2) + = RootFields (a1 <> a2) (b1 <> b2) instance Monoid RootFields where mempty = RootFields Map.empty Map.empty diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs index e85184bbe04..6646bb63d31 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs @@ -29,6 +29,7 @@ import qualified Database.PG.Query as Q import Data.Aeson +import qualified Hasura.GraphQL.Context as GC import qualified Hasura.GraphQL.Schema as GS import Hasura.Db @@ -153,6 +154,9 @@ buildSchemaCacheWithOptions withSetup = do -- remote schemas forM_ remoteSchemas resolveSingleRemoteSchema + -- validate tables' custom root fields + validateTablesCustomRootFields + where permHelper setup sqlGenCtx qt rn pDef pa = do qCtx <- mkAdminQCtx sqlGenCtx <$> askSchemaCache @@ -180,6 +184,16 @@ buildSchemaCacheWithOptions withSetup = do , scDefaultRemoteGCtx = mergedDefGCtx } + validateTablesCustomRootFields = do + sc <- askSchemaCache + let tables = M.elems $ scTables sc + defRemoteGCtx = scDefaultRemoteGCtx sc + forM_ tables $ \table -> do + let GC.TableCustomRootFields sel selByPk selAgg ins upd del = + _tcCustomRootFields $ _tiCustomConfig table + rootFldNames = catMaybes [sel, selByPk, selAgg, ins, upd, del] + forM_ rootFldNames $ GS.checkConflictingNode defRemoteGCtx + -- | Rebuilds the schema cache. If an object with the given object id became newly inconsistent, -- raises an error about it specifically. Otherwise, raises a generic metadata inconsistency error. buildSchemaCacheFor :: (CacheBuildM m) => MetadataObjId -> m () diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index d7143352f78..c149bd0b338 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -98,35 +98,6 @@ trackExistingTableOrViewP1 qt = do when (M.member qf $ scFunctions rawSchemaCache) $ throw400 NotSupported $ "function with name " <> qt <<> " already exists" -validateCustomRootFlds - :: (MonadError QErr m) - => GS.GCtx - -> GC.TableCustomRootFields - -> m () -validateCustomRootFlds defRemoteGCtx rootFlds = - forM_ rootFldNames $ GS.checkConflictingNode defRemoteGCtx - where - GC.TableCustomRootFields sel selByPk selAgg ins upd del = rootFlds - rootFldNames = catMaybes [sel, selByPk, selAgg, ins, upd, del] - -validateTableConfig - :: (QErrM m, CacheRM m) - => TableInfo a -> TableConfig -> m () -validateTableConfig tableInfo (TableConfig rootFlds colFlds) = do - withPathK "custom_root_fields" $ do - sc <- askSchemaCache - let defRemoteGCtx = scDefaultRemoteGCtx sc - validateCustomRootFlds defRemoteGCtx rootFlds - withPathK "custom_column_names" $ - forM_ (M.toList colFlds) $ \(col, customName) -> do - void $ askPGColInfo (_tiFieldInfoMap tableInfo) col "" - withPathK (getPGColTxt col) $ - checkForFieldConflict tableInfo $ FieldName $ G.unName customName - when (not $ null duplicateNames) $ throw400 NotSupported $ - "the following names are duplicated: " <> showNames duplicateNames - where - duplicateNames = duplicates $ M.elems colFlds - trackExistingTableOrViewP2 :: (CacheBuildM m) => QualifiedTable -> SystemDefined -> Bool -> TableConfig -> m EncJSON trackExistingTableOrViewP2 tableName systemDefined isEnum config = do @@ -182,11 +153,20 @@ instance FromJSON SetTableCustomFields where runSetTableCustomFieldsQV2 :: (CacheBuildM m, UserInfoM m) => SetTableCustomFields -> m EncJSON runSetTableCustomFieldsQV2 (SetTableCustomFields tableName rootFields columnNames) = do adminOnly - void $ askTabInfo tableName + fields <- _tiFieldInfoMap <$> askTabInfo tableName let tableConfig = TableConfig rootFields columnNames + withPathK "custom_column_names" $ validateWithNonColumnFields fields updateTableConfig tableName tableConfig buildSchemaCacheFor (MOTable tableName) return successMsg + where + validateWithNonColumnFields fields = do + let customNames = M.elems columnNames + nonColumnFields = possibleNonColumnGraphQLFields fields + conflictingNames = customNames `intersect` nonColumnFields + when (not $ null conflictingNames) $ throw400 NotSupported $ + "the following custom column names conflict with existing non-column fields: " + <> showNames conflictingNames unTrackExistingTableOrViewP1 :: (CacheRM m, UserInfoM m, QErrM m) => UntrackTable -> m () @@ -419,8 +399,9 @@ buildTableCache = processTableCache <=< buildRawTableCache , _tiDescription = maybeDesc } - -- validate tableConfig - withPathK "configuration" $ validateTableConfig info config + -- validate custom column names with existing columns + withPathK "configuration" $ + validateWithExistingColumns columnFields $ _tcCustomColumnNames config pure (name, info) -- Step 2: Process the raw table cache to replace Postgres column types with logical column @@ -435,6 +416,23 @@ buildTableCache = processTableCache <=< buildRawTableCache where enumTables = M.mapMaybe _tiEnumValues rawTables + validateWithExistingColumns :: FieldInfoMap PGRawColumnInfo -> CustomColumnNames -> m () + validateWithExistingColumns columnFields customColumnNames = do + withPathK "custom_column_names" $ do + -- Check all keys are valid columns + forM_ (M.keys customColumnNames) $ \col -> void $ askPGColInfo columnFields col "" + let columns = getCols columnFields + defaultNameMap = M.fromList $ flip map columns $ + \col -> ( prciName col + , G.Name $ getPGColTxt $ prciName col + ) + customNames = M.elems $ defaultNameMap `M.union` customColumnNames + conflictingCustomNames = duplicates customNames + + when (not $ null conflictingCustomNames) $ throw400 NotSupported $ + "the following custom column names are conflicting: " <> showNames conflictingCustomNames + + -- | “Processes” a 'PGRawColumnInfo' into a 'PGColumnInfo' by resolving its type using a map of known -- enum tables. diff --git a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs index 49f8ba400fa..6a80f28e394 100644 --- a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs +++ b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs @@ -55,6 +55,7 @@ module Hasura.RQL.Types.SchemaCache , getCols , getRels , getComputedFieldInfos + , possibleNonColumnGraphQLFields , isPGColInfo , RelInfo(..) @@ -144,6 +145,7 @@ import Language.Haskell.TH.Syntax (Lift) import qualified Data.HashMap.Strict as M import qualified Data.HashSet as HS import qualified Data.Text as T +import qualified Language.GraphQL.Draft.Syntax as G reportSchemaObjs :: [SchemaObjId] -> T.Text reportSchemaObjs = T.intercalate ", " . map reportSchemaObj @@ -176,6 +178,18 @@ $(makePrisms ''FieldInfo) type FieldInfoMap columnInfo = M.HashMap FieldName (FieldInfo columnInfo) +possibleNonColumnGraphQLFields :: FieldInfoMap PGColumnInfo -> [G.Name] +possibleNonColumnGraphQLFields fields = + flip concatMap (M.toList fields) $ \case + (_, FIColumn _) -> [] + (_, FIRelationship relInfo) -> + let relationshipName = G.Name $ relNameToTxt $ riName relInfo + in case riType relInfo of + ObjRel -> [relationshipName] + ArrRel -> [relationshipName, relationshipName <> "_aggregate"] + (_, FIComputedField info) -> + pure $ G.Name $ computedFieldNameToText $ _cfiName info + getCols :: FieldInfoMap columnInfo -> [columnInfo] getCols = mapMaybe (^? _FIColumn) . M.elems @@ -366,17 +380,25 @@ $(makeLenses ''TableInfo) checkForFieldConflict :: (MonadError QErr m) - => TableInfo a + => TableInfo PGColumnInfo -> FieldName -> m () -checkForFieldConflict tabInfo f = - case M.lookup f (_tiFieldInfoMap tabInfo) of +checkForFieldConflict tableInfo f = do + case M.lookup f fieldInfoMap of Just _ -> throw400 AlreadyExists $ mconcat - [ "column/relationship " <>> f - , " of table " <>> _tiName tabInfo + [ "column/relationship/computed field " <>> f + , " of table " <>> tableName , " already exists" ] Nothing -> return () + when (f `elem` customColumnFields) $ + throw400 AlreadyExists $ + "custom column name " <> f <<> " of table " <> tableName <<> " already exists" + where + tableName = _tiName tableInfo + fieldInfoMap = _tiFieldInfoMap tableInfo + customColumnFields = + map (FieldName . G.unName . pgiName) $ getCols fieldInfoMap type TableCache columnInfo = M.HashMap QualifiedTable (TableInfo columnInfo) -- info of all tables type FunctionCache = M.HashMap QualifiedFunction FunctionInfo -- info of all functions diff --git a/server/tests-py/queries/v1/computed_fields/add_computed_field_errors.yaml b/server/tests-py/queries/v1/computed_fields/add_computed_field_errors.yaml index 31adb664b1f..d70acb6855c 100644 --- a/server/tests-py/queries/v1/computed_fields/add_computed_field_errors.yaml +++ b/server/tests-py/queries/v1/computed_fields/add_computed_field_errors.yaml @@ -25,7 +25,7 @@ function: full_name response: path: "$.args.name" - error: column/relationship "first_name" of table "author" already exists + error: column/relationship/computed field "first_name" of table "author" already exists code: already-exists - description: Try adding computed field with invalid function diff --git a/server/tests-py/queries/v1/set_table_custom_fields/column_field_swap.yaml b/server/tests-py/queries/v1/set_table_custom_fields/column_field_swap.yaml new file mode 100644 index 00000000000..1ef1cd168b3 --- /dev/null +++ b/server/tests-py/queries/v1/set_table_custom_fields/column_field_swap.yaml @@ -0,0 +1,71 @@ +- description: Set custom column names for article table by swaping the column names + url: /v1/query + status: 200 + response: + message: success + query: + type: set_table_custom_fields + version: 2 + args: + table: article + custom_root_fields: {} + custom_column_names: + title: content + content: title + +- description: Perform graphql query + url: /v1/graphql + status: 200 + response: + data: + article: + - id: 1 + title: Article 1 content + content: Article 1 title + - id: 2 + title: Article 2 content + content: Article 2 title + query: + query: | + query { + article{ + id + title + content + } + } + +- description: Unset the custom column names + url: /v1/query + status: 200 + response: + message: success + query: + type: set_table_custom_fields + version: 2 + args: + table: article + custom_root_fields: {} + custom_column_names: {} + +- description: Peform graphql query + url: /v1/graphql + status: 200 + response: + data: + article: + - id: 1 + title: Article 1 title + content: Article 1 content + - id: 2 + title: Article 2 title + content: Article 2 content + query: + query: | + query { + article{ + id + title + content + } + } diff --git a/server/tests-py/queries/v1/set_table_custom_fields/conflict_with_relationship.yaml b/server/tests-py/queries/v1/set_table_custom_fields/conflict_with_relationship.yaml new file mode 100644 index 00000000000..5e9caa83f60 --- /dev/null +++ b/server/tests-py/queries/v1/set_table_custom_fields/conflict_with_relationship.yaml @@ -0,0 +1,17 @@ +description: Set custom column names conflicting with existing relationship +url: /v1/query +status: 400 +response: + path: "$.args.custom_column_names" + error: 'the following custom column names conflict with existing non-column fields: + articles_aggregate, articles' + code: not-supported +query: + type: set_table_custom_fields + version: 2 + args: + table: author + custom_root_fields: {} + custom_column_names: + name: articles + age: articles_aggregate diff --git a/server/tests-py/queries/v1/set_table_custom_fields/relationship_conflict_with_custom_column.yaml b/server/tests-py/queries/v1/set_table_custom_fields/relationship_conflict_with_custom_column.yaml new file mode 100644 index 00000000000..1a63e3d5ae1 --- /dev/null +++ b/server/tests-py/queries/v1/set_table_custom_fields/relationship_conflict_with_custom_column.yaml @@ -0,0 +1,16 @@ +description: Try to define a relationship with custom column name +url: /v1/query +status: 400 +response: + path: "$.args" + error: custom column name "AuthorId" of table "author" already exists + code: already-exists +query: + type: create_array_relationship + args: + name: AuthorId + table: author + using: + foreign_key_constraint_on: + table: article + column: author_id diff --git a/server/tests-py/queries/v1/set_table_custom_fields/set_and_unset.yaml b/server/tests-py/queries/v1/set_table_custom_fields/set_and_unset.yaml index 47ba3333835..30977687fe6 100644 --- a/server/tests-py/queries/v1/set_table_custom_fields/set_and_unset.yaml +++ b/server/tests-py/queries/v1/set_table_custom_fields/set_and_unset.yaml @@ -14,6 +14,7 @@ custom_column_names: id: AuthorId name: AuthorName + age: age - description: Check that above query has changed the schema url: /v1/graphql diff --git a/server/tests-py/queries/v1/set_table_custom_fields/setup.yaml b/server/tests-py/queries/v1/set_table_custom_fields/setup.yaml index 86705c653f7..c98741e42a7 100644 --- a/server/tests-py/queries/v1/set_table_custom_fields/setup.yaml +++ b/server/tests-py/queries/v1/set_table_custom_fields/setup.yaml @@ -14,6 +14,17 @@ args: ('Clarke', 23), ('Bellamy', NULL); + CREATE TABLE article ( + id SERIAL PRIMARY KEY, + title TEXT NOT NULL, + content TEXT, + author_id INTEGER NOT NULL REFERENCES author(id) + ); + + INSERT INTO article (title, content, author_id) VALUES + ('Article 1 title', 'Article 1 content', 1), + ('Article 2 title', 'Article 2 content', 2); + - type: track_table version: 2 args: @@ -21,3 +32,20 @@ args: configuration: custom_root_fields: select: Authors + custom_column_names: + id: AuthorId + +- type: track_table + version: 2 + args: + table: article + configuration: {} + +- type: create_array_relationship + args: + name: articles + table: author + using: + foreign_key_constraint_on: + table: article + column: author_id diff --git a/server/tests-py/queries/v1/set_table_custom_fields/teardown.yaml b/server/tests-py/queries/v1/set_table_custom_fields/teardown.yaml index 68160e26124..3e33734a3b5 100644 --- a/server/tests-py/queries/v1/set_table_custom_fields/teardown.yaml +++ b/server/tests-py/queries/v1/set_table_custom_fields/teardown.yaml @@ -3,5 +3,6 @@ args: - type: run_sql args: sql: | + DROP TABLE article; DROP TABLE author; cascade: true diff --git a/server/tests-py/test_v1_queries.py b/server/tests-py/test_v1_queries.py index f1a5c34cd9d..a89b41b93d1 100644 --- a/server/tests-py/test_v1_queries.py +++ b/server/tests-py/test_v1_queries.py @@ -686,6 +686,15 @@ class TestSetTableCustomFields(DefaultTestQueries): def test_alter_column(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/alter_column.yaml') + def test_conflict_with_relationship(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/conflict_with_relationship.yaml') + + def test_column_field_swap(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + "/column_field_swap.yaml") + + def test_relationship_conflict_with_custom_column(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + "/relationship_conflict_with_custom_column.yaml") + class TestComputedFields(DefaultTestQueries): @classmethod def dir(cls):