Make @node mandatory to generate Queries and Mutations. (#5820)

.changeset/perfect-zoos-push.md

@@ -0,0 +1,6 @@
+---
+"@neo4j/graphql": major
+---
+
+Change the way how `@node` behaves, `@node` is now required, and GraphQL Object types without the directive `@node` will no longer considered as a Neo4j Nodes representation. 
+Queries and Mutations will be generated only for types with the `@node` directive.

packages/apollo-federation-subgraph-compatibility/src/type-defs.ts

@@ -42,7 +42,7 @@ export const typeDefs = gql`
 
     directive @custom on OBJECT
 
-    type Product @custom @key(fields: "id") @key(fields: "sku package") @key(fields: "sku variation { id }") @node {
+    type Product @node @custom @key(fields: "id") @key(fields: "sku package") @key(fields: "sku variation { id }") {
         id: ID!
         sku: String
         package: String
@@ -75,7 +75,7 @@ export const typeDefs = gql`
         research: [ProductResearch!]! @relationship(type: "HAS_RESEARCH", direction: OUT)
     }
 
-    type DeprecatedProduct @key(fields: "sku package") @node {
+    type DeprecatedProduct @node @key(fields: "sku package") {
         sku: String!
         package: String!
         reason: String
@@ -137,7 +137,7 @@ export const typeDefs = gql`
         yearsOfEmployment: Int! @external
     }
 
-    type Inventory @interfaceObject @key(fields: "id") {
+    type Inventory @node @interfaceObject @key(fields: "id") {
         id: ID!
         deprecatedProducts: [DeprecatedProduct!]! @relationship(type: "HAS_DEPRECATED_PRODUCT", direction: OUT)
     }

packages/graphql/src/classes/Neo4jGraphQL.ts

@@ -23,13 +23,13 @@ import type { IExecutableSchemaDefinition } from "@graphql-tools/schema";
 import { addResolversToSchema, makeExecutableSchema } from "@graphql-tools/schema";
 import { forEachField, getResolversFromSchema } from "@graphql-tools/utils";
 import Debug from "debug";
-import type { DocumentNode, GraphQLSchema } from "graphql";
+import { type DocumentNode, type GraphQLSchema } from "graphql";
 import type { Driver, SessionConfig } from "neo4j-driver";
 import { DEBUG_ALL } from "../constants";
 import { makeAugmentedSchema } from "../schema";
 import type { Neo4jGraphQLSchemaModel } from "../schema-model/Neo4jGraphQLSchemaModel";
 import { generateModel } from "../schema-model/generate-model";
-import { getDefinitionNodes } from "../schema/get-definition-nodes";
+import { getDefinitionCollection } from "../schema-model/parser/definition-collection";
 import { makeDocumentToAugment } from "../schema/make-document-to-augment";
 import type { WrapResolverArguments } from "../schema/resolvers/composition/wrap-query-and-mutation";
 import { wrapQueryAndMutation } from "../schema/resolvers/composition/wrap-query-and-mutation";
@@ -365,12 +365,17 @@ class Neo4jGraphQL {
                     interfaceTypes: interfaces,
                     unionTypes: unions,
                     objectTypes: objects,
-                } = getDefinitionNodes(initialDocument);
+                } = getDefinitionCollection(initialDocument);
 
                 validateDocument({
                     document: initialDocument,
                     features: this.features,
-                    additionalDefinitions: { enums, interfaces, unions, objects },
+                    additionalDefinitions: {
+                        enums: [...enums.values()],
+                        interfaces: [...interfaces.values()],
+                        unions: [...unions.values()],
+                        objects: [...objects.values()],
+                    },
                     userCustomResolvers: this.resolvers,
                 });
             }
@@ -421,18 +426,18 @@ class Neo4jGraphQL {
                 interfaceTypes: interfaces,
                 unionTypes: unions,
                 objectTypes: objects,
-            } = getDefinitionNodes(initialDocument);
+            } = getDefinitionCollection(initialDocument);
 
             validateDocument({
                 document: initialDocument,
                 features: this.features,
                 additionalDefinitions: {
                     additionalDirectives: directives,
                     additionalTypes: types,
-                    enums,
-                    interfaces,
-                    unions,
-                    objects,
+                    enums: [...enums.values()],
+                    interfaces: [...interfaces.values()],
+                    unions: [...unions.values()],
+                    objects: [...objects.values()],
                 },
                 userCustomResolvers: this.resolvers,
             });

packages/graphql/src/classes/utils/asserts-indexes-and-constraints.ts

@@ -133,7 +133,7 @@ async function checkIndexesAndConstraints({
     schemaModel: Neo4jGraphQLSchemaModel;
     session: Session;
 }): Promise<void> {
-    const missingConstraints = await getMissingConstraints({ schemaModel, session });
+    const missingConstraints = await getMissingConstraints({ session });
 
     if (missingConstraints.length) {
         const missingConstraintMessages = missingConstraints.map(
@@ -193,13 +193,7 @@ async function checkIndexesAndConstraints({
 
 type MissingConstraint = { constraintName: string; label: string; property: string };
 
-async function getMissingConstraints({
-    schemaModel,
-    session,
-}: {
-    schemaModel: Neo4jGraphQLSchemaModel;
-    session: Session;
-}): Promise<MissingConstraint[]> {
+async function getMissingConstraints({ session }: { session: Session }): Promise<MissingConstraint[]> {
     const existingConstraints: Record<string, string[]> = {};
 
     const constraintsCypher = "SHOW UNIQUE CONSTRAINTS";

packages/graphql/src/schema-model/Neo4jGraphQLSchemaModel.ts

@@ -70,8 +70,7 @@ export class Neo4jGraphQLSchemaModel {
     }
 
     public getConcreteEntity(name: string): ConcreteEntity | undefined {
-        const concreteEntity = this.concreteEntities.find((entity) => entity.name === name);
-        return concreteEntity;
+        return this.concreteEntities.find((entity) => entity.name === name);
     }
 
     public getEntitiesByLabels(labels: string[]): ConcreteEntity[] {

packages/graphql/src/schema-model/annotation/CustomResolverAnnotation.ts

@@ -19,9 +19,9 @@
 
 import type { DocumentNode, FieldDefinitionNode } from "graphql";
 import { parse } from "graphql";
-import { selectionSetToResolveTree } from "../../schema/get-custom-resolver-meta";
-import { getDefinitionNodes } from "../../schema/get-definition-nodes";
 import type { ResolveTree } from "graphql-parse-resolve-info";
+import { selectionSetToResolveTree } from "../../schema/get-custom-resolver-meta";
+import { getDefinitionCollection } from "../parser/definition-collection";
 import type { Annotation } from "./Annotation";
 
 export class CustomResolverAnnotation implements Annotation {
@@ -37,16 +37,18 @@ export class CustomResolverAnnotation implements Annotation {
         if (!this.requires) {
             return;
         }
-        const definitionNodes = getDefinitionNodes(document);
+        const definitionCollection = getDefinitionCollection(document);
 
-        const { interfaceTypes, objectTypes, unionTypes } = definitionNodes;
+        const { interfaceTypes, objectTypes, unionTypes } = definitionCollection;
 
         const selectionSetDocument = parse(`{ ${this.requires} }`);
+        // TODO: likely selectionSetToResolveTree could be change to accept Maps instead of Arrays.
+        // initially these were arrays as they were coming from getDefinitionNodes that was returning arrays
         this.parsedRequires = selectionSetToResolveTree(
             objectFields || [],
-            objectTypes,
-            interfaceTypes,
-            unionTypes,
+            [...objectTypes.values()],
+            [...interfaceTypes.values()],
+            [...unionTypes.values()],
             selectionSetDocument
         );
     }

packages/graphql/src/schema-model/attribute/model-adapters/AttributeAdapter.ts

@@ -126,7 +126,7 @@ export class AttributeAdapter {
             (this.typeHelper.isEnum() ||
                 this.typeHelper.isSpatial() ||
                 this.typeHelper.isScalar() ||
-                this.isCypherRelationshipField()) &&
+                (this.isCypherRelationshipField() && !this.typeHelper.isList())) &&
             this.isFilterable() &&
             !this.isCustomResolvable()
         );

packages/graphql/src/schema-model/generate-model.ts

@@ -39,7 +39,8 @@ import type { Operations } from "./Neo4jGraphQLSchemaModel";
 import { Neo4jGraphQLSchemaModel } from "./Neo4jGraphQLSchemaModel";
 import { Operation } from "./Operation";
 import type { Attribute } from "./attribute/Attribute";
-import { ObjectType } from "./attribute/AttributeType";
+import type { AttributeType } from "./attribute/AttributeType";
+import { ListType, ObjectType } from "./attribute/AttributeType";
 import type { CompositeEntity } from "./entity/CompositeEntity";
 import { ConcreteEntity } from "./entity/ConcreteEntity";
 import type { Entity } from "./entity/Entity";
@@ -72,9 +73,6 @@ export function generateModel(document: DocumentNode): Neo4jGraphQLSchemaModel {
     );
 
     const concreteEntitiesMap = concreteEntities.reduce((acc, entity) => {
-        if (acc.has(entity.name)) {
-            throw new Neo4jGraphQLSchemaValidationError(`Duplicate node ${entity.name}`);
-        }
         acc.set(entity.name, entity);
         return acc;
     }, new Map<string, ConcreteEntity>());
@@ -131,20 +129,31 @@ function addCompositeEntitiesToConcreteEntity(compositeEntities: CompositeEntity
     });
 }
 
+function getCypherTarget(schema: Neo4jGraphQLSchemaModel, attributeType: AttributeType): ConcreteEntity | undefined {
+    if (attributeType instanceof ListType) {
+        return getCypherTarget(schema, attributeType.ofType);
+    }
+    if (attributeType instanceof ObjectType) {
+        const foundConcreteEntity = schema.getConcreteEntity(attributeType.name);
+        if (!foundConcreteEntity) {
+            throw new Neo4jGraphQLSchemaValidationError(
+                `@cypher field must target type annotated with the @node directive${attributeType.name}, `
+            );
+        }
+        return schema.getConcreteEntity(attributeType.name);
+    }
+    if (attributeType instanceof InterfaceEntity || attributeType instanceof UnionEntity) {
+        throw new Error("@cypher field target cannot be an interface or an union");
+    }
+}
+
+// TODO: currently the below is used only for Filtering purposes, and therefore the target is set only for ObjectTypes but in the future we might want to use it for other types as well
 function hydrateCypherAnnotations(schema: Neo4jGraphQLSchemaModel, concreteEntities: ConcreteEntity[]) {
     for (const concreteEntity of concreteEntities) {
         for (const attributeField of concreteEntity.attributes.values()) {
             if (attributeField.annotations.cypher) {
-                if (attributeField.type instanceof ObjectType) {
-                    const foundConcreteEntity = schema.getConcreteEntity(attributeField.type.name);
-                    if (!foundConcreteEntity) {
-                        throw new Neo4jGraphQLSchemaValidationError(
-                            `Could not find concrete entity with name ${attributeField.type.name}`
-                        );
-                    }
-
-                    attributeField.annotations.cypher.targetEntity = foundConcreteEntity;
-                }
+                const target = getCypherTarget(schema, attributeField.type);
+                attributeField.annotations.cypher.targetEntity = target;
             }
         }
     }
@@ -225,21 +234,12 @@ function generateCompositeEntity(
     entityImplementingTypeNames: string[],
     concreteEntities: Map<string, ConcreteEntity>
 ): { name: string; concreteEntities: ConcreteEntity[] } {
-    const compositeFields = entityImplementingTypeNames.map((type) => {
-        const concreteEntity = concreteEntities.get(type);
-        if (!concreteEntity) {
-            throw new Neo4jGraphQLSchemaValidationError(`Could not find concrete entity with name ${type}`);
-        }
-        return concreteEntity;
-    });
-    /*
-   // This is commented out because is currently possible to have leaf interfaces as demonstrated in the test
-   // packages/graphql/tests/integration/aggregations/where/node/string.int.test.ts
-   if (!compositeFields.length) {
-        throw new Neo4jGraphQLSchemaValidationError(
-            `Composite entity ${entityDefinitionName} has no concrete entities`
-        );
-    } */
+    const compositeFields = filterTruthy(
+        entityImplementingTypeNames.map((type) => {
+            return concreteEntities.get(type);
+        })
+    );
+
     return {
         name: entityDefinitionName,
         concreteEntities: compositeFields,
@@ -538,7 +538,7 @@ function generateConcreteEntity(
     });
 
     // schema configuration directives are propagated onto concrete entities
-    const schemaDirectives = definitionCollection.schemaExtension?.directives?.filter((x) =>
+    const schemaDirectives = definitionCollection.schemaExtensions?.directives?.filter((x) =>
         isInArray(SCHEMA_CONFIGURATION_OBJECT_DIRECTIVES, x.name.value)
     );
     const annotations = parseAnnotations((definition.directives || []).concat(schemaDirectives || []));

packages/graphql/src/schema-model/parser/definition-collection.ts

@@ -30,20 +30,22 @@ import type {
     UnionTypeDefinitionNode,
 } from "graphql";
 import { Kind } from "graphql";
-import { jwt, relationshipPropertiesDirective } from "../../graphql/directives";
+import { jwt, nodeDirective, relationshipPropertiesDirective } from "../../graphql/directives";
 import { isRootType } from "../../utils/is-root-type";
 import { findDirective } from "./utils";
 
 export type DefinitionCollection = {
-    nodes: Map<string, ObjectTypeDefinitionNode>; // this does not include @jwtPayload type.
+    nodes: Map<string, ObjectTypeDefinitionNode>; // includes all object types marked with @node
+    objectTypes: Map<string, ObjectTypeDefinitionNode>; // includes all objects
+    userDefinedObjectTypes: Map<string, ObjectTypeDefinitionNode>; // includes objects not reserved by the library
     scalarTypes: Map<string, ScalarTypeDefinitionNode>;
     enumTypes: Map<string, EnumTypeDefinitionNode>;
     interfaceTypes: Map<string, InterfaceTypeDefinitionNode>;
     unionTypes: Map<string, UnionTypeDefinitionNode>;
     directives: Map<string, DirectiveDefinitionNode>;
     relationshipProperties: Map<string, ObjectTypeDefinitionNode>;
     inputTypes: Map<string, InputObjectTypeDefinitionNode>;
-    schemaExtension: SchemaExtensionNode | undefined;
+    schemaExtensions: SchemaExtensionNode | undefined;
     jwtPayload: ObjectTypeDefinitionNode | undefined;
     interfaceToImplementingTypeNamesMap: Map<string, string[]>; // TODO: change this logic, this was the logic contained in initInterfacesToTypeNamesMap but potentially can be simplified now.
     operations: ObjectTypeDefinitionNode[];
@@ -58,18 +60,27 @@ export function getDefinitionCollection(document: DocumentNode): DefinitionColle
                 case Kind.SCALAR_TYPE_DEFINITION:
                     definitionCollection.scalarTypes.set(definition.name.value, definition);
                     break;
-                case Kind.OBJECT_TYPE_DEFINITION:
+                case Kind.OBJECT_TYPE_DEFINITION: {
+                    definitionCollection.objectTypes.set(definition.name.value, definition);
                     if (findDirective(definition.directives, relationshipPropertiesDirective.name)) {
                         definitionCollection.relationshipProperties.set(definition.name.value, definition);
-                    } else if (findDirective(definition.directives, jwt.name)) {
+                        break;
+                    }
+                    if (findDirective(definition.directives, jwt.name)) {
                         definitionCollection.jwtPayload = definition;
-                    } else if (!isRootType(definition)) {
-                        definitionCollection.nodes.set(definition.name.value, definition);
-                    } else {
+                        break;
+                    }
+                    if (isRootType(definition)) {
                         definitionCollection.operations.push(definition);
+                        break;
                     }
-
+                    if (findDirective(definition.directives, nodeDirective.name)) {
+                        definitionCollection.nodes.set(definition.name.value, definition);
+                        break;
+                    }
+                    definitionCollection.userDefinedObjectTypes.set(definition.name.value, definition);
                     break;
+                }
                 case Kind.ENUM_TYPE_DEFINITION:
                     definitionCollection.enumTypes.set(definition.name.value, definition);
                     break;
@@ -88,7 +99,7 @@ export function getDefinitionCollection(document: DocumentNode): DefinitionColle
                     break;
                 case Kind.SCHEMA_EXTENSION:
                     // This is based on the assumption that mergeTypeDefs is used and therefore there is only one schema extension (merged), this assumption is currently used as well for object extensions.
-                    definitionCollection.schemaExtension = definition;
+                    definitionCollection.schemaExtensions = definition;
                     definitionCollection.schemaDirectives = definition.directives
                         ? Array.from(definition.directives)
                         : [];
@@ -99,14 +110,16 @@ export function getDefinitionCollection(document: DocumentNode): DefinitionColle
         },
         {
             nodes: new Map(),
+            objectTypes: new Map(),
+            userDefinedObjectTypes: new Map(),
             enumTypes: new Map(),
             scalarTypes: new Map(),
             interfaceTypes: new Map(),
             directives: new Map(),
             unionTypes: new Map(),
             relationshipProperties: new Map(),
             inputTypes: new Map(),
-            schemaExtension: undefined,
+            schemaExtensions: undefined,
             jwtPayload: undefined,
             interfaceToImplementingTypeNamesMap: new Map(),
             operations: [],

packages/graphql/src/schema-model/parser/parse-attribute.ts

@@ -146,7 +146,7 @@ export function isUserScalar(definitionCollection: DefinitionCollection, name: s
 }
 
 export function isObject(definitionCollection: DefinitionCollection, name: string) {
-    return definitionCollection.nodes.has(name);
+    return definitionCollection.objectTypes.has(name);
 }
 
 function isInput(definitionCollection: DefinitionCollection, name: string) {

packages/graphql/src/schema/create-relationship-fields/create-relationship-fields.ts

@@ -316,7 +316,7 @@ function createRelationshipFieldsForTarget({
     });
 
     if (relationshipAdapter.target instanceof InterfaceEntityAdapter) {
-        withFieldInputType({ relationshipAdapter, composer, userDefinedFieldDirectives, features });
+        withFieldInputType({ relationshipAdapter, composer, userDefinedFieldDirectives });
     }
     composeNode.addFields(
         augmentObjectOrInterfaceTypeWithRelationshipField({