From 69b5aa43370398d024d01cafe5b1e50ce575a14e Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Thu, 27 Aug 2020 11:20:22 +0530 Subject: [PATCH] fix(GraphQL): fix internal error when doing GraphQL schema introspection after drop all (#6268) This PR fixes the "Internal error" response when querying the GraphQL schema after performing drop_all operation. (cherry picked from commit d3bee33ba70370bfdedf532cb0facce3dcc2caca) --- graphql/admin/admin.go | 19 ++++++++++++------- graphql/e2e/schema/schema_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/graphql/admin/admin.go b/graphql/admin/admin.go index 9a98724b489..122c404a02f 100644 --- a/graphql/admin/admin.go +++ b/graphql/admin/admin.go @@ -742,13 +742,18 @@ func (as *adminServer) resetSchema(gqlSchema schema.Schema) { // set status as updating schema mainHealthStore.updatingSchema() - resolverFactory := resolverFactoryWithErrorMsg(errResolverNotFound) - // it is nil after drop_all - if gqlSchema != nil { - resolverFactory = resolverFactory.WithConventionResolvers(gqlSchema, as.fns) - } - if as.withIntrospection { - resolverFactory.WithSchemaIntrospection() + var resolverFactory resolve.ResolverFactory + // If schema is nil (which becomes after drop_all) then do not attach Resolver for + // introspection operations, and set GQL schema to empty. + if gqlSchema == nil { + resolverFactory = resolverFactoryWithErrorMsg(errNoGraphQLSchema) + gqlSchema, _ = schema.FromString("") + } else { + resolverFactory = resolverFactoryWithErrorMsg(errResolverNotFound). + WithConventionResolvers(gqlSchema, as.fns) + if as.withIntrospection { + resolverFactory.WithSchemaIntrospection() + } } // Increment the Epoch when you get a new schema. So, that subscription's local epoch diff --git a/graphql/e2e/schema/schema_test.go b/graphql/e2e/schema/schema_test.go index 1b7749bdf07..210292dc831 100644 --- a/graphql/e2e/schema/schema_test.go +++ b/graphql/e2e/schema/schema_test.go @@ -278,6 +278,35 @@ func TestConcurrentSchemaUpdates(t *testing.T) { require.Equal(t, finalGraphQLSchema, resp.GqlSchema[0].Schema) } +// TestIntrospectionQueryAfterDropAll make sure that Introspection query after drop_all doesn't give any internal error +func TestIntrospectionQueryAfterDropAll(t *testing.T) { + // First Do the drop_all operation + dg, err := testutil.DgraphClient(groupOnegRPC) + require.NoError(t, err) + testutil.DropAll(t, dg) + // wait for a bit + time.Sleep(time.Second) + + introspectionQuery := ` + query{ + __schema{ + types{ + name + } + } + }` + introspect := &common.GraphQLParams{ + Query: introspectionQuery, + } + + // On doing Introspection Query Now, We should get the Expected Error Message, not the Internal Error. + introspectionResult := introspect.ExecuteAsPost(t, groupOneServer) + require.Len(t, introspectionResult.Errors, 1) + gotErrorMessage := introspectionResult.Errors[0].Message + expectedErrorMessage := "Not resolving __schema. There's no GraphQL schema in Dgraph. Use the /admin API to add a GraphQL schema" + require.Equal(t, expectedErrorMessage, gotErrorMessage) +} + // TestUpdateGQLSchemaAfterDropAll makes sure that updating the GraphQL schema after drop_all works func TestUpdateGQLSchemaAfterDropAll(t *testing.T) { updateGQLSchemaRequireNoErrors(t, `