From 13c7bd90dae9e5d6ffd33a8813b2cdfcc75ae131 Mon Sep 17 00:00:00 2001 From: doug-martin Date: Fri, 1 May 2020 19:27:45 -0500 Subject: [PATCH] fix(graphql): Fix paging to properly check next/previous page --- .run/jest.config.js.run.xml | 2 +- .../e2e/sub-task.resolver.spec.ts | 2 +- .../tag-graphql/e2e/tag.resolver.spec.ts | 2 +- .../e2e/todo-item.resolver.spec.ts | 2 +- .../e2e/sub-task.resolver.spec.ts | 2 +- .../e2e/tag.resolver.spec.ts | 2 +- .../e2e/todo-item.resolver.spec.ts | 2 +- .../e2e/todo-item.resolver.spec.ts | 2 +- .../e2e/sub-task.resolver.spec.ts | 2 +- .../e2e/tag.resolver.spec.ts | 2 +- .../e2e/todo-item.resolver.spec.ts | 2 +- .../__tests__/resolvers/read.resolver.spec.ts | 7 +- .../relations/read-relation.resolver.spec.ts | 31 ++- .../types/connection/connection.type.spec.ts | 214 ++++++++++++------ .../src/metadata/metadata-storage.ts | 2 +- .../src/resolvers/read.resolver.ts | 2 +- .../relations/read-relations.resolver.ts | 2 +- .../src/types/connection/connection.type.ts | 59 ++--- .../src/types/connection/page-info.type.ts | 32 +-- .../src/types/connection/pager/index.ts | 7 + .../src/types/connection/pager/interfaces.ts | 27 +++ .../connection/pager/limit-offset.pager.ts | 96 ++++++++ 22 files changed, 349 insertions(+), 152 deletions(-) create mode 100644 packages/query-graphql/src/types/connection/pager/index.ts create mode 100644 packages/query-graphql/src/types/connection/pager/interfaces.ts create mode 100644 packages/query-graphql/src/types/connection/pager/limit-offset.pager.ts diff --git a/.run/jest.config.js.run.xml b/.run/jest.config.js.run.xml index 145c15aa6..fab4e2e76 100644 --- a/.run/jest.config.js.run.xml +++ b/.run/jest.config.js.run.xml @@ -5,7 +5,7 @@ - + diff --git a/examples/federation/sub-task-graphql/e2e/sub-task.resolver.spec.ts b/examples/federation/sub-task-graphql/e2e/sub-task.resolver.spec.ts index dac222fd2..2fbfe1a1c 100644 --- a/examples/federation/sub-task-graphql/e2e/sub-task.resolver.spec.ts +++ b/examples/federation/sub-task-graphql/e2e/sub-task.resolver.spec.ts @@ -313,7 +313,7 @@ describe('Federated - SubTaskResolver (e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/federation/tag-graphql/e2e/tag.resolver.spec.ts b/examples/federation/tag-graphql/e2e/tag.resolver.spec.ts index c7d43fe3d..2a6ed9c11 100644 --- a/examples/federation/tag-graphql/e2e/tag.resolver.spec.ts +++ b/examples/federation/tag-graphql/e2e/tag.resolver.spec.ts @@ -238,7 +238,7 @@ describe('Federated - TagResolver (e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/federation/todo-item-graphql/e2e/todo-item.resolver.spec.ts b/examples/federation/todo-item-graphql/e2e/todo-item.resolver.spec.ts index 44030172e..f32fb5399 100644 --- a/examples/federation/todo-item-graphql/e2e/todo-item.resolver.spec.ts +++ b/examples/federation/todo-item-graphql/e2e/todo-item.resolver.spec.ts @@ -231,7 +231,7 @@ describe('Federated - TodoItemResolver (e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/nest-graphql-sequelize/e2e/sub-task.resolver.spec.ts b/examples/nest-graphql-sequelize/e2e/sub-task.resolver.spec.ts index e0296a751..1a768ff9e 100644 --- a/examples/nest-graphql-sequelize/e2e/sub-task.resolver.spec.ts +++ b/examples/nest-graphql-sequelize/e2e/sub-task.resolver.spec.ts @@ -317,7 +317,7 @@ describe('SubTaskResolver (sequelize - e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/nest-graphql-sequelize/e2e/tag.resolver.spec.ts b/examples/nest-graphql-sequelize/e2e/tag.resolver.spec.ts index 8245653bc..7f606a434 100644 --- a/examples/nest-graphql-sequelize/e2e/tag.resolver.spec.ts +++ b/examples/nest-graphql-sequelize/e2e/tag.resolver.spec.ts @@ -238,7 +238,7 @@ describe('TagResolver (sequelize - e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/nest-graphql-sequelize/e2e/todo-item.resolver.spec.ts b/examples/nest-graphql-sequelize/e2e/todo-item.resolver.spec.ts index 72e71283b..9e7b8c863 100644 --- a/examples/nest-graphql-sequelize/e2e/todo-item.resolver.spec.ts +++ b/examples/nest-graphql-sequelize/e2e/todo-item.resolver.spec.ts @@ -306,7 +306,7 @@ describe('TodoItemResolver (sequelize - e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/nest-graphql-typeorm-soft-delete/e2e/todo-item.resolver.spec.ts b/examples/nest-graphql-typeorm-soft-delete/e2e/todo-item.resolver.spec.ts index b2fddefbc..cbe17deb9 100644 --- a/examples/nest-graphql-typeorm-soft-delete/e2e/todo-item.resolver.spec.ts +++ b/examples/nest-graphql-typeorm-soft-delete/e2e/todo-item.resolver.spec.ts @@ -226,7 +226,7 @@ describe('SoftDelete - TodoItemResolver (e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/nest-graphql-typeorm/e2e/sub-task.resolver.spec.ts b/examples/nest-graphql-typeorm/e2e/sub-task.resolver.spec.ts index e697ab0c2..9435b783a 100644 --- a/examples/nest-graphql-typeorm/e2e/sub-task.resolver.spec.ts +++ b/examples/nest-graphql-typeorm/e2e/sub-task.resolver.spec.ts @@ -317,7 +317,7 @@ describe('SubTaskResolver (typeorm - e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/nest-graphql-typeorm/e2e/tag.resolver.spec.ts b/examples/nest-graphql-typeorm/e2e/tag.resolver.spec.ts index 0be8ea550..21317554a 100644 --- a/examples/nest-graphql-typeorm/e2e/tag.resolver.spec.ts +++ b/examples/nest-graphql-typeorm/e2e/tag.resolver.spec.ts @@ -238,7 +238,7 @@ describe('TagResolver (typeorm - e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/examples/nest-graphql-typeorm/e2e/todo-item.resolver.spec.ts b/examples/nest-graphql-typeorm/e2e/todo-item.resolver.spec.ts index d3e8bce72..4781b8be8 100644 --- a/examples/nest-graphql-typeorm/e2e/todo-item.resolver.spec.ts +++ b/examples/nest-graphql-typeorm/e2e/todo-item.resolver.spec.ts @@ -306,7 +306,7 @@ describe('TodoItemResolver (typeorm - e2e)', () => { expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjM=', hasNextPage: true, - hasPreviousPage: false, + hasPreviousPage: true, startCursor: 'YXJyYXljb25uZWN0aW9uOjI=', }); expect(edges).toHaveLength(2); diff --git a/packages/query-graphql/__tests__/resolvers/read.resolver.spec.ts b/packages/query-graphql/__tests__/resolvers/read.resolver.spec.ts index d6f39ac5d..ac8c9c013 100644 --- a/packages/query-graphql/__tests__/resolvers/read.resolver.spec.ts +++ b/packages/query-graphql/__tests__/resolvers/read.resolver.spec.ts @@ -2,7 +2,7 @@ import 'reflect-metadata'; import * as nestGraphql from '@nestjs/graphql'; import { instance, mock, when, objectContaining } from 'ts-mockito'; import { CanActivate, ExecutionContext } from '@nestjs/common'; -import { QueryService, Query } from '@nestjs-query/core'; +import { QueryService } from '@nestjs-query/core'; import { ReturnTypeFuncValue, QueryOptions, ArgsOptions } from '@nestjs/graphql'; import * as decorators from '../../src/decorators'; import { ConnectionType, QueryArgsType, ReadResolver } from '../../src'; @@ -122,10 +122,11 @@ describe('ReadResolver', () => { it('should call the service query with the provided input', async () => { const mockService = mock>(); - const input: Query = { + const input: QueryArgsType = { filter: { stringField: { eq: 'foo' }, }, + paging: { first: 1 }, }; const output: TestResolverDTO[] = [ { @@ -134,7 +135,7 @@ describe('ReadResolver', () => { }, ]; const resolver = new TestResolver(instance(mockService)); - when(mockService.query(objectContaining(input))).thenResolve(output); + when(mockService.query(objectContaining({ ...input, paging: { limit: 2, offset: 0 } }))).thenResolve(output); const result = await resolver.queryMany(input); return expect(result).toEqual({ edges: [ diff --git a/packages/query-graphql/__tests__/resolvers/relations/read-relation.resolver.spec.ts b/packages/query-graphql/__tests__/resolvers/relations/read-relation.resolver.spec.ts index 687afbdb3..0249a9f68 100644 --- a/packages/query-graphql/__tests__/resolvers/relations/read-relation.resolver.spec.ts +++ b/packages/query-graphql/__tests__/resolvers/relations/read-relation.resolver.spec.ts @@ -1,11 +1,12 @@ import 'reflect-metadata'; -import { Query, QueryService } from '@nestjs-query/core'; +import { QueryService } from '@nestjs-query/core'; import * as nestGraphql from '@nestjs/graphql'; import { mock, instance, when, objectContaining, deepEqual } from 'ts-mockito'; import { CanActivate, ExecutionContext } from '@nestjs/common'; import { ReturnTypeFuncValue, ResolveFieldOptions } from '@nestjs/graphql'; import { ReadRelationsResolver } from '../../../src/resolvers/relations'; import * as decorators from '../../../src/decorators'; +import { QueryArgsType } from '../../../src/types'; import * as types from '../../../src/types'; const { ID, ObjectType } = nestGraphql; @@ -249,8 +250,9 @@ describe('ReadRelationsResolver', () => { id: 'id-1', stringField: 'foo', }; - const query: Query = { + const query: QueryArgsType = { filter: { id: { eq: 'id-2' } }, + paging: { first: 1 }, }; const output: RelationDTO[] = [ { @@ -260,9 +262,14 @@ describe('ReadRelationsResolver', () => { ]; const R = ReadRelationsResolver(ReadRelationDTO, { many: { relation: { DTO: RelationDTO } } }); const resolver = new R(instance(mockService)); - when(mockService.queryRelations(RelationDTO, 'relations', deepEqual([dto]), objectContaining(query))).thenResolve( - new Map([[dto, output]]), - ); + when( + mockService.queryRelations( + RelationDTO, + 'relations', + deepEqual([dto]), + objectContaining({ ...query, paging: { limit: 2, offset: 0 } }), + ), + ).thenResolve(new Map([[dto, output]])); // @ts-ignore const result = await resolver.queryRelations(dto, query, {}); return expect(result).toEqual({ @@ -290,8 +297,9 @@ describe('ReadRelationsResolver', () => { id: 'id-1', stringField: 'foo', }; - const query: Query = { + const query: QueryArgsType = { filter: { id: { eq: 'id-2' } }, + paging: { first: 1 }, }; const output: RelationDTO[] = [ { @@ -303,9 +311,14 @@ describe('ReadRelationsResolver', () => { many: { relation: { DTO: RelationDTO, relationName: 'other' } }, }); const resolver = new R(instance(mockService)); - when(mockService.queryRelations(RelationDTO, 'other', deepEqual([dto]), objectContaining(query))).thenResolve( - new Map([[dto, output]]), - ); + when( + mockService.queryRelations( + RelationDTO, + 'other', + deepEqual([dto]), + objectContaining({ ...query, paging: { limit: 2, offset: 0 } }), + ), + ).thenResolve(new Map([[dto, output]])); // @ts-ignore const result = await resolver.queryRelations(dto, query, {}); return expect(result).toEqual({ diff --git a/packages/query-graphql/__tests__/types/connection/connection.type.spec.ts b/packages/query-graphql/__tests__/types/connection/connection.type.spec.ts index b315af615..b64908281 100644 --- a/packages/query-graphql/__tests__/types/connection/connection.type.spec.ts +++ b/packages/query-graphql/__tests__/types/connection/connection.type.spec.ts @@ -1,6 +1,7 @@ import 'reflect-metadata'; import { Field, ObjectType, Query, Resolver } from '@nestjs/graphql'; -import { ConnectionType } from '../../../src'; +import { plainToClass } from 'class-transformer'; +import { ConnectionType, CursorPagingType } from '../../../src'; import { connectionObjectTypeSDL, expectSDL } from '../../__fixtures__'; describe('ConnectionType', (): void => { @@ -12,6 +13,10 @@ describe('ConnectionType', (): void => { const TestConnection = ConnectionType(TestDto); + const createPage = (paging: CursorPagingType): CursorPagingType => { + return plainToClass(CursorPagingType(), paging); + }; + it('should store metadata', async () => { @Resolver() class TestConnectionTypeResolver { @@ -20,6 +25,7 @@ describe('ConnectionType', (): void => { return undefined; } } + return expectSDL([TestConnectionTypeResolver], connectionObjectTypeSDL); }); @@ -34,100 +40,170 @@ describe('ConnectionType', (): void => { ); }); + it('should create an empty connection when created with new', () => { + expect(new TestConnection()).toEqual({ + pageInfo: { hasNextPage: false, hasPreviousPage: false }, + edges: [], + }); + }); + describe('.createFromPromise', () => { - it('should create a connections response with no connection args', async () => { - const response = await TestConnection.createFromPromise( - Promise.resolve([{ stringField: 'foo1' }, { stringField: 'foo2' }]), - {}, - ); + it('should create a connections response with an empty query', async () => { + const queryMany = jest.fn(); + const response = await TestConnection.createFromPromise(queryMany, {}); + expect(queryMany).toHaveBeenCalledTimes(0); expect(response).toEqual({ - edges: [ - { - cursor: 'YXJyYXljb25uZWN0aW9uOjA=', - node: { - stringField: 'foo1', - }, - }, - { - cursor: 'YXJyYXljb25uZWN0aW9uOjE=', - node: { - stringField: 'foo2', - }, - }, - ], + edges: [], pageInfo: { - endCursor: 'YXJyYXljb25uZWN0aW9uOjE=', hasNextPage: false, hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', }, }); }); - it('should create a connections response with just a first arg', async () => { - const response = await TestConnection.createFromPromise( - Promise.resolve([{ stringField: 'foo1' }, { stringField: 'foo2' }]), - { first: 2 }, - ); + it('should create a connections response with an empty paging', async () => { + const queryMany = jest.fn(); + const response = await TestConnection.createFromPromise(queryMany, { paging: {} }); + expect(queryMany).toHaveBeenCalledTimes(0); expect(response).toEqual({ - edges: [ - { - cursor: 'YXJyYXljb25uZWN0aW9uOjA=', - node: { - stringField: 'foo1', + edges: [], + pageInfo: { + hasNextPage: false, + hasPreviousPage: false, + }, + }); + }); + + describe('with first', () => { + it('should return hasNextPage and hasPreviousPage false when there are the exact number of records', async () => { + const queryMany = jest.fn(); + queryMany.mockResolvedValueOnce([{ stringField: 'foo1' }, { stringField: 'foo2' }]); + const response = await TestConnection.createFromPromise(queryMany, { paging: createPage({ first: 2 }) }); + expect(queryMany).toHaveBeenCalledTimes(1); + expect(queryMany).toHaveBeenCalledWith({ paging: { limit: 3, offset: 0 } }); + expect(response).toEqual({ + edges: [ + { + cursor: 'YXJyYXljb25uZWN0aW9uOjA=', + node: { + stringField: 'foo1', + }, + }, + { + cursor: 'YXJyYXljb25uZWN0aW9uOjE=', + node: { + stringField: 'foo2', + }, }, + ], + pageInfo: { + endCursor: 'YXJyYXljb25uZWN0aW9uOjE=', + hasNextPage: false, + hasPreviousPage: false, + startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', }, - { - cursor: 'YXJyYXljb25uZWN0aW9uOjE=', - node: { - stringField: 'foo2', + }); + }); + + it('should return hasNextPage true and hasPreviousPage false when the number of records more than the first', async () => { + const queryMany = jest.fn(); + queryMany.mockResolvedValueOnce([{ stringField: 'foo1' }, { stringField: 'foo2' }, { stringField: 'foo3' }]); + const response = await TestConnection.createFromPromise(queryMany, { paging: createPage({ first: 2 }) }); + expect(queryMany).toHaveBeenCalledTimes(1); + expect(queryMany).toHaveBeenCalledWith({ paging: { limit: 3, offset: 0 } }); + expect(response).toEqual({ + edges: [ + { + cursor: 'YXJyYXljb25uZWN0aW9uOjA=', + node: { + stringField: 'foo1', + }, + }, + { + cursor: 'YXJyYXljb25uZWN0aW9uOjE=', + node: { + stringField: 'foo2', + }, }, + ], + pageInfo: { + endCursor: 'YXJyYXljb25uZWN0aW9uOjE=', + hasNextPage: true, + hasPreviousPage: false, + startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', }, - ], - pageInfo: { - endCursor: 'YXJyYXljb25uZWN0aW9uOjE=', - hasNextPage: true, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, + }); }); }); - it('should create a connections response with just a last arg', async () => { - const response = await TestConnection.createFromPromise( - Promise.resolve([{ stringField: 'foo1' }, { stringField: 'foo2' }]), - { - last: 2, - }, - ); - expect(response).toEqual({ - edges: [ - { - cursor: 'YXJyYXljb25uZWN0aW9uOjA=', - node: { - stringField: 'foo1', + describe('with last', () => { + it("should return hasPreviousPage false if paging backwards and we're on the first page", async () => { + const queryMany = jest.fn(); + queryMany.mockResolvedValueOnce([{ stringField: 'foo1' }]); + const response = await TestConnection.createFromPromise(queryMany, { + paging: createPage({ last: 2, before: 'YXJyYXljb25uZWN0aW9uOjE=' }), + }); + expect(queryMany).toHaveBeenCalledTimes(1); + expect(queryMany).toHaveBeenCalledWith({ paging: { limit: 1, offset: 0 } }); + expect(response).toEqual({ + edges: [ + { + cursor: 'YXJyYXljb25uZWN0aW9uOjA=', + node: { + stringField: 'foo1', + }, }, + ], + pageInfo: { + endCursor: 'YXJyYXljb25uZWN0aW9uOjA=', + hasNextPage: true, + hasPreviousPage: false, + startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', }, - { - cursor: 'YXJyYXljb25uZWN0aW9uOjE=', - node: { - stringField: 'foo2', + }); + }); + + it('should return hasPreviousPage true if paging backwards and there is an additional node', async () => { + const queryMany = jest.fn(); + queryMany.mockResolvedValueOnce([{ stringField: 'foo1' }, { stringField: 'foo2' }, { stringField: 'foo3' }]); + const response = await TestConnection.createFromPromise(queryMany, { + paging: createPage({ last: 2, before: 'YXJyYXljb25uZWN0aW9uOjM=' }), + }); + expect(queryMany).toHaveBeenCalledTimes(1); + expect(queryMany).toHaveBeenCalledWith({ paging: { limit: 2, offset: 0 } }); + expect(response).toEqual({ + edges: [ + { + cursor: 'YXJyYXljb25uZWN0aW9uOjE=', + node: { + stringField: 'foo2', + }, + }, + { + cursor: 'YXJyYXljb25uZWN0aW9uOjI=', + node: { + stringField: 'foo3', + }, }, + ], + pageInfo: { + endCursor: 'YXJyYXljb25uZWN0aW9uOjI=', + hasNextPage: true, + hasPreviousPage: true, + startCursor: 'YXJyYXljb25uZWN0aW9uOjE=', }, - ], - pageInfo: { - endCursor: 'YXJyYXljb25uZWN0aW9uOjE=', - hasNextPage: false, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=', - }, + }); }); }); it('should create an empty connection', async () => { - const response = await TestConnection.createFromPromise(Promise.resolve([]), { - first: 2, + const queryMany = jest.fn(); + queryMany.mockResolvedValueOnce([]); + const response = await TestConnection.createFromPromise(queryMany, { + paging: createPage({ first: 2 }), }); + expect(queryMany).toHaveBeenCalledTimes(1); + expect(queryMany).toHaveBeenCalledWith({ paging: { limit: 3, offset: 0 } }); expect(response).toEqual({ edges: [], pageInfo: { diff --git a/packages/query-graphql/src/metadata/metadata-storage.ts b/packages/query-graphql/src/metadata/metadata-storage.ts index abb6db850..4b9ac5803 100644 --- a/packages/query-graphql/src/metadata/metadata-storage.ts +++ b/packages/query-graphql/src/metadata/metadata-storage.ts @@ -78,7 +78,7 @@ export class GraphQLQueryMetadataStorage { } addConnectionType(type: Class, connectionType: StaticConnectionType): void { - this.connectionTypeStorage.set(type, connectionType); + this.connectionTypeStorage.set(type, connectionType as StaticConnectionType); } getConnectionType(type: Class): StaticConnectionType | undefined { diff --git a/packages/query-graphql/src/resolvers/read.resolver.ts b/packages/query-graphql/src/resolvers/read.resolver.ts index a16b6064a..f9b913add 100644 --- a/packages/query-graphql/src/resolvers/read.resolver.ts +++ b/packages/query-graphql/src/resolvers/read.resolver.ts @@ -45,7 +45,7 @@ export const Readable = (DTOClass: Class, opts: ReadResolverOpts) @ResolverQuery(() => Connection, { name: pluralBaseNameLower }, commonResolverOpts, opts.many ?? {}) async queryMany(@Args() query: QA): Promise> { const qa = await transformAndValidate(QA, query); - return Connection.createFromPromise(this.service.query(qa), qa.paging || {}); + return Connection.createFromPromise((q) => this.service.query(q), qa); } } return ReadResolverBase; diff --git a/packages/query-graphql/src/resolvers/relations/read-relations.resolver.ts b/packages/query-graphql/src/resolvers/relations/read-relations.resolver.ts index 53f4a0151..391e1b98f 100644 --- a/packages/query-graphql/src/resolvers/relations/read-relations.resolver.ts +++ b/packages/query-graphql/src/resolvers/relations/read-relations.resolver.ts @@ -63,7 +63,7 @@ const ReadManyRelationMixin = (DTOClass: Class, relation: Re ): Promise> { const qa = await transformAndValidate(RelationQA, q); const loader = DataLoaderFactory.getOrCreateLoader(context, loaderName, queryLoader.createLoader(this.service)); - return CT.createFromPromise(loader.load({ dto, query: qa }), qa.paging || {}); + return CT.createFromPromise((query) => loader.load({ dto, query }), qa); } } return ReadManyMixin; diff --git a/packages/query-graphql/src/types/connection/connection.type.ts b/packages/query-graphql/src/types/connection/connection.type.ts index d51112138..31de66402 100644 --- a/packages/query-graphql/src/types/connection/connection.type.ts +++ b/packages/query-graphql/src/types/connection/connection.type.ts @@ -1,17 +1,18 @@ -import { offsetToCursor } from 'graphql-relay'; import { Field, ObjectType } from '@nestjs/graphql'; -import { Class } from '@nestjs-query/core'; -import { CursorPagingType } from '../query'; +import { Class, Query } from '@nestjs-query/core'; +import { QueryArgsType } from '../query'; import { PageInfoType } from './page-info.type'; import { EdgeType } from './edge.type'; import { getMetadataStorage } from '../../metadata'; import { UnregisteredObjectType } from '../type.errors'; - -export interface StaticConnectionType { - createFromPromise(findMany: Promise, pagingInfo: CursorPagingType): Promise>; - createFromArray(findMany: TItem[], pagingInfo: CursorPagingType): ConnectionType; - empty(): ConnectionType; - new (): ConnectionType; +import { createPager, QueryMany } from './pager'; + +export interface StaticConnectionType { + createFromPromise( + queryMany: (query: Query) => Promise, + query: QueryArgsType, + ): Promise>; + new (): ConnectionType; } export interface ConnectionType { @@ -29,42 +30,18 @@ export function ConnectionType(TItemClass: Class): StaticConnectionTyp if (!objMetadata) { throw new UnregisteredObjectType(TItemClass, 'Unable to make ConnectionType.'); } + const pager = createPager(); const E = EdgeType(TItemClass); const PIT = PageInfoType(); @ObjectType(`${objMetadata.name}Connection`) class AbstractConnection implements ConnectionType { - static async createFromPromise(items: Promise, pagingInfo: CursorPagingType): Promise { - return this.createFromArray(await items, pagingInfo); - } - - static createFromArray(dtos: DTO[], pagingInfo: CursorPagingType): AbstractConnection { - if (!dtos.length) { - return this.empty(); - } - const baseOffset = pagingInfo.offset || 0; - const pageInfo: PageInfoType = this.createPageInfo(dtos, pagingInfo); - const edges: EdgeType[] = dtos.map((dto, i) => this.createEdge(dto, i, baseOffset)); - return new AbstractConnection(pageInfo, edges); - } - - static empty(): AbstractConnection { - return new AbstractConnection(); - } - - private static createPageInfo(dtos: DTO[], pagingInfo: CursorPagingType): PageInfoType { - const { length } = dtos; - const { first, after } = pagingInfo; - const isForwardPaging = !!first || !!after; - const baseOffset = pagingInfo.offset || 0; - const startCursor = offsetToCursor(baseOffset); - const endCursor = offsetToCursor(baseOffset + length - 1); - const hasNextPage = isForwardPaging ? length >= (pagingInfo.limit || 0) : false; - const hasPreviousPage = isForwardPaging ? false : baseOffset > 0; - return new PIT(hasNextPage, hasPreviousPage, startCursor, endCursor); - } - - private static createEdge(dto: DTO, index: number, initialOffset: number): EdgeType { - return new E(dto, offsetToCursor(initialOffset + index)); + static async createFromPromise(queryMany: QueryMany, query: QueryArgsType): Promise { + const { pageInfo, edges } = await pager.page(queryMany, query); + return new AbstractConnection( + // create the appropriate graphql instance + new PIT(pageInfo.hasNextPage, pageInfo.hasPreviousPage, pageInfo.startCursor, pageInfo.endCursor), + edges.map(({ node, cursor }) => new E(node, cursor)), + ); } constructor(pageInfo?: PageInfoType, edges?: EdgeType[]) { diff --git a/packages/query-graphql/src/types/connection/page-info.type.ts b/packages/query-graphql/src/types/connection/page-info.type.ts index 26d698b9d..5bf35102a 100644 --- a/packages/query-graphql/src/types/connection/page-info.type.ts +++ b/packages/query-graphql/src/types/connection/page-info.type.ts @@ -4,18 +4,18 @@ import { ConnectionCursorType, ConnectionCursorScalar } from '../cursor.scalar'; export interface PageInfoTypeConstructor { new ( - hasNextPage: boolean | null, - hasPreviousPage: boolean | null, - startCursor: ConnectionCursorType | null, - endCursor: ConnectionCursorType | null, + hasNextPage: boolean, + hasPreviousPage: boolean, + startCursor: ConnectionCursorType | undefined, + endCursor: ConnectionCursorType | undefined, ): PageInfoType; } export interface PageInfoType { - hasNextPage?: boolean | null; - hasPreviousPage?: boolean | null; - startCursor?: ConnectionCursorType | null; - endCursor?: ConnectionCursorType | null; + hasNextPage: boolean; + hasPreviousPage: boolean; + startCursor?: ConnectionCursorType | undefined; + endCursor?: ConnectionCursorType | undefined; } /** @internal */ @@ -28,10 +28,10 @@ export const PageInfoType = (): PageInfoTypeConstructor => { @ObjectType('PageInfo') class PageInfoTypeImpl { constructor( - hasNextPage: boolean | null, - hasPreviousPage: boolean | null, - startCursor: ConnectionCursorType | null, - endCursor: ConnectionCursorType | null, + hasNextPage: boolean, + hasPreviousPage: boolean, + startCursor: ConnectionCursorType | undefined, + endCursor: ConnectionCursorType | undefined, ) { this.hasNextPage = hasNextPage; this.hasPreviousPage = hasPreviousPage; @@ -40,19 +40,19 @@ export const PageInfoType = (): PageInfoTypeConstructor => { } @Field(() => Boolean, { nullable: true, description: 'true if paging forward and there are more records.' }) - hasNextPage?: boolean | null; + hasNextPage: boolean; @Field(() => Boolean, { nullable: true, description: 'true if paging backwards and there are more records.' }) - hasPreviousPage?: boolean | null; + hasPreviousPage: boolean; @Field(() => ConnectionCursorScalar, { nullable: true, description: 'The cursor of the first returned record.' }) - startCursor?: ConnectionCursorType | null; + startCursor?: ConnectionCursorType | undefined; @Field(() => ConnectionCursorScalar, { nullable: true, description: 'The cursor of the last returned record.', }) - endCursor?: ConnectionCursorType | null; + endCursor?: ConnectionCursorType | undefined; } pageInfoType = PageInfoTypeImpl; diff --git a/packages/query-graphql/src/types/connection/pager/index.ts b/packages/query-graphql/src/types/connection/pager/index.ts new file mode 100644 index 000000000..5e8c2e2ce --- /dev/null +++ b/packages/query-graphql/src/types/connection/pager/index.ts @@ -0,0 +1,7 @@ +import { Pager } from './interfaces'; +import { LimitOffsetPager } from './limit-offset.pager'; + +export { Pager, PagingResults, QueryMany } from './interfaces'; + +// default pager factory to plug in addition paging strategies later on. +export const createPager = (): Pager => new LimitOffsetPager(); diff --git a/packages/query-graphql/src/types/connection/pager/interfaces.ts b/packages/query-graphql/src/types/connection/pager/interfaces.ts new file mode 100644 index 000000000..9ec1fd073 --- /dev/null +++ b/packages/query-graphql/src/types/connection/pager/interfaces.ts @@ -0,0 +1,27 @@ +import { Query } from '@nestjs-query/core'; +import { QueryArgsType } from '../../query'; +import { EdgeType } from '../edge.type'; +import { PageInfoType } from '../page-info.type'; + +export interface PagingMeta { + offset: number; + limit: number; + isBackward: boolean; + isForward: boolean; + hasBefore: boolean; +} + +export interface QueryResults { + nodes: DTO[]; + hasExtraNode: boolean; +} + +export interface PagingResults { + pageInfo: PageInfoType; + edges: EdgeType[]; +} +export type QueryMany = (query: Query) => Promise; + +export interface Pager { + page(queryMany: QueryMany, query: QueryArgsType): Promise>; +} diff --git a/packages/query-graphql/src/types/connection/pager/limit-offset.pager.ts b/packages/query-graphql/src/types/connection/pager/limit-offset.pager.ts new file mode 100644 index 000000000..ae33f128d --- /dev/null +++ b/packages/query-graphql/src/types/connection/pager/limit-offset.pager.ts @@ -0,0 +1,96 @@ +import { Query } from '@nestjs-query/core'; +import { offsetToCursor } from 'graphql-relay'; +import { QueryArgsType } from '../../query'; +import { EdgeType } from '../edge.type'; +import { PagingMeta, PagingResults, QueryMany, QueryResults } from './interfaces'; + +const EMPTY_PAGING_RESULTS = (): PagingResults => ({ + edges: [], + pageInfo: { hasNextPage: false, hasPreviousPage: false }, +}); + +const DEFAULT_PAGING_META = (): PagingMeta => ({ + offset: 0, + limit: 0, + isBackward: false, + isForward: true, + hasBefore: false, +}); + +export class LimitOffsetPager { + async page(queryMany: QueryMany, query: QueryArgsType): Promise> { + const pagingMeta = this.getPageMeta(query); + if (!LimitOffsetPager.pagingMetaHasLimitOrOffset(pagingMeta)) { + return EMPTY_PAGING_RESULTS(); + } + const results = await this.runQuery(queryMany, query, pagingMeta); + if (this.isEmptyPage(results, pagingMeta)) { + return EMPTY_PAGING_RESULTS(); + } + return this.createPagingResult(results, pagingMeta); + } + + private static pagingMetaHasLimitOrOffset(pagingMeta: PagingMeta): boolean { + return pagingMeta.offset > 0 || pagingMeta.limit > 0; + } + + async runQuery(queryMany: QueryMany, query: Query, pagingMeta: PagingMeta): Promise> { + // if paging forward add 1 to limit for check for an additional page. + const limit = pagingMeta.isForward ? pagingMeta.limit + 1 : pagingMeta.limit; + // if paging backwards remove one from the offset to check for a previous page. + const offset = Math.max(0, pagingMeta.isBackward ? pagingMeta.offset - 1 : pagingMeta.offset); + const nodes = await queryMany({ ...query, paging: { limit, offset } }); + // check if we have an additional node + // if paging forward that indicates we have a next page + // if paging backward that indicates we have a previous page. + const hasExtraNode = nodes.length > pagingMeta.limit; + if (hasExtraNode) { + // remove the additional node so its not returned in the results. + if (pagingMeta.isForward) { + nodes.pop(); + } else { + nodes.shift(); + } + } + return { nodes, hasExtraNode }; + } + + getPageMeta(query: QueryArgsType): PagingMeta { + const { paging } = query; + if (!paging) { + return DEFAULT_PAGING_META(); + } + const offset = paging.offset ?? 0; + const limit = paging.limit ?? 0; + const isBackward = typeof paging.last !== 'undefined'; + const isForward = !isBackward; + const hasBefore = isBackward && !!paging.before; + return { offset, limit, isBackward, isForward, hasBefore }; + } + + createPagingResult(results: QueryResults, pagingMeta: PagingMeta): PagingResults { + const { nodes, hasExtraNode } = results; + const { offset, hasBefore, isBackward, isForward } = pagingMeta; + const endOffset = Math.max(0, offset + nodes.length - 1); + const pageInfo = { + startCursor: offsetToCursor(offset), + endCursor: offsetToCursor(endOffset), + // if we have are going forward and have an extra node or there was a before cursor + hasNextPage: isForward ? hasExtraNode : hasBefore, + // we have a previous page if we are going backwards and have an extra node. + hasPreviousPage: isBackward ? hasExtraNode : offset > 0, + }; + + const edges: EdgeType[] = nodes.map((node, i) => ({ node, cursor: offsetToCursor(offset + i) })); + return { edges, pageInfo }; + } + + isEmptyPage(results: QueryResults, pagingMeta: PagingMeta): boolean { + // it is an empty page if + // 1. we dont have an extra node + // 2. there were no nodes returned + // 3. we're paging forward + // 4. and we dont have an offset + return !results.hasExtraNode && !results.nodes.length && pagingMeta.isForward && !pagingMeta.offset; + } +}