From 6153d686471741b9a35710ced8872ca664263dae Mon Sep 17 00:00:00 2001 From: Darrell Warde <8117355+darrellwarde@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:39:29 +0000 Subject: [PATCH] Fix Cypher when filtering by aggregations over different relationship properties types (#5943) * Fix Cypher when filtering by aggregations over different relationship properites types * Update friendly-pigs-wait.md * Fix issues * Add small explanation * Fix edge case --- .changeset/friendly-pigs-wait.md | 5 + .../queryAST/factory/FilterFactory.ts | 14 +- .../where/edge/interfaces.int.test.ts | 156 +++++++++++++ .../where/edge/string.int.test.ts | 166 +++++++------ .../where/edge/interface-relationship.test.ts | 219 ++++++++++++++++++ 5 files changed, 472 insertions(+), 88 deletions(-) create mode 100644 .changeset/friendly-pigs-wait.md create mode 100644 packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts create mode 100644 packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts diff --git a/.changeset/friendly-pigs-wait.md b/.changeset/friendly-pigs-wait.md new file mode 100644 index 0000000000..1f2799fc5d --- /dev/null +++ b/.changeset/friendly-pigs-wait.md @@ -0,0 +1,5 @@ +--- +"@neo4j/graphql": patch +--- + +Fix Cypher when filtering by aggregations over different relationship properties types diff --git a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts index 98aecca30c..ac867b4f57 100644 --- a/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts +++ b/packages/graphql/src/translate/queryAST/factory/FilterFactory.ts @@ -683,7 +683,19 @@ export class FilterFactory { ); } - if (fieldName === "edge") { + if (fieldName === "edge" && relationship.propertiesTypeName) { + // This conditional handles when the relationship is an interface which is also being accessed through an interface + if ( + isInterfaceEntity(relationship.target) && + Object.keys(value).some((v) => relationship.siblings?.includes(v)) + ) { + return Object.entries(value).flatMap(([k, v]) => { + if (k === relationship.propertiesTypeName) { + return this.createAggregationNodeFilters(v as Record, relationship); + } + return []; + }); + } return this.createAggregationNodeFilters(value as Record, relationship); } diff --git a/packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts b/packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts new file mode 100644 index 0000000000..4b47dca55f --- /dev/null +++ b/packages/graphql/tests/integration/aggregations/where/edge/interfaces.int.test.ts @@ -0,0 +1,156 @@ +import type { UniqueType } from "../../../../utils/graphql-types"; +import { TestHelper } from "../../../../utils/tests-helper"; + +describe("aggregations-where-edge-string interface relationships of interface types", () => { + let testHelper: TestHelper; + + let Movie: UniqueType; + let Series: UniqueType; + let Production: UniqueType; + let Actor: UniqueType; + let Cameo: UniqueType; + let Person: UniqueType; + + beforeEach(async () => { + testHelper = new TestHelper(); + + Movie = testHelper.createUniqueType("Movie"); + Series = testHelper.createUniqueType("Series"); + Production = testHelper.createUniqueType("Production"); + Actor = testHelper.createUniqueType("Actor"); + Cameo = testHelper.createUniqueType("Cameo"); + Person = testHelper.createUniqueType("Person"); + + const typeDefs = /* GraphQL */ ` + interface ${Production} { + title: String + } + + type ${Movie} implements ${Production} @node { + title: String + } + + type ${Series} implements ${Production} @node { + title: String + } + + interface ${Person} { + name: String + productions: [${Production}!]! @declareRelationship + } + + type ${Actor} implements ${Person} @node { + name: String + productions: [${Production}!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn") + } + + type ${Cameo} implements ${Person} @node { + name: String + productions: [${Production}!]! @relationship(type: "APPEARED_IN", direction: OUT, properties: "AppearedIn") + } + + type ActedIn @relationshipProperties { + role: String + } + + type AppearedIn @relationshipProperties { + role: String + } + `; + + await testHelper.initNeo4jGraphQL({ typeDefs }); + }); + + afterEach(async () => { + await testHelper.close(); + }); + + test("should return nodes aggregated across different relationship properties types", async () => { + await testHelper.executeCypher( + ` + CREATE (a:${Actor} { name: "A" })-[:ACTED_IN { role: "definitely too long" }]->(g:${Movie} { title: "G" }) + CREATE (a)-[:ACTED_IN { role: "extremely long" }]->(g) + CREATE (b:${Actor} { name: "B" })-[:ACTED_IN { role: "a" }]->(h:${Series} { title: "H" }) + CREATE (b)-[:ACTED_IN { role: "b" }]->(h) + CREATE (c:${Actor} { name: "C" }) + CREATE (d:${Cameo} { name: "D" })-[:APPEARED_IN { role: "too long" }]->(i:${Movie} { title: "I" }) + CREATE (d)-[:APPEARED_IN { role: "also too long" }]->(i) + CREATE (e:${Cameo} { name: "E" })-[:APPEARED_IN { role: "s" }]->(j:${Series} { title: "J" }) + CREATE (e)-[:APPEARED_IN { role: "very long" }]->(j) + CREATE (f:${Cameo} { name: "F" }) + ` + ); + + const query = /* GraphQL */ ` + query People { + ${Person.plural}( + where: { + productionsAggregate: { + edge: { + AppearedIn: { role_SHORTEST_LENGTH_LT: 3 } + ActedIn: { role_AVERAGE_LENGTH_LT: 5 } + } + } + } + ) { + name + } + } + `; + + const gqlResult = await testHelper.executeGraphQL(query); + + if (gqlResult.errors) { + console.log(JSON.stringify(gqlResult.errors, null, 2)); + } + + expect(gqlResult.errors).toBeUndefined(); + + expect((gqlResult.data as any)[Person.plural]).toIncludeSameMembers([{ name: "E" }, { name: "B" }]); + }); + + test("should return nodes aggregated across relationship properties and count", async () => { + await testHelper.executeCypher( + ` + CREATE (a:${Actor} { name: "A" })-[:ACTED_IN { role: "definitely too long" }]->(g:${Movie} { title: "G" }) + CREATE (a)-[:ACTED_IN { role: "extremely long" }]->(g) + CREATE (b:${Actor} { name: "B" })-[:ACTED_IN { role: "a" }]->(h:${Series} { title: "H" }) + CREATE (b)-[:ACTED_IN { role: "b" }]->(h) + CREATE (b2:${Actor} { name: "B2" })-[:ACTED_IN { role: "a" }]->(h2:${Series} { title: "H2" }) + CREATE (b2)-[:ACTED_IN { role: "b" }]->(h2) + CREATE (b2)-[:ACTED_IN { role: "b" }]->(h2) + CREATE (c:${Actor} { name: "C" }) + CREATE (d:${Cameo} { name: "D" })-[:APPEARED_IN { role: "too long" }]->(i:${Movie} { title: "I" }) + CREATE (d)-[:APPEARED_IN { role: "also too long" }]->(i) + CREATE (e:${Cameo} { name: "E" })-[:APPEARED_IN { role: "s" }]->(j:${Series} { title: "J" }) + CREATE (e)-[:APPEARED_IN { role: "very long" }]->(j) + CREATE (e)-[:APPEARED_IN { role: "another very long" }]->(j) + CREATE (f:${Cameo} { name: "F" }) + ` + ); + + const query = /* GraphQL */ ` + query People { + ${Person.plural}( + where: { productionsAggregate: { edge: { ActedIn: { role_AVERAGE_LENGTH_LT: 5 } }, count_LT: 3 } } + ) { + name + } + } + `; + + const gqlResult = await testHelper.executeGraphQL(query); + + if (gqlResult.errors) { + console.log(JSON.stringify(gqlResult.errors, null, 2)); + } + + expect(gqlResult.errors).toBeUndefined(); + + expect((gqlResult.data as any)[Person.plural]).toIncludeSameMembers([ + { name: "D" }, + { name: "B" }, + { name: "F" }, + ]); + }); +}); diff --git a/packages/graphql/tests/integration/aggregations/where/edge/string.int.test.ts b/packages/graphql/tests/integration/aggregations/where/edge/string.int.test.ts index 044c4c1b30..1cc17f1320 100644 --- a/packages/graphql/tests/integration/aggregations/where/edge/string.int.test.ts +++ b/packages/graphql/tests/integration/aggregations/where/edge/string.int.test.ts @@ -575,42 +575,39 @@ describe("aggregations-where-edge-string interface relationships of concrete typ ]); }); - - test( - "should return posts where the LONGEST edge like String is EQUAL to", - async () => { - const testString = generate({ - charset: "alphabetic", - readable: true, - }); - - const shortestTestString = generate({ - charset: "alphabetic", - readable: true, - length: 10, - }); - - const testString2 = generate({ - charset: "alphabetic", - readable: true, - length: 11, - }); - - const longestTestString = generate({ - charset: "alphabetic", - readable: true, - length: 12, - }); - - await testHelper.executeCypher( - ` + test("should return posts where the LONGEST edge like String is EQUAL to", async () => { + const testString = generate({ + charset: "alphabetic", + readable: true, + }); + + const shortestTestString = generate({ + charset: "alphabetic", + readable: true, + length: 10, + }); + + const testString2 = generate({ + charset: "alphabetic", + readable: true, + length: 11, + }); + + const longestTestString = generate({ + charset: "alphabetic", + readable: true, + length: 12, + }); + + await testHelper.executeCypher( + ` CREATE (:${Post} {testString: "${testString}"})<-[:LIKES { testString: "${shortestTestString}" }]-(:${User} {testString: "${shortestTestString}"}) CREATE (:${Post} {testString: "${testString}"})<-[:LIKES { testString: "${testString2}" }]-(:${User} {testString: "${testString2}"}) CREATE (:${Post} {testString: "${testString}"})<-[:LIKES { testString: "${longestTestString}" }]-(:${User} {testString: "${longestTestString}"}) ` - ); + ); - const query = ` + const query = ` { ${Post.plural}(where: { testString_EQ: "${testString}", likesAggregate: { edge: { testString_LONGEST_LENGTH_EQUAL: ${longestTestString.length} } } }) { testString @@ -621,64 +618,60 @@ describe("aggregations-where-edge-string interface relationships of concrete typ } `; - const gqlResult = await testHelper.executeGraphQL(query); - - if (gqlResult.errors) { - console.log(JSON.stringify(gqlResult.errors, null, 2)); - } + const gqlResult = await testHelper.executeGraphQL(query); - expect(gqlResult.errors).toBeUndefined(); + if (gqlResult.errors) { + console.log(JSON.stringify(gqlResult.errors, null, 2)); + } - expect((gqlResult.data as any)[Post.plural]).toEqual([ - { - testString, - likes: [{ testString: longestTestString }], - }, - ]); - } - ); + expect(gqlResult.errors).toBeUndefined(); + expect((gqlResult.data as any)[Post.plural]).toEqual([ + { + testString, + likes: [{ testString: longestTestString }], + }, + ]); + }); describe("AVERAGE", () => { - test( - "should return posts where the %s of edge like Strings is EQUAL to", - async () => { - const testString = generate({ - charset: "alphabetic", - readable: true, - }); - - const testString1 = generate({ - charset: "alphabetic", - readable: true, - length: 10, - }); - - const testString2 = generate({ - charset: "alphabetic", - readable: true, - length: 11, - }); - - const testString3 = generate({ - charset: "alphabetic", - readable: true, - length: 12, - }); - - const avg = (10 + 11 + 12) / 3; - - await testHelper.executeCypher( - ` + test("should return posts where the %s of edge like Strings is EQUAL to", async () => { + const testString = generate({ + charset: "alphabetic", + readable: true, + }); + + const testString1 = generate({ + charset: "alphabetic", + readable: true, + length: 10, + }); + + const testString2 = generate({ + charset: "alphabetic", + readable: true, + length: 11, + }); + + const testString3 = generate({ + charset: "alphabetic", + readable: true, + length: 12, + }); + + const avg = (10 + 11 + 12) / 3; + + await testHelper.executeCypher( + ` CREATE (p:${Post} {testString: "${testString}"}) CREATE(p)<-[:LIKES { testString: "${testString1}" }]-(:${User} {testString: "${testString}"}) CREATE(p)<-[:LIKES { testString: "${testString2}" }]-(:${User} {testString: "${testString}"}) CREATE(p)<-[:LIKES { testString: "${testString3}" }]-(:${User} {testString: "${testString}"}) CREATE (:${Post} {testString: "${testString}"}) ` - ); + ); - const query = ` + const query = ` { ${Post.plural}(where: { testString_EQ: "${testString}", likesAggregate: { edge: { testString_AVERAGE_LENGTH_EQUAL: ${avg} } } }) { testString @@ -689,19 +682,18 @@ describe("aggregations-where-edge-string interface relationships of concrete typ } `; - const gqlResult = await testHelper.executeGraphQL(query); + const gqlResult = await testHelper.executeGraphQL(query); - if (gqlResult.errors) { - console.log(JSON.stringify(gqlResult.errors, null, 2)); - } + if (gqlResult.errors) { + console.log(JSON.stringify(gqlResult.errors, null, 2)); + } - expect(gqlResult.errors).toBeUndefined(); + expect(gqlResult.errors).toBeUndefined(); - const [post] = (gqlResult.data as any)[Post.plural] as any[]; - expect(post.testString).toEqual(testString); - expect(post.likes).toHaveLength(3); - } - ); + const [post] = (gqlResult.data as any)[Post.plural] as any[]; + expect(post.testString).toEqual(testString); + expect(post.likes).toHaveLength(3); + }); test("should return posts where the average of edge like Strings is GT than", async () => { const testString = generate({ diff --git a/packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts b/packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts new file mode 100644 index 0000000000..a1adb49279 --- /dev/null +++ b/packages/graphql/tests/tck/aggregations/where/edge/interface-relationship.test.ts @@ -0,0 +1,219 @@ +/* + * 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 { Neo4jGraphQL } from "../../../../../src"; +import { formatCypher, formatParams, translateQuery } from "../../../utils/tck-test-utils"; + +describe("Cypher Aggregations where edge with String", () => { + let typeDefs: string; + let neoSchema: Neo4jGraphQL; + + beforeAll(() => { + typeDefs = /* GraphQL */ ` + interface Production { + title: String + } + + type Movie implements Production @node { + title: String + } + + type Series implements Production @node { + title: String + } + + interface Person { + name: String + productions: [Production!]! @declareRelationship + } + + type Actor implements Person @node { + name: String + productions: [Production!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn") + } + + type Cameo implements Person @node { + name: String + productions: [Production!]! @relationship(type: "APPEARED_IN", direction: OUT, properties: "AppearedIn") + } + + type ActedIn @relationshipProperties { + role: String + } + + type AppearedIn @relationshipProperties { + role: String + } + `; + + neoSchema = new Neo4jGraphQL({ + typeDefs, + }); + }); + + test("should count number of interface relationships", async () => { + const query = /* GraphQL */ ` + query ActorsAggregate { + actors(where: { productionsAggregate: { count_LT: 3 } }) { + name + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "MATCH (this:Actor) + CALL { + WITH this + MATCH (this)-[this0:ACTED_IN]->(this1) + WHERE (this1:Movie OR this1:Series) + RETURN count(this1) < $param0 AS var2 + } + WITH * + WHERE var2 = true + RETURN this { .name } AS this" + `); + + expect(formatParams(result.params)).toMatchInlineSnapshot(` + "{ + \\"param0\\": { + \\"low\\": 3, + \\"high\\": 0 + } + }" + `); + }); + + test("should generate Cypher to aggregate over multiple relationship properties types", async () => { + const query = /* GraphQL */ ` + query People { + people( + where: { + productionsAggregate: { + edge: { AppearedIn: { role_SHORTEST_LENGTH_LT: 3 }, ActedIn: { role_AVERAGE_LENGTH_LT: 5 } } + } + } + ) { + name + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "CALL { + MATCH (this0:Actor) + CALL { + WITH this0 + MATCH (this0)-[this1:ACTED_IN]->(this2) + WHERE (this2:Movie OR this2:Series) + RETURN avg(size(this1.role)) < $param0 AS var3 + } + WITH * + WHERE var3 = true + WITH this0 { .name, __resolveType: \\"Actor\\", __id: id(this0) } AS this0 + RETURN this0 AS this + UNION + MATCH (this4:Cameo) + CALL { + WITH this4 + MATCH (this4)-[this5:APPEARED_IN]->(this6) + WHERE (this6:Movie OR this6:Series) + RETURN min(size(this5.role)) < $param1 AS var7 + } + WITH * + WHERE var7 = true + WITH this4 { .name, __resolveType: \\"Cameo\\", __id: id(this4) } AS this4 + RETURN this4 AS this + } + WITH this + RETURN this AS this" + `); + + expect(formatParams(result.params)).toMatchInlineSnapshot(` + "{ + \\"param0\\": 5, + \\"param1\\": { + \\"low\\": 3, + \\"high\\": 0 + } + }" + `); + }); + + test("should generate Cypher to aggregate over edge properties and count", async () => { + const query = /* GraphQL */ ` + query People { + people( + where: { productionsAggregate: { edge: { ActedIn: { role_AVERAGE_LENGTH_LT: 5 } }, count_LTE: 10 } } + ) { + name + } + } + `; + + const result = await translateQuery(neoSchema, query); + + expect(formatCypher(result.cypher)).toMatchInlineSnapshot(` + "CALL { + MATCH (this0:Actor) + CALL { + WITH this0 + MATCH (this0)-[this1:ACTED_IN]->(this2) + WHERE (this2:Movie OR this2:Series) + RETURN (count(this2) <= $param0 AND avg(size(this1.role)) < $param1) AS var3 + } + WITH * + WHERE var3 = true + WITH this0 { .name, __resolveType: \\"Actor\\", __id: id(this0) } AS this0 + RETURN this0 AS this + UNION + MATCH (this4:Cameo) + CALL { + WITH this4 + MATCH (this4)-[this5:APPEARED_IN]->(this6) + WHERE (this6:Movie OR this6:Series) + RETURN count(this6) <= $param2 AS var7 + } + WITH * + WHERE var7 = true + WITH this4 { .name, __resolveType: \\"Cameo\\", __id: id(this4) } AS this4 + RETURN this4 AS this + } + WITH this + RETURN this AS this" + `); + + expect(formatParams(result.params)).toMatchInlineSnapshot(` + "{ + \\"param0\\": { + \\"low\\": 10, + \\"high\\": 0 + }, + \\"param1\\": 5, + \\"param2\\": { + \\"low\\": 10, + \\"high\\": 0 + } + }" + `); + }); +});