diff --git a/.changeset/khaki-experts-decide.md b/.changeset/khaki-experts-decide.md new file mode 100644 index 0000000000..2f13dfe2df --- /dev/null +++ b/.changeset/khaki-experts-decide.md @@ -0,0 +1,21 @@ +--- +"@neo4j/graphql": major +--- + +It was possible to define schemas with types that have multiple relationship fields connected by the same type of relationships. Instances of this scenario are now detected during schema generation and an error is thrown so developers are informed to remedy the type definitions. + +An example of what is now considered invalid with these checks: + +```graphql +type Team { + player1: Person! @relationship(type: "PLAYS_IN", direction: IN) + player2: Person! @relationship(type: "PLAYS_IN", direction: IN) + backupPlayers: [Person!]! @relationship(type: "PLAYS_IN", direction: IN) +} + +type Person { + teams: [Team!]! @relationship(type: "PLAYS_IN", direction: OUT) +} +``` + +For more information about this change and how to disable this validation please see the [4.0.0 migration guide](https://neo4j.com/docs/graphql-manual/current/guides/v4-migration/) diff --git a/docs/modules/ROOT/pages/guides/v4-migration/index.adoc b/docs/modules/ROOT/pages/guides/v4-migration/index.adoc index 00fe6fa045..945c8d0775 100644 --- a/docs/modules/ROOT/pages/guides/v4-migration/index.adoc +++ b/docs/modules/ROOT/pages/guides/v4-migration/index.adoc @@ -526,12 +526,48 @@ type User { } ---- +=== Duplicate relationship fields are now checked for + +It was possible to define schemas with types that have multiple relationship fields connected by the same type of relationships. Instances of this scenario are now detected during schema generation and an error is thrown so developers are informed to remedy the type definitions. + +An example of what is now considered invalid with these checks: + +[source, graphql, indent=0] +---- +type Team { + player1: Person! @relationship(type: "PLAYS_IN", direction: IN) + player2: Person! @relationship(type: "PLAYS_IN", direction: IN) + backupPlayers: [Person!]! @relationship(type: "PLAYS_IN", direction: IN) +} + +type Person { + teams: [Team!]! @relationship(type: "PLAYS_IN", direction: OUT) +} +---- + +In this example, there are multiple fields in the `Team` type which have the same `Person` type, the same `@relationship` type and ("PLAYS_IN") direction (IN). This is an issue when returning data from the database, as there would be no difference between `player1`, `player2` and `backupPlayers`. Selecting these fields would then return the same data. + +To disable checks for duplicate relationship fields, the `noDuplicateRelationshipFields` config option should be used: + +[source, javascript, indent=0] +---- +const neoSchema = new Neo4jGraphQL({ + typeDefs, + config: { + startupValidation: { + noDuplicateRelationshipFields: false, + }, + }, +}); +---- + + == Miscellaneous changes [[startup-validation]] === Startup validation -In version 4.0.0, startup xref::guides/v4-migration/index.adoc#customResolver-checks[checks for custom resolvers] have been added. As a result, a new configuration option has been added that can disable these checks. +In version 4.0.0, startup xref::guides/v4-migration/index.adoc#customResolver-checks[checks for custom resolvers], and checks for duplicate relationship fields have been added. As a result, a new configuration option has been added that can disable these checks. This new option has been combined with the option to `skipValidateTypeDefs`. As a result, `skipValidateTypeDefs` will be removed and replaced by `startupValidation`. To only disable strict type definition validation, the following config option should be used: @@ -562,6 +598,21 @@ const neoSchema = new Neo4jGraphQL({ }) ---- +To only disable checks for duplicate relationship fields, the following config option should be used: + +[source, javascript, indent=0] +---- +const neoSchema = new Neo4jGraphQL({ + typeDefs, + config: { + startupValidation: { + noDuplicateRelationshipFields: false + }, + }, +}) +---- + + To disable all startup checks, the following config option should be used: [source, javascript, indent=0] diff --git a/packages/graphql/src/classes/Neo4jGraphQL.ts b/packages/graphql/src/classes/Neo4jGraphQL.ts index 9fea6bf47d..60ec3a72af 100644 --- a/packages/graphql/src/classes/Neo4jGraphQL.ts +++ b/packages/graphql/src/classes/Neo4jGraphQL.ts @@ -67,6 +67,12 @@ export interface Neo4jGraphQLConfig { callbacks?: Neo4jGraphQLCallbacks; } +export type ValidationConfig = { + validateTypeDefs: boolean; + validateResolvers: boolean; + validateDuplicateRelationshipFields: boolean; +}; + export interface Neo4jGraphQLConstructor { typeDefs: TypeSource; resolvers?: IExecutableSchemaDefinition["resolvers"]; @@ -76,6 +82,12 @@ export interface Neo4jGraphQLConstructor { plugins?: Neo4jGraphQLPlugins; } +export const defaultValidationConfig: ValidationConfig = { + validateTypeDefs: true, + validateResolvers: true, + validateDuplicateRelationshipFields: true, +}; + class Neo4jGraphQL { typeDefs: TypeSource; resolvers?: IExecutableSchemaDefinition["resolvers"]; @@ -238,17 +250,16 @@ class Neo4jGraphQL { return getNeo4jDatabaseInfo(new Executor(executorConstructorParam)); } - private wrapResolvers(resolvers: NonNullable) { - if (!this.schemaModel) { - throw new Error("Schema Model is not defined"); - } - + private wrapResolvers( + resolvers: NonNullable, + schemaModel: Neo4jGraphQLSchemaModel + ) { const wrapResolverArgs = { driver: this.driver, config: this.config, nodes: this.nodes, relationships: this.relationships, - schemaModel: this.schemaModel, + schemaModel: schemaModel, plugins: this.plugins, }; @@ -263,17 +274,16 @@ class Neo4jGraphQL { return composeResolvers(mergedResolvers, resolversComposition); } - private wrapFederationResolvers(resolvers: NonNullable) { - if (!this.schemaModel) { - throw new Error("Schema Model is not defined"); - } - + private wrapFederationResolvers( + resolvers: NonNullable, + schemaModel: Neo4jGraphQLSchemaModel + ) { const wrapResolverArgs = { driver: this.driver, config: this.config, nodes: this.nodes, relationships: this.relationships, - schemaModel: this.schemaModel, + schemaModel: schemaModel, plugins: this.plugins, }; @@ -286,22 +296,27 @@ class Neo4jGraphQL { return composeResolvers(mergedResolvers, resolversComposition); } + private generateSchemaModel(document: DocumentNode): Neo4jGraphQLSchemaModel { + // This can be run several times but it will always be the same result, + // so we memoize the schemaModel. + if (!this.schemaModel) { + this.schemaModel = generateModel(document); + } + return this.schemaModel; + } + private generateExecutableSchema(): Promise { return new Promise((resolve) => { const document = this.getDocument(this.typeDefs); - const { validateTypeDefs, validateResolvers } = this.parseStartupValidationConfig(); - - validateDocument(document, validateTypeDefs); + const validationConfig = this.parseStartupValidationConfig(); - if (!this.schemaModel) { - this.schemaModel = generateModel(document); - } + validateDocument({ document, validationConfig }); const { nodes, relationships, typeDefs, resolvers } = makeAugmentedSchema(document, { features: this.features, enableRegex: this.config?.enableRegex, - validateResolvers, + validateResolvers: validationConfig.validateResolvers, generateSubscriptions: Boolean(this.plugins?.subscriptions), callbacks: this.config.callbacks, userCustomResolvers: this.resolvers, @@ -310,8 +325,10 @@ class Neo4jGraphQL { this._nodes = nodes; this._relationships = relationships; + const schemaModel = this.generateSchemaModel(document); + // Wrap the generated and custom resolvers, which adds a context including the schema to every request - const wrappedResolvers = this.wrapResolvers(resolvers); + const wrappedResolvers = this.wrapResolvers(resolvers, schemaModel); const schema = makeExecutableSchema({ typeDefs, @@ -330,18 +347,14 @@ class Neo4jGraphQL { const { directives, types } = subgraph.getValidationDefinitions(); - const { validateTypeDefs, validateResolvers } = this.parseStartupValidationConfig(); + const validationConfig = this.parseStartupValidationConfig(); - validateDocument(document, validateTypeDefs, directives, types); - - if (!this.schemaModel) { - this.schemaModel = generateModel(document); - } + validateDocument({ document, validationConfig, additionalDirectives: directives, additionalTypes: types }); const { nodes, relationships, typeDefs, resolvers } = makeAugmentedSchema(document, { features: this.features, enableRegex: this.config?.enableRegex, - validateResolvers, + validateResolvers: validationConfig.validateResolvers, generateSubscriptions: Boolean(this.plugins?.subscriptions), callbacks: this.config.callbacks, userCustomResolvers: this.resolvers, @@ -354,7 +367,9 @@ class Neo4jGraphQL { // TODO: Move into makeAugmentedSchema, add resolvers alongside other resolvers const referenceResolvers = subgraph.getReferenceResolvers(this._nodes); - const wrappedResolvers = this.wrapResolvers([resolvers, referenceResolvers]); + const schemaModel = this.generateSchemaModel(document); + + const wrappedResolvers = this.wrapResolvers([resolvers, referenceResolvers], schemaModel); const schema = subgraph.buildSchema({ typeDefs, @@ -365,7 +380,7 @@ class Neo4jGraphQL { const subgraphResolvers = getResolversFromSchema(schema); // Wrap the _entities and _service Query resolvers - const wrappedSubgraphResolvers = this.wrapFederationResolvers(subgraphResolvers); + const wrappedSubgraphResolvers = this.wrapFederationResolvers(subgraphResolvers, schemaModel); // Add the wrapped resolvers back to the schema, context will now be populated addResolversToSchema({ schema, resolvers: wrappedSubgraphResolvers, updateResolversInPlace: true }); @@ -373,32 +388,28 @@ class Neo4jGraphQL { return this.addDefaultFieldResolvers(schema); } - private parseStartupValidationConfig(): { - validateTypeDefs: boolean; - validateResolvers: boolean; - } { - let validateTypeDefs = true; - let validateResolvers = true; + private parseStartupValidationConfig(): ValidationConfig { + const validationConfig: ValidationConfig = { ...defaultValidationConfig }; if (this.config?.startupValidation === false) { return { validateTypeDefs: false, validateResolvers: false, + validateDuplicateRelationshipFields: false, }; } // TODO - remove in 4.0.0 when skipValidateTypeDefs is removed - if (this.config?.skipValidateTypeDefs === true) validateTypeDefs = false; + if (this.config?.skipValidateTypeDefs === true) validationConfig.validateTypeDefs = false; if (typeof this.config?.startupValidation === "object") { - if (this.config?.startupValidation.typeDefs === false) validateTypeDefs = false; - if (this.config?.startupValidation.resolvers === false) validateResolvers = false; + if (this.config?.startupValidation.typeDefs === false) validationConfig.validateTypeDefs = false; + if (this.config?.startupValidation.resolvers === false) validationConfig.validateResolvers = false; + if (this.config?.startupValidation.noDuplicateRelationshipFields === false) + validationConfig.validateDuplicateRelationshipFields = false; } - return { - validateTypeDefs, - validateResolvers, - }; + return validationConfig; } private pluginsSetup(): Promise { diff --git a/packages/graphql/src/schema/validation/validate-custom-resolver-requires.ts b/packages/graphql/src/schema/validation/validate-custom-resolver-requires.ts index d16e6a4742..ed814b3ddc 100644 --- a/packages/graphql/src/schema/validation/validate-custom-resolver-requires.ts +++ b/packages/graphql/src/schema/validation/validate-custom-resolver-requires.ts @@ -18,35 +18,36 @@ */ import { mergeSchemas } from "@graphql-tools/schema"; -import type { - DocumentNode, - GraphQLSchema, - InterfaceTypeDefinitionNode, - ObjectTypeDefinitionNode} from "graphql"; -import { - Kind, - parse, - validate, -} from "graphql"; -import { getDefinitionNodes } from "../get-definition-nodes"; +import type { DocumentNode, GraphQLSchema, InterfaceTypeDefinitionNode, ObjectTypeDefinitionNode } from "graphql"; +import { Kind, parse, validate } from "graphql"; -export function validateCustomResolverRequires(document: DocumentNode, schema: GraphQLSchema) { - const definitionNodes = getDefinitionNodes(document); - definitionNodes.objectTypes.forEach((objType) => { - objType.fields?.forEach((field) => { - const customResolverDirective = field.directives?.find( - (directive) => directive.name.value === "customResolver" - ); - const requiresArg = customResolverDirective?.arguments?.find((arg) => arg.name.value === "requires"); - if (requiresArg) { - if (requiresArg?.value.kind !== Kind.STRING) { - throw new Error("@customResolver requires expects a string"); - } - const selectionSetDocument = parse(`{ ${requiresArg.value.value} }`); - validateSelectionSet(schema, objType, selectionSetDocument); - } - }); - }); +export function validateCustomResolverRequires(objType: ObjectTypeDefinitionNode, schema: GraphQLSchema) { + if (!objType.fields) { + return; + } + + for (const field of objType.fields) { + if (!field.directives) { + continue; + } + + const customResolverDirective = field.directives.find((directive) => directive.name.value === "customResolver"); + if (!customResolverDirective || !customResolverDirective.arguments) { + continue; + } + + const requiresArg = customResolverDirective.arguments.find((arg) => arg.name.value === "requires"); + if (!requiresArg) { + continue; + } + + if (requiresArg.value.kind !== Kind.STRING) { + throw new Error("@customResolver requires expects a string"); + } + + const selectionSetDocument = parse(`{ ${requiresArg.value.value} }`); + validateSelectionSet(schema, objType, selectionSetDocument); + } } function validateSelectionSet( diff --git a/packages/graphql/src/schema/validation/validate-document.test.ts b/packages/graphql/src/schema/validation/validate-document.test.ts index 19f0bc7dcf..3ffe593a6f 100644 --- a/packages/graphql/src/schema/validation/validate-document.test.ts +++ b/packages/graphql/src/schema/validation/validate-document.test.ts @@ -29,7 +29,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow('Directive "@coalesce" may not be used on OBJECT.'); + expect(() => validateDocument({ document: doc })).toThrow('Directive "@coalesce" may not be used on OBJECT.'); }); test("should throw an error if a directive is missing an argument", () => { @@ -39,7 +39,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Directive "@coalesce" argument "value" of type "ScalarOrEnum!" is required, but it was not provided.' ); }); @@ -51,7 +51,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow('Unknown type "Unknown".'); + expect(() => validateDocument({ document: doc })).toThrow('Unknown type "Unknown".'); }); test("should throw an error if a user tries to pass in their own Point definition", () => { @@ -66,7 +66,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Type "Point" already exists in the schema. It cannot also be defined in this type definition.' ); }); @@ -80,7 +80,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Type "DateTime" already exists in the schema. It cannot also be defined in this type definition.' ); }); @@ -94,7 +94,7 @@ describe("validateDocument", () => { extend type User @fulltext `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Directive "@fulltext" argument "indexes" of type "[FullTextInput]!" is required, but it was not provided.' ); }); @@ -111,7 +111,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Type "PointInput" already exists in the schema. It cannot also be defined in this type definition.' ); }); @@ -127,7 +127,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( "Interface field UserInterface.age expected but User does not provide it." ); }); @@ -141,7 +141,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Directive "@relationship" already exists in the schema. It cannot be redefined.' ); }); @@ -153,7 +153,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); test("should not throw error on use of internal node input types", () => { @@ -181,7 +181,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); describe("relationshipProperties directive", () => { @@ -202,7 +202,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); test("should throw if used on an object type", () => { @@ -212,7 +212,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Directive "@relationshipProperties" may not be used on OBJECT.' ); }); @@ -224,7 +224,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Directive "@relationshipProperties" may not be used on FIELD_DEFINITION.' ); }); @@ -307,7 +307,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); describe("Github Issue 158", () => { @@ -322,7 +322,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); }); @@ -339,7 +339,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); test("should not throw error on validation of schema if SortDirection used", () => { @@ -354,7 +354,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); }); @@ -487,7 +487,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); }); @@ -509,7 +509,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); }); @@ -520,7 +520,7 @@ describe("validateDocument", () => { name: String @alias } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( 'Directive "@alias" argument "property" of type "String!" is required, but it was not provided.' ); }); @@ -530,7 +530,7 @@ describe("validateDocument", () => { name: String } `; - expect(() => validateDocument(doc)).toThrow('Directive "@alias" may not be used on OBJECT.'); + expect(() => validateDocument({ document: doc })).toThrow('Directive "@alias" may not be used on OBJECT.'); }); test("should not throw when used correctly", () => { const doc = gql` @@ -538,7 +538,7 @@ describe("validateDocument", () => { name: String @alias(property: "dbName") } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); }); @@ -551,7 +551,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( RESERVED_TYPE_NAMES.find((x) => x.regex.test("PageInfo"))?.error ); }); @@ -563,7 +563,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( RESERVED_TYPE_NAMES.find((x) => x.regex.test("NodeConnection"))?.error ); }); @@ -575,7 +575,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( RESERVED_TYPE_NAMES.find((x) => x.regex.test("Node"))?.error ); }); @@ -598,7 +598,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( RESERVED_TYPE_NAMES.find((x) => x.regex.test("PageInfo"))?.error ); }); @@ -619,7 +619,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( RESERVED_TYPE_NAMES.find((x) => x.regex.test("NodeConnection"))?.error ); }); @@ -640,7 +640,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).toThrow( + expect(() => validateDocument({ document: doc })).toThrow( RESERVED_TYPE_NAMES.find((x) => x.regex.test("Node"))?.error ); }); @@ -655,7 +655,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); }); @@ -675,7 +675,7 @@ describe("validateDocument", () => { } `; - expect(() => validateDocument(doc)).not.toThrow(); + expect(() => validateDocument({ document: doc })).not.toThrow(); }); }); }); diff --git a/packages/graphql/src/schema/validation/validate-document.ts b/packages/graphql/src/schema/validation/validate-document.ts index 929c5a38bb..e249273b3d 100644 --- a/packages/graphql/src/schema/validation/validate-document.ts +++ b/packages/graphql/src/schema/validation/validate-document.ts @@ -17,6 +17,7 @@ * limitations under the License. */ +import { GraphQLSchema, extendSchema, validateSchema, specifiedDirectives, Kind } from "graphql"; import type { DefinitionNode, DocumentNode, @@ -27,7 +28,6 @@ import type { GraphQLDirective, GraphQLNamedType, } from "graphql"; -import { GraphQLSchema, extendSchema, validateSchema, specifiedDirectives, Kind } from "graphql"; import pluralize from "pluralize"; import * as scalars from "../../graphql/scalars"; import * as directives from "../../graphql/directives"; @@ -40,7 +40,9 @@ import { PointDistance } from "../../graphql/input-objects/PointDistance"; import { CartesianPointDistance } from "../../graphql/input-objects/CartesianPointDistance"; import { RESERVED_TYPE_NAMES } from "../../constants"; import { isRootType } from "../../utils/is-root-type"; -import { validateCustomResolverRequires } from "./validate-custom-resolver-requires"; +import { validateSchemaCustomizations } from "./validate-schema-customizations"; +import type { ValidationConfig } from "../../classes/Neo4jGraphQL"; +import { defaultValidationConfig } from "../../classes/Neo4jGraphQL"; function filterDocument(document: DocumentNode): DocumentNode { const nodeNames = document.definitions @@ -159,12 +161,17 @@ function filterDocument(document: DocumentNode): DocumentNode { }; } -function getBaseSchema( - document: DocumentNode, +function getBaseSchema({ + document, validateTypeDefs = true, - additionalDirectives: Array = [], - additionalTypes: Array = [] -): GraphQLSchema { + additionalDirectives = [], + additionalTypes = [], +}: { + document: DocumentNode; + validateTypeDefs: boolean; + additionalDirectives: Array; + additionalTypes: Array; +}): GraphQLSchema { const doc = filterDocument(document); const schemaToExtend = new GraphQLSchema({ @@ -185,21 +192,31 @@ function getBaseSchema( return extendSchema(schemaToExtend, doc, { assumeValid: !validateTypeDefs }); } -function validateDocument( - document: DocumentNode, - validateTypeDefs = true, - additionalDirectives: Array = [], - additionalTypes: Array = [] -): void { - const schema = getBaseSchema(document, validateTypeDefs, additionalDirectives, additionalTypes); - if (validateTypeDefs) { +function validateDocument({ + document, + validationConfig = defaultValidationConfig, + additionalDirectives = [], + additionalTypes = [], +}: { + document: DocumentNode; + validationConfig?: ValidationConfig; + additionalDirectives?: Array; + additionalTypes?: Array; +}): void { + const schema = getBaseSchema({ + document, + validateTypeDefs: validationConfig.validateTypeDefs, + additionalDirectives, + additionalTypes, + }); + if (validationConfig.validateTypeDefs) { const errors = validateSchema(schema); const filteredErrors = errors.filter((e) => e.message !== "Query root type must be provided."); if (filteredErrors.length) { throw new Error(filteredErrors.join("\n")); } } - validateCustomResolverRequires(document, schema); + validateSchemaCustomizations({ document, schema, validationConfig }); } export default validateDocument; diff --git a/packages/graphql/src/schema/validation/validate-duplicate-relationship-fields.test.ts b/packages/graphql/src/schema/validation/validate-duplicate-relationship-fields.test.ts new file mode 100644 index 0000000000..380ce1cdb7 --- /dev/null +++ b/packages/graphql/src/schema/validation/validate-duplicate-relationship-fields.test.ts @@ -0,0 +1,76 @@ +/* + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { gql } from "apollo-server"; +import validateDocument from "./validate-document"; + +describe("validateDuplicateRelationshipFields", () => { + test("should throw an error if multiple relationship fields in the same type have the same relationship type.", () => { + const doc = gql` + type Team { + name: String! + player1: Person! @relationship(type: "PLAYS_IN", direction: IN) + player2: Person! @relationship(type: "PLAYS_IN", direction: IN) + backupPlayers: [Person!]! @relationship(type: "PLAYS_IN", direction: IN) + } + + type Person { + name: String! + teams: [Team!]! @relationship(type: "PLAYS_IN", direction: OUT) + } + `; + + expect(() => validateDocument({ document: doc })).toThrow( + "Multiple relationship fields with the same type and direction may not have the same relationship type." + ); + }); + + test("should not throw an error if multiple relationship fields of different types have the same relationship type.", () => { + const doc = gql` + type Team { + name: String! + player: Person! @relationship(type: "PLAYS_IN", direction: IN) + venue: Venue! @relationship(type: "PLAYS_IN", direction: IN) + } + + type Person { + name: String! + teams: [Team!]! @relationship(type: "PLAYS_IN", direction: OUT) + } + + type Venue { + location: String! + } + `; + + expect(() => validateDocument({ document: doc })).not.toThrow(); + }); + + test("should not throw an error if multiple relationship fields in the same type have the same relationship type but have different directions.", () => { + const doc = gql` + type Person { + name: String! + knows: [Person!]! @relationship(type: "KNOWS", direction: OUT) + knowedBy: [Person!]! @relationship(type: "KNOWS", direction: IN) + } + `; + + expect(() => validateDocument({ document: doc })).not.toThrow(); + }); +}); diff --git a/packages/graphql/src/schema/validation/validate-duplicate-relationship-fields.ts b/packages/graphql/src/schema/validation/validate-duplicate-relationship-fields.ts new file mode 100644 index 0000000000..8e22f6c037 --- /dev/null +++ b/packages/graphql/src/schema/validation/validate-duplicate-relationship-fields.ts @@ -0,0 +1,65 @@ +/* + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { ObjectTypeDefinitionNode } from "graphql"; +import getFieldTypeMeta from "../get-field-type-meta"; +import { Kind } from "graphql"; + +export function validateDuplicateRelationshipFields(objType: ObjectTypeDefinitionNode) { + if (!objType.fields) { + return; + } + + const relationshipUsages = new Set(); + + for (const field of objType.fields) { + if (!field.directives) { + continue; + } + + const relationshipDirective = field.directives.find((directive) => directive.name.value === "relationship"); + if (!relationshipDirective || !relationshipDirective.arguments) { + continue; + } + + const typeArg = relationshipDirective.arguments.find((arg) => arg.name.value === "type"); + const directionArg = relationshipDirective.arguments.find((arg) => arg.name.value === "direction"); + if (!typeArg || !directionArg) { + continue; + } + + if (typeArg.value.kind !== Kind.STRING) { + throw new Error("@relationship type expects a string"); + } + + if (directionArg.value.kind !== Kind.ENUM) { + throw new Error("@relationship direction expects an enum"); + } + + const typeMeta = getFieldTypeMeta(field.type); + + if (relationshipUsages.has(`${typeMeta.name}__${typeArg.value.value}__${directionArg.value.value}`)) { + throw new Error( + "Multiple relationship fields with the same type and direction may not have the same relationship type." + ); + } + + relationshipUsages.add(`${typeMeta.name}__${typeArg.value.value}__${directionArg.value.value}`); + } +} diff --git a/packages/graphql/src/schema/validation/validate-schema-customizations.ts b/packages/graphql/src/schema/validation/validate-schema-customizations.ts new file mode 100644 index 0000000000..1b9bc99eaa --- /dev/null +++ b/packages/graphql/src/schema/validation/validate-schema-customizations.ts @@ -0,0 +1,43 @@ +/* + * Copyright (c) "Neo4j" + * Neo4j Sweden AB [http://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { DocumentNode, GraphQLSchema } from "graphql"; +import type { ValidationConfig } from "../../classes/Neo4jGraphQL"; +import { getDefinitionNodes } from "../get-definition-nodes"; +import { validateCustomResolverRequires } from "./validate-custom-resolver-requires"; +import { validateDuplicateRelationshipFields } from "./validate-duplicate-relationship-fields"; + +export function validateSchemaCustomizations({ + document, + schema, + validationConfig, +}: { + document: DocumentNode; + schema: GraphQLSchema; + validationConfig: ValidationConfig; +}) { + const definitionNodes = getDefinitionNodes(document); + + for (const objectType of definitionNodes.objectTypes) { + validateCustomResolverRequires(objectType, schema); + if (validationConfig.validateDuplicateRelationshipFields) { + validateDuplicateRelationshipFields(objectType); + } + } +} diff --git a/packages/graphql/src/types.ts b/packages/graphql/src/types.ts index 5a805b15de..2d4ac5f892 100644 --- a/packages/graphql/src/types.ts +++ b/packages/graphql/src/types.ts @@ -366,6 +366,7 @@ export interface CypherQueryOptions { export interface StartupValidationOptions { typeDefs?: boolean; resolvers?: boolean; + noDuplicateRelationshipFields?: boolean; } /** diff --git a/packages/graphql/tests/integration/config-options/startup-validation.int.test.ts b/packages/graphql/tests/integration/config-options/startup-validation.int.test.ts index 3eeb4e9fef..350b231f01 100644 --- a/packages/graphql/tests/integration/config-options/startup-validation.int.test.ts +++ b/packages/graphql/tests/integration/config-options/startup-validation.int.test.ts @@ -27,6 +27,8 @@ describe("Startup Validation", () => { const invalidTypeDefsError = 'Type "Point" already exists in the schema.'; const missingCustomResolverError = "Custom resolver for fullName has not been provided"; + const duplicateRelationshipFieldsError = + "Multiple relationship fields with the same type and direction may not have the same relationship type."; const customResolverTypeDefs = ` type User { @@ -66,6 +68,32 @@ describe("Startup Validation", () => { } `; + const invalidDuplicateRelationship = ` + type User { + id: ID! + firstName: String! + lastName: String! + friend1: User! @relationship(type: "FRIENDS_WITH", direction: IN) + friend2: User! @relationship(type: "FRIENDS_WITH", direction: IN) + } + `; + + const invalidAll = ` + type User { + id: ID! + firstName: String! + lastName: String! + fullName: String @customResolver(requires: "firstName lastName") + friend1: User! @relationship(type: "FRIENDS_WITH", direction: IN) + friend2: User! @relationship(type: "FRIENDS_WITH", direction: IN) + } + + type Point { + latitude: Float! + longitude: Float! + } + `; + beforeAll(async () => { neo4j = new Neo4j(); driver = await neo4j.getDriver(); @@ -318,4 +346,48 @@ describe("Startup Validation", () => { await expect(neoSchema.getSchema()).resolves.not.toThrow(); }); + + test("should throw an error for duplicate relationship fields when startupValidation.noDuplicateRelationshipFields is true", async () => { + const neoSchema = new Neo4jGraphQL({ + typeDefs: invalidDuplicateRelationship, + driver, + config: { + startupValidation: { + typeDefs: true, + resolvers: true, + noDuplicateRelationshipFields: true, + }, + }, + }); + + await expect(neoSchema.getSchema()).rejects.toThrow(duplicateRelationshipFieldsError); + }); + + test("should not throw an error for duplicate relationship fields when startupValidation.noDuplicateRelationshipFields is false", async () => { + const neoSchema = new Neo4jGraphQL({ + typeDefs: invalidDuplicateRelationship, + driver, + config: { + startupValidation: { + typeDefs: true, + resolvers: true, + noDuplicateRelationshipFields: false, + }, + }, + }); + + await expect(neoSchema.getSchema()).resolves.not.toThrow(); + }); + + test("should throw no errors when startupValidation false", async () => { + const neoSchema = new Neo4jGraphQL({ + typeDefs: invalidAll, + driver, + config: { + startupValidation: false, + }, + }); + + await expect(neoSchema.getSchema()).resolves.not.toThrow(); + }); }); diff --git a/packages/graphql/tests/schema/rfcs/rfc-003.test.ts b/packages/graphql/tests/schema/rfcs/rfc-003.test.ts index c87038ba9a..7dfab683ed 100644 --- a/packages/graphql/tests/schema/rfcs/rfc-003.test.ts +++ b/packages/graphql/tests/schema/rfcs/rfc-003.test.ts @@ -29,13 +29,21 @@ describe("schema/rfc/003", () => { const typeDefs = gql` type Source { targets: [Target!]! @relationship(type: "HAS_TARGET", direction: OUT) - target1: Target! @relationship(type: "HAS_TARGET", direction: OUT) - target2: Target @relationship(type: "HAS_TARGET", direction: OUT) + target1: SecondTarget! @relationship(type: "HAS_TARGET", direction: OUT) + target2: ThirdTarget @relationship(type: "HAS_TARGET", direction: OUT) } type Target { id: ID @id } + + type SecondTarget { + id: ID @id + } + + type ThirdTarget { + id: ID @id + } `; const neoSchema = new Neo4jGraphQL({ typeDefs });