From 0ba38d0cf36251aab615b1cc015826d2bcf4d4c2 Mon Sep 17 00:00:00 2001 From: Brandon Simmons Date: Sat, 3 Oct 2020 18:31:00 -0400 Subject: [PATCH] Support tracking VOLATILE SQL functions as mutations. Closes #1514 --- CHANGELOG.md | 1 + .../schema-metadata-api/custom-functions.rst | 40 +++++++-- docs/graphql/core/schema/custom-functions.rst | 27 +++++- server/src-lib/Hasura/GraphQL/Context.hs | 3 + .../Hasura/GraphQL/Execute/Mutation.hs | 29 ++----- .../src-lib/Hasura/GraphQL/Execute/Query.hs | 30 +++++++ server/src-lib/Hasura/GraphQL/Schema.hs | 82 +++++++++++++------ server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs | 4 +- .../src-lib/Hasura/RQL/DDL/Schema/Function.hs | 43 ++++++++-- server/src-lib/Hasura/RQL/Types/Function.hs | 18 ++-- .../src-lib/Hasura/RQL/Types/SchemaCache.hs | 2 +- .../functions/function_as_mutations.yaml | 65 +++++++++++++++ .../function_as_mutations_permissions.yaml | 61 ++++++++++++++ .../functions/schema_setup.yaml | 64 +++++++++++++++ .../functions/schema_teardown.yaml | 8 ++ .../functions/values_setup.yaml | 11 +++ .../functions/values_teardown.yaml | 7 ++ .../track_function_v2_mutation_errors.yaml | 80 ++++++++++++++++++ server/tests-py/test_graphql_mutations.py | 18 ++++ server/tests-py/test_graphql_queries.py | 4 + server/tests-py/validate.py | 2 +- 21 files changed, 527 insertions(+), 72 deletions(-) create mode 100644 server/tests-py/queries/graphql_mutation/functions/function_as_mutations.yaml create mode 100644 server/tests-py/queries/graphql_mutation/functions/function_as_mutations_permissions.yaml create mode 100644 server/tests-py/queries/graphql_mutation/functions/schema_setup.yaml create mode 100644 server/tests-py/queries/graphql_mutation/functions/schema_teardown.yaml create mode 100644 server/tests-py/queries/graphql_mutation/functions/values_setup.yaml create mode 100644 server/tests-py/queries/graphql_mutation/functions/values_teardown.yaml create mode 100644 server/tests-py/queries/graphql_query/functions/track_function_v2_mutation_errors.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f86a03e64cb1..6a42d78bbb4a1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ This release contains the [PDV refactor (#4111)](https://github.com/hasura/graph - server: allow remote relationships joining `type` column with `[type]` input argument as spec allows this coercion (fixes #5133) - server: add action-like URL templating for event triggers and remote schemas (fixes #2483) - server: change `created_at` column type from `timestamp` to `timestamptz` for scheduled triggers tables (fix #5722) +- server: Support tracking VOLATILE SQL functions as mutations. (closing #1514) - server: allow configuring timeouts for actions (fixes #4966) - server: accept only non-negative integers for batch size and refetch interval (close #5653) (#5759) - server: limit the length of event trigger names (close #5786) diff --git a/docs/graphql/core/api-reference/schema-metadata-api/custom-functions.rst b/docs/graphql/core/api-reference/schema-metadata-api/custom-functions.rst index a71bb557a52cce..b23c741e08ff4b 100644 --- a/docs/graphql/core/api-reference/schema-metadata-api/custom-functions.rst +++ b/docs/graphql/core/api-reference/schema-metadata-api/custom-functions.rst @@ -24,7 +24,7 @@ Only tracked custom functions are available for querying/mutating/subscribing da track_function -------------- -``track_function`` is used to add a custom SQL function to the GraphQL schema. +``track_function`` is used to add a custom SQL function to the ``query`` root field of the GraphQL schema. Also refer a note :ref:`here `. Add an SQL function ``search_articles``: @@ -48,10 +48,12 @@ Add an SQL function ``search_articles``: track_function v2 ----------------- -Version 2 of ``track_function`` is used to add a custom SQL function to the GraphQL schema with configuration. +Version 2 of ``track_function`` is used to add a custom SQL function to the GraphQL schema. +Its support more configuration options than v1, and also supports tracking +``VOLATILE`` functions as mutations. Also refer a note :ref:`here `. -Add an SQL function called ``search_articles`` with a Hasura session argument. +Track a SQL function called ``search_articles`` with a Hasura session argument: .. code-block:: http @@ -73,6 +75,29 @@ Add an SQL function called ``search_articles`` with a Hasura session argument. } } +Track ``VOLATILE`` SQL function ``reset_widget`` as a mutation, so it appears +as a top-level field under the ``mutation`` root field: + +.. code-block:: http + + POST /v1/query HTTP/1.1 + Content-Type: application/json + X-Hasura-Role: admin + + { + "type": "track_function", + "version": 2, + "args": { + "function": { + "schema": "public", + "name": "reset_widget" + }, + "configuration": { + "as_mutation": true + } + } + } + .. _track_function_args_syntax_v2: Args syntax @@ -110,6 +135,10 @@ Function Configuration - false - `String` - Function argument which accepts session info JSON + * - as_mutation + - false + - `Bool` + - Should this function be exposed as a mutation? If true, the function must be VOLATILE. .. _note: @@ -118,8 +147,9 @@ Function Configuration Currently, only functions which satisfy the following constraints can be exposed over the GraphQL API (*terminology from* `Postgres docs `__): - - **Function behaviour**: ONLY ``STABLE`` or ``IMMUTABLE`` - - **Return type**: MUST be ``SETOF `` + - **Function behaviour**: ``STABLE`` or ``IMMUTABLE`` functions may *only* be exposed as queries + (i.e. with ``as_mutation: false``), while ``VOLATILE`` functions may only be exposed as mutations. + - **Return type**: MUST be ``SETOF `` where ```` is already tracked - **Argument modes**: ONLY ``IN`` .. _untrack_function: diff --git a/docs/graphql/core/schema/custom-functions.rst b/docs/graphql/core/schema/custom-functions.rst index cc401cea4c063f..25ac7a788c6b3a 100644 --- a/docs/graphql/core/schema/custom-functions.rst +++ b/docs/graphql/core/schema/custom-functions.rst @@ -20,7 +20,7 @@ that can be used to either encapsulate some custom business logic or extend the are also referred to as **stored procedures**. Hasura GraphQL engine lets you expose certain types of custom functions as top level fields in the GraphQL API to allow -querying them using both ``queries`` and ``subscriptions``. +querying them as either ``queries`` or ``subscriptions``, or (for ``VOLATILE`` functions) as ``mutations``. .. note:: @@ -34,8 +34,9 @@ Supported SQL functions Currently, only functions which satisfy the following constraints can be exposed as top level fields in the GraphQL API (*terminology from* `Postgres docs `__): -- **Function behaviour**: ONLY ``STABLE`` or ``IMMUTABLE`` -- **Return type**: MUST be ``SETOF `` +- **Function behaviour**: ``STABLE`` or ``IMMUTABLE`` functions may *only* be exposed as queries + (i.e. with ``as_mutation: false``), while ``VOLATILE`` functions may only be exposed as mutations. +- **Return type**: MUST be ``SETOF `` where ```` is already tracked - **Argument modes**: ONLY ``IN`` .. _create_sql_functions: @@ -551,6 +552,26 @@ following example. The specified session argument will not be included in the ``_args`` input object in the GraphQL schema. + +Using functions as mutations +**************************** + +You can also use the :ref:`track_function_v2 ` API to track +functions as mutations. In this case the SQL function *must* be marked +``VOLATILE`` (this is the default if volatility isn't specified; see +`the postgres docs `). + +Aside from showing up under the ``mutation`` root (and presumably having +side-effects), these behave the same as described above for ``queries``. + +.. note:: + + It's easy to accidentally give a SQL function the wrong volatility (or for a + function to end up with ``VOLATILE`` mistakenly, since it's the default). + That's one reason we require specifying ``as_mutation``: it lets us raise an + error rather than do something unintended. + + Permissions for custom function queries --------------------------------------- diff --git a/server/src-lib/Hasura/GraphQL/Context.hs b/server/src-lib/Hasura/GraphQL/Context.hs index 742364d665e468..5503080428eea6 100644 --- a/server/src-lib/Hasura/GraphQL/Context.hs +++ b/server/src-lib/Hasura/GraphQL/Context.hs @@ -106,6 +106,9 @@ data MutationDB (b :: Backend) v = MDBInsert (AnnInsert b v) | MDBUpdate (RQL.AnnUpdG b v) | MDBDelete (RQL.AnnDelG b v) + | MDBFunction (RQL.AnnSimpleSelG b v) + -- ^ This represents a VOLATILE function, and is AnnSimpleSelG for easy + -- re-use of non-VOLATILE function tracking code. data ActionMutation (b :: Backend) v = AMSync !(RQL.AnnActionExecution b v) diff --git a/server/src-lib/Hasura/GraphQL/Execute/Mutation.hs b/server/src-lib/Hasura/GraphQL/Execute/Mutation.hs index bd4da97aac4035..a61f9002cb9467 100644 --- a/server/src-lib/Hasura/GraphQL/Execute/Mutation.hs +++ b/server/src-lib/Hasura/GraphQL/Execute/Mutation.hs @@ -29,6 +29,8 @@ import Hasura.GraphQL.Context import Hasura.GraphQL.Execute.Action import Hasura.GraphQL.Execute.Insert import Hasura.GraphQL.Execute.Prepare +-- We borrow some Query code to handle the case of VOLATILE functions: +import Hasura.GraphQL.Execute.Query import Hasura.GraphQL.Execute.Remote import Hasura.GraphQL.Execute.Resolve import Hasura.GraphQL.Parser @@ -92,24 +94,6 @@ convertInsert env usrVars remoteJoinCtx insertOperation stringifyNum = do validateSessionVariables expectedVariables usrVars pure $ convertToSQLTransaction env preparedInsert remoteJoinCtx Seq.empty stringifyNum -convertMutationDB - :: ( HasVersion - , MonadIO m - , MonadError QErr m - , Tracing.MonadTrace tx - , MonadIO tx - , MonadTx tx - ) - => Env.Environment - -> SessionVariables - -> RQL.MutationRemoteJoinCtx - -> Bool - -> MutationDB 'Postgres UnpreparedValue - -> m (tx EncJSON, HTTP.ResponseHeaders) -convertMutationDB env userSession remoteJoinCtx stringifyNum = \case - MDBInsert s -> noResponseHeaders <$> convertInsert env userSession remoteJoinCtx s stringifyNum - MDBUpdate s -> noResponseHeaders <$> convertUpdate env userSession remoteJoinCtx s stringifyNum - MDBDelete s -> noResponseHeaders <$> convertDelete env userSession remoteJoinCtx s stringifyNum noResponseHeaders :: tx EncJSON -> (tx EncJSON, HTTP.ResponseHeaders) noResponseHeaders rTx = (rTx, []) @@ -159,7 +143,7 @@ convertMutationSelectionSet -> [G.VariableDefinition] -> Maybe GH.VariableValues -> m (ExecutionPlan (tx EncJSON, HTTP.ResponseHeaders)) -convertMutationSelectionSet env logger gqlContext sqlGenCtx userInfo manager reqHeaders fields varDefs varValsM = do +convertMutationSelectionSet env logger gqlContext SQLGenCtx{stringifyNum} userInfo manager reqHeaders fields varDefs varValsM = do mutationParser <- onNothing (gqlMutationParser gqlContext) $ throw400 ValidationFailed "no mutations exist" -- Parse the GraphQL query into the RQL AST @@ -172,7 +156,12 @@ convertMutationSelectionSet env logger gqlContext sqlGenCtx userInfo manager req let userSession = _uiSession userInfo remoteJoinCtx = (manager, reqHeaders, userInfo) txs <- for unpreparedQueries \case - RFDB db -> ExecStepDB <$> convertMutationDB env userSession remoteJoinCtx (stringifyNum sqlGenCtx) db + RFDB db -> ExecStepDB . noResponseHeaders <$> case db of + MDBInsert s -> convertInsert env userSession remoteJoinCtx s stringifyNum + MDBUpdate s -> convertUpdate env userSession remoteJoinCtx s stringifyNum + MDBDelete s -> convertDelete env userSession remoteJoinCtx s stringifyNum + MDBFunction s -> convertFunction env userInfo manager reqHeaders s + RFRemote (remoteSchemaInfo, remoteField) -> pure $ buildExecStepRemote remoteSchemaInfo diff --git a/server/src-lib/Hasura/GraphQL/Execute/Query.hs b/server/src-lib/Hasura/GraphQL/Execute/Query.hs index 8364175566f73c..60cafbe0c70592 100644 --- a/server/src-lib/Hasura/GraphQL/Execute/Query.hs +++ b/server/src-lib/Hasura/GraphQL/Execute/Query.hs @@ -1,5 +1,6 @@ module Hasura.GraphQL.Execute.Query ( convertQuerySelSet + , convertFunction -- , queryOpFromPlan -- , ReusableQueryPlan , PreparedSql(..) @@ -294,6 +295,35 @@ convertQuerySelSet env logger gqlContext userInfo manager reqHeaders directives AQAsync s -> AQPAsyncQuery <$> DS.traverseAnnSimpleSelect prepareWithPlan (resolveAsyncActionQuery userInfo s) +-- | A pared-down version of 'convertQuerySelSet', for use in execution of +-- special case of SQL function mutations (see 'MDBFunction'). +convertFunction + :: forall m tx . + ( MonadError QErr m + , HasVersion + , MonadIO tx + , MonadTx tx + , Tracing.MonadTrace tx + ) + => Env.Environment + -> UserInfo + -> HTTP.Manager + -> HTTP.RequestHeaders + -> DS.AnnSimpleSelG 'Postgres UnpreparedValue + -- ^ VOLATILE function as 'SelectExp' + -> m (tx EncJSON) +convertFunction env userInfo manager reqHeaders unpreparedQuery = do + -- Transform the RQL AST into a prepared SQL query + (preparedQuery, PlanningSt _ _ planVals expectedVariables) + <- flip runStateT initPlanningSt + $ DS.traverseAnnSimpleSelect prepareWithPlan unpreparedQuery + validateSessionVariables expectedVariables $ _uiSession userInfo + + pure $! + fst $ -- forget (Maybe PreparedSql) + mkCurPlanTx env manager reqHeaders userInfo id noProfile $ + RFPPostgres $ irToRootFieldPlan planVals $ QDBSimple preparedQuery + -- See Note [Temporarily disabling query plan caching] -- use the existing plan and new variables to create a pg query -- queryOpFromPlan diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index 36ec80ef316eba..e907ad2647bc3a 100644 --- a/server/src-lib/Hasura/GraphQL/Schema.hs +++ b/server/src-lib/Hasura/GraphQL/Schema.hs @@ -15,6 +15,7 @@ import Control.Arrow.Extended import Control.Lens.Extended import Control.Monad.Unique import Data.Has +import Data.List (partition) import Data.List.Extended (duplicates) import qualified Hasura.GraphQL.Parser as P @@ -74,11 +75,15 @@ buildGQLContext = functionFilter = not . isSystemDefined . fiSystemDefined validTables = Map.filter (tableFilter . _tiCoreInfo) allTables - validFunctions = Map.elems $ Map.filter functionFilter allFunctions + -- VOLATILE functions end up under the mutation root; see 'mkFunctionInfo': + validFunctions@(validVolatileFunctions, _) = + partition ((== FTVOLATILE) . fiType) $ + Map.elems $ Map.filter functionFilter allFunctions allActionInfos = Map.elems allActions queryRemotesMap = fmap (map fDefinition . piQuery . rscParsed . fst) allRemoteSchemas + buildFullestDBSchema :: m ( Parser 'Output (P.ParseT Identity) (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)) , Maybe (Parser 'Output (P.ParseT Identity) (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue))) @@ -90,7 +95,8 @@ buildGQLContext = <$> queryWithIntrospection (Set.fromMap $ validTables $> ()) validFunctions mempty mempty allActionInfos nonObjectCustomTypes - <*> mutation (Set.fromMap $ validTables $> ()) mempty + <*> mutation (Set.fromMap $ validTables $> ()) + validVolatileFunctions mempty allActionInfos nonObjectCustomTypes flip runReaderT (adminRoleName, validTables, Frontend, QueryContext stringifyNum queryType queryRemotesMap) $ P.runSchemaT gqlContext @@ -104,6 +110,7 @@ buildGQLContext = P.TNamed (P.Definition _ _ _ (P.TIObject (P.ObjectInfo rootFields _interfaces))) -> pure $ fmap P.dName rootFields _ -> throw500 "We encountered an root query of unexpected GraphQL type. It should be an object type." + let mutationFieldNames :: [G.Name] mutationFieldNames = case P.discardNullability . P.parserType <$> snd adminHasuraContext of @@ -174,7 +181,8 @@ buildGQLContext = SQLGenCtx{ stringifyNum } <- askSQLGenCtx let gqlContext = GQLContext <$> (finalizeParser <$> queryHasuraOrRelay) - <*> (fmap finalizeParser <$> mutation (Set.fromList $ Map.keys validTables) mutationRemotes + <*> (fmap finalizeParser <$> mutation (Set.fromList $ Map.keys validTables) + validVolatileFunctions mutationRemotes allActionInfos nonObjectCustomTypes) flip runReaderT (roleName, validTables, scenario, QueryContext stringifyNum queryType queryRemotesMap) $ P.runSchemaT gqlContext @@ -211,7 +219,7 @@ query' -> [ActionInfo 'Postgres] -> NonObjectTypeMap -> m [P.FieldParser n (QueryRootField UnpreparedValue)] -query' allTables allFunctions allRemotes allActions nonObjectCustomTypes = do +query' allTables nonVolatileFunctions allRemotes allActions nonObjectCustomTypes = do tableSelectExpParsers <- for (toList allTables) \table -> do selectPerms <- tableSelectPermissions table customRootFields <- _tcCustomRootFields . _tciCustomConfig . _tiCoreInfo <$> askTableInfo table @@ -227,7 +235,8 @@ query' allTables allFunctions allRemotes allActions nonObjectCustomTypes = do , mapMaybeFieldParser (RFDB . QDBPrimaryKey) $ selectTableByPk table (fromMaybe pkName $ _tcrfSelectByPk customRootFields) (Just pkDesc) perms , mapMaybeFieldParser (RFDB . QDBAggregation) $ selectTableAggregate table (fromMaybe aggName $ _tcrfSelectAggregate customRootFields) (Just aggDesc) perms ] - functionSelectExpParsers <- for allFunctions \function -> do + + functionSelectExpParsers <- for nonVolatileFunctions \function -> do let targetTable = fiReturnType function functionName = fiName function selectPerms <- tableSelectPermissions targetTable @@ -250,13 +259,14 @@ query' allTables allFunctions allRemotes allActions nonObjectCustomTypes = do pure $ (concat . catMaybes) (tableSelectExpParsers <> functionSelectExpParsers <> toRemoteFieldParser allRemotes) <> catMaybes actionParsers where - requiredFieldParser :: (a -> b) -> m (P.FieldParser n a) -> m (Maybe (P.FieldParser n b)) - requiredFieldParser f = fmap $ Just . fmap f - mapMaybeFieldParser :: (a -> b) -> m (Maybe (P.FieldParser n a)) -> m (Maybe (P.FieldParser n b)) mapMaybeFieldParser f = fmap $ fmap $ fmap f toRemoteFieldParser p = [Just $ fmap (fmap RFRemote) p] + +requiredFieldParser :: (Functor n, Functor m)=> (a -> b) -> m (P.FieldParser n a) -> m (Maybe (P.FieldParser n b)) +requiredFieldParser f = fmap $ Just . fmap f + -- | Similar to @query'@ but for Relay. relayQuery' @@ -269,7 +279,7 @@ relayQuery' => HashSet QualifiedTable -> [FunctionInfo] -> m [P.FieldParser n (QueryRootField UnpreparedValue)] -relayQuery' allTables allFunctions = do +relayQuery' allTables nonVolatileFunctions = do tableConnectionSelectParsers <- for (toList allTables) $ \table -> runMaybeT do pkeyColumns <- MaybeT $ (^? tiCoreInfo.tciPrimaryKey._Just.pkColumns) @@ -281,7 +291,7 @@ relayQuery' allTables allFunctions = do lift $ selectTableConnection table fieldName fieldDesc pkeyColumns selectPerms functionConnectionSelectParsers <- - for allFunctions $ \function -> runMaybeT do + for nonVolatileFunctions $ \function -> runMaybeT do let returnTable = fiReturnType function functionName = fiName function pkeyColumns <- MaybeT $ (^? tiCoreInfo.tciPrimaryKey._Just.pkColumns) @@ -307,8 +317,8 @@ query -> [ActionInfo 'Postgres] -> NonObjectTypeMap -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))) -query name allTables allFunctions allRemotes allActions nonObjectCustomTypes = do - queryFieldsParser <- query' allTables allFunctions allRemotes allActions nonObjectCustomTypes +query name allTables nonVolatileFunctions allRemotes allActions nonObjectCustomTypes = do + queryFieldsParser <- query' allTables nonVolatileFunctions allRemotes allActions nonObjectCustomTypes P.safeSelectionSet name Nothing queryFieldsParser <&> fmap (fmap (P.handleTypename (RFRaw . J.String . G.unName))) @@ -319,8 +329,8 @@ subscription -> [FunctionInfo] -> [ActionInfo 'Postgres] -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))) -subscription allTables allFunctions asyncActions = - query $$(G.litName "subscription_root") allTables allFunctions [] asyncActions mempty +subscription allTables nonVolatileFunctions asyncActions = + query $$(G.litName "subscription_root") allTables nonVolatileFunctions [] asyncActions mempty queryRootFromFields :: forall n m @@ -402,16 +412,17 @@ queryWithIntrospection , Has Scenario r ) => HashSet QualifiedTable - -> [FunctionInfo] + -> ([FunctionInfo], [FunctionInfo]) -> [P.FieldParser n RemoteField] -> [P.FieldParser n RemoteField] -> [ActionInfo 'Postgres] -> NonObjectTypeMap -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))) -queryWithIntrospection allTables allFunctions queryRemotes mutationRemotes allActions nonObjectCustomTypes = do - basicQueryFP <- query' allTables allFunctions queryRemotes allActions nonObjectCustomTypes - mutationP <- mutation allTables mutationRemotes allActions nonObjectCustomTypes - subscriptionP <- subscription allTables allFunctions $ +queryWithIntrospection allTables (volatileFunctions, nonVolatileFunctions) queryRemotes + mutationRemotes allActions nonObjectCustomTypes = do + basicQueryFP <- query' allTables nonVolatileFunctions queryRemotes allActions nonObjectCustomTypes + mutationP <- mutation allTables volatileFunctions mutationRemotes allActions nonObjectCustomTypes + subscriptionP <- subscription allTables nonVolatileFunctions $ filter (has (aiDefinition.adType._ActionMutation._ActionAsynchronous)) allActions queryWithIntrospectionHelper basicQueryFP mutationP subscriptionP @@ -424,12 +435,12 @@ relayWithIntrospection , Has Scenario r ) => HashSet QualifiedTable - -> [FunctionInfo] + -> ([FunctionInfo], [FunctionInfo]) -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))) -relayWithIntrospection allTables allFunctions = do +relayWithIntrospection allTables (volatileFunctions, nonVolatileFunctions) = do nodeFP <- fmap (RFDB . QDBPrimaryKey) <$> nodeField - basicQueryFP <- relayQuery' allTables allFunctions - mutationP <- mutation allTables [] [] mempty + basicQueryFP <- relayQuery' allTables nonVolatileFunctions + mutationP <- mutation allTables volatileFunctions [] [] mempty emptyIntro <- emptyIntrospection let relayQueryFP = nodeFP:basicQueryFP basicQueryP <- queryRootFromFields relayQueryFP @@ -478,12 +489,14 @@ mutation :: forall m n r . (MonadSchema n m, MonadTableInfo r m, MonadRole r m, Has QueryContext r, Has Scenario r) => HashSet QualifiedTable + -> [FunctionInfo] + -- ^ valid VOLATILE functions to be added under the graphql schema mutation root -> [P.FieldParser n RemoteField] -> [ActionInfo 'Postgres] -> NonObjectTypeMap -> m (Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue)))) -mutation allTables allRemotes allActions nonObjectCustomTypes = do - mutationParsers <- for (toList allTables) \table -> do +mutation allTables volatileFunctions allRemotes allActions nonObjectCustomTypes = do + tableMutationParsers <- for (toList allTables) \table -> do tableCoreInfo <- _tiCoreInfo <$> askTableInfo table displayName <- qualifiedObjectToName table tablePerms <- tablePermissions table @@ -539,6 +552,20 @@ mutation allTables allRemotes allActions nonObjectCustomTypes = do pure $ fmap (RFDB . MDBDelete) delete : maybe [] (pure . fmap (RFDB . MDBDelete)) deleteByPk pure $ concat $ catMaybes [inserts, updates, deletes] + + -- NOTE: this is basically copied from functionSelectExpParsers body + functionMutationExpParsers <- for volatileFunctions \function@FunctionInfo{..} -> do + selectPerms <- tableSelectPermissions fiReturnType + for selectPerms \perms -> do + displayName <- qualifiedObjectToName fiName + let functionDesc = G.Description $ + "execute VOLATILE function " <> fiName <<> " which returns " <>> fiReturnType + catMaybes <$> sequenceA + [ requiredFieldParser (RFDB . MDBFunction) $ + selectFunction function displayName (Just functionDesc) perms + -- FWIW: The equivalent of this is possible for mutations; do we want that?: + -- , mapMaybeFieldParser (RFDB . QDBAggregation) $ selectFunctionAggregate function aggName (Just aggDesc) perms + ] actionParsers <- for allActions $ \actionInfo -> case _adType (_aiDefinition actionInfo) of @@ -548,7 +575,10 @@ mutation allTables allRemotes allActions nonObjectCustomTypes = do fmap (fmap (RFAction . AMAsync)) <$> actionAsyncMutation nonObjectCustomTypes actionInfo ActionQuery -> pure Nothing - let mutationFieldsParser = concat (catMaybes mutationParsers) <> catMaybes actionParsers <> fmap (fmap RFRemote) allRemotes + let mutationFieldsParser = + concat (catMaybes tableMutationParsers) <> + concat (catMaybes functionMutationExpParsers) <> + catMaybes actionParsers <> fmap (fmap RFRemote) allRemotes if null mutationFieldsParser then pure Nothing else fmap Just $ P.safeSelectionSet $$(G.litName "mutation_root") (Just $ G.Description "mutation root") mutationFieldsParser diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs index 11cacdf3a0fc29..2dbf821a05e4b0 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs @@ -40,7 +40,7 @@ data FunctionMeta = FunctionMeta { fmOid :: !OID , fmFunction :: !QualifiedFunction - , fmType :: !FunctionType + , fmType :: !FunctionVolatility } deriving (Show, Eq) $(deriveJSON (aesonDrop 2 snakeCase) ''FunctionMeta) @@ -217,7 +217,7 @@ fetchFunctionMeta = data FunctionDiff = FunctionDiff { fdDropped :: ![QualifiedFunction] - , fdAltered :: ![(QualifiedFunction, FunctionType)] + , fdAltered :: ![(QualifiedFunction, FunctionVolatility)] } deriving (Show, Eq) getFuncDiff :: [FunctionMeta] -> [FunctionMeta] -> FunctionDiff diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs index c1c05952d7792a..636a41caab0207 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs @@ -31,7 +31,7 @@ import Hasura.Server.Utils (englishList, makeReasonMess data RawFunctionInfo = RawFunctionInfo { rfiHasVariadic :: !Bool - , rfiFunctionType :: !FunctionType + , rfiFunctionType :: !FunctionVolatility , rfiReturnTypeSchema :: !SchemaName , rfiReturnTypeName :: !PGScalarType , rfiReturnTypeType :: !PGTypeKind @@ -78,7 +78,8 @@ data FunctionIntegrityError | FunctionReturnNotCompositeType | FunctionReturnNotSetof | FunctionReturnNotSetofTable - | FunctionVolatile + | FunctionVolatilityMismatch !Bool + -- ^ Bool indicates: user requested a mutation | FunctionSessionArgumentNotJSON !FunctionArgName | FunctionInvalidSessionArgument !FunctionArgName | FunctionInvalidArgumentNames [FunctionArgName] @@ -91,7 +92,7 @@ mkFunctionInfo -> FunctionConfig -> RawFunctionInfo -> m (FunctionInfo, SchemaDependency) -mkFunctionInfo qf systemDefined config rawFuncInfo = +mkFunctionInfo qf systemDefined FunctionConfig{..} rawFuncInfo = either (throw400 NotSupported . showErrors) pure =<< MV.runValidateT validateFunction where @@ -110,7 +111,12 @@ mkFunctionInfo qf systemDefined config rawFuncInfo = when (retTyTyp /= PGKindComposite) $ throwValidateError FunctionReturnNotCompositeType unless retSet $ throwValidateError FunctionReturnNotSetof unless returnsTab $ throwValidateError FunctionReturnNotSetofTable - when (funTy == FTVOLATILE) $ throwValidateError FunctionVolatile + -- Only VOLATILE functions can be exposed as mutations; after validating + -- here we just use the volatility info in 'FunctionInfo' to decide + -- whether it should go under the query or mutation root: + let askedMutation = _fcAsMutation == Just True + when (funTy == FTVOLATILE && not askedMutation || funTy /= FTVOLATILE && askedMutation) $ + throwValidateError $ FunctionVolatilityMismatch askedMutation -- validate function argument names validateFunctionArgNames @@ -130,7 +136,7 @@ mkFunctionInfo qf systemDefined config rawFuncInfo = throwValidateError $ FunctionInvalidArgumentNames invalidArgs makeInputArguments = - case _fcSessionArgument config of + case _fcSessionArgument of Nothing -> pure $ Seq.fromList $ map IAUserProvided functionArgs Just sessionArgName -> do when (not $ any (\arg -> (Just sessionArgName) == faName arg) functionArgs) $ @@ -152,7 +158,9 @@ mkFunctionInfo qf systemDefined config rawFuncInfo = FunctionReturnNotCompositeType -> "the function does not return a \"COMPOSITE\" type" FunctionReturnNotSetof -> "the function does not return a SETOF" FunctionReturnNotSetofTable -> "the function does not return a SETOF table" - FunctionVolatile -> "function of type \"VOLATILE\" is not supported now" + FunctionVolatilityMismatch requestedMutation + | requestedMutation -> "only \"VOLATILE\" functions can be tracked as mutations" + | otherwise -> "the function is \"VOLATILE\" (the default) and can only be tracked as a mutation, with \"as_mutation: true\"" FunctionSessionArgumentNotJSON argName -> "given session argument " <> argName <<> " is not of type json" FunctionInvalidSessionArgument argName -> @@ -188,13 +196,28 @@ newtype TrackFunction data FunctionConfig = FunctionConfig { _fcSessionArgument :: !(Maybe FunctionArgName) + -- ^ The argument, if any, to the _tfv2Function to which we'll pass the + -- user's session variables during execution. This must be a JSON type + -- argument, and disappears from the resulting graphql schema. + , _fcAsMutation :: !(Maybe Bool) + -- ^ Do we intend this function to end up under the 'mutation' top-level field? + -- + -- We require the 'as_mutation' parameter from users in order to verify their + -- intent; since VOLATILE is the default when creating a function, it would + -- be easy for users to create a function then track it expecting it to be a + -- query, only to find it ended up under the 'mutation' root field. + -- + -- This is Maybe so we can derive instances, with Nothing implying False. } deriving (Show, Eq, Generic, Lift) instance NFData FunctionConfig instance Cacheable FunctionConfig $(deriveJSON (aesonDrop 3 snakeCase){omitNothingFields = True} ''FunctionConfig) +-- ^ TODO consider rejectUnknownFields for this and all API JSON (this could +-- easily be hiding bugs in our test suite, for one thing) +-- | The default function config; v1 of the API implies this. emptyFunctionConfig :: FunctionConfig -emptyFunctionConfig = FunctionConfig Nothing +emptyFunctionConfig = FunctionConfig Nothing Nothing -- | Track function, Phase 1: -- Validate function tracking operation. Fails if function is already being @@ -244,6 +267,9 @@ runTrackFunc (TrackFunction qf)= do trackFunctionP1 qf trackFunctionP2 qf emptyFunctionConfig +-- | JSON API payload for v2 of 'track_function': +-- +-- https://hasura.io/docs/1.0/graphql/core/api-reference/schema-metadata-api/custom-functions.html#track-function-v2 data TrackFunctionV2 = TrackFunctionV2 { _tfv2Function :: !QualifiedFunction @@ -266,6 +292,9 @@ runTrackFunctionV2 (TrackFunctionV2 qf config) = do trackFunctionP1 qf trackFunctionP2 qf config +-- | JSON API payload for 'untrack_function': +-- +-- https://hasura.io/docs/1.0/graphql/core/api-reference/schema-metadata-api/custom-functions.html#untrack-function newtype UnTrackFunction = UnTrackFunction { utfName :: QualifiedFunction } diff --git a/server/src-lib/Hasura/RQL/Types/Function.hs b/server/src-lib/Hasura/RQL/Types/Function.hs index 81d824e03faf35..8913f0ba2f0984 100644 --- a/server/src-lib/Hasura/RQL/Types/Function.hs +++ b/server/src-lib/Hasura/RQL/Types/Function.hs @@ -17,21 +17,22 @@ import Hasura.Incremental (Cacheable) import Hasura.RQL.Types.Common -data FunctionType +-- | https://www.postgresql.org/docs/current/xfunc-volatility.html +data FunctionVolatility = FTVOLATILE | FTIMMUTABLE | FTSTABLE deriving (Eq, Generic) -instance NFData FunctionType -instance Cacheable FunctionType -$(deriveJSON defaultOptions{constructorTagModifier = drop 2} ''FunctionType) +instance NFData FunctionVolatility +instance Cacheable FunctionVolatility +$(deriveJSON defaultOptions{constructorTagModifier = drop 2} ''FunctionVolatility) -funcTypToTxt :: FunctionType -> Text +funcTypToTxt :: FunctionVolatility -> Text funcTypToTxt FTVOLATILE = "VOLATILE" funcTypToTxt FTIMMUTABLE = "IMMUTABLE" funcTypToTxt FTSTABLE = "STABLE" -instance Show FunctionType where +instance Show FunctionVolatility where show = T.unpack . funcTypToTxt newtype FunctionArgName = @@ -68,9 +69,12 @@ data FunctionInfo = FunctionInfo { fiName :: !QualifiedFunction , fiSystemDefined :: !SystemDefined - , fiType :: !FunctionType + , fiType :: !FunctionVolatility , fiInputArgs :: !(Seq.Seq FunctionInputArgument) , fiReturnType :: !QualifiedTable + -- ^ NOTE: when a table is created, a new composite type of the same name is + -- automatically created; so strictly speaking this field means "the function + -- returns the composite type corresponding to this table". , fiDescription :: !(Maybe PGDescription) } deriving (Show, Eq) $(deriveToJSON (aesonDrop 2 snakeCase) ''FunctionInfo) diff --git a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs index 46631b6653f372..2f0fa6bf17a640 100644 --- a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs +++ b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs @@ -104,7 +104,7 @@ module Hasura.RQL.Types.SchemaCache , getDependentObjs , getDependentObjsWith - , FunctionType(..) + , FunctionVolatility(..) , FunctionArg(..) , FunctionArgName(..) , FunctionName(..) diff --git a/server/tests-py/queries/graphql_mutation/functions/function_as_mutations.yaml b/server/tests-py/queries/graphql_mutation/functions/function_as_mutations.yaml new file mode 100644 index 00000000000000..424a2c2053a316 --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/functions/function_as_mutations.yaml @@ -0,0 +1,65 @@ +- description: Test that a tracked VOLATILE function works as a mutation + url: /v1/graphql + status: 200 + query: + query: | + mutation { + add_to_score(args: {search: "Bla", increment: 3}){ + name + score + session_echo + } + } + response: + data: + add_to_score: + - name: Starke Blake + score: 3 + session_echo: + x-hasura-role: admin + - name: Bellamy Blake + score: 13 + session_echo: + x-hasura-role: admin + - name: Dora Black + score: 53 + session_echo: + x-hasura-role: admin + +- description: Test that defaults in SQL function become default parameters in schema + url: /v1/graphql + status: 200 + query: + # We omit 'increment' here, defaulting to 1 + query: | + mutation { + add_to_score(args: {search: "Blake"}){ + name + score + } + } + response: + data: + add_to_score: + - name: Starke Blake + score: 4 + - name: Bellamy Blake + score: 14 + +- description: Sanity check that VOLATILE function didn't end up in query root too + url: /v1/graphql + # TODO shouldnt this return fail status? + status: 200 + query: + query: | + query { + add_to_score(args: {search: "Bla", increment: 3}){ + name + } + } + response: + errors: + - extensions: + path: $.selectionSet.add_to_score + code: validation-failed + message: "field \"add_to_score\" not found in type: 'query_root'" diff --git a/server/tests-py/queries/graphql_mutation/functions/function_as_mutations_permissions.yaml b/server/tests-py/queries/graphql_mutation/functions/function_as_mutations_permissions.yaml new file mode 100644 index 00000000000000..eed80cb9bf7bdc --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/functions/function_as_mutations_permissions.yaml @@ -0,0 +1,61 @@ +- description: Works as admin + url: /v1/graphql + status: 200 + query: + query: | + mutation { + add_to_score(args: {search: "Black"}){ + name + score + session_echo + } + } + response: + data: + add_to_score: + - name: Dora Black + score: 51 + session_echo: + x-hasura-role: admin + +- description: Fails as anonymous due to permissions set up previously + headers: + X-Hasura-Role: anonymous + url: /v1/graphql + status: 200 + query: + query: | + mutation { + add_to_score(args: {search: "Black"}){ + name + score + session_echo + } + } + response: + # We also expect that the side-effectful function wasn't run (see below) + errors: + - extensions: + path: $.selectionSet.add_to_score.selectionSet.session_echo + code: validation-failed + message: "field \"session_echo\" not found in type: 'user'" + +- description: Works with permitted columns + headers: + X-Hasura-Role: anonymous + url: /v1/graphql + status: 200 + query: + query: | + mutation { + add_to_score(args: {search: "Black"}){ + name + score + } + } + response: + data: + add_to_score: + - name: Dora Black + # NOTE: the function didn't run above (good): + score: 52 diff --git a/server/tests-py/queries/graphql_mutation/functions/schema_setup.yaml b/server/tests-py/queries/graphql_mutation/functions/schema_setup.yaml new file mode 100644 index 00000000000000..91e951a6ddd07d --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/functions/schema_setup.yaml @@ -0,0 +1,64 @@ +# Test VOLATILE SQL functions exposed as top-level fields under the mutation root field +# (#1514) +type: bulk +args: + +# test functions having multiple defaults +- type: run_sql + args: + sql: | + CREATE TABLE "user" ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL, + score INTEGER, + /* We just return the session vars as this column from our function + * to show they're passed through properly. + * + * NOTE: with the addition of function "tracking" we probably want to + * logically be defining permissions on composite types (which might + * or might not have been created implicitly in a CREATE TABLE). + * + * See: https://github.com/hasura/graphql-engine-internal/issues/502 + */ + session_echo JSON DEFAULT '{}' + ); + +# Adds a value (defaulting to 1) to users matching 'search', returning updated +# rows and echoing the hasura session vars. +- type: run_sql + args: + sql: | + CREATE FUNCTION add_to_score(hasura_session json, search text, increment integer default 1) + RETURNS SETOF "user" AS $$ + UPDATE "user" + SET score = score + increment + WHERE name ilike ('%' || search || '%') + RETURNING id, name, score, hasura_session AS session_echo + $$ LANGUAGE sql VOLATILE; + +- type: track_table + args: + name: user + schema: public + +- type: track_function + version: 2 + args: + function: + schema: public + name: add_to_score + configuration: + as_mutation: true + session_argument: hasura_session + +# We'll use this to check that permissions on "user" table are applied to the +# return set of the tracked function: +- type: create_select_permission + args: + table: user + role: anonymous + permission: + filter: {} + columns: + - name + - score diff --git a/server/tests-py/queries/graphql_mutation/functions/schema_teardown.yaml b/server/tests-py/queries/graphql_mutation/functions/schema_teardown.yaml new file mode 100644 index 00000000000000..727c776d025a8e --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/functions/schema_teardown.yaml @@ -0,0 +1,8 @@ +type: bulk +args: +# Drop table and function from postgres +- type: run_sql + args: + sql: | + DROP TABLE "user" cascade; + cascade: true diff --git a/server/tests-py/queries/graphql_mutation/functions/values_setup.yaml b/server/tests-py/queries/graphql_mutation/functions/values_setup.yaml new file mode 100644 index 00000000000000..2de2b90ee8e327 --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/functions/values_setup.yaml @@ -0,0 +1,11 @@ +type: bulk +args: + +- type: run_sql + args: + sql: | + INSERT INTO "user" (name, score) VALUES + ('Starke Blake', 0) + , ('Bellamy Blake', 10) + , ('Dora Black', 50) + ; diff --git a/server/tests-py/queries/graphql_mutation/functions/values_teardown.yaml b/server/tests-py/queries/graphql_mutation/functions/values_teardown.yaml new file mode 100644 index 00000000000000..f66721fcb9653a --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/functions/values_teardown.yaml @@ -0,0 +1,7 @@ +type: bulk +args: + +- type: run_sql + args: + sql: | + TRUNCATE "user"; diff --git a/server/tests-py/queries/graphql_query/functions/track_function_v2_mutation_errors.yaml b/server/tests-py/queries/graphql_query/functions/track_function_v2_mutation_errors.yaml new file mode 100644 index 00000000000000..d8ec317746ed4d --- /dev/null +++ b/server/tests-py/queries/graphql_query/functions/track_function_v2_mutation_errors.yaml @@ -0,0 +1,80 @@ +- description: setup a custom SQL function + url: /v1/query + status: 200 + query: + type: run_sql + args: + sql: | + /* STABLE function we'll try to add as mutation */ + CREATE FUNCTION stable_func() + RETURNS SETOF text_result AS $$ + SELECT * FROM text_result; + $$ LANGUAGE sql STABLE; + /* VOLATILE function (by default) we'll try to add as query */ + CREATE FUNCTION volatile_func() + RETURNS SETOF text_result AS $$ + SELECT * FROM text_result; + $$ LANGUAGE sql; + +# We take the 'as_mutation' parameter from users in order to verify their +# intent; since VOLATILE is the default when creating a function, it would be +# easy for users to create a function then track it expecting it to be a query, +# only to find it ended up under the 'mutation' root field. +- description: Track function v2 as query, but function is VOLATILE + url: /v1/query + status: 400 + response: + internal: + - definition: + schema: public + name: volatile_func + reason: 'in function "volatile_func": the function "volatile_func" cannot be + tracked because the function is "VOLATILE" (the default) and can only be tracked + as a mutation, with "as_mutation: true"' + type: function + path: $.args + error: 'in function "volatile_func": the function "volatile_func" cannot be tracked + because the function is "VOLATILE" (the default) and can only be tracked as + a mutation, with "as_mutation: true"' + code: constraint-violation + query: + version: 2 + type: track_function + args: + function: volatile_func + configuration: + as_mutation: false + +- description: Track function v2 as mutation, but function is non-VOLATILE + url: /v1/query + status: 400 + response: + internal: + - definition: + schema: public + name: stable_func + reason: 'in function "stable_func": the function "stable_func" cannot be tracked + because only "VOLATILE" functions can be tracked as mutations' + type: function + path: $.args + error: 'in function "stable_func": the function "stable_func" cannot be tracked + because only "VOLATILE" functions can be tracked as mutations' + code: constraint-violation + query: + version: 2 + type: track_function + args: + function: stable_func + configuration: + as_mutation: true + +- description: teardown function + url: /v1/query + status: 200 + query: + type: run_sql + args: + sql: | + DROP FUNCTION stable_func; + DROP FUNCTION volatile_func; + cascade: true diff --git a/server/tests-py/test_graphql_mutations.py b/server/tests-py/test_graphql_mutations.py index 32637904b997cd..9c013838e87b16 100644 --- a/server/tests-py/test_graphql_mutations.py +++ b/server/tests-py/test_graphql_mutations.py @@ -567,3 +567,21 @@ def test_update_where_enum_field(self, hge_ctx, transport): def test_delete_where_enum_field(self, hge_ctx, transport): check_query_f(hge_ctx, self.dir() + '/delete_where_enum_field.yaml', transport) + +# Tracking VOLATILE SQL functions as mutations (#1514) +@pytest.mark.parametrize('transport', ['http', 'websocket']) +@use_mutation_fixtures +class TestGraphQLMutationFunctions: + @classmethod + def dir(cls): + return 'queries/graphql_mutation/functions' + + # Test tracking a VOLATILE function as top-level field of mutation root + # field, also smoke testing basic permissions on the table return type. + def test_functions_as_mutations(self, hge_ctx, transport): + check_query_f(hge_ctx, self.dir() + '/function_as_mutations.yaml', transport) + + # Ensure select permissions on the corresponding SETOF table apply to + # the return set of the mutation field backed by the tracked function. + def test_functions_as_mutations_permissions(self, hge_ctx, transport): + check_query_f(hge_ctx, self.dir() + '/function_as_mutations_permissions.yaml', transport) diff --git a/server/tests-py/test_graphql_queries.py b/server/tests-py/test_graphql_queries.py index aaa50f19246e41..3f864dc700c990 100644 --- a/server/tests-py/test_graphql_queries.py +++ b/server/tests-py/test_graphql_queries.py @@ -541,6 +541,10 @@ def test_query_get_session_var(self, hge_ctx, transport): def test_track_function_v2_errors(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/track_function_v2_errors.yaml') + # Additional error cases after extending v2 API to support mutations: + def test_track_function_v2_mutation_errors(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/track_function_v2_mutation_errors.yaml') + @pytest.mark.parametrize("transport", ['http', 'websocket']) def test_query_get_test_session_id(self, hge_ctx, transport): check_query_f(hge_ctx, self.dir() + '/query_get_test_session_id.yaml') diff --git a/server/tests-py/validate.py b/server/tests-py/validate.py index a18b311364acf5..c3cced8fc8b5a9 100644 --- a/server/tests-py/validate.py +++ b/server/tests-py/validate.py @@ -267,7 +267,7 @@ def validate_gql_ws_q(hge_ctx, conf, headers, retry=False, via_subscription=Fals def validate_http_anyq(hge_ctx, url, query, headers, exp_code, exp_response): code, resp, resp_hdrs = hge_ctx.anyq(url, query, headers) print(headers) - assert code == exp_code, resp + assert code == exp_code, (code, exp_code, resp) print('http resp: ', resp) if exp_response: return assert_graphql_resp_expected(resp, exp_response, query, resp_hdrs, hge_ctx.avoid_err_msg_checks)