diff --git a/documentation/docs/concepts/queries.mdx b/documentation/docs/concepts/queries.mdx index 5d8d2a6b74..f52f9b7dd1 100644 --- a/documentation/docs/concepts/queries.mdx +++ b/documentation/docs/concepts/queries.mdx @@ -171,8 +171,12 @@ const q: Query = { :::note -When using filters on relations in combination with paging, performance can be degraded on large result sets. -For more info see this [issue](https://github.com/doug-martin/nestjs-query/issues/954) +When using filters on relations with `typeorm` in combination with paging, performance can be degraded on large result +sets. For more info see this [issue](https://github.com/doug-martin/nestjs-query/issues/954) + +In short two queries will be executed: +* The first one fetching a distinct list of primary keys with paging applied. +* The second uses primary keys from the first query to fetch the actual records. ::: --- diff --git a/examples/basic/e2e/todo-item.resolver.spec.ts b/examples/basic/e2e/todo-item.resolver.spec.ts index a9a7ca2cc0..8dc939aef2 100644 --- a/examples/basic/e2e/todo-item.resolver.spec.ts +++ b/examples/basic/e2e/todo-item.resolver.spec.ts @@ -199,222 +199,6 @@ describe('TodoItemResolver (basic - e2e)', () => { ]); })); - it(`should allow querying relations`, () => - request(app.getHttpServer()) - .post('/graphql') - .send({ - operationName: null, - variables: {}, - query: `{ - todoItems(filter: { subTasks: { title: { eq: "Create Nest App - Sub Task 1" } } }, sorting: [{field: id, direction: ASC}]) { - ${pageInfoField} - ${edgeNodes(` - ${todoItemFields} - subTasks (sorting: [{field: id, direction: ASC}]) { - ${pageInfoField} - ${edgeNodes(subTaskFields)} - } - `)} - } - }`, - }) - .expect(200) - .then(({ body }) => { - const { edges, pageInfo }: CursorConnectionType = body.data.todoItems; - expect(pageInfo).toEqual({ - endCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - hasNextPage: false, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }); - expect(edges).toHaveLength(1); - - expect(edges.map((e) => e.node)).toEqual([ - { - id: '1', - title: 'Create Nest App', - completed: true, - description: null, - subTasks: { - pageInfo: { - endCursor: 'YXJyYXljb25uZWN0aW9uOjI=', - hasNextPage: false, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, - edges: [ - { - node: { - id: '1', - completed: true, - title: `Create Nest App - Sub Task 1`, - description: null, - todoItemId: '1', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, - { - node: { - id: '2', - completed: false, - title: `Create Nest App - Sub Task 2`, - description: null, - todoItemId: '1', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjE=', - }, - { - node: { - id: '3', - completed: false, - title: `Create Nest App - Sub Task 3`, - description: null, - todoItemId: '1', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjI=', - }, - ], - }, - }, - ]); - })); - - it(`should allow querying relations with overlapping matches on the results and correct pagination (issue 954)`, () => - request(app.getHttpServer()) - .post('/graphql') - .send({ - operationName: null, - variables: {}, - query: `{ - todoItems( - paging: {first: 2} - filter: { - or: [ - { id: { eq: 2 } }, - { subTasks: { title: { like: "Create Nest App%" } } } - ] - }, - sorting: [{field: id, direction: ASC}] - ) { - ${pageInfoField} - ${edgeNodes(` - ${todoItemFields} - subTasks (sorting: [{field: id, direction: ASC}]) { - ${pageInfoField} - ${edgeNodes(subTaskFields)} - } - `)} - } - }`, - }) - .expect(200) - .then(({ body }) => { - const { edges, pageInfo }: CursorConnectionType = body.data.todoItems; - expect(edges).toHaveLength(2); - - expect(pageInfo).toEqual({ - endCursor: 'YXJyYXljb25uZWN0aW9uOjE=', - hasNextPage: false, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }); - - expect(edges.map((e) => e.node)).toEqual([ - { - id: '1', - title: 'Create Nest App', - completed: true, - description: null, - subTasks: { - pageInfo: { - endCursor: 'YXJyYXljb25uZWN0aW9uOjI=', - hasNextPage: false, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, - edges: [ - { - node: { - id: '1', - completed: true, - title: `Create Nest App - Sub Task 1`, - description: null, - todoItemId: '1', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, - { - node: { - id: '2', - completed: false, - title: `Create Nest App - Sub Task 2`, - description: null, - todoItemId: '1', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjE=', - }, - { - node: { - id: '3', - completed: false, - title: `Create Nest App - Sub Task 3`, - description: null, - todoItemId: '1', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjI=', - }, - ], - }, - }, - { - id: '2', - title: 'Create Entity', - completed: false, - description: null, - subTasks: { - pageInfo: { - endCursor: 'YXJyYXljb25uZWN0aW9uOjI=', - hasNextPage: false, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, - edges: [ - { - node: { - id: '4', - completed: true, - title: `Create Entity - Sub Task 1`, - description: null, - todoItemId: '2', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, - { - node: { - id: '5', - completed: false, - title: `Create Entity - Sub Task 2`, - description: null, - todoItemId: '2', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjE=', - }, - { - node: { - id: '6', - completed: false, - title: `Create Entity - Sub Task 3`, - description: null, - todoItemId: '2', - }, - cursor: 'YXJyYXljb25uZWN0aW9uOjI=', - }, - ], - }, - }, - ]); - })); - it(`should allow sorting`, () => request(app.getHttpServer()) .post('/graphql') diff --git a/examples/basic/src/todo-item/dto/todo-item.dto.ts b/examples/basic/src/todo-item/dto/todo-item.dto.ts index 61687e15da..a5f5590344 100644 --- a/examples/basic/src/todo-item/dto/todo-item.dto.ts +++ b/examples/basic/src/todo-item/dto/todo-item.dto.ts @@ -1,11 +1,11 @@ -import { FilterableCursorConnection, FilterableField } from '@nestjs-query/query-graphql'; +import { CursorConnection, FilterableField } from '@nestjs-query/query-graphql'; import { ObjectType, ID, GraphQLISODateTime } from '@nestjs/graphql'; import { SubTaskDTO } from '../../sub-task/dto/sub-task.dto'; import { TagDTO } from '../../tag/dto/tag.dto'; @ObjectType('TodoItem') -@FilterableCursorConnection('subTasks', () => SubTaskDTO, { disableRemove: true }) -@FilterableCursorConnection('tags', () => TagDTO) +@CursorConnection('subTasks', () => SubTaskDTO, { disableRemove: true }) +@CursorConnection('tags', () => TagDTO) export class TodoItemDTO { @FilterableField(() => ID) id!: number; diff --git a/packages/query-typeorm/__tests__/__fixtures__/connection.fixture.ts b/packages/query-typeorm/__tests__/__fixtures__/connection.fixture.ts index 3f168e16af..4098a03c9b 100644 --- a/packages/query-typeorm/__tests__/__fixtures__/connection.fixture.ts +++ b/packages/query-typeorm/__tests__/__fixtures__/connection.fixture.ts @@ -12,7 +12,7 @@ export const CONNECTION_OPTIONS: ConnectionOptions = { dropSchema: true, entities: [TestEntity, TestSoftDeleteEntity, TestRelation, TestEntityRelationEntity], synchronize: true, - logging: false, + logging: true, }; export function createTestConnection(): Promise { diff --git a/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts b/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts index e8524bd790..f12a1881fa 100644 --- a/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts +++ b/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts @@ -1,4 +1,4 @@ -import { Filter } from '@nestjs-query/core'; +import { Filter, SortDirection } from '@nestjs-query/core'; import { Test, TestingModule } from '@nestjs/testing'; import { plainToClass } from 'class-transformer'; import { Repository } from 'typeorm'; @@ -81,6 +81,28 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResult).toEqual([entity]); }); + + it('should allow filtering on a one to one relation with an OR clause', async () => { + const entity = TEST_ENTITIES[0]; + const queryService = moduleRef.get(TestEntityService); + const queryResult = await queryService.query({ + filter: { + or: [ + { testEntityPk: { eq: TEST_ENTITIES[1].testEntityPk } }, + { + oneTestRelation: { + testRelationPk: { + in: [`test-relations-${entity.testEntityPk}-1`, `test-relations-${entity.testEntityPk}-3`], + }, + }, + }, + ], + }, + sorting: [{ field: 'testEntityPk', direction: SortDirection.ASC }], + paging: { limit: 2 }, + }); + expect(queryResult).toEqual([entity, TEST_ENTITIES[1]]); + }); }); describe('manyToOne', () => { @@ -111,6 +133,27 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResults).toEqual(TEST_RELATIONS.slice(0, 6)); }); + + it('should allow filtering on a many to one relation with an OR clause', async () => { + const queryService = moduleRef.get(TestRelationService); + const queryResults = await queryService.query({ + filter: { + or: [ + { testRelationPk: { eq: TEST_RELATIONS[6].testRelationPk } }, + { + testEntity: { + testEntityPk: { + in: [TEST_ENTITIES[0].testEntityPk, TEST_ENTITIES[1].testEntityPk], + }, + }, + }, + ], + }, + sorting: [{ field: 'testRelationPk', direction: SortDirection.ASC }], + paging: { limit: 3 }, + }); + expect(queryResults).toEqual(TEST_RELATIONS.slice(0, 3)); + }); }); describe('oneToMany', () => { @@ -128,6 +171,27 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResult).toEqual([entity]); }); + it('should allow filtering on a one to one relation with an OR clause', async () => { + const entity = TEST_ENTITIES[0]; + const queryService = moduleRef.get(TestEntityService); + const queryResult = await queryService.query({ + filter: { + or: [ + { testEntityPk: { eq: TEST_ENTITIES[1].testEntityPk } }, + { + testRelations: { + testRelationPk: { + in: [`test-relations-${entity.testEntityPk}-1`, `test-relations-${entity.testEntityPk}-3`], + }, + }, + }, + ], + }, + sorting: [{ field: 'testEntityPk', direction: SortDirection.ASC }], + paging: { limit: 2 }, + }); + expect(queryResult).toEqual([entity, TEST_ENTITIES[1]]); + }); }); describe('manyToMany', () => { @@ -164,6 +228,33 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResult).toEqual([TEST_ENTITIES[2], TEST_ENTITIES[5], TEST_ENTITIES[8]]); }); + it('should allow filtering on a many to many relation with an OR clause', async () => { + const queryService = moduleRef.get(TestEntityService); + const queryResult = await queryService.query({ + filter: { + or: [ + { testEntityPk: { eq: TEST_ENTITIES[2].testEntityPk } }, + { + manyTestRelations: { + relationName: { + in: [TEST_RELATIONS[1].relationName, TEST_RELATIONS[4].relationName], + }, + }, + }, + ], + }, + sorting: [{ field: 'numberType', direction: SortDirection.ASC }], + paging: { limit: 6 }, + }); + expect(queryResult).toEqual([ + TEST_ENTITIES[1], + TEST_ENTITIES[2], // additional one from the or + TEST_ENTITIES[3], + TEST_ENTITIES[5], + TEST_ENTITIES[7], + TEST_ENTITIES[9], + ]); + }); }); }); }); diff --git a/packages/query-typeorm/src/query/filter-query.builder.ts b/packages/query-typeorm/src/query/filter-query.builder.ts index 15797083de..424868b143 100644 --- a/packages/query-typeorm/src/query/filter-query.builder.ts +++ b/packages/query-typeorm/src/query/filter-query.builder.ts @@ -50,9 +50,9 @@ export class FilterQueryBuilder { * @param query - the query to apply. */ select(query: Query): SelectQueryBuilder { + const hasRelations = this.filterHasRelations(query.filter); let qb = this.createQueryBuilder(); - let hasRelations = false; - [qb, hasRelations] = this.applyRelationJoins(qb, query.filter); + qb = hasRelations ? this.applyRelationJoins(qb, query.filter) : qb; qb = this.applyFilter(qb, query.filter, qb.alias); qb = this.applySorting(qb, query.sorting, qb.alias); qb = this.applyPaging(qb, query.paging, hasRelations); @@ -60,9 +60,9 @@ export class FilterQueryBuilder { } selectById(id: string | number | (string | number)[], query: Query): SelectQueryBuilder { + const hasRelations = this.filterHasRelations(query.filter); let qb = this.createQueryBuilder(); - let hasRelations = false; - [qb, hasRelations] = this.applyRelationJoins(qb, query.filter); + qb = hasRelations ? this.applyRelationJoins(qb, query.filter) : qb; qb = qb.andWhereInIds(id); qb = this.applyFilter(qb, query.filter, qb.alias); qb = this.applySorting(qb, query.sorting, qb.alias); @@ -182,18 +182,19 @@ export class FilterQueryBuilder { * * @returns the query builder for chaining and a boolean indicating if relations have been added to the filter */ - private applyRelationJoins( - qb: SelectQueryBuilder, - filter?: Filter, - ): [SelectQueryBuilder, boolean] { + private applyRelationJoins(qb: SelectQueryBuilder, filter?: Filter): SelectQueryBuilder { if (!filter) { - return [qb, false]; + return qb; } const referencedRelations = this.getReferencedRelations(filter); - return [ - referencedRelations.reduce((rqb, relation) => rqb.leftJoin(`${rqb.alias}.${relation}`, relation), qb), - referencedRelations.length > 0, - ]; + return referencedRelations.reduce((rqb, relation) => rqb.leftJoin(`${rqb.alias}.${relation}`, relation), qb); + } + + private filterHasRelations(filter?: Filter): boolean { + if (!filter) { + return false; + } + return this.getReferencedRelations(filter).length > 0; } private getReferencedRelations(filter: Filter): string[] {