Skip to content

Commit

Permalink
Merge pull request #6028 from neo4j/fix-4615
Browse files Browse the repository at this point in the history
Fix 4615 with new aggregations
  • Loading branch information
angrykoala authored Feb 27, 2025
2 parents da37476 + da0d5f6 commit 270f5ef
Show file tree
Hide file tree
Showing 20 changed files with 650 additions and 359 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ export class CompositeConnectionReadOperation extends Operation {

const union = new Cypher.Union(...nestedSubqueries);

const nestedSubquery = new Cypher.Call(union);
const nestedSubquery = new Cypher.Call(new Cypher.Call(union).return([Cypher.collect(edgeVar), edgesVar]));
if (context.target) {
nestedSubquery.importWith(context.target);
}

let orderSubquery: Cypher.Call | undefined;

Expand Down Expand Up @@ -104,7 +107,7 @@ export class CompositeConnectionReadOperation extends Operation {
} = this.transpileAggregation(context);

const aggregateVariables = aggregateFields.map((c) => c[1]);
const subqueryWith = new Cypher.With([Cypher.collect(edgeVar), edgesVar], ...aggregateFields).with(
const subqueryWith = new Cypher.With(edgesVar, ...aggregateFields).with(
edgesVar,
[Cypher.size(edgesVar), totalCount],
...aggregateVariables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ export class AggregateFactory {
entityOrRel,
resolveTree,
context,
extraWhereArgs = {},
}: {
entityOrRel: ConcreteEntityAdapter | RelationshipAdapter | InterfaceEntityAdapter;
resolveTree: ResolveTree;
context: Neo4jGraphQLTranslationContext;
extraWhereArgs?: Record<string, any>;
}): AggregationOperation | CompositeAggregationOperation {
let entity: ConcreteEntityAdapter | InterfaceEntityAdapter;
if (entityOrRel instanceof RelationshipAdapter) {
Expand All @@ -59,7 +61,10 @@ export class AggregateFactory {
entity = entityOrRel;
}

const resolveTreeWhere = this.queryASTFactory.operationsFactory.getWhereArgs(resolveTree);
const resolveTreeWhere = {
...this.queryASTFactory.operationsFactory.getWhereArgs(resolveTree),
...extraWhereArgs,
};

if (entityOrRel instanceof RelationshipAdapter) {
if (isConcreteEntity(entity)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export class ConnectionFactory {
relationship,
context,
operation: compositeConnectionOp,
whereArgs: resolveTreeWhere.node, // Cascades the filters from connection down to the aggregation generation, to appply them to aggregation match
});
}
}
Expand Down Expand Up @@ -236,6 +237,7 @@ export class ConnectionFactory {
relationship,
context,
operation,
whereArgs: resolveTreeWhere.node, // Cascades the filters from connection down to the aggregation generation, to appply them to aggregation match
});

return operation;
Expand All @@ -247,12 +249,14 @@ export class ConnectionFactory {
relationship,
context,
operation,
whereArgs,
}: {
target: ConcreteEntityAdapter | InterfaceEntityAdapter;
resolveTreeAggregate: ResolveTree | undefined;
relationship: RelationshipAdapter | undefined;
context: Neo4jGraphQLTranslationContext;
operation: ConnectionReadOperation | CompositeConnectionReadOperation;
whereArgs: Record<string, any>;
}) {
if (relationship) {
const resolveTreeAggregateFields =
Expand All @@ -262,6 +266,7 @@ export class ConnectionFactory {
entityOrRel: relationship ?? target,
resolveTree: resolveTreeAggregate,
context,
extraWhereArgs: whereArgs,
});
// NOTE: This will always be true on 7.x and this attribute should be removed
aggregationOperation.isInConnectionField = true;
Expand All @@ -282,6 +287,7 @@ export class ConnectionFactory {
entityOrRel: relationship ?? target,
resolveTree: resolveTreeAggregate,
context,
extraWhereArgs: whereArgs,
});
// NOTE: This will always be true on 7.x and this attribute should be removed
aggregationOperation.isInConnectionField = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* 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 { UniqueType } from "../../../utils/graphql-types";
import { TestHelper } from "../../../utils/tests-helper";

describe("https://github.com/neo4j/graphql/issues/4615", () => {
const testHelper = new TestHelper();

let Movie: UniqueType;
let Series: UniqueType;
let Actor: UniqueType;

beforeAll(async () => {
Movie = testHelper.createUniqueType("Movie");
Series = testHelper.createUniqueType("Series");
Actor = testHelper.createUniqueType("Actor");

const typeDefs = /* GraphQL */ `
interface Show {
title: String!
release: DateTime!
actors: [${Actor}!]! @declareRelationship
}
type ${Movie} implements Show @node {
title: String!
runtime: Int
release: DateTime!
actors: [${Actor}!]! @relationship(type: "ACTED_IN", direction: IN, properties: "ActedIn")
}
type ${Series} implements Show @node {
title: String!
episodes: Int
release: DateTime!
actors: [${Actor}!]! @relationship(type: "ACTED_IN", direction: IN, properties: "ActedIn")
}
type ${Actor} @node {
name: String!
actedIn: [Show!]! @relationship(type: "ACTED_IN", direction: OUT, properties: "ActedIn")
}
type ActedIn @relationshipProperties {
screenTime: Int
}
`;
await testHelper.initNeo4jGraphQL({
typeDefs,
});

await testHelper.executeCypher(
`
// Create Movies
CREATE (m1:${Movie} { title: "The Movie One", cost: 10000000, runtime: 120, release: dateTime('2007-08-31T16:47+00:00') })
CREATE (m2:${Movie} { title: "The Movie Two", cost: 20000000, runtime: 90, release: dateTime('2009-08-31T16:47+00:00') })
CREATE (m3:${Movie} { title: "The Movie Three", cost: 12000000, runtime: 70, release: dateTime('2010-08-31T16:47+00:00') })
// Create Series
CREATE (s1:${Series} { title: "The Series One", cost: 10000000, episodes: 10, release: dateTime('2011-08-31T16:47+00:00') })
CREATE (s2:${Series} { title: "The Series Two", cost: 20000000, episodes: 20, release: dateTime('2012-08-31T16:47+00:00') })
CREATE (s3:${Series} { title: "The Series Three", cost: 20000000, episodes: 15, release: dateTime('2013-08-31T16:47+00:00') })
// Create Actors
CREATE (a1:${Actor} { name: "Actor One" })
CREATE (a2:${Actor} { name: "Actor Two" })
// Associate Actor 1 with Movies and Series
CREATE (a1)-[:ACTED_IN { screenTime: 100 }]->(m1)
CREATE (a1)-[:ACTED_IN { screenTime: 82 }]->(s1)
CREATE (a1)-[:ACTED_IN { screenTime: 20 }]->(m3)
CREATE (a1)-[:ACTED_IN { screenTime: 22 }]->(s3)
// Associate Actor 2 with Movies and Series
CREATE (a2)-[:ACTED_IN { screenTime: 240 }]->(m2)
CREATE (a2)-[:ACTED_IN { screenTime: 728 }]->(s2)
CREATE (a2)-[:ACTED_IN { screenTime: 728 }]->(m3)
CREATE (a2)-[:ACTED_IN { screenTime: 88 }]->(s3)
`
);
});

afterAll(async () => {
await testHelper.close();
});

test("should return null aggregations - deprecated", async () => {
const query = /* GraphQL */ `
query {
showsAggregate(where: { title_STARTS_WITH: "asdasdasd" }) {
title {
longest
}
release {
min
}
}
}
`;

const response = await testHelper.executeGraphQL(query);
expect(response.errors).toBeFalsy();
expect(response.data).toEqual({
showsAggregate: {
title: {
longest: null,
},
release: {
min: null,
},
},
});
});
});
32 changes: 20 additions & 12 deletions packages/graphql/tests/integration/issues/4615.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,16 @@ describe("https://github.com/neo4j/graphql/issues/4615", () => {
test("should return null aggregations", async () => {
const query = /* GraphQL */ `
query {
showsAggregate(where: { title_STARTS_WITH: "asdasdasd" }) {
title {
longest
}
release {
min
showsConnection(where: { title_STARTS_WITH: "asdasdasd" }) {
aggregate {
node {
title {
longest
}
release {
min
}
}
}
}
}
Expand All @@ -118,12 +122,16 @@ describe("https://github.com/neo4j/graphql/issues/4615", () => {
const response = await testHelper.executeGraphQL(query);
expect(response.errors).toBeFalsy();
expect(response.data).toEqual({
showsAggregate: {
title: {
longest: null,
},
release: {
min: null,
showsConnection: {
aggregate: {
node: {
title: {
longest: null,
},
release: {
min: null,
},
},
},
},
});
Expand Down
Loading

0 comments on commit 270f5ef

Please sign in to comment.