Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server: Don't throw validation error while creating remote relationship joining singular type with array type #5603

Merged
Merged
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ This release contains the [PDV refactor (#4111)](https://github.com/hasura/graph
- if a query selects table `bar` through table `foo` via a relationship, the required permissions headers will be the union of the required headers of table `foo` and table `bar` (we used to only check the headers of the root table);
- if an insert does not have an `on_conflict` clause, it will not require the update permissions headers.


### Bug fixes and improvements

(Add entries here in the order of: server, console, cli, docs, others)

- server: some mutations that cannot be performed will no longer be in the schema (for instance, `delete_by_pk` mutations won't be shown to users that do not have select permissions on all primary keys) (#4111)
- server: miscellaneous description changes (#4111)
- server: treat the absence of `backend_only` configuration and `backend_only: false` equally (closing #5059) (#4111)
- server: allow remote relationships joining `type` column with `[type]` input argument as spec allows this coercion (fixes #5133)
- console: allow user to cascade Postgres dependencies when dropping Postgres objects (close #5109) (#5248)
- console: mark inconsistent remote schemas in the UI (close #5093) (#5181)
- cli: add missing global flags for seeds command (#5565)
Expand Down
10 changes: 10 additions & 0 deletions server/src-lib/Hasura/GraphQL/Utils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module Hasura.GraphQL.Utils
, mkMapWith
, showNames
, simpleGraphQLQuery
, getBaseTyWithNestedLevelsCount
) where

import Hasura.Prelude
Expand All @@ -17,6 +18,15 @@ import qualified Language.GraphQL.Draft.Syntax as G
showName :: G.Name -> Text
showName name = "\"" <> G.unName name <> "\""

getBaseTyWithNestedLevelsCount :: G.GType -> (G.Name, Int)
getBaseTyWithNestedLevelsCount ty = go ty 0
where
go :: G.GType -> Int -> (G.Name, Int)
go gType ctr =
case gType of
G.TypeNamed _ n -> (n, ctr)
G.TypeList _ gType' -> flip go (ctr + 1) gType'

groupListWith
:: (Eq k, Hashable k, Foldable t, Functor t)
=> (v -> k) -> t v -> Map.HashMap k (NE.NonEmpty v)
Expand Down
41 changes: 23 additions & 18 deletions server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module Hasura.RQL.DDL.RemoteRelationship.Validate
import Data.Foldable
import Hasura.GraphQL.Schema.Remote
import Hasura.GraphQL.Parser.Column
import Hasura.GraphQL.Utils (getBaseTyWithNestedLevelsCount)
import Hasura.Prelude hiding (first)
import Hasura.RQL.Types
import Hasura.SQL.Types
Expand Down Expand Up @@ -333,20 +334,20 @@ validateType permittedVariables value expectedGType schemaDocument =
Nothing -> throwError (InvalidVariable variable permittedVariables)
Just fieldInfo -> do
namedType <- columnInfoToNamedType fieldInfo
assertType (mkGraphQLType namedType) expectedGType
isTypeCoercible (mkGraphQLType namedType) expectedGType
G.VInt {} -> do
intScalarGType <- (mkGraphQLType <$> mkScalarTy PGInteger)
assertType intScalarGType expectedGType
isTypeCoercible intScalarGType expectedGType
G.VFloat {} -> do
floatScalarGType <- (mkGraphQLType <$> mkScalarTy PGFloat)
assertType floatScalarGType expectedGType
isTypeCoercible floatScalarGType expectedGType
G.VBoolean {} -> do
boolScalarGType <- (mkGraphQLType <$> mkScalarTy PGBoolean)
assertType boolScalarGType expectedGType
isTypeCoercible boolScalarGType expectedGType
G.VNull -> throwError NullNotAllowedHere
G.VString {} -> do
stringScalarGType <- (mkGraphQLType <$> mkScalarTy PGText)
assertType stringScalarGType expectedGType
isTypeCoercible stringScalarGType expectedGType
G.VEnum _ -> throwError UnsupportedEnum
G.VList values -> do
case values of
Expand Down Expand Up @@ -390,23 +391,27 @@ validateType permittedVariables value expectedGType schemaDocument =
Left _ -> throwError $ InvalidGraphQLName $ toSQLTxt scalarType
Right s -> pure s

assertType
isTypeCoercible
:: (MonadError ValidationError m)
=> G.GType
-> G.GType
-> m ()
assertType actualType expectedType = do
-- check if both are list types or both are named types
(when
(G.isListType actualType /= G.isListType expectedType)
(throwError $ ExpectedTypeButGot expectedType actualType))
-- if list type then check over unwrapped type, else check base types
if G.isListType actualType
then assertType (unwrapGraphQLType actualType) (unwrapGraphQLType expectedType)
else (when
(G.getBaseType actualType /= G.getBaseType expectedType)
(throwError $ ExpectedTypeButGot expectedType actualType))
pure ()
isTypeCoercible actualType expectedType =
-- The GraphQL spec says that, a singleton type can be coerced into an array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think technically "singleton" type is called named type but we can fix this comment later.

-- type. Which means that if the 'actualType' is a singleton type, like
-- 'Int' we should be able to join this with a remote node, which expects an
-- input argument of type '[Int]'
-- http://spec.graphql.org/June2018/#sec-Type-System.List
let (actualBaseType, actualNestingLevel) = getBaseTyWithNestedLevelsCount actualType
(expectedBaseType, expectedNestingLevel) = getBaseTyWithNestedLevelsCount expectedType
in
if | actualBaseType /= expectedBaseType -> raiseValidationError
-- we cannot coerce two types with different nesting levels,
-- for example, we cannot coerce [Int] to [[Int]]
| (actualNestingLevel == expectedNestingLevel || actualNestingLevel == 0) -> pure ()
| otherwise -> raiseValidationError
where
raiseValidationError = throwError $ ExpectedTypeButGot expectedType actualType

assertListType :: (MonadError ValidationError m) => G.GType -> m ()
assertListType actualType =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
description: Simple Remote relationship with singleton input value type joining to an expected array input type
url: /v1/graphql
status: 200
response:
data:
profiles:
- id: 1
remoteUsers:
- user_id: 1
userMessages:
- id: 1
msg: "You win!"
query:
query: |
query {
profiles(where:{ id: { _eq: 1}}) {
id
remoteUsers {
user_id
userMessages {
id
msg
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
type: bulk
args:
- type: create_remote_relationship
args:
name: remoteUsers
table: profiles
hasura_fields:
- id
remote_schema: my-remote-schema
remote_field:
users:
arguments:
user_ids: "$id"

- type: create_remote_relationship
args:
name: remoteUsersMultipleIds
table: profiles
hasura_fields:
- id
remote_schema: my-remote-schema
remote_field:
users:
arguments:
user_ids: ["$id"]
tirumaraiselvan marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 8 additions & 0 deletions server/tests-py/remote_schemas/nodejs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const typeDefs = gql`
hello: String
messages(where: MessageWhereInpObj, includes: IncludeInpObj): [Message]
user(user_id: Int!): User
users(user_ids: [Int]!): [User]
message(id: Int!) : Message
communications(id: Int): [Communication]
}
Expand Down Expand Up @@ -175,6 +176,13 @@ const resolvers = {
user: (_, { user_id }) => {
return { "user_id": user_id };
},
users: (parent, args, context, info) => {
var results = []
for (userId of args.user_ids) {
results.push({"user_id":userId})
}
return results;
},
communications: (_, { id }) => {
var result = allMessages;
if(id) {
Expand Down
8 changes: 8 additions & 0 deletions server/tests-py/test_remote_relationships.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ def test_create_valid(self, hge_ctx):
st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_multiple_fields.yaml')
assert st_code == 200, resp

st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_joining_singleton_with_array.yaml')
assert st_code == 200, resp

# st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_interface.yaml')
# assert st_code == 200, resp

Expand Down Expand Up @@ -157,6 +160,11 @@ def test_basic_relationship_on_object(self, hge_ctx):
assert st_code == 200, resp
check_query_f(hge_ctx, self.dir() + 'query_with_arr_rel.yaml')

def test_basic_relationship_joining_singleton_to_array(self, hge_ctx):
st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_joining_singleton_with_array.yaml')
assert st_code == 200, resp
check_query_f(hge_ctx, self.dir() + 'basic_relationship_joining_singleton_with_array.yaml')

def test_basic_array(self, hge_ctx):
st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_array.yaml')
assert st_code == 200, resp
Expand Down