Fixing URL parameter for variable <name> not supported bug in REST endpoints for Dates

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4345
GitOrigin-RevId: 0b739530757ee1081d764d9582e3c0e648861d4e

CHANGELOG.md

@@ -10,6 +10,8 @@
 - server: fix parsing remote relationship json definition from 1.x server catalog on migration (fix #7906)
 - server: Don't drop nested typed null fields in actions (fix #8237)
 - server: fixes remote relationships on actions (fix #8399)
+- server: fixes url/query date variable bug in REST endpoints
+- server: makes url/query variables in REST endpoints assume string if other types not applicable
 - console: add remote database relationships for views
 - console: bug fixes for RS-to-RS relationships
 - cli: avoid exporting hasura-specific schemas during hasura init (#8352)

docs-old/graphql/core/api-reference/restified.rst

@@ -65,12 +65,8 @@ Variables can be supplied via route patterns, url query variables, and request b
   * Missing nullable variables are interpreted as ``NULL``
   * Boolean variables are interpreted as ``Boolean``
   * Boolean flags without values e.g. ``/api/rest/myquery?mybool`` are interpreted as ``true``
-  * String variables are interpreted as ``String``
-  * UUID variables are interpreted as ``String``
-  * ID variables are interpreted as ``String``
   * Number, Int, Float, and Double variables are interpreted as ``Number``
-  * **All other types or mismatches currently report variable type errors**
-
+  * **All other types are currently interpreted as ``String``** (Note: Further validation is performed during query execution)
 
 When making a request to this API only one endpoint should match. If multiple endpoints match your request this is considered an error and will report so via a 500 status code. If endpoints exist with your path, but none matching your HTTP method then a 405 error will be returned
 listing the methods that you can use.

server/src-lib/Hasura/Server/Rest.hs

@@ -51,45 +51,23 @@ alignVars defVars parseVars =
     (M.fromList (map (\v -> (G._vdName v, v)) defVars))
     (M.fromList (mapMaybe (\(k, v) -> (,v) <$> G.mkName k) parseVars))
 
+-- | `resolveVar` is responsible for decoding variables sent via REST request.
+-- These can either be via body (represented by Right) or via query-param or URL param (represented by Left).
+-- A variable can be expected, unexpected, or missing (represented by These, This, and That).
 resolveVar :: G.Name -> These G.VariableDefinition (Either Text J.Value) -> Either Text (Maybe Value)
-resolveVar _ (This _expectedVar) = Right Nothing
-resolveVar varName (That _providedVar) = Left $ "Unexpected variable " <> toTxt @G.Name varName
-resolveVar varName (These expectedVar providedVar) =
-  -- TODO: See CustomTypes.hs for SCALAR types
+resolveVar _ (This _expectedVar) = Right Nothing -- If a variable is expected but missing, assign a missing value `Nothing` to it for resolution in query execution. This allows Null defaulting.
+resolveVar varName (That _providedVar) = Left $ "Unexpected variable " <> toTxt @G.Name varName -- If a variable is unexpected but present, throw an error.
+resolveVar _varName (These _expectedVar (Right bodyVar)) = Right (Just bodyVar) -- Variables sent via body can be passed through to execution without parsing.
+resolveVar varName (These expectedVar (Left l)) =
   case G._vdType expectedVar of
-    G.TypeNamed (G.Nullability nullable) typeName -> case providedVar of
-      Right r -> Right (Just r)
-      Left l
-        | typeName == stringScalar -> Right $ Just $ J.String l -- "String" -- Note: Strings don't need to be decoded since the format already matches.
-        | otherwise ->
-          case (J.decodeStrict (T.encodeUtf8 l), nullable) of
-            (Just J.Null, True) -> pure Nothing
-            (decoded, _)
-              | typeName == boolScalar && T.null l -> Right $ Just $ J.Bool True -- Key present but value missing for bools defaults to True.
-              | typeName == G._UUID -> Right $ Just $ J.String l
-              | typeName == G._uuid -> Right $ Just $ J.String l
-              | typeName == idScalar -> Right $ Just $ J.String l -- "ID" -- Note: Console doesn't expose this as a column type.
-              | otherwise -> case decoded of
-                (Just J.Null) -> Left $ "Null or missing value for non-nullable variable: " <> G.unName varName
-                (Just x@(J.Bool _))
-                  | typeName == boolScalar -> pure $ Just x -- "Boolean"
-                  | typeName == G._Bool -> pure $ Just x
-                  | otherwise -> Left $ "Expected " <> toTxt typeName <> " for variable " <> G.unName varName <> " got Bool"
-                (Just x@(J.Number _))
-                  | typeName == intScalar -> pure $ Just x -- "Int"
-                  | typeName == floatScalar -> pure $ Just x -- "Float"
-                  | typeName == G._Number -> pure $ Just x
-                  | typeName == G._Double -> pure $ Just x
-                  | typeName == G._float8 -> pure $ Just x
-                  | typeName == G._numeric -> pure $ Just x
-                  | otherwise -> Left $ "Expected " <> toTxt typeName <> " for variable " <> G.unName varName <> " got Number"
-                _ -> Left ("Type of URL parameter for variable " <> G.unName varName <> " not supported - Consider putting it in the request body: " <> tshow l)
-    -- TODO: This is a fallthrough case and is still required
-    --       but we can move checks for template variables being
-    --       scalars into the schema-cache construction.
-    G.TypeList _ _ -> case providedVar of
-      Right r -> Right (Just r)
-      Left l -> Left ("The variable type for the expected variable " <> toTxt @G.Name varName <> ", with value " <> tshow l <> " is not supported.")
+    G.TypeList _ _ -> Left $ "List variables are not currently supported in URL or Query parameters. (Variable " <> toTxt @G.Name varName <> ", with value " <> tshow l <> ")"
+    G.TypeNamed (G.Nullability nullable) typeName
+      | typeName == boolScalar && T.null l -> Right $ Just $ J.Bool True -- Booleans indicated true by a standalone key.
+      | nullable && T.null l -> Right Nothing -- Missing value, but nullable variable sets value to null.
+      | otherwise -> case J.decodeStrict (T.encodeUtf8 l) of -- We special case parsing of bools and numbers and pass the rest through as literal strings.
+        Just v@(J.Bool _) | typeName `elem` [G._Bool, boolScalar] -> Right $ Just v
+        Just v@(J.Number _) | typeName `elem` [intScalar, floatScalar, G._Number, G._Double, G._float8, G._numeric] -> Right $ Just v
+        _ -> Right $ Just $ J.String l
 
 mkPassthroughRequest :: EndpointMetadata GQLQueryWithText -> VariableValues -> GQLReq GQLQueryText
 mkPassthroughRequest queryx resolvedVariables =

server/tests-py/queries/endpoints/endpoint_pg_date_url_variable.yaml

@@ -0,0 +1,7 @@
+description: Successful call of an endpoint with date and point
+url: /api/rest/with_date/2010-1-1/{"type":"Point","coordinates":[-48.23456,20.12345]}
+method: GET
+status: 200
+query:
+response:
+  test_table: []

server/tests-py/queries/endpoints/setup.yaml

@@ -3,9 +3,12 @@ args:
 - type: run_sql
   args:
     sql: |
+      CREATE EXTENSION IF NOT EXISTS postgis;
       create table test_table(
           first_name text,
           last_name text,
+          created_at date,
+          point GEOMETRY,
           id UUID NOT NULL DEFAULT gen_random_uuid()
       );
 
@@ -38,6 +41,8 @@ args:
         query: "query ($first_name: String!, $last_name:String!) { test_table(where: {first_name: { _eq: $first_name } last_name: { _eq: $last_name }}) { first_name last_name } }"
       - name: query_with_uuid_arg
         query: "query ($id: uuid!) { test_table(where: {id: { _neq: $id }}) { first_name last_name } }"
+      - name: query_with_date_arg
+        query: "query ($date: date!, $point: geometry!) { test_table(where: {created_at: {_gt: $date}, point: {_st_equals: $point}}) { created_at } }"
       - name: query_with_uuid_args
         query: "query ($ids: [uuid!]!) { test_table(where: {id: { _in: $ids }}) { first_name last_name } }"
       - name: simple_subscription
@@ -110,6 +115,17 @@ args:
         collection_name: test_collection
         query_name: query_with_uuid_arg
 
+- type: create_rest_endpoint
+  args:
+    url: with_date/:date/:point
+    name: with_date
+    methods:
+      - GET
+    definition:
+      query:
+        collection_name: test_collection
+        query_name: query_with_date_arg
+
 - type: create_rest_endpoint
   args:
     url: with_uuids
@@ -121,7 +137,6 @@ args:
         collection_name: test_collection
         query_name: query_with_uuid_args
 
-
 - type: create_rest_endpoint
   args:
     url: to_be_dropped

server/tests-py/queries/endpoints/teardown.yaml

@@ -18,6 +18,9 @@ args:
 - type: drop_rest_endpoint
   args:
     name: with_uuid
+- type: drop_rest_endpoint
+  args:
+    name: with_date
 - type: drop_rest_endpoint
   args:
     name: with_template
@@ -28,4 +31,4 @@ args:
 - type: run_sql
   args:
     sql: |
-      drop table test_table
+      drop table test_table

server/tests-py/test_endpoints.py

@@ -87,3 +87,6 @@ class TestCustomEndpoints:
 
     def test_query_validation(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/endpoint_query_validation.yaml', transport)
+
+    def test_endpoint_with_pg_date_url_variable(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + '/endpoint_pg_date_url_variable.yaml', transport)