Skip to content

Commit 15a7f04

Browse files
mjfwebbmjfwebb
authored
Restrict multiple relationship fields with same type (#3043)
* feat: add validation for duplicate relationship fields * feat: add validation config for noDuplicateRelationshipFields
1 parent 0fb2592 commit 15a7f04

File tree

12 files changed

+486
-120
lines changed

12 files changed

+486
-120
lines changed

‎.changeset/khaki-experts-decide.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@neo4j/graphql": major
3+
---
4+
5+
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.
6+
7+
An example of what is now considered invalid with these checks:
8+
9+
```graphql
10+
type Team {
11+
player1: Person! @relationship(type: "PLAYS_IN", direction: IN)
12+
player2: Person! @relationship(type: "PLAYS_IN", direction: IN)
13+
backupPlayers: [Person!]! @relationship(type: "PLAYS_IN", direction: IN)
14+
}
15+
16+
type Person {
17+
teams: [Team!]! @relationship(type: "PLAYS_IN", direction: OUT)
18+
}
19+
```
20+
21+
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/)

‎docs/modules/ROOT/pages/guides/v4-migration/index.adoc‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,12 +526,48 @@ type User {
526526
}
527527
----
528528

529+
=== Duplicate relationship fields are now checked for
530+
531+
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.
532+
533+
An example of what is now considered invalid with these checks:
534+
535+
[source, graphql, indent=0]
536+
----
537+
type Team {
538+
player1: Person! @relationship(type: "PLAYS_IN", direction: IN)
539+
player2: Person! @relationship(type: "PLAYS_IN", direction: IN)
540+
backupPlayers: [Person!]! @relationship(type: "PLAYS_IN", direction: IN)
541+
}
542+
543+
type Person {
544+
teams: [Team!]! @relationship(type: "PLAYS_IN", direction: OUT)
545+
}
546+
----
547+
548+
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.
549+
550+
To disable checks for duplicate relationship fields, the `noDuplicateRelationshipFields` config option should be used:
551+
552+
[source, javascript, indent=0]
553+
----
554+
const neoSchema = new Neo4jGraphQL({
555+
typeDefs,
556+
config: {
557+
startupValidation: {
558+
noDuplicateRelationshipFields: false,
559+
},
560+
},
561+
});
562+
----
563+
564+
529565
== Miscellaneous changes
530566

531567
[[startup-validation]]
532568
=== Startup validation
533569

534-
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.
570+
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.
535571
This new option has been combined with the option to `skipValidateTypeDefs`. As a result, `skipValidateTypeDefs` will be removed and replaced by `startupValidation`.
536572

537573
To only disable strict type definition validation, the following config option should be used:
@@ -562,6 +598,21 @@ const neoSchema = new Neo4jGraphQL({
562598
})
563599
----
564600

601+
To only disable checks for duplicate relationship fields, the following config option should be used:
602+
603+
[source, javascript, indent=0]
604+
----
605+
const neoSchema = new Neo4jGraphQL({
606+
typeDefs,
607+
config: {
608+
startupValidation: {
609+
noDuplicateRelationshipFields: false
610+
},
611+
},
612+
})
613+
----
614+
615+
565616
To disable all startup checks, the following config option should be used:
566617

567618
[source, javascript, indent=0]

‎packages/graphql/src/classes/Neo4jGraphQL.ts‎

Lines changed: 53 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ export interface Neo4jGraphQLConfig {
6767
callbacks?: Neo4jGraphQLCallbacks;
6868
}
6969

70+
export type ValidationConfig = {
71+
validateTypeDefs: boolean;
72+
validateResolvers: boolean;
73+
validateDuplicateRelationshipFields: boolean;
74+
};
75+
7076
export interface Neo4jGraphQLConstructor {
7177
typeDefs: TypeSource;
7278
resolvers?: IExecutableSchemaDefinition["resolvers"];
@@ -76,6 +82,12 @@ export interface Neo4jGraphQLConstructor {
7682
plugins?: Neo4jGraphQLPlugins;
7783
}
7884

85+
export const defaultValidationConfig: ValidationConfig = {
86+
validateTypeDefs: true,
87+
validateResolvers: true,
88+
validateDuplicateRelationshipFields: true,
89+
};
90+
7991
class Neo4jGraphQL {
8092
typeDefs: TypeSource;
8193
resolvers?: IExecutableSchemaDefinition["resolvers"];
@@ -238,17 +250,16 @@ class Neo4jGraphQL {
238250
return getNeo4jDatabaseInfo(new Executor(executorConstructorParam));
239251
}
240252

241-
private wrapResolvers(resolvers: NonNullable<IExecutableSchemaDefinition["resolvers"]>) {
242-
if (!this.schemaModel) {
243-
throw new Error("Schema Model is not defined");
244-
}
245-
253+
private wrapResolvers(
254+
resolvers: NonNullable<IExecutableSchemaDefinition["resolvers"]>,
255+
schemaModel: Neo4jGraphQLSchemaModel
256+
) {
246257
const wrapResolverArgs = {
247258
driver: this.driver,
248259
config: this.config,
249260
nodes: this.nodes,
250261
relationships: this.relationships,
251-
schemaModel: this.schemaModel,
262+
schemaModel: schemaModel,
252263
plugins: this.plugins,
253264
};
254265

@@ -263,17 +274,16 @@ class Neo4jGraphQL {
263274
return composeResolvers(mergedResolvers, resolversComposition);
264275
}
265276

266-
private wrapFederationResolvers(resolvers: NonNullable<IExecutableSchemaDefinition["resolvers"]>) {
267-
if (!this.schemaModel) {
268-
throw new Error("Schema Model is not defined");
269-
}
270-
277+
private wrapFederationResolvers(
278+
resolvers: NonNullable<IExecutableSchemaDefinition["resolvers"]>,
279+
schemaModel: Neo4jGraphQLSchemaModel
280+
) {
271281
const wrapResolverArgs = {
272282
driver: this.driver,
273283
config: this.config,
274284
nodes: this.nodes,
275285
relationships: this.relationships,
276-
schemaModel: this.schemaModel,
286+
schemaModel: schemaModel,
277287
plugins: this.plugins,
278288
};
279289

@@ -286,22 +296,27 @@ class Neo4jGraphQL {
286296
return composeResolvers(mergedResolvers, resolversComposition);
287297
}
288298

299+
private generateSchemaModel(document: DocumentNode): Neo4jGraphQLSchemaModel {
300+
// This can be run several times but it will always be the same result,
301+
// so we memoize the schemaModel.
302+
if (!this.schemaModel) {
303+
this.schemaModel = generateModel(document);
304+
}
305+
return this.schemaModel;
306+
}
307+
289308
private generateExecutableSchema(): Promise<GraphQLSchema> {
290309
return new Promise((resolve) => {
291310
const document = this.getDocument(this.typeDefs);
292311

293-
const { validateTypeDefs, validateResolvers } = this.parseStartupValidationConfig();
294-
295-
validateDocument(document, validateTypeDefs);
312+
const validationConfig = this.parseStartupValidationConfig();
296313

297-
if (!this.schemaModel) {
298-
this.schemaModel = generateModel(document);
299-
}
314+
validateDocument({ document, validationConfig });
300315

301316
const { nodes, relationships, typeDefs, resolvers } = makeAugmentedSchema(document, {
302317
features: this.features,
303318
enableRegex: this.config?.enableRegex,
304-
validateResolvers,
319+
validateResolvers: validationConfig.validateResolvers,
305320
generateSubscriptions: Boolean(this.plugins?.subscriptions),
306321
callbacks: this.config.callbacks,
307322
userCustomResolvers: this.resolvers,
@@ -310,8 +325,10 @@ class Neo4jGraphQL {
310325
this._nodes = nodes;
311326
this._relationships = relationships;
312327

328+
const schemaModel = this.generateSchemaModel(document);
329+
313330
// Wrap the generated and custom resolvers, which adds a context including the schema to every request
314-
const wrappedResolvers = this.wrapResolvers(resolvers);
331+
const wrappedResolvers = this.wrapResolvers(resolvers, schemaModel);
315332

316333
const schema = makeExecutableSchema({
317334
typeDefs,
@@ -330,18 +347,14 @@ class Neo4jGraphQL {
330347

331348
const { directives, types } = subgraph.getValidationDefinitions();
332349

333-
const { validateTypeDefs, validateResolvers } = this.parseStartupValidationConfig();
350+
const validationConfig = this.parseStartupValidationConfig();
334351

335-
validateDocument(document, validateTypeDefs, directives, types);
336-
337-
if (!this.schemaModel) {
338-
this.schemaModel = generateModel(document);
339-
}
352+
validateDocument({ document, validationConfig, additionalDirectives: directives, additionalTypes: types });
340353

341354
const { nodes, relationships, typeDefs, resolvers } = makeAugmentedSchema(document, {
342355
features: this.features,
343356
enableRegex: this.config?.enableRegex,
344-
validateResolvers,
357+
validateResolvers: validationConfig.validateResolvers,
345358
generateSubscriptions: Boolean(this.plugins?.subscriptions),
346359
callbacks: this.config.callbacks,
347360
userCustomResolvers: this.resolvers,
@@ -354,7 +367,9 @@ class Neo4jGraphQL {
354367
// TODO: Move into makeAugmentedSchema, add resolvers alongside other resolvers
355368
const referenceResolvers = subgraph.getReferenceResolvers(this._nodes);
356369

357-
const wrappedResolvers = this.wrapResolvers([resolvers, referenceResolvers]);
370+
const schemaModel = this.generateSchemaModel(document);
371+
372+
const wrappedResolvers = this.wrapResolvers([resolvers, referenceResolvers], schemaModel);
358373

359374
const schema = subgraph.buildSchema({
360375
typeDefs,
@@ -365,40 +380,36 @@ class Neo4jGraphQL {
365380
const subgraphResolvers = getResolversFromSchema(schema);
366381

367382
// Wrap the _entities and _service Query resolvers
368-
const wrappedSubgraphResolvers = this.wrapFederationResolvers(subgraphResolvers);
383+
const wrappedSubgraphResolvers = this.wrapFederationResolvers(subgraphResolvers, schemaModel);
369384

370385
// Add the wrapped resolvers back to the schema, context will now be populated
371386
addResolversToSchema({ schema, resolvers: wrappedSubgraphResolvers, updateResolversInPlace: true });
372387

373388
return this.addDefaultFieldResolvers(schema);
374389
}
375390

376-
private parseStartupValidationConfig(): {
377-
validateTypeDefs: boolean;
378-
validateResolvers: boolean;
379-
} {
380-
let validateTypeDefs = true;
381-
let validateResolvers = true;
391+
private parseStartupValidationConfig(): ValidationConfig {
392+
const validationConfig: ValidationConfig = { ...defaultValidationConfig };
382393

383394
if (this.config?.startupValidation === false) {
384395
return {
385396
validateTypeDefs: false,
386397
validateResolvers: false,
398+
validateDuplicateRelationshipFields: false,
387399
};
388400
}
389401

390402
// TODO - remove in 4.0.0 when skipValidateTypeDefs is removed
391-
if (this.config?.skipValidateTypeDefs === true) validateTypeDefs = false;
403+
if (this.config?.skipValidateTypeDefs === true) validationConfig.validateTypeDefs = false;
392404

393405
if (typeof this.config?.startupValidation === "object") {
394-
if (this.config?.startupValidation.typeDefs === false) validateTypeDefs = false;
395-
if (this.config?.startupValidation.resolvers === false) validateResolvers = false;
406+
if (this.config?.startupValidation.typeDefs === false) validationConfig.validateTypeDefs = false;
407+
if (this.config?.startupValidation.resolvers === false) validationConfig.validateResolvers = false;
408+
if (this.config?.startupValidation.noDuplicateRelationshipFields === false)
409+
validationConfig.validateDuplicateRelationshipFields = false;
396410
}
397411

398-
return {
399-
validateTypeDefs,
400-
validateResolvers,
401-
};
412+
return validationConfig;
402413
}
403414

404415
private pluginsSetup(): Promise<void> {

‎packages/graphql/src/schema/validation/validate-custom-resolver-requires.ts‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,35 +18,36 @@
1818
*/
1919

2020
import { mergeSchemas } from "@graphql-tools/schema";
21-
import type {
22-
DocumentNode,
23-
GraphQLSchema,
24-
InterfaceTypeDefinitionNode,
25-
ObjectTypeDefinitionNode} from "graphql";
26-
import {
27-
Kind,
28-
parse,
29-
validate,
30-
} from "graphql";
31-
import { getDefinitionNodes } from "../get-definition-nodes";
21+
import type { DocumentNode, GraphQLSchema, InterfaceTypeDefinitionNode, ObjectTypeDefinitionNode } from "graphql";
22+
import { Kind, parse, validate } from "graphql";
3223

33-
export function validateCustomResolverRequires(document: DocumentNode, schema: GraphQLSchema) {
34-
const definitionNodes = getDefinitionNodes(document);
35-
definitionNodes.objectTypes.forEach((objType) => {
36-
objType.fields?.forEach((field) => {
37-
const customResolverDirective = field.directives?.find(
38-
(directive) => directive.name.value === "customResolver"
39-
);
40-
const requiresArg = customResolverDirective?.arguments?.find((arg) => arg.name.value === "requires");
41-
if (requiresArg) {
42-
if (requiresArg?.value.kind !== Kind.STRING) {
43-
throw new Error("@customResolver requires expects a string");
44-
}
45-
const selectionSetDocument = parse(`{ ${requiresArg.value.value} }`);
46-
validateSelectionSet(schema, objType, selectionSetDocument);
47-
}
48-
});
49-
});
24+
export function validateCustomResolverRequires(objType: ObjectTypeDefinitionNode, schema: GraphQLSchema) {
25+
if (!objType.fields) {
26+
return;
27+
}
28+
29+
for (const field of objType.fields) {
30+
if (!field.directives) {
31+
continue;
32+
}
33+
34+
const customResolverDirective = field.directives.find((directive) => directive.name.value === "customResolver");
35+
if (!customResolverDirective || !customResolverDirective.arguments) {
36+
continue;
37+
}
38+
39+
const requiresArg = customResolverDirective.arguments.find((arg) => arg.name.value === "requires");
40+
if (!requiresArg) {
41+
continue;
42+
}
43+
44+
if (requiresArg.value.kind !== Kind.STRING) {
45+
throw new Error("@customResolver requires expects a string");
46+
}
47+
48+
const selectionSetDocument = parse(`{ ${requiresArg.value.value} }`);
49+
validateSelectionSet(schema, objType, selectionSetDocument);
50+
}
5051
}
5152

5253
function validateSelectionSet(

0 commit comments

Comments
 (0)