Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 1-1 validation on nested update #5616

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Fix 1-1 validation on nested update #5616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-icons-marry.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@neo4j/graphql": patch
---

Fix cardinality validation on nested unions
55 changes: 55 additions & 0 deletions packages/graphql/src/translate/create-update-and-params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,30 @@ export default function createUpdateAndParams({
}

if (update.connect) {
if (relationField.interface) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be the case also for unions?

if (!relationField.typeMeta.array) {
const inStr = relationField.direction === "IN" ? "<-" : "-";
const outStr = relationField.direction === "OUT" ? "->" : "-";

const validatePredicates: string[] = [];
refNodes.forEach((refNode) => {
const validateRelationshipExistence = `EXISTS((${varName})${inStr}[:${relationField.type}]${outStr}(:${refNode.name}))`;
validatePredicates.push(validateRelationshipExistence);
});

if (validatePredicates.length) {
subquery.push("WITH *");
subquery.push(
`WHERE apoc.util.validatePredicate(${validatePredicates.join(
" OR "
)},'Relationship field "%s.%s" cannot have more than one node linked',["${
relationField.connectionPrefix
}","${relationField.fieldName}"])`
);
}
}
}

const connectAndParams = createConnectAndParams({
context,
callbackBucket,
Expand Down Expand Up @@ -402,6 +426,37 @@ export default function createUpdateAndParams({
};
}

if (!relationField.typeMeta.array) {
subquery.push("WITH *");

const validatePredicateTemplate = (condition: string) =>
`WHERE apoc.util.validatePredicate(${condition},'Relationship field "%s.%s" cannot have more than one node linked',["${relationField.connectionPrefix}","${relationField.fieldName}"])`;

const singleCardinalityValidationTemplate = (nodeName) =>
`EXISTS((${varName})${inStr}[:${relationField.type}]${outStr}(:${nodeName}))`;

if (relationField.union && relationField.union.nodes) {
const validateRelationshipExistence = relationField.union.nodes.map(
singleCardinalityValidationTemplate
);
subquery.push(
validatePredicateTemplate(validateRelationshipExistence.join(" OR "))
);
} else if (relationField.interface && relationField.interface.implementations) {
const validateRelationshipExistence = relationField.interface.implementations.map(
singleCardinalityValidationTemplate
);
subquery.push(
validatePredicateTemplate(validateRelationshipExistence.join(" OR "))
);
} else {
const validateRelationshipExistence = singleCardinalityValidationTemplate(
refNode.name
);
subquery.push(validatePredicateTemplate(validateRelationshipExistence));
}
}

const {
create: nestedCreate,
params,
Expand Down
12 changes: 6 additions & 6 deletions packages/graphql/tests/integration/issues/1430.int.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe("https://github.com/neo4j/graphql/issues/1430", () => {
const updateMutation = `
mutation ddfs{
${testAbce.operations.update}(where: { id: "${abcesId}" }
create: { interface: { node: { ${testChildOne.name}: { name: "childone name2" } } } }
update: { interface: { create: { node: { ${testChildOne.name}: { name: "childone name2" } } } } }
){
${testAbce.plural} {
id
Expand Down Expand Up @@ -232,10 +232,10 @@ describe("https://github.com/neo4j/graphql/issues/1430", () => {
const abcesId = (createMutationResults.data as any)[testAbce.operations.create][testAbce.plural][0].id;

const updateMutation = `
mutation {
${testAbce.operations.update}(
where: { id: "${abcesId}" }
connect: { interface: { where: { node: { name: "childone name connect" } } } }
mutation {
${testAbce.operations.update}(
where: { id: "${abcesId}" }
update: { interface: { connect: { where: { node: { name: "childone name connect" } } } } }
) {
${testAbce.plural} {
id
Expand Down Expand Up @@ -312,7 +312,7 @@ describe("https://github.com/neo4j/graphql/issues/1430", () => {
mutation {
${testAbce.operations.update}(
where: { id: "${abcesId}" }
create: { interface: { node: { ${testChildOne.name}: { name: "childone anme nested create" } } } }
update: { interface: { create: { node: { ${testChildOne.name}: { name: "childone anme nested create" } } } } }
) {
${testAbce.plural} {
id
Expand Down
47 changes: 29 additions & 18 deletions packages/graphql/tests/tck/issues/1430.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe("https://github.com/neo4j/graphql/issues/1430", () => {
mutation {
updateAbces(
where: { id: "TestId" }
connect: { interface: { where: { node: { name: "childone name connect" } } } }
update: { interface: { connect: { where: { node: { name: "childone name connect" } } } } }
) {
abces {
id
Expand All @@ -138,42 +138,54 @@ describe("https://github.com/neo4j/graphql/issues/1430", () => {
expect(formatCypher(result.cypher)).toMatchInlineSnapshot(`
"MATCH (this:ABCE)
WHERE this.id = $param0
WITH this
CALL {
WITH this
WITH *
WHERE apoc.util.validatePredicate(EXISTS((this)-[:HAS_INTERFACE]->(:ChildOne)) OR EXISTS((this)-[:HAS_INTERFACE]->(:ChildTwo)),'Relationship field \\"%s.%s\\" cannot have more than one node linked',[\\"ABCE\\",\\"interface\\"])
WITH *
CALL {
WITH this
OPTIONAL MATCH (this_connect_interface0_node:ChildOne)
WHERE this_connect_interface0_node.name = $this_connect_interface0_node_param0
OPTIONAL MATCH (this_interface0_connect0_node:ChildOne)
WHERE this_interface0_connect0_node.name = $this_interface0_connect0_node_param0
CALL {
WITH *
WITH collect(this_connect_interface0_node) as connectedNodes, collect(this) as parentNodes
WITH collect(this_interface0_connect0_node) as connectedNodes, collect(this) as parentNodes
CALL {
WITH connectedNodes, parentNodes
UNWIND parentNodes as this
UNWIND connectedNodes as this_connect_interface0_node
MERGE (this)-[:HAS_INTERFACE]->(this_connect_interface0_node)
UNWIND connectedNodes as this_interface0_connect0_node
MERGE (this)-[:HAS_INTERFACE]->(this_interface0_connect0_node)
}
}
WITH this, this_connect_interface0_node
RETURN count(*) AS connect_this_connect_interface_ChildOne0
WITH this, this_interface0_connect0_node
RETURN count(*) AS connect_this_interface0_connect_ChildOne0
}
RETURN count(*) AS update_this_ChildOne
}
CALL {
WITH this
OPTIONAL MATCH (this_connect_interface1_node:ChildTwo)
WHERE this_connect_interface1_node.name = $this_connect_interface1_node_param0
WITH this
WITH *
WHERE apoc.util.validatePredicate(EXISTS((this)-[:HAS_INTERFACE]->(:ChildOne)) OR EXISTS((this)-[:HAS_INTERFACE]->(:ChildTwo)),'Relationship field \\"%s.%s\\" cannot have more than one node linked',[\\"ABCE\\",\\"interface\\"])
WITH *
CALL {
WITH this
OPTIONAL MATCH (this_interface0_connect0_node:ChildTwo)
WHERE this_interface0_connect0_node.name = $this_interface0_connect0_node_param0
CALL {
WITH *
WITH collect(this_connect_interface1_node) as connectedNodes, collect(this) as parentNodes
WITH collect(this_interface0_connect0_node) as connectedNodes, collect(this) as parentNodes
CALL {
WITH connectedNodes, parentNodes
UNWIND parentNodes as this
UNWIND connectedNodes as this_connect_interface1_node
MERGE (this)-[:HAS_INTERFACE]->(this_connect_interface1_node)
UNWIND connectedNodes as this_interface0_connect0_node
MERGE (this)-[:HAS_INTERFACE]->(this_interface0_connect0_node)
}
}
WITH this, this_connect_interface1_node
RETURN count(*) AS connect_this_connect_interface_ChildTwo1
WITH this, this_interface0_connect0_node
RETURN count(*) AS connect_this_interface0_connect_ChildTwo0
}
RETURN count(*) AS update_this_ChildTwo
}
WITH *
CALL {
Expand All @@ -198,8 +210,7 @@ describe("https://github.com/neo4j/graphql/issues/1430", () => {
expect(formatParams(result.params)).toMatchInlineSnapshot(`
"{
\\"param0\\": \\"TestId\\",
\\"this_connect_interface0_node_param0\\": \\"childone name connect\\",
\\"this_connect_interface1_node_param0\\": \\"childone name connect\\",
\\"this_interface0_connect0_node_param0\\": \\"childone name connect\\",
\\"resolvedCallbacks\\": {}
}"
`);
Expand Down
2 changes: 2 additions & 0 deletions packages/graphql/tests/tck/rfcs/rfc-003.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ describe("tck/rfs/003", () => {
"MATCH (this:Movie)
WHERE this.id = $param0
WITH this
WITH *
WHERE apoc.util.validatePredicate(EXISTS((this)<-[:DIRECTED]-(:Director)),'Relationship field \\"%s.%s\\" cannot have more than one node linked',[\\"Movie\\",\\"director\\"])
CREATE (this_director0_create0_node:Director)
SET this_director0_create0_node.id = $this_director0_create0_node_id
MERGE (this)<-[:DIRECTED]-(this_director0_create0_node)
Expand Down
Loading