Skip to content

Commit

Permalink
Add validation on single relationship
Browse files Browse the repository at this point in the history
  • Loading branch information
angrykoala committed Nov 20, 2024
1 parent 8748230 commit 4936eec
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 49 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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 { ASTVisitor, FieldDefinitionNode, ListTypeNode, NonNullTypeNode } from "graphql";
import { Kind } from "graphql";
import type { SDLValidationContext } from "graphql/validation/ValidationContext";
import { relationshipDirective } from "../../../graphql/directives";
import { createGraphQLError } from "./utils/document-validation-error";

export function ErrorIfSingleRelationships(context: SDLValidationContext): ASTVisitor {
return {
FieldDefinition(field: FieldDefinitionNode) {
let isRelationship = false;
for (const directive of field.directives ?? []) {
if (directive.name.value === relationshipDirective.name) {
isRelationship = true;
}
}

const isList = Boolean(getListTypeNode(field));
if (isRelationship && !isList) {
context.reportError(
createGraphQLError({
errorMsg: `Using @relationship directive on a non-list property "${field.name.value}" is not supported.`,
})
);
}
},
};
}

function getListTypeNode(definition: FieldDefinitionNode | ListTypeNode | NonNullTypeNode): ListTypeNode | undefined {
if (definition.type.kind === Kind.NON_NULL_TYPE) {
return getListTypeNode(definition.type);
}

if (definition.type.kind === Kind.LIST_TYPE) {
return definition.type;
}

return;
}
86 changes: 54 additions & 32 deletions packages/graphql/src/schema/validation/validate-document.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ describe("validation 2.0", () => {
test("@relationship properties required", () => {
const doc = gql`
type User @node {
name: Post @relationship
name: [Post!]! @relationship
}
type Post @node {
title: String
Expand All @@ -700,7 +700,7 @@ describe("validation 2.0", () => {
test("@relationship type required", () => {
const doc = gql`
type User @node {
name: Post @relationship(direction: IN)
name: [Post!]! @relationship(direction: IN)
}
type Post @node {
title: String
Expand All @@ -720,7 +720,7 @@ describe("validation 2.0", () => {
test("@relationship direction required", () => {
const doc = gql`
type User @node {
name: Post @relationship(type: "HAS_POST")
name: [Post!]! @relationship(type: "HAS_POST")
}
type Post @node {
title: String
Expand All @@ -740,7 +740,7 @@ describe("validation 2.0", () => {
test("@relationship ok", () => {
const doc = gql`
type User @node {
name: Post @relationship(direction: IN, type: "HAS_POST")
name: [Post!]! @relationship(direction: IN, type: "HAS_POST")
}
type Post @node {
title: String
Expand All @@ -755,6 +755,34 @@ describe("validation 2.0", () => {
});
expect(executeValidate).not.toThrow();
});

test("Error on 1-1 relationships", () => {
const doc = gql`
type Movie @node {
id: ID
actors: Actor @relationship(type: "ACTED_IN", direction: OUT)
}
type Actor @node {
name: String
movie: Movie! @relationship(type: "ACTED_IN", direction: IN)
}
`;

const executeValidate = () => validateDocument({ document: doc, features: {}, additionalDefinitions });
const errors = getError(executeValidate);
expect(errors).toHaveLength(2);
expect(errors[0]).not.toBeInstanceOf(NoErrorThrownError);
expect(errors[1]).not.toBeInstanceOf(NoErrorThrownError);
expect(errors[0]).toHaveProperty(
"message",
`Using @relationship directive on a non-list property "actors" is not supported.`
);
expect(errors[1]).toHaveProperty(
"message",
`Using @relationship directive on a non-list property "movie" is not supported.`
);
});
});
});

Expand Down Expand Up @@ -799,7 +827,7 @@ describe("validation 2.0", () => {
test("@relationship.direction property must be enum value", () => {
const doc = gql`
type User @node {
post: Post @relationship(direction: "EVERYWHERE", type: "HAS_NAME")
post: [Post!]! @relationship(direction: "EVERYWHERE", type: "HAS_NAME")
}
type Post @node {
title: String
Expand Down Expand Up @@ -828,7 +856,7 @@ describe("validation 2.0", () => {
id: ID
}
extend type User {
post: Post @relationship(direction: "EVERYWHERE", type: "HAS_NAME")
post: [Post!]! @relationship(direction: "EVERYWHERE", type: "HAS_NAME")
}
type Post @node {
title: String
Expand All @@ -854,7 +882,7 @@ describe("validation 2.0", () => {
test("@relationship.type property must be string", () => {
const doc = gql`
type User @node {
post: Post @relationship(type: 42, direction: IN)
post: [Post!]! @relationship(type: 42, direction: IN)
}
type Post @node {
title: String
Expand Down Expand Up @@ -885,7 +913,7 @@ describe("validation 2.0", () => {
`;
const doc = gql`
type User implements Person @node {
post: Post @relationship(type: 42, direction: IN)
post: [Post!]! @relationship(type: 42, direction: IN)
}
type Post @node {
title: String
Expand Down Expand Up @@ -922,7 +950,7 @@ describe("validation 2.0", () => {
const doc = gql`
type User implements Person @node {
id: ID
post: Post @relationship(type: 42, direction: IN)
post: [Post!]! @relationship(type: 42, direction: IN)
}
type Post @node {
title: String
Expand Down Expand Up @@ -2854,7 +2882,7 @@ describe("validation 2.0", () => {
posts: [Post!]! @relationship(type: "HAS_POST", direction: OUT, properties: "Poster")
archived: [Post!]!
@relationship(type: "HAS_ARCHIVED_POST", direction: OUT, properties: "Poster")
favorite: Post @relationship(type: "HAS_FAVORITE", direction: OUT)
favorite: [Post!]! @relationship(type: "HAS_FAVORITE", direction: OUT)
}
type Post @node {
title: String
Expand Down Expand Up @@ -5023,7 +5051,7 @@ describe("validation 2.0", () => {
const doc = gql`
${relationshipProperties}
type Movie @node {
actors: Actor! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
type Actor @node {
Expand Down Expand Up @@ -5065,7 +5093,7 @@ describe("validation 2.0", () => {
const doc = gql`
${relationshipProperties}
type Movie @node {
actors: Actor! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
type Actor @node {
Expand Down Expand Up @@ -5103,7 +5131,7 @@ describe("validation 2.0", () => {
const doc = gql`
${relationshipProperties}
type Movie @node {
actors: Actor! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
type Actor @node {
Expand Down Expand Up @@ -5141,7 +5169,7 @@ describe("validation 2.0", () => {
const doc = gql`
${relationshipProperties}
type Movie @node {
actors: Actor! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
type Actor @node {
Expand Down Expand Up @@ -5174,13 +5202,13 @@ describe("validation 2.0", () => {
test("should throw error if @relationship is used on relationship property", () => {
const relationshipProperties = gql`
type ActedIn @relationshipProperties {
actors: Actor! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
`;
const doc = gql`
${relationshipProperties}
type Movie @node {
actors: Actor! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
type Actor @node {
Expand Down Expand Up @@ -5220,7 +5248,7 @@ describe("validation 2.0", () => {
const doc = gql`
${relationshipProperties}
type Movie @node {
actors: Actor! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
type Actor @node {
Expand Down Expand Up @@ -5460,7 +5488,6 @@ describe("validation 2.0", () => {
const doc = gql`
type User @node {
name: String
posts: Int! @relationship(type: "HAS_POST", direction: OUT)
allPosts: [Int!] @relationship(type: "HAS_POST", direction: OUT)
}
`;
Expand All @@ -5473,19 +5500,14 @@ describe("validation 2.0", () => {
});

const errors = getError(executeValidate);
expect(errors).toHaveLength(2);
expect(errors).toHaveLength(1);
expect(errors[0]).not.toBeInstanceOf(NoErrorThrownError);
expect(errors[1]).not.toBeInstanceOf(NoErrorThrownError);

expect(errors[0]).toHaveProperty(
"message",
"Invalid field type: Scalar types cannot be relationship targets. Please use an Object type instead."
);
expect(errors[1]).toHaveProperty(
"message",
"Invalid field type: Scalar types cannot be relationship targets. Please use an Object type instead."
);
expect(errors[0]).toHaveProperty("path", ["User", "posts"]);
expect(errors[1]).toHaveProperty("path", ["User", "allPosts"]);
expect(errors[0]).toHaveProperty("path", ["User", "allPosts"]);
});
});

Expand Down Expand Up @@ -6093,7 +6115,7 @@ describe("validation 2.0", () => {
eligibleForBonus: Boolean
bonusPercentage: Float
salaryReviewDate: DateTime
pays_salary: EmploymentRecord! @relationship(type: "PAYS_SALARY", direction: IN)
pays_salary: [EmploymentRecord!]! @relationship(type: "PAYS_SALARY", direction: IN)
}
type EmploymentRecord @node {
Expand Down Expand Up @@ -6321,8 +6343,8 @@ describe("validation 2.0", () => {
type Order @node {
orderID: ID! @id
placedAt: DateTime @timestamp
shipTo: Address! @relationship(type: "SHIPS_TO", direction: OUT)
customer: Customer! @relationship(type: "PLACED", direction: IN)
shipTo: [Address!]! @relationship(type: "SHIPS_TO", direction: OUT)
customer: [Customer!]! @relationship(type: "PLACED", direction: IN)
books: [Book!]! @relationship(type: "CONTAINS", direction: OUT)
}
Expand Down Expand Up @@ -6354,7 +6376,7 @@ describe("validation 2.0", () => {
type Address @node {
address: String
location: Point
order: Order @relationship(type: "SHIPS_TO", direction: IN)
order: [Order!]! @relationship(type: "SHIPS_TO", direction: IN)
}
extend type Address {
Expand Down Expand Up @@ -6403,8 +6425,8 @@ describe("validation 2.0", () => {
rating: Int
text: String
createdAt: DateTime @timestamp
book: Book! @relationship(type: "REVIEWS", direction: OUT)
author: Customer! @relationship(type: "WROTE", direction: IN)
book: [Book!]! @relationship(type: "REVIEWS", direction: OUT)
author: [Customer!]! @relationship(type: "WROTE", direction: IN)
}
type Author @node {
Expand Down
2 changes: 2 additions & 0 deletions packages/graphql/src/schema/validation/validate-document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import { isRootType } from "../../utils/is-root-type";
import { DirectiveArgumentOfCorrectType } from "./custom-rules/directive-argument-of-correct-type";
import { directiveIsValid } from "./custom-rules/directives/valid-directive";
import { ValidDirectiveAtFieldLocation } from "./custom-rules/directives/valid-directive-field-location";
import { ErrorIfSingleRelationships } from "./custom-rules/error-single-relationships";
import { ValidJwtDirectives } from "./custom-rules/features/valid-jwt-directives";
import { ValidRelationshipDeclaration } from "./custom-rules/features/valid-relationship-declaration";
import { ValidRelationshipProperties } from "./custom-rules/features/valid-relationship-properties";
Expand Down Expand Up @@ -225,6 +226,7 @@ function runValidationRulesOnFilteredDocument({
DirectiveArgumentOfCorrectType(false),
WarnIfAuthorizationFeatureDisabled(features?.authorization),
WarnIfListOfListsFieldDefinition,
ErrorIfSingleRelationships,
WarnIfAMaxLimitCanBeBypassedThroughInterface(),
WarnObjectFieldsWithoutResolver({
customResolvers: asArray(userCustomResolvers ?? []),
Expand Down
17 changes: 0 additions & 17 deletions packages/graphql/tests/schema/directives/relationship.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,21 +516,4 @@ describe("Relationship", () => {
}"
`);
});

test("Fails on non list relationship", async () => {
const typeDefs = gql`
type Actor @node {
name: String
}
type Movie @node {
id: ID
actors: Actor @relationship(type: "ACTED_IN", direction: IN)
}
`;
const neoSchema = new Neo4jGraphQL({ typeDefs });
await expect(async () => {
await neoSchema.getSchema();
}).rejects.toThrow("@relationship on non-list field [Movie.actors] not supported");
});
});

0 comments on commit 4936eec

Please sign in to comment.