From 34ffe55864cd12b309d996b7e2d0319bd8534008 Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Tue, 27 Oct 2020 17:41:25 +0530 Subject: [PATCH 1/8] add support for joining to remote interface and union fields --- server/graphql-engine.cabal | 1 + server/src-lib/Hasura/GraphQL/Schema/Remote.hs | 7 ++++--- .../Hasura/RQL/DDL/RemoteRelationship/Validate.hs | 14 ++++++++++---- server/tests-py/test_remote_relationships.py | 13 +++++++------ 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/server/graphql-engine.cabal b/server/graphql-engine.cabal index 3979b6e9b2e87..d04bffa1a3c54 100644 --- a/server/graphql-engine.cabal +++ b/server/graphql-engine.cabal @@ -224,6 +224,7 @@ library -- pretty printer , ansi-wl-pprint + , pretty-simple -- for capturing various metrics , ekg-core diff --git a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs index c2f0d873e36cb..d1b374e61074c 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs @@ -11,6 +11,7 @@ import Hasura.Prelude import qualified Data.HashMap.Strict as Map import qualified Data.HashMap.Strict.Extended as Map import qualified Data.HashMap.Strict.InsOrd as OMap +import qualified Data.HashMap.Strict.InsOrd.Extended as OMap import qualified Data.List.NonEmpty as NE import Data.Foldable (sequenceA_) @@ -278,9 +279,9 @@ remoteSchemaInterface schemaDoc defn@(G.InterfaceTypeDefinition description name -- selection set provided -- #1 of Note [Querying remote schema Interfaces] commonInterfaceFields = - Map.elems $ - Map.mapMaybe allTheSame $ - Map.groupOn G._fName $ + OMap.elems $ + OMap.mapMaybe (allTheSame . toList) $ + OMap.groupListWith G._fName $ concatMap (filter ((`elem` interfaceFieldNames) . G._fName) . snd) $ objNameAndFields diff --git a/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs b/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs index 8fc273b1bcca9..515b0fd42ed96 100644 --- a/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs +++ b/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs @@ -123,9 +123,15 @@ validateRemoteRelationship remoteRelationship remoteSchemaMap pgColumns = do getObjTyInfoFromField schemaDoc field = let baseTy = G.getBaseType (G._fldType field) in lookupObject schemaDoc baseTy - isScalarType schemaDoc field = + isValidType schemaDoc field = let baseTy = G.getBaseType (G._fldType field) - in isJust $ lookupScalar schemaDoc baseTy + in + case (lookupType schemaDoc baseTy) of + Nothing -> False + Just (G.TypeDefinitionScalar _) -> True + Just (G.TypeDefinitionInterface _) -> True + Just (G.TypeDefinitionUnion _) -> True + _ -> False buildRelationshipTypeInfo pgColumnsVariablesMap schemaDoc (objTyInfo,(_,typeMap)) fieldCall = do objFldDefinition <- lookupField (fcName fieldCall) objTyInfo let providedArguments = getRemoteArguments $ fcArguments fieldCall @@ -145,9 +151,9 @@ validateRemoteRelationship remoteRelationship remoteSchemaMap pgColumns = do (newParamMap, newTypeMap) <- onLeft eitherParamAndTypeMap $ throwError innerObjTyInfo <- onNothing (getObjTyInfoFromField schemaDoc objFldDefinition) $ bool (throwError $ - (InvalidType (G._fldType objFldDefinition) "only objects or scalar types expected")) + (InvalidType (G._fldType objFldDefinition) "only objects,scalar,interface or union types expected")) (pure objTyInfo) - (isScalarType schemaDoc objFldDefinition) + (isValidType schemaDoc objFldDefinition) pure ( innerObjTyInfo , (newParamMap,newTypeMap)) diff --git a/server/tests-py/test_remote_relationships.py b/server/tests-py/test_remote_relationships.py index d2c2d78b9a748..5e4a3cf9862f9 100644 --- a/server/tests-py/test_remote_relationships.py +++ b/server/tests-py/test_remote_relationships.py @@ -57,8 +57,8 @@ def test_create_valid(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 - # st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_interface.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 def test_create_invalid(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_invalid_remote_rel_hasura_field.yaml') @@ -202,10 +202,11 @@ def test_with_variables(self, hge_ctx): # assert st_code == 200, resp # check_query_f(hge_ctx, self.dir() + 'remote_rel_fragments.yaml') - # TODO: Support interface in remote relationships - # def test_with_interface(self, hge_ctx): - # check_query_f(hge_ctx, self.dir() + 'mixed_interface.yaml') - # check_query_f(hge_ctx, self.dir() + 'remote_rel_interface.yaml') + def test_with_interface(self, hge_ctx): + st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_interface.yaml') + assert st_code == 200, resp + check_query_f(hge_ctx, self.dir() + 'mixed_interface.yaml') + check_query_f(hge_ctx, self.dir() + 'remote_rel_interface.yaml') def test_with_errors(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_basic.yaml') From d01042c75d1accbba829b64d0711eb5805ce14ba Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Tue, 27 Oct 2020 18:29:18 +0530 Subject: [PATCH 2/8] add test for making a remote relationship with an Union type --- .../remote_rel_union.yaml | 30 ++++++++++++++++ .../setup_remote_rel_with_union.yaml | 11 ++++++ .../tests-py/remote_schemas/nodejs/index.js | 34 ++++++++++++++++++- server/tests-py/test_remote_relationships.py | 8 +++++ 4 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_union.yaml create mode 100644 server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_union.yaml diff --git a/server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_union.yaml b/server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_union.yaml new file mode 100644 index 0000000000000..d57c972e109b3 --- /dev/null +++ b/server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_union.yaml @@ -0,0 +1,30 @@ +description: Simple remote relationship GraphQL query with union +url: /v1/graphql +status: 200 +response: + data: + profiles: + - id: 1 + searchUnion: + title: "Harry Porter and the prisoner of Azkaban" + - id: 2 + searchUnion: + name: "J.K. Rowling" + - id: 3 + searchUnion: + title: "Harry Porter and the philoshoper's stone" +query: + query: | + query { + profiles { + id + searchUnion { + ... on Author { + name + } + ... on Book { + title + } + } + } + } diff --git a/server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_union.yaml b/server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_union.yaml new file mode 100644 index 0000000000000..b3167e3aadf64 --- /dev/null +++ b/server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_union.yaml @@ -0,0 +1,11 @@ +type: create_remote_relationship +args: + name: searchUnion + table: profiles + hasura_fields: + - id + remote_schema: my-remote-schema + remote_field: + search: + arguments: + id: $id diff --git a/server/tests-py/remote_schemas/nodejs/index.js b/server/tests-py/remote_schemas/nodejs/index.js index 7ea925e9c0590..3144419d3a56b 100644 --- a/server/tests-py/remote_schemas/nodejs/index.js +++ b/server/tests-py/remote_schemas/nodejs/index.js @@ -9,6 +9,12 @@ const allMessages = [ { id: 3, name: "alice", msg: "Another alice"}, ]; +const data = { + 1: { title: 'Harry Porter and the prisoner of Azkaban' }, + 2: { name: 'J.K. Rowling' }, + 3: { title: "Harry Porter and the philoshoper's stone"} +} + const typeDefs = gql` type User { @@ -29,6 +35,16 @@ const typeDefs = gql` errorMsg: String } + union SearchResult = Book | Author + + type Book { + title: String + } + + type Author { + name: String + } + input MessageWhereInpObj { id: IntCompareObj name: StringCompareObj @@ -56,6 +72,7 @@ const typeDefs = gql` users(user_ids: [Int]!): [User] message(id: Int!) : Message communications(id: Int): [Communication] + search(id: Int!): SearchResult } `; @@ -116,7 +133,6 @@ const resolvers = { } } }, - Message: { errorMsg : () => { throw new ApolloError("intentional-error", "you asked for it"); @@ -190,12 +206,28 @@ const resolvers = { } return result; }, + search: (_, { id }) => { + return data[id] + } }, Communication: { __resolveType(communication, context, info){ if(communication.name) { return "Message"; } + return null; + } + }, + SearchResult: { + __resolveType(obj, context, info) { + if(obj.name) { + return "Author"; + } + + if(obj.title) { + return "Book"; + } + return null; } } diff --git a/server/tests-py/test_remote_relationships.py b/server/tests-py/test_remote_relationships.py index 5e4a3cf9862f9..9a64f40563213 100644 --- a/server/tests-py/test_remote_relationships.py +++ b/server/tests-py/test_remote_relationships.py @@ -60,6 +60,9 @@ def test_create_valid(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_interface.yaml') assert st_code == 200, resp + st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_union.yaml') + assert st_code == 200, resp + def test_create_invalid(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_invalid_remote_rel_hasura_field.yaml') assert st_code == 400, resp @@ -208,6 +211,11 @@ def test_with_interface(self, hge_ctx): check_query_f(hge_ctx, self.dir() + 'mixed_interface.yaml') check_query_f(hge_ctx, self.dir() + 'remote_rel_interface.yaml') + def test_with_union(self, hge_ctx): + st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_union.yaml') + assert st_code == 200, resp + check_query_f(hge_ctx, self.dir() + 'remote_rel_union.yaml') + def test_with_errors(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_basic.yaml') assert st_code == 200, resp From a4136964e5ee86843f6108e1d71bfd97d2ab547d Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Tue, 27 Oct 2020 18:34:37 +0530 Subject: [PATCH 3/8] add CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e3de7c6ad3c6..2527dcbf02a7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,7 @@ This release contains the [PDV refactor (#4111)](https://github.com/hasura/graph **NOTE:** If you have event triggers with names greater than 42 chars, then you should update their names to avoid running into Postgres identifier limit bug (#5786) - server: validate remote schema queries (fixes #4143) - server: fix issue with tracking custom functions that return `SETOF` materialized view (close #5294) (#5945) +- server: allow remote relationships with union and interface type fields as well (fixes #5875) (#6080) - 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 seed command (#5565) From 72dd913dadc951a9e903a983b79d88e8c171b9d9 Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Tue, 27 Oct 2020 18:41:05 +0530 Subject: [PATCH 4/8] remove the pretty-simple library --- server/graphql-engine.cabal | 1 - 1 file changed, 1 deletion(-) diff --git a/server/graphql-engine.cabal b/server/graphql-engine.cabal index d04bffa1a3c54..3979b6e9b2e87 100644 --- a/server/graphql-engine.cabal +++ b/server/graphql-engine.cabal @@ -224,7 +224,6 @@ library -- pretty printer , ansi-wl-pprint - , pretty-simple -- for capturing various metrics , ekg-core From f9c89f524117b1a79b8d4c6de45f5f162b309e02 Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Tue, 27 Oct 2020 19:21:01 +0530 Subject: [PATCH 5/8] remove unused imports from Remote.hs --- server/src-lib/Hasura/GraphQL/Schema/Remote.hs | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs index d1b374e61074c..c8d2de82ecc2f 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs @@ -8,8 +8,6 @@ module Hasura.GraphQL.Schema.Remote import Hasura.Prelude -import qualified Data.HashMap.Strict as Map -import qualified Data.HashMap.Strict.Extended as Map import qualified Data.HashMap.Strict.InsOrd as OMap import qualified Data.HashMap.Strict.InsOrd.Extended as OMap import qualified Data.List.NonEmpty as NE From 6a0ae34bb075800fe51ec9adc4aac9b7d12dc678 Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Wed, 28 Oct 2020 12:26:31 +0530 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Brandon Simmons --- server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs b/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs index 515b0fd42ed96..9825397ef4854 100644 --- a/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs +++ b/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs @@ -127,7 +127,6 @@ validateRemoteRelationship remoteRelationship remoteSchemaMap pgColumns = do let baseTy = G.getBaseType (G._fldType field) in case (lookupType schemaDoc baseTy) of - Nothing -> False Just (G.TypeDefinitionScalar _) -> True Just (G.TypeDefinitionInterface _) -> True Just (G.TypeDefinitionUnion _) -> True @@ -151,7 +150,7 @@ validateRemoteRelationship remoteRelationship remoteSchemaMap pgColumns = do (newParamMap, newTypeMap) <- onLeft eitherParamAndTypeMap $ throwError innerObjTyInfo <- onNothing (getObjTyInfoFromField schemaDoc objFldDefinition) $ bool (throwError $ - (InvalidType (G._fldType objFldDefinition) "only objects,scalar,interface or union types expected")) + (InvalidType (G._fldType objFldDefinition) "only objects, scalar, interface or union types expected")) (pure objTyInfo) (isValidType schemaDoc objFldDefinition) pure From 9b1c2d026895bf132a516b3914dc706d9e54b67e Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Wed, 28 Oct 2020 13:15:44 +0530 Subject: [PATCH 7/8] support remote relationships against enum fields as well --- .../RQL/DDL/RemoteRelationship/Validate.hs | 3 ++- .../remote_relationships/remote_rel_enum.yaml | 20 +++++++++++++++++++ .../setup_remote_rel_with_enum.yaml | 11 ++++++++++ .../tests-py/remote_schemas/nodejs/index.js | 17 ++++++++++++++++ server/tests-py/test_remote_relationships.py | 8 ++++++++ 5 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_enum.yaml create mode 100644 server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_enum.yaml diff --git a/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs b/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs index 87e0e797d11a4..1b4bf796a67c7 100644 --- a/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs +++ b/server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs @@ -130,6 +130,7 @@ validateRemoteRelationship remoteRelationship remoteSchemaMap pgColumns = do Just (G.TypeDefinitionScalar _) -> True Just (G.TypeDefinitionInterface _) -> True Just (G.TypeDefinitionUnion _) -> True + Just (G.TypeDefinitionEnum _) -> True _ -> False buildRelationshipTypeInfo pgColumnsVariablesMap schemaDoc (objTyInfo,(_,typeMap)) fieldCall = do objFldDefinition <- lookupField (fcName fieldCall) objTyInfo @@ -150,7 +151,7 @@ validateRemoteRelationship remoteRelationship remoteSchemaMap pgColumns = do (newParamMap, newTypeMap) <- onLeft eitherParamAndTypeMap $ throwError innerObjTyInfo <- onNothing (getObjTyInfoFromField schemaDoc objFldDefinition) $ bool (throwError $ - (InvalidType (G._fldType objFldDefinition) "only objects, scalar, interface or union types expected")) + (InvalidType (G._fldType objFldDefinition) "only output type is expected")) (pure objTyInfo) (isValidType schemaDoc objFldDefinition) pure diff --git a/server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_enum.yaml b/server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_enum.yaml new file mode 100644 index 0000000000000..f359ad79c8701 --- /dev/null +++ b/server/tests-py/queries/remote_schemas/remote_relationships/remote_rel_enum.yaml @@ -0,0 +1,20 @@ +description: Simple remote relationship GraphQL query with union +url: /v1/graphql +status: 200 +response: + data: + profiles: + - id: 1 + getOccupationEnum: "SINGER" + - id: 2 + getOccupationEnum: "ARTIST" + - id: 3 + getOccupationEnum: "SINGER" +query: + query: | + query { + profiles { + id + getOccupationEnum + } + } diff --git a/server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_enum.yaml b/server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_enum.yaml new file mode 100644 index 0000000000000..9a1d1ac812984 --- /dev/null +++ b/server/tests-py/queries/remote_schemas/remote_relationships/setup_remote_rel_with_enum.yaml @@ -0,0 +1,11 @@ +type: create_remote_relationship +args: + name: getOccupationEnum + table: profiles + hasura_fields: + - name + remote_schema: my-remote-schema + remote_field: + getOccupation: + arguments: + name: $name diff --git a/server/tests-py/remote_schemas/nodejs/index.js b/server/tests-py/remote_schemas/nodejs/index.js index 3144419d3a56b..180f549e8c0df 100644 --- a/server/tests-py/remote_schemas/nodejs/index.js +++ b/server/tests-py/remote_schemas/nodejs/index.js @@ -65,6 +65,12 @@ const typeDefs = gql` name: [String] } + enum Occupation { + SINGER + COMEDIAN + ARTIST + } + type Query { hello: String messages(where: MessageWhereInpObj, includes: IncludeInpObj): [Message] @@ -73,6 +79,7 @@ const typeDefs = gql` message(id: Int!) : Message communications(id: Int): [Communication] search(id: Int!): SearchResult + getOccupation(name: String!): Occupation! } `; @@ -208,6 +215,16 @@ const resolvers = { }, search: (_, { id }) => { return data[id] + }, + getOccupation: (_, { name }) => { + switch (name) { + case "alice": + return 'SINGER' + case "bob": + return 'ARTIST' + default: + throw new ApolloError("invalid argument - " + name, "invalid "); + } } }, Communication: { diff --git a/server/tests-py/test_remote_relationships.py b/server/tests-py/test_remote_relationships.py index 9a64f40563213..08e25a8b229a6 100644 --- a/server/tests-py/test_remote_relationships.py +++ b/server/tests-py/test_remote_relationships.py @@ -63,6 +63,9 @@ def test_create_valid(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_union.yaml') assert st_code == 200, resp + st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_enum.yaml') + assert st_code == 200, resp + def test_create_invalid(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_invalid_remote_rel_hasura_field.yaml') assert st_code == 400, resp @@ -216,6 +219,11 @@ def test_with_union(self, hge_ctx): assert st_code == 200, resp check_query_f(hge_ctx, self.dir() + 'remote_rel_union.yaml') + def test_with_enum(self, hge_ctx): + st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_with_enum.yaml') + assert st_code == 200, resp + check_query_f(hge_ctx, self.dir() + 'remote_rel_enum.yaml') + def test_with_errors(self, hge_ctx): st_code, resp = hge_ctx.v1q_f(self.dir() + 'setup_remote_rel_basic.yaml') assert st_code == 200, resp From e76fd67f14a78889e8fab031b51ecadabe3aea0e Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Wed, 28 Oct 2020 13:16:52 +0530 Subject: [PATCH 8/8] update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 202c69db88510..27f78110329d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,7 +84,7 @@ This release contains the [PDV refactor (#4111)](https://github.com/hasura/graph **NOTE:** If you have event triggers with names greater than 42 chars, then you should update their names to avoid running into Postgres identifier limit bug (#5786) - server: validate remote schema queries (fixes #4143) - server: fix issue with tracking custom functions that return `SETOF` materialized view (close #5294) (#5945) -- server: allow remote relationships with union and interface type fields as well (fixes #5875) (#6080) +- server: allow remote relationships with union, interface and enum type fields as well (fixes #5875) (#6080) - 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 seed command (#5565)