allow reusing Postgres scalars in custom types & actions (close #4125) (

#4333)

* allow re-using Postgres scalars in custom types, close #4125

* add pytest tests

* update CHANGELOG.md

* add a doc pointer for reusable postgres scalars

* document the code, improve the CHANGELOG entry

As suggested by @lexi-lambda

* a bit more source code documentation, use WriterT to collect reused scalars

* Apply suggestions from code review

Co-Authored-By: Marion Schleifer <marion@hasura.io>

* improve doc for Postgres scalars in custom graphql types

* Add some more references to Note; fix Haddock syntax

Also a few very minor tweaks:
  * Use HashSet instead of [] more pervasively
  * Export execWriterT from Hasura.Prelude
  * Use pattern guards in multi-way if
  * Tweak a few names/comments

* Pull buildActions out of buildAndCollectInfo, use buildInfoMap

* Tweak wording in documentation

* incorporate changes in console code

* account Postgres scalars for action input arguments

-> Avoid unnecessary 'throw500' in making action schema

* Review changes

Co-authored-by: Marion Schleifer <marion@hasura.io>
Co-authored-by: Alexis King <lexi.lambda@gmail.com>
Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com>
Co-authored-by: Aleksandra Sikora <ola.zxcvbnm@gmail.com>

CHANGELOG.md

@@ -10,6 +10,7 @@ The order and collapsed state of columns is now persisted across page navigation
 
 ### Bug fixes and improvements
 
+- server: support reusing Postgres scalars in custom types (close #4125)
 - cli: set_table_is_enum metadata type for squashing migrations (close #4394) (#4395)
 - console: query support for actions (#4318)
 - cli: query support for actions (#4318)

console/src/components/Services/Actions/Common/utils.js

@@ -231,7 +231,7 @@ export const getActionTypes = (currentAction, allTypes) => {
     const type = findType(allTypes, typename);
     actionTypes[typename] = type;
 
-    if (type.fields) {
+    if (type && type.fields) {
       type.fields.forEach(f => {
         getDependentTypes(f.type);
         if (f.arguments) {
@@ -268,7 +268,7 @@ export const getOverlappingTypeConfirmation = (
     const action = otherActions[i];
     const actionTypes = getActionTypes(action, allTypes);
     actionTypes.forEach(t => {
-      if (typeCollisionMap[t.name]) return;
+      if (!t || typeCollisionMap[t.name]) return;
       overlappingTypenames.forEach(ot => {
         if (ot === t.name) {
           typeCollisionMap[ot] = true;

console/src/shared/utils/deriveAction.js

@@ -117,6 +117,10 @@ const deriveAction = (
   const allHasuraTypes = clientSchema._typeMap;
   const operationType = getOperationType(clientSchema, operation);
 
+  const isHasuraScalar = name => {
+    return isScalarType(allHasuraTypes[name]);
+  };
+
   const actionArguments = [];
   const newTypes = {};
 
@@ -128,7 +132,7 @@ const deriveAction = (
     newType.name = typename;
 
     if (isScalarType(type)) {
-      if (!inbuiltTypes[type.name]) {
+      if (!inbuiltTypes[type.name] && !allHasuraTypes[type.name]) {
         newType.kind = 'scalar';
         newTypes[typename] = newType;
       }
@@ -156,7 +160,10 @@ const deriveAction = (
           type: underLyingType,
           wraps: fieldTypeWraps,
         } = getUnderlyingType(tf.type);
-        if (inbuiltTypes[underLyingType.name]) {
+        if (
+          inbuiltTypes[underLyingType.name] ||
+          isHasuraScalar(underLyingType.name)
+        ) {
           _tf.type = wrapTypename(underLyingType.name, fieldTypeWraps);
         } else {
           _tf.type = wrapTypename(
@@ -177,7 +184,10 @@ const deriveAction = (
       name: v.variable.name.value,
     };
     const argTypeMetadata = getAstTypeMetadata(v.type);
-    if (!inbuiltTypes[argTypeMetadata.typename]) {
+    if (
+      !inbuiltTypes[argTypeMetadata.typename] &&
+      !isHasuraScalar(argTypeMetadata.typename)
+    ) {
       const argTypename = prefixTypename(argTypeMetadata.typename);
       generatedArg.type = wrapTypename(argTypename, argTypeMetadata.stack);
       const typeInSchema = allHasuraTypes[argTypeMetadata.typename];
@@ -208,19 +218,10 @@ const deriveAction = (
       outputTypeField => {
         const fieldTypeMetadata = getUnderlyingType(outputTypeField.type);
         if (isScalarType(fieldTypeMetadata.type)) {
-          if (inbuiltTypes[fieldTypeMetadata.type.name]) {
-            outputTypeFields[outputTypeField.name] = wrapTypename(
-              fieldTypeMetadata.type.name,
-              fieldTypeMetadata.wraps
-            );
-          } else {
-            const fieldTypename = prefixTypename(fieldTypeMetadata.type.name);
-            outputTypeFields[outputTypeField.name] = wrapTypename(
-              fieldTypename,
-              fieldTypeMetadata.wraps
-            );
-            handleType(fieldTypeMetadata.type, fieldTypename);
-          }
+          outputTypeFields[outputTypeField.name] = wrapTypename(
+            fieldTypeMetadata.type.name,
+            fieldTypeMetadata.wraps
+          );
         }
       }
     );

console/src/shared/utils/sdlUtils.js

@@ -258,6 +258,7 @@ ${enumValuesSdl.join('\n')}
 };
 
 const getTypeSdl = type => {
+  if (!type) return '';
   switch (type.kind) {
     case 'scalar':
       return getScalarTypeSdl(type);

docs/graphql/manual/actions/types/index.rst

@@ -141,6 +141,22 @@ a scalar called ``Date``, you can define it like.
 These scalars can be used as arguments of the mutation or as fields of object
 types and input types.
 
+.. admonition:: Postgres scalars
+
+   Postgres base types are implicitly made available as GraphQL scalars; there
+   is no need to declare them separately. For example, in the definition
+
+   .. code-block:: graphql
+
+       type User {
+         id: uuid!
+         name: String!
+         location: geography
+       }
+
+   the ``uuid`` and ``geography`` types are assumed to refer to Postgres
+   scalars (assuming no other definition for them is provided).
+
 Enum types
 ----------
 
@@ -165,4 +181,3 @@ This means that wherever we use the type ``Color`` in our schema, we expect it
 to be exactly one of RED, GREEN, or BLUE.
 
 `See reference <https://graphql.org/learn/schema/#enumeration-types>`__
-

server/src-lib/Hasura/GraphQL/Resolve/Action.hs

@@ -295,7 +295,7 @@ asyncActionsProcessor cacheRef pgPool httpManager = forever $ do
         Nothing -> return ()
         Just actionInfo -> do
           let definition = _aiDefinition actionInfo
-              outputFields = _aiOutputFields actionInfo
+              outputFields = getActionOutputFields $ _aiOutputObject actionInfo
               webhookUrl = _adHandler definition
               forwardClientHeaders = _adForwardClientHeaders definition
               confHeaders = _adHeaders definition

server/src-lib/Hasura/GraphQL/Schema.hs

@@ -720,11 +720,11 @@ noFilter = annBoolExpTrue
 
 mkGCtxMap
   :: forall m. (MonadError QErr m)
-  => AnnotatedObjects -> TableCache -> FunctionCache -> ActionCache -> m GCtxMap
-mkGCtxMap annotatedObjects tableCache functionCache actionCache = do
+  => TableCache -> FunctionCache -> ActionCache -> m GCtxMap
+mkGCtxMap tableCache functionCache actionCache = do
   typesMapL <- mapM (mkGCtxMapTable tableCache functionCache) $
                filter (tableFltr . _tiCoreInfo) $ Map.elems tableCache
-  actionsSchema <- mkActionsSchema annotatedObjects actionCache
+  let actionsSchema = mkActionsSchema actionCache
   typesMap <- combineTypes actionsSchema typesMapL
   let gCtxMap  = flip Map.map typesMap $
                  \(ty, flds, insCtxMap) -> mkGCtx ty flds insCtxMap

server/src-lib/Hasura/GraphQL/Schema/Action.hs

@@ -3,6 +3,7 @@ module Hasura.GraphQL.Schema.Action
   ) where
 
 import qualified Data.HashMap.Strict           as Map
+import qualified Data.HashSet                  as Set
 import qualified Language.GraphQL.Draft.Syntax as G
 
 import           Data.Coerce                   (coerce)
@@ -68,14 +69,14 @@ mkMutationField actionName actionInfo definitionList =
         ActionSynchronous  ->
           ActionExecutionSyncWebhook $ SyncActionExecutionContext actionName
           (_adOutputType definition)
-          (_aiOutputFields actionInfo)
+          (getActionOutputFields $ _aiOutputObject actionInfo)
           definitionList
           (_adHandler definition)
           (_adHeaders definition)
           (_adForwardClientHeaders definition)
         ActionAsynchronous -> ActionExecutionAsync
 
-    description = mkDescriptionWith (PGDescription <$> (_aiComment actionInfo)) $
+    description = mkDescriptionWith (PGDescription <$> _aiComment actionInfo) $
                   "perform the action: " <>> actionName
 
     fieldInfo =
@@ -123,22 +124,21 @@ mkQueryField actionName comment definition definitionList =
         idDescription = G.Description $ "id of the action: " <>> actionName
 
 mkActionFieldsAndTypes
-  :: (QErrM m)
-  => ActionInfo
-  -> AnnotatedObjectType
+  :: ActionInfo
   -> ActionPermissionInfo
-  -> m ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo)
+  -> ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo)
        -- context, field, response type info
      , (ActionExecutionContext, ObjFldInfo) -- mutation field
      , FieldMap
      )
-mkActionFieldsAndTypes actionInfo annotatedOutputType permission =
-  return ( mkQueryField actionName comment definition definitionList
-         , mkMutationField actionName actionInfo definitionList
-         , fieldMap
-         )
+mkActionFieldsAndTypes actionInfo permission =
+  ( mkQueryField actionName comment definition definitionList
+  , mkMutationField actionName actionInfo definitionList
+  , fieldMap
+  )
   where
     actionName = _aiName actionInfo
+    annotatedOutputType = _aiOutputObject actionInfo
     definition = _aiDefinition actionInfo
     roleName = _apiRole permission
     comment = _aiComment actionInfo
@@ -220,46 +220,38 @@ mkActionFieldsAndTypes actionInfo annotatedOutputType permission =
       G.getBaseType $ unGraphQLType $ _adOutputType $ _aiDefinition actionInfo
 
 mkActionSchemaOne
-  :: (QErrM m)
-  => AnnotatedObjects
-  -> ActionInfo
-  -> m (Map.HashMap RoleName
-         ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo)
-         , (ActionExecutionContext, ObjFldInfo)
-         , FieldMap
-         )
+  :: ActionInfo
+  -> Map.HashMap RoleName
+       ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo)
+       , (ActionExecutionContext, ObjFldInfo)
+       , FieldMap
        )
-mkActionSchemaOne annotatedObjects actionInfo = do
-  annotatedOutputType <- onNothing
-      (Map.lookup (ObjectTypeName actionOutputBaseType) annotatedObjects) $
-      throw500 $ "missing annotated type for: " <> showNamedTy actionOutputBaseType
-  forM permissions $ \permission ->
-    mkActionFieldsAndTypes actionInfo annotatedOutputType permission
+mkActionSchemaOne actionInfo =
+  flip Map.map permissions $ \permission ->
+    mkActionFieldsAndTypes actionInfo permission
   where
     adminPermission = ActionPermissionInfo adminRole
     permissions = Map.insert adminRole adminPermission $ _aiPermissions actionInfo
-    actionOutputBaseType =
-      G.getBaseType $ unGraphQLType $ _adOutputType $ _aiDefinition actionInfo
 
 mkActionsSchema
-  :: (QErrM m)
-  => AnnotatedObjects
-  -> ActionCache
-  -> m (Map.HashMap RoleName (RootFields, TyAgg))
-mkActionsSchema annotatedObjects =
-  foldM
+  :: ActionCache
+  -> Map.HashMap RoleName (RootFields, TyAgg)
+mkActionsSchema =
+  foldl'
   (\aggregate actionInfo ->
-     Map.foldrWithKey f aggregate <$>
-     mkActionSchemaOne annotatedObjects actionInfo
+     Map.foldrWithKey (accumulate (_aiPgScalars actionInfo)) aggregate $ mkActionSchemaOne actionInfo
   )
   mempty
   where
     -- we'll need to add uuid and timestamptz for actions
-    newRoleState = (mempty, addScalarToTyAgg PGJSON $
-                            addScalarToTyAgg PGTimeStampTZ $
-                            addScalarToTyAgg PGUUID mempty)
-    f roleName (queryFieldM, mutationField, fields) =
-      Map.alter (Just . addToState . fromMaybe newRoleState) roleName
+    mkNewRoleState pgScalars =
+      ( mempty
+      , foldr addScalarToTyAgg mempty $
+        pgScalars <> Set.fromList [PGJSON, PGTimeStampTZ, PGUUID]
+      )
+
+    accumulate pgScalars roleName (queryFieldM, mutationField, fields) =
+      Map.alter (Just . addToState . fromMaybe (mkNewRoleState pgScalars)) roleName
       where
         addToState = case queryFieldM of
           Just (fldCtx, fldDefinition, responseTypeInfo) ->

server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs

@@ -31,7 +31,7 @@ buildObjectTypeInfo roleName annotatedObjectType =
       \(TypeRelationship name ty remoteTableInfo _) ->
         if isJust (getSelectPermissionInfoM remoteTableInfo roleName) ||
            roleName == adminRole
-        then Just (relationshipToFieldInfo name ty $ _tciName $ _tiCoreInfo $ remoteTableInfo)
+        then Just (relationshipToFieldInfo name ty $ _tciName $ _tiCoreInfo remoteTableInfo)
         else Nothing
       where
         relationshipToFieldInfo name relTy remoteTableName =
@@ -116,14 +116,18 @@ annotateObjectType tableCache nonObjectTypeMap objectDefinition = do
 
 buildCustomTypesSchemaPartial
   :: (QErrM m)
-  => TableCache -> CustomTypes -> m (NonObjectTypeMap, AnnotatedObjects)
-buildCustomTypesSchemaPartial tableCache customTypes = do
+  => TableCache
+  -> CustomTypes
+  -> HashSet PGScalarType
+  -- ^ Postgres base types used in the custom type definitions;
+  -- see Note [Postgres scalars in custom types].
+  -> m (NonObjectTypeMap, AnnotatedObjects)
+buildCustomTypesSchemaPartial tableCache customTypes pgScalars = do
   let typeInfos =
         map (VT.TIEnum . convertEnumDefinition) enumDefinitions <>
-        -- map (VT.TIObj . convertObjectDefinition) objectDefinitions <>
         map (VT.TIInpObj . convertInputObjectDefinition) inputObjectDefinitions <>
-        map (VT.TIScalar . convertScalarDefinition) scalarDefinitions
-        -- <> defaultTypes
+        map (VT.TIScalar . convertScalarDefinition) scalarDefinitions <>
+        map (VT.TIScalar . VT.mkHsraScalarTyInfo) (toList pgScalars)
       nonObjectTypeMap = NonObjectTypeMap $ mapFromL VT.getNamedTy typeInfos
 
   annotatedObjectTypes <- mapFromL (_otdName . _aotDefinition) <$>

server/src-lib/Hasura/Prelude.hs

@@ -28,7 +28,8 @@ import           Control.Monad.Fail                as M (MonadFail)
 import           Control.Monad.Identity            as M
 import           Control.Monad.Reader              as M
 import           Control.Monad.State.Strict        as M
-import           Control.Monad.Writer.Strict       as M (MonadWriter (..), WriterT (..))
+import           Control.Monad.Writer.Strict       as M (MonadWriter (..), WriterT (..),
+                                                         execWriterT, runWriterT)
 import           Data.Align                        as M (Align (align, alignWith))
 import           Data.Align.Key                    as M (AlignWithKey (..))
 import           Data.Bool                         as M (bool)