From f38b19cb96f4c7741d5d680374fbadb796905400 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 3 Feb 2023 09:14:46 +0000 Subject: [PATCH 1/5] Failing test for schema printing --- src/__testUtils__/viralSDL.ts | 19 +++++++++ src/__testUtils__/viralSchema.ts | 44 +++++++++++++++++++++ src/utilities/__tests__/printSchema-test.ts | 6 +++ 3 files changed, 69 insertions(+) create mode 100644 src/__testUtils__/viralSDL.ts create mode 100644 src/__testUtils__/viralSchema.ts diff --git a/src/__testUtils__/viralSDL.ts b/src/__testUtils__/viralSDL.ts new file mode 100644 index 0000000000..e66ff169a7 --- /dev/null +++ b/src/__testUtils__/viralSDL.ts @@ -0,0 +1,19 @@ +export const viralSDL = `\ +schema { + query: Query +} + +type Query { + viruses: [Virus!] +} + +type Virus { + name: String! + knownMutations: [Mutation!]! +} + +type Mutation { + name: String! + geneSequence: String! +}\ +`; diff --git a/src/__testUtils__/viralSchema.ts b/src/__testUtils__/viralSchema.ts new file mode 100644 index 0000000000..62613dc710 --- /dev/null +++ b/src/__testUtils__/viralSchema.ts @@ -0,0 +1,44 @@ +import { + GraphQLList, + GraphQLNonNull, + GraphQLObjectType, +} from '../type/definition.js'; +import { GraphQLString } from '../type/scalars.js'; +import { GraphQLSchema } from '../type/schema.js'; + +const Mutation = new GraphQLObjectType({ + name: 'Mutation', + fields: { + name: { + type: new GraphQLNonNull(GraphQLString), + }, + geneSequence: { + type: new GraphQLNonNull(GraphQLString), + }, + }, +}); + +const Virus = new GraphQLObjectType({ + name: 'Virus', + fields: { + name: { + type: new GraphQLNonNull(GraphQLString), + }, + knownMutations: { + type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(Mutation))), + }, + }, +}); + +const Query = new GraphQLObjectType({ + name: 'Query', + fields: { + viruses: { + type: new GraphQLList(new GraphQLNonNull(Virus)), + }, + }, +}); + +export const viralSchema = new GraphQLSchema({ + query: Query, +}); diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 7248d747b6..29799a4881 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -2,6 +2,8 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; import { dedent, dedentString } from '../../__testUtils__/dedent.js'; +import { viralSchema } from '../../__testUtils__/viralSchema.js'; +import { viralSDL } from '../../__testUtils__/viralSDL.js'; import { DirectiveLocation } from '../../language/directiveLocation.js'; @@ -867,4 +869,8 @@ describe('Type System Printer', () => { } `); }); + it('prints viral schema correctly', () => { + const printed = printSchema(viralSchema); + expect(printed).to.equal(viralSDL); + }); }); From f6576dcf49e4bb9d183268a63404014f78c7f313 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 3 Feb 2023 09:22:40 +0000 Subject: [PATCH 2/5] Add passing test for viral schema parsing --- src/utilities/__tests__/buildASTSchema-test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/utilities/__tests__/buildASTSchema-test.ts b/src/utilities/__tests__/buildASTSchema-test.ts index 204ceb800c..6e152af36c 100644 --- a/src/utilities/__tests__/buildASTSchema-test.ts +++ b/src/utilities/__tests__/buildASTSchema-test.ts @@ -2,6 +2,7 @@ import { assert, expect } from 'chai'; import { describe, it } from 'mocha'; import { dedent } from '../../__testUtils__/dedent.js'; +import { viralSDL } from '../../__testUtils__/viralSDL.js'; import type { Maybe } from '../../jsutils/Maybe.js'; @@ -1092,4 +1093,14 @@ describe('Schema Builder', () => { 'Unknown type: "UnknownType".', ); }); + + it('correctly processes viral schema', () => { + const schema = buildSchema(viralSDL); + expect(schema.getQueryType()).to.contain({ name: 'Query' }); + expect(schema.getType('Virus')).to.contain({ name: 'Virus' }); + expect(schema.getType('Mutation')).to.contain({ name: 'Mutation' }); + // Though the viral schema has a 'Mutation' type, it is not used for the + // 'mutation' operation. + expect(schema.getMutationType()).to.equal(undefined); + }); }); From 04eb23298ce2b1ad715b7bd07b0e092bad635aa7 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 3 Feb 2023 09:29:24 +0000 Subject: [PATCH 3/5] Fix printSchema --- src/utilities/printSchema.ts | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index 527cf4be17..edca2a7bff 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -107,25 +107,23 @@ function printSchemaDefinition(schema: GraphQLSchema): Maybe { * } * ``` * - * When using this naming convention, the schema description can be omitted. + * When using this naming convention, the schema description can be omitted so + * long as these names are only used for operation types. */ function isSchemaOfCommonNames(schema: GraphQLSchema): boolean { - const queryType = schema.getQueryType(); - if (queryType && queryType.name !== 'Query') { - return false; - } + const queryOperationType = schema.getQueryType() ?? null; + const mutationOperationType = schema.getMutationType() ?? null; + const subscriptionOperationType = schema.getSubscriptionType() ?? null; - const mutationType = schema.getMutationType(); - if (mutationType && mutationType.name !== 'Mutation') { - return false; - } + const queryType = schema.getType('Query') ?? null; + const mutationType = schema.getType('Mutation') ?? null; + const subscriptionType = schema.getType('Subscription') ?? null; - const subscriptionType = schema.getSubscriptionType(); - if (subscriptionType && subscriptionType.name !== 'Subscription') { - return false; - } - - return true; + return ( + queryOperationType === queryType && + mutationOperationType === mutationType && + subscriptionOperationType === subscriptionType + ); } export function printType(type: GraphQLNamedType): string { From 662b489b37f97119971b43ddc3d932b1cdce483c Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 3 Feb 2023 09:43:40 +0000 Subject: [PATCH 4/5] Treat schema with no operation types as if it uses common names --- src/utilities/printSchema.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index edca2a7bff..55a1758140 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -115,6 +115,15 @@ function isSchemaOfCommonNames(schema: GraphQLSchema): boolean { const mutationOperationType = schema.getMutationType() ?? null; const subscriptionOperationType = schema.getSubscriptionType() ?? null; + // Special case for when there are no operation types + if ( + !queryOperationType && + !mutationOperationType && + !subscriptionOperationType + ) { + return true; + } + const queryType = schema.getType('Query') ?? null; const mutationType = schema.getType('Mutation') ?? null; const subscriptionType = schema.getType('Subscription') ?? null; From 8a266f3013093692dcf5a7b52e03422f77285f7d Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Wed, 8 Feb 2023 16:39:55 -0800 Subject: [PATCH 5/5] Attempt to simplify and rename things Moves the "no operation types" special case up to the caller. Renames the function to match the spec text --- src/utilities/printSchema.ts | 64 +++++++++++++++--------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index 55a1758140..0b9b1638d7 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -70,28 +70,28 @@ function printFilteredSchema( } function printSchemaDefinition(schema: GraphQLSchema): Maybe { - if (schema.description == null && isSchemaOfCommonNames(schema)) { - return; - } - - const operationTypes = []; - const queryType = schema.getQueryType(); - if (queryType) { - operationTypes.push(` query: ${queryType.name}`); - } - const mutationType = schema.getMutationType(); - if (mutationType) { - operationTypes.push(` mutation: ${mutationType.name}`); - } - const subscriptionType = schema.getSubscriptionType(); - if (subscriptionType) { - operationTypes.push(` subscription: ${subscriptionType.name}`); + + // Special case: When a schema has no root operation types, no valid schema + // definition can be printed. + if (!queryType && !mutationType && !subscriptionType) { + return; } - return printDescription(schema) + `schema {\n${operationTypes.join('\n')}\n}`; + // Only print a schema definition if there is a description or if it should + // not be omitted because of having default type names. + if (schema.description || !hasDefaultRootOperationTypes(schema)) { + return ( + printDescription(schema) + + 'schema {\n' + + (queryType ? ` query: ${queryType.name}\n` : '') + + (mutationType ? ` mutation: ${mutationType.name}\n` : '') + + (subscriptionType ? ` subscription: ${subscriptionType.name}\n` : '') + + '}' + ); + } } /** @@ -109,29 +109,17 @@ function printSchemaDefinition(schema: GraphQLSchema): Maybe { * * When using this naming convention, the schema description can be omitted so * long as these names are only used for operation types. + * + * Note however that if any of these default names are used elsewhere in the + * schema but not as a root operation type, the schema definition must still + * be printed to avoid ambiguity. */ -function isSchemaOfCommonNames(schema: GraphQLSchema): boolean { - const queryOperationType = schema.getQueryType() ?? null; - const mutationOperationType = schema.getMutationType() ?? null; - const subscriptionOperationType = schema.getSubscriptionType() ?? null; - - // Special case for when there are no operation types - if ( - !queryOperationType && - !mutationOperationType && - !subscriptionOperationType - ) { - return true; - } - - const queryType = schema.getType('Query') ?? null; - const mutationType = schema.getType('Mutation') ?? null; - const subscriptionType = schema.getType('Subscription') ?? null; - +function hasDefaultRootOperationTypes(schema: GraphQLSchema): boolean { + /* eslint-disable eqeqeq */ return ( - queryOperationType === queryType && - mutationOperationType === mutationType && - subscriptionOperationType === subscriptionType + schema.getQueryType() == schema.getType('Query') && + schema.getMutationType() == schema.getType('Mutation') && + schema.getSubscriptionType() == schema.getType('Subscription') ); }