fix(typeorm, #954): Filtering on relations with pagination

* Added tests to typeorm-query.service.spec for filtering on relations with paging * Reverted basic example.
doug-martin · Mar 16, 2021 · c3e43f6 · c3e43f6
1 parent 6e3d99d
commit c3e43f6
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 236 deletions.
diff --git a/documentation/docs/concepts/queries.mdx b/documentation/docs/concepts/queries.mdx
@@ -171,8 +171,12 @@ const q: Query<MyClass> = {
 </Tabs>
 
 :::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
@@ -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<TodoItemDTO> = 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<TodoItemDTO> = 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
@@ -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
@@ -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<Connection> {

diff --git 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],
+          ]);
+        });
       });
     });
   });