From 6bfc92f6ea3781f5619247023fef419c6d1b4299 Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Sat, 18 Feb 2023 15:21:35 +0800 Subject: [PATCH 1/7] feat(query-graphql): Adds enableFetchAllWithNegative option --- .../src/types/connection/cursor/pager/index.ts | 8 ++++++-- .../src/types/connection/cursor/pager/pager.ts | 8 +++++--- .../pager/strategies/keyset.pager-strategy.ts | 14 ++++++++++---- .../strategies/limit-offset.pager-strategy.ts | 15 ++++++++++++--- .../src/types/connection/interfaces.ts | 1 + .../connection/offset/offset-connection.type.ts | 2 +- .../src/types/connection/offset/pager/index.ts | 3 ++- .../src/types/connection/offset/pager/pager.ts | 9 +++++++-- .../src/types/query/paging/cursor-paging.type.ts | 12 +++++++----- .../query/query-args/cursor-query-args.type.ts | 9 ++++++--- .../src/types/query/query-args/interfaces.ts | 7 +++++++ .../query/query-args/offset-query-args.type.ts | 5 ++++- 12 files changed, 68 insertions(+), 25 deletions(-) diff --git a/packages/query-graphql/src/types/connection/cursor/pager/index.ts b/packages/query-graphql/src/types/connection/cursor/pager/index.ts index 4ac05a4cb..9f084d87e 100644 --- a/packages/query-graphql/src/types/connection/cursor/pager/index.ts +++ b/packages/query-graphql/src/types/connection/cursor/pager/index.ts @@ -8,13 +8,17 @@ import { KeysetPagerStrategy, LimitOffsetPagerStrategy } from './strategies' export type PagerOpts = { disableKeySetPagination?: boolean + enableFetchAllWithNegative?: boolean } // default pager factory to plug in addition paging strategies later on. export const createPager = (DTOClass: Class, opts: PagerOpts): Pager> => { const keySet = opts.disableKeySetPagination ? undefined : getKeySet(DTOClass) if (keySet && keySet.length) { - return new CursorPager(new KeysetPagerStrategy(DTOClass, keySet)) + return new CursorPager( + new KeysetPagerStrategy(DTOClass, keySet, opts.enableFetchAllWithNegative), + opts.enableFetchAllWithNegative + ) } - return new CursorPager(new LimitOffsetPagerStrategy()) + return new CursorPager(new LimitOffsetPagerStrategy(opts.enableFetchAllWithNegative), opts.enableFetchAllWithNegative) } diff --git a/packages/query-graphql/src/types/connection/cursor/pager/pager.ts b/packages/query-graphql/src/types/connection/cursor/pager/pager.ts index 2e5e0a140..a52bfa8dc 100644 --- a/packages/query-graphql/src/types/connection/cursor/pager/pager.ts +++ b/packages/query-graphql/src/types/connection/cursor/pager/pager.ts @@ -17,7 +17,7 @@ const DEFAULT_PAGING_META = (query: Query): PagingMeta implements Pager> { - constructor(readonly strategy: PagerStrategy) {} + constructor(readonly strategy: PagerStrategy, private readonly enableFetchAllWithNegative?: boolean) {} async page>( queryMany: QueryMany, @@ -36,10 +36,12 @@ export class CursorPager implements Pager> { } private isValidPaging(pagingMeta: PagingMeta>): boolean { + const minimumLimit = this.enableFetchAllWithNegative ? -1 : 1 + const isValidLimit = pagingMeta.opts.limit >= minimumLimit if ('offset' in pagingMeta.opts) { - return pagingMeta.opts.offset > 0 || pagingMeta.opts.limit > 0 + return pagingMeta.opts.offset > 0 && isValidLimit } - return pagingMeta.opts.limit > 0 + return isValidLimit } private async runQuery>( diff --git a/packages/query-graphql/src/types/connection/cursor/pager/strategies/keyset.pager-strategy.ts b/packages/query-graphql/src/types/connection/cursor/pager/strategies/keyset.pager-strategy.ts index 6d4339358..7e61bcc90 100644 --- a/packages/query-graphql/src/types/connection/cursor/pager/strategies/keyset.pager-strategy.ts +++ b/packages/query-graphql/src/types/connection/cursor/pager/strategies/keyset.pager-strategy.ts @@ -7,7 +7,11 @@ import { decodeBase64, encodeBase64, hasBeforeCursor, isBackwardPaging, isForwar import { KeySetCursorPayload, KeySetPagingOpts, PagerStrategy } from './pager-strategy' export class KeysetPagerStrategy implements PagerStrategy { - constructor(readonly DTOClass: Class, readonly pageFields: (keyof DTO)[]) {} + constructor( + readonly DTOClass: Class, + readonly pageFields: (keyof DTO)[], + private readonly enableFetchAllWithNegative?: boolean + ) {} fromCursorArgs(cursor: CursorPagingType): KeySetPagingOpts { const { defaultSort } = this @@ -38,7 +42,7 @@ export class KeysetPagerStrategy implements PagerStrategy { createQuery>(query: Q, opts: KeySetPagingOpts, includeExtraNode: boolean): Q { const paging = { limit: opts.limit } - if (includeExtraNode) { + if (includeExtraNode && (!this.enableFetchAllWithNegative || opts.limit !== -1)) { // Add 1 to the limit so we will fetch an additional node paging.limit += 1 } @@ -46,11 +50,13 @@ export class KeysetPagerStrategy implements PagerStrategy { // Add 1 to the limit so we will fetch an additional node with the current node const sorting = this.getSortFields(query, opts) const filter = mergeFilter(query.filter ?? {}, this.createFieldsFilter(sorting, payload)) - return { ...query, filter, paging, sorting } + const createdQuery = { ...query, filter, sorting, paging } + if (this.enableFetchAllWithNegative && opts.limit === -1) delete createdQuery.paging + return createdQuery } checkForExtraNode(nodes: DTO[], opts: KeySetPagingOpts): DTO[] { - const hasExtraNode = nodes.length > opts.limit + const hasExtraNode = nodes.length > opts.limit && !(this.enableFetchAllWithNegative && opts.limit === -1) const returnNodes = [...nodes] if (hasExtraNode) { returnNodes.pop() diff --git a/packages/query-graphql/src/types/connection/cursor/pager/strategies/limit-offset.pager-strategy.ts b/packages/query-graphql/src/types/connection/cursor/pager/strategies/limit-offset.pager-strategy.ts index cae8fe662..f2fddeb86 100644 --- a/packages/query-graphql/src/types/connection/cursor/pager/strategies/limit-offset.pager-strategy.ts +++ b/packages/query-graphql/src/types/connection/cursor/pager/strategies/limit-offset.pager-strategy.ts @@ -5,6 +5,8 @@ import { decodeBase64, encodeBase64, hasBeforeCursor, isBackwardPaging, isForwar import { OffsetPagingOpts, PagerStrategy } from './pager-strategy' export class LimitOffsetPagerStrategy implements PagerStrategy { + constructor(private readonly enableFetchAllWithNegative?: boolean) {} + private static CURSOR_PREFIX = 'arrayconnection:' toCursor(dto: DTO, index: number, pagingOpts: OffsetPagingOpts): string { @@ -25,7 +27,7 @@ export class LimitOffsetPagerStrategy implements PagerStrategy { createQuery>(query: Q, opts: OffsetPagingOpts, includeExtraNode: boolean): Q { const { isBackward } = opts const paging = { limit: opts.limit, offset: opts.offset } - if (includeExtraNode) { + if (includeExtraNode && (!this.enableFetchAllWithNegative || opts.limit !== -1)) { // Add 1 to the limit so we will fetch an additional node paging.limit += 1 // if paging backwards remove one from the offset to check for a previous page. @@ -38,10 +40,15 @@ export class LimitOffsetPagerStrategy implements PagerStrategy { paging.limit = opts.limit } } + if (this.enableFetchAllWithNegative && paging.limit === -1) delete paging.limit return { ...query, paging } } checkForExtraNode(nodes: DTO[], opts: OffsetPagingOpts): DTO[] { + if (opts.limit === -1) { + // If we are fetching all the nodes we don't need to check for an extra node. + return nodes + } const returnNodes = [...nodes] // check if we have an additional node // if paging forward that indicates we have a next page @@ -58,10 +65,11 @@ export class LimitOffsetPagerStrategy implements PagerStrategy { return returnNodes } - private getLimit(cursor: CursorPagingType): number { + private getLimit(cursor: CursorPagingType): number | null { if (isBackwardPaging(cursor)) { const { last = 0, before } = cursor const offsetFromCursor = before ? LimitOffsetPagerStrategy.cursorToOffset(before) : 0 + if (this.enableFetchAllWithNegative && last === -1) return offsetFromCursor const offset = offsetFromCursor - last // Check to see if our before-page is underflowing past the 0th item if (offset < 0) { @@ -70,11 +78,12 @@ export class LimitOffsetPagerStrategy implements PagerStrategy { } return last } - return cursor.first || 0 + return cursor.first ?? 0 } private getOffset(cursor: CursorPagingType): number { if (isBackwardPaging(cursor)) { + if (this.enableFetchAllWithNegative && cursor.last === -1) return 0 const { last, before } = cursor const beforeOffset = before ? LimitOffsetPagerStrategy.cursorToOffset(before) : 0 const offset = last ? beforeOffset - last : 0 diff --git a/packages/query-graphql/src/types/connection/interfaces.ts b/packages/query-graphql/src/types/connection/interfaces.ts index 3ac75fe59..aab895e6b 100644 --- a/packages/query-graphql/src/types/connection/interfaces.ts +++ b/packages/query-graphql/src/types/connection/interfaces.ts @@ -8,6 +8,7 @@ interface BaseConnectionOptions { enableTotalCount?: boolean connectionName?: string disableKeySetPagination?: boolean + enableFetchAllWithNegative?: boolean } export interface CursorConnectionOptions extends BaseConnectionOptions { diff --git a/packages/query-graphql/src/types/connection/offset/offset-connection.type.ts b/packages/query-graphql/src/types/connection/offset/offset-connection.type.ts index 28c848f89..293f1f175 100644 --- a/packages/query-graphql/src/types/connection/offset/offset-connection.type.ts +++ b/packages/query-graphql/src/types/connection/offset/offset-connection.type.ts @@ -36,7 +36,7 @@ export function getOrCreateOffsetConnectionType( ): StaticConnectionType { const connectionName = getOrCreateConnectionName(TItemClass, opts) return reflector.memoize(TItemClass, connectionName, () => { - const pager = createPager() + const pager = createPager(opts.enableFetchAllWithNegative) const PIT = getOrCreateOffsetPageInfoType() @ObjectType(connectionName) diff --git a/packages/query-graphql/src/types/connection/offset/pager/index.ts b/packages/query-graphql/src/types/connection/offset/pager/index.ts index f467949aa..e28b3528c 100644 --- a/packages/query-graphql/src/types/connection/offset/pager/index.ts +++ b/packages/query-graphql/src/types/connection/offset/pager/index.ts @@ -5,4 +5,5 @@ import { OffsetPager } from './pager' export { OffsetPagerResult } from './interfaces' // default pager factory to plug in addition paging strategies later on. -export const createPager = (): Pager> => new OffsetPager() +export const createPager = (enableFetchAllWithNegative?: boolean): Pager> => + new OffsetPager(enableFetchAllWithNegative) diff --git a/packages/query-graphql/src/types/connection/offset/pager/pager.ts b/packages/query-graphql/src/types/connection/offset/pager/pager.ts index 64d4fe323..00311062b 100644 --- a/packages/query-graphql/src/types/connection/offset/pager/pager.ts +++ b/packages/query-graphql/src/types/connection/offset/pager/pager.ts @@ -16,6 +16,8 @@ const DEFAULT_PAGING_META = (query: Query): OffsetPagingMeta => ( }) export class OffsetPager implements Pager> { + constructor(private readonly enableFetchAllWithNegative?: boolean) {} + async page>(queryMany: QueryMany, query: Q, count: Count): Promise> { const pagingMeta = this.getPageMeta(query) if (!this.isValidPaging(pagingMeta)) { @@ -26,7 +28,7 @@ export class OffsetPager implements Pager> { } private isValidPaging(pagingMeta: OffsetPagingMeta): boolean { - return pagingMeta.opts.limit > 0 + return pagingMeta.opts.limit >= (this.enableFetchAllWithNegative ? -1 : 1) } private async runQuery>( @@ -66,10 +68,13 @@ export class OffsetPager implements Pager> { private createQuery>(query: Q, pagingMeta: OffsetPagingMeta): Q { const { limit, offset } = pagingMeta.opts - return { ...query, paging: { limit: limit + 1, offset } } + const paging = { limit: limit + 1, offset } + if (this.enableFetchAllWithNegative && limit === -1) delete paging.limit + return { ...query, paging } } private checkForExtraNode(nodes: DTO[], opts: OffsetPagingOpts): DTO[] { + if (this.enableFetchAllWithNegative && opts.limit === -1) return nodes const returnNodes = [...nodes] const hasExtraNode = nodes.length > opts.limit if (hasExtraNode) { diff --git a/packages/query-graphql/src/types/query/paging/cursor-paging.type.ts b/packages/query-graphql/src/types/query/paging/cursor-paging.type.ts index ba7dd29bd..1b1b39f9f 100644 --- a/packages/query-graphql/src/types/query/paging/cursor-paging.type.ts +++ b/packages/query-graphql/src/types/query/paging/cursor-paging.type.ts @@ -2,15 +2,17 @@ import { Field, InputType, Int } from '@nestjs/graphql' import { Class } from '@ptc-org/nestjs-query-core' import { IsPositive, Min, Validate } from 'class-validator' +import { SkipIf } from '../../../decorators' import { ConnectionCursorScalar, ConnectionCursorType } from '../../cursor.scalar' import { CannotUseWith, CannotUseWithout, IsUndefined } from '../../validators' +import { CursorQueryArgsTypeOpts } from '..' import { PagingStrategies } from './constants' import { CursorPagingType } from './interfaces' /** @internal */ let graphQLCursorPaging: Class | null = null // eslint-disable-next-line @typescript-eslint/no-redeclare -- intentional -export const getOrCreateCursorPagingType = (): Class => { +export const getOrCreateCursorPagingType = (opts: CursorQueryArgsTypeOpts): Class => { if (graphQLCursorPaging) { return graphQLCursorPaging } @@ -40,8 +42,8 @@ export const getOrCreateCursorPagingType = (): Class => { @Field(() => Int, { nullable: true, description: 'Paginate first' }) @IsUndefined() - @IsPositive() - @Min(1) + @SkipIf(() => opts.enableFetchAllWithNegative, IsPositive()) + @Min(opts.enableFetchAllWithNegative ? -1 : 1) @Validate(CannotUseWith, ['before', 'last']) first?: number @@ -52,8 +54,8 @@ export const getOrCreateCursorPagingType = (): Class => { // We'll just ignore it for now. @Validate(CannotUseWithout, ['before']) @Validate(CannotUseWith, ['after', 'first']) - @Min(1) - @IsPositive() + @SkipIf(() => opts.enableFetchAllWithNegative, IsPositive()) + @Min(opts.enableFetchAllWithNegative ? -1 : 1) last?: number } diff --git a/packages/query-graphql/src/types/query/query-args/cursor-query-args.type.ts b/packages/query-graphql/src/types/query/query-args/cursor-query-args.type.ts index a0190b350..3190ac879 100644 --- a/packages/query-graphql/src/types/query/query-args/cursor-query-args.type.ts +++ b/packages/query-graphql/src/types/query/query-args/cursor-query-args.type.ts @@ -20,7 +20,7 @@ export function createCursorQueryArgsType( ): StaticQueryType { const F = FilterType(DTOClass) const S = getOrCreateSortType(DTOClass) - const P = getOrCreateCursorPagingType() + const P = getOrCreateCursorPagingType(opts) const C = getOrCreateCursorConnectionType(DTOClass, opts) @ArgsType() @@ -38,8 +38,11 @@ export function createCursorQueryArgsType( description: 'Limit or page results.' }) @ValidateNested() - @Validate(PropertyMax, ['first', opts.maxResultsSize ?? DEFAULT_QUERY_OPTS.maxResultsSize]) - @Validate(PropertyMax, ['last', opts.maxResultsSize ?? DEFAULT_QUERY_OPTS.maxResultsSize]) + @SkipIf( + () => opts.enableFetchAllWithNegative, + Validate(PropertyMax, ['first', opts.maxResultsSize ?? DEFAULT_QUERY_OPTS.maxResultsSize]), + Validate(PropertyMax, ['last', opts.maxResultsSize ?? DEFAULT_QUERY_OPTS.maxResultsSize]) + ) @Type(() => P) paging?: CursorPagingType diff --git a/packages/query-graphql/src/types/query/query-args/interfaces.ts b/packages/query-graphql/src/types/query/query-args/interfaces.ts index be4596f90..f87cd07aa 100644 --- a/packages/query-graphql/src/types/query/query-args/interfaces.ts +++ b/packages/query-graphql/src/types/query/query-args/interfaces.ts @@ -13,6 +13,9 @@ export type BaseQueryArgsTypeOpts = { /** * The maximum number of results that can be returned from a query. * [Default=50] + * + * If `-1` it means there is no limit to the number of results that can be + * returned. */ maxResultsSize?: number /** @@ -33,6 +36,10 @@ export type BaseQueryArgsTypeOpts = { * Disable the filtering */ disableFilter?: boolean + /** + * Enable the fetch all with negative limit. + */ + enableFetchAllWithNegative?: boolean } & FilterTypeOptions export interface CursorQueryArgsTypeOpts extends BaseQueryArgsTypeOpts, CursorConnectionOptions { diff --git a/packages/query-graphql/src/types/query/query-args/offset-query-args.type.ts b/packages/query-graphql/src/types/query/query-args/offset-query-args.type.ts index 57b2e2eea..00634acdd 100644 --- a/packages/query-graphql/src/types/query/query-args/offset-query-args.type.ts +++ b/packages/query-graphql/src/types/query/query-args/offset-query-args.type.ts @@ -38,7 +38,10 @@ export function createOffsetQueryArgs( description: 'Limit or page results.' }) @ValidateNested() - @Validate(PropertyMax, ['limit', opts.maxResultsSize ?? DEFAULT_QUERY_OPTS.maxResultsSize]) + @SkipIf( + () => opts.maxResultsSize === -1, + Validate(PropertyMax, ['limit', opts.maxResultsSize ?? DEFAULT_QUERY_OPTS.maxResultsSize]) + ) @Type(() => P) paging?: OffsetPagingType From a954f81c0727bc65c02023e189f8f0702b406713 Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Sat, 18 Feb 2023 15:22:15 +0800 Subject: [PATCH 2/7] test(nestjs-graphql): Add tests on enableFetchAllWithNegative option --- .../fetch-all-with-negative/e2e/fixtures.ts | 22 +++++ .../e2e/graphql-fragments.ts | 36 +++++++ ...-cursor-fetch-all-disable.resolver.spec.ts | 70 +++++++++++++ ...o-cursor-fetch-all-enable.resolver.spec.ts | 99 +++++++++++++++++++ ...-offset-fetch-all-disable.resolver.spec.ts | 68 +++++++++++++ ...o-offset-fetch-all-enable.resolver.spec.ts | 70 +++++++++++++ .../fetch-all-with-negative/src/app.module.ts | 20 ++++ .../todo-item-cursor-fetch-all-disable.dto.ts | 15 +++ .../todo-item-cursor-fetch-all-enable.dto.ts | 15 +++ .../todo-item-offset-fetch-all-disable.dto.ts | 15 +++ .../todo-item-offset-fetch-all-enable.dto.ts | 15 +++ .../src/todo-item/todo-item.entity.ts | 13 +++ .../src/todo-item/todo-item.module.ts | 36 +++++++ .../postgres/init-fetch-all-with-negative.sql | 3 + examples/nest-cli.json | 9 ++ .../connection/cursor-connection.type.spec.ts | 2 +- .../__tests__/types/query/paging.type.spec.ts | 2 +- 17 files changed, 508 insertions(+), 2 deletions(-) create mode 100644 examples/fetch-all-with-negative/e2e/fixtures.ts create mode 100644 examples/fetch-all-with-negative/e2e/graphql-fragments.ts create mode 100644 examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts create mode 100644 examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts create mode 100644 examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-disable.resolver.spec.ts create mode 100644 examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts create mode 100644 examples/fetch-all-with-negative/src/app.module.ts create mode 100644 examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-disable.dto.ts create mode 100644 examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-enable.dto.ts create mode 100644 examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-disable.dto.ts create mode 100644 examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-enable.dto.ts create mode 100644 examples/fetch-all-with-negative/src/todo-item/todo-item.entity.ts create mode 100644 examples/fetch-all-with-negative/src/todo-item/todo-item.module.ts create mode 100644 examples/init-scripts/postgres/init-fetch-all-with-negative.sql diff --git a/examples/fetch-all-with-negative/e2e/fixtures.ts b/examples/fetch-all-with-negative/e2e/fixtures.ts new file mode 100644 index 000000000..2b43e0896 --- /dev/null +++ b/examples/fetch-all-with-negative/e2e/fixtures.ts @@ -0,0 +1,22 @@ +import { Connection } from 'typeorm' + +import { executeTruncate } from '../../helpers' +import { TodoItemEntity } from '../src/todo-item/todo-item.entity' + +const tables = ['todo_item'] +export const truncate = async (connection: Connection): Promise => executeTruncate(connection, tables) + +export let todoItems: TodoItemEntity[] + +export const refresh = async (connection: Connection): Promise => { + await truncate(connection) + + const todoRepo = connection.getRepository(TodoItemEntity) + + todoItems = await todoRepo.save( + Array.from({ length: 100 }, (_, index) => ({ + title: `todoTitle${index + 1}`, + completed: index % 2 === 0 + })) + ) +} diff --git a/examples/fetch-all-with-negative/e2e/graphql-fragments.ts b/examples/fetch-all-with-negative/e2e/graphql-fragments.ts new file mode 100644 index 000000000..ca563c5a1 --- /dev/null +++ b/examples/fetch-all-with-negative/e2e/graphql-fragments.ts @@ -0,0 +1,36 @@ +export const todoItemFields = ` + id + title + completed + ` + +export const cursorPageInfoField = ` +pageInfo{ + hasNextPage + hasPreviousPage + startCursor + endCursor +} +` + +export const offsetPageInfoField = ` +pageInfo{ + hasNextPage + hasPreviousPage +} +` + +export const nodes = (fields: string): string => ` + nodes{ + ${fields} + } + ` + +export const edgeNodes = (fields: string): string => ` + edges { + node{ + ${fields} + } + cursor + } + ` diff --git a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts new file mode 100644 index 000000000..03ee1e869 --- /dev/null +++ b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts @@ -0,0 +1,70 @@ +import { INestApplication, ValidationPipe } from '@nestjs/common' +import { Test } from '@nestjs/testing' +import { CursorConnectionType } from '@ptc-org/nestjs-query-graphql' +import request from 'supertest' +import { Connection } from 'typeorm' + +import { AppModule } from '../src/app.module' +import { TodoItemCursorFetchWithNegativeDisableDTO } from '../src/todo-item/dto/todo-item-cursor-fetch-all-disable.dto' +import { refresh } from './fixtures' +import { cursorPageInfoField, edgeNodes, todoItemFields } from './graphql-fragments' + +describe('TodoItemResolver (cursor paginatino - fetch all with negative disabled)', () => { + let app: INestApplication + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [AppModule] + }).compile() + + app = moduleRef.createNestApplication() + app.useGlobalPipes( + new ValidationPipe({ + transform: true, + whitelist: true, + forbidNonWhitelisted: true, + skipMissingProperties: false, + forbidUnknownValues: true + }) + ) + + await app.init() + await refresh(app.get(Connection)) + }) + + afterAll(() => refresh(app.get(Connection))) + + describe('query', () => { + describe('paging', () => { + it('should not allow to fetch all the nodes', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemCursorFetchWithNegativeDisables(paging: {first: -1, after: "YXJyYXljb25uZWN0aW9uOjE="}) { + ${cursorPageInfoField} + ${edgeNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + const { edges, pageInfo }: CursorConnectionType = + body.data.todoItemCursorFetchWithNegativeDisables + expect(pageInfo).toEqual({ + endCursor: null, + hasNextPage: false, + hasPreviousPage: false, + startCursor: null + }) + expect(edges).toHaveLength(0) + })) + }) + }) + + afterAll(async () => { + await app.close() + }) +}) diff --git a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts new file mode 100644 index 000000000..2a4561a7e --- /dev/null +++ b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts @@ -0,0 +1,99 @@ +import { INestApplication, ValidationPipe } from '@nestjs/common' +import { Test } from '@nestjs/testing' +import { CursorConnectionType } from '@ptc-org/nestjs-query-graphql' +import request from 'supertest' +import { Connection } from 'typeorm' + +import { AppModule } from '../src/app.module' +import { TodoItemCursorFetchWithNegativeEnableDTO } from '../src/todo-item/dto/todo-item-cursor-fetch-all-enable.dto' +import { refresh, todoItems } from './fixtures' +import { cursorPageInfoField, edgeNodes, todoItemFields } from './graphql-fragments' + +describe('TodoItemResolver (cursor paginatino - fetch all with negative enabled)', () => { + let app: INestApplication + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [AppModule] + }).compile() + + app = moduleRef.createNestApplication() + app.useGlobalPipes( + new ValidationPipe({ + transform: true, + whitelist: true, + forbidNonWhitelisted: true, + skipMissingProperties: false, + forbidUnknownValues: true + }) + ) + + await app.init() + await refresh(app.get(Connection)) + }) + + afterAll(() => refresh(app.get(Connection))) + + describe('query', () => { + describe('paging', () => { + it('should return all the nodes after the given cursor', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemCursorFetchWithNegativeEnables(paging: {first: -1, after: "YXJyYXljb25uZWN0aW9uOjE="}) { + ${cursorPageInfoField} + ${edgeNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + const { edges, pageInfo }: CursorConnectionType = + body.data.todoItemCursorFetchWithNegativeEnables + expect(pageInfo).toEqual({ + endCursor: 'YXJyYXljb25uZWN0aW9uOjk5', + hasNextPage: false, + hasPreviousPage: true, + startCursor: 'YXJyYXljb25uZWN0aW9uOjI=' + }) + expect(edges).toHaveLength(98) + + expect(edges.map((e) => e.node)).toEqual(todoItems.slice(2)) + })) + it('should return all the nodes before the given cursor', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemCursorFetchWithNegativeEnables(paging: {last: -1, before: "YXJyYXljb25uZWN0aW9uOjk4"}) { + ${cursorPageInfoField} + ${edgeNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + const { edges, pageInfo }: CursorConnectionType = + body.data.todoItemCursorFetchWithNegativeEnables + expect(pageInfo).toEqual({ + endCursor: 'YXJyYXljb25uZWN0aW9uOjk3', + hasNextPage: true, + hasPreviousPage: false, + startCursor: 'YXJyYXljb25uZWN0aW9uOjA=' + }) + expect(edges).toHaveLength(98) + + expect(edges.map((e) => e.node)).toEqual(todoItems.slice(0, 98)) + })) + }) + }) + + afterAll(async () => { + await app.close() + }) +}) diff --git a/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-disable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-disable.resolver.spec.ts new file mode 100644 index 000000000..4bae8a29b --- /dev/null +++ b/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-disable.resolver.spec.ts @@ -0,0 +1,68 @@ +import { INestApplication, ValidationPipe } from '@nestjs/common' +import { Test } from '@nestjs/testing' +import { OffsetConnectionType } from '@ptc-org/nestjs-query-graphql' +import request from 'supertest' +import { Connection } from 'typeorm' + +import { AppModule } from '../src/app.module' +import { TodoItemOffsetFetchWithNegativeDisableDTO } from '../src/todo-item/dto/todo-item-offset-fetch-all-disable.dto' +import { refresh } from './fixtures' +import { nodes as graphqlNodes, offsetPageInfoField, todoItemFields } from './graphql-fragments' + +describe('TodoItemResolver (offset pagination - fetch all with negative disabled)', () => { + let app: INestApplication + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [AppModule] + }).compile() + + app = moduleRef.createNestApplication() + app.useGlobalPipes( + new ValidationPipe({ + transform: true, + whitelist: true, + forbidNonWhitelisted: true, + skipMissingProperties: false, + forbidUnknownValues: true + }) + ) + + await app.init() + await refresh(app.get(Connection)) + }) + + afterAll(() => refresh(app.get(Connection))) + + describe('query', () => { + describe('paging', () => { + it('should not allow to fetch all the nodes', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemOffsetFetchWithNegativeDisables(paging: {limit: -1, offset: 2}) { + ${offsetPageInfoField} + ${graphqlNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + const { nodes, pageInfo }: OffsetConnectionType = + body.data.todoItemOffsetFetchWithNegativeDisables + expect(pageInfo).toEqual({ + hasNextPage: false, + hasPreviousPage: false + }) + expect(nodes).toHaveLength(0) + })) + }) + }) + + afterAll(async () => { + await app.close() + }) +}) diff --git a/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts new file mode 100644 index 000000000..9a0af59b6 --- /dev/null +++ b/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts @@ -0,0 +1,70 @@ +import { INestApplication, ValidationPipe } from '@nestjs/common' +import { Test } from '@nestjs/testing' +import { OffsetConnectionType } from '@ptc-org/nestjs-query-graphql' +import request from 'supertest' +import { Connection } from 'typeorm' + +import { AppModule } from '../src/app.module' +import { TodoItemOffsetFetchWithNegativeEnableDTO } from '../src/todo-item/dto/todo-item-offset-fetch-all-enable.dto' +import { refresh, todoItems } from './fixtures' +import { nodes as graphqlNodes, offsetPageInfoField, todoItemFields } from './graphql-fragments' + +describe('TodoItemResolver (offset pagination - fetch all with negative enabled)', () => { + let app: INestApplication + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [AppModule] + }).compile() + + app = moduleRef.createNestApplication() + app.useGlobalPipes( + new ValidationPipe({ + transform: true, + whitelist: true, + forbidNonWhitelisted: true, + skipMissingProperties: false, + forbidUnknownValues: true + }) + ) + + await app.init() + await refresh(app.get(Connection)) + }) + + afterAll(() => refresh(app.get(Connection))) + + describe('query', () => { + describe('paging', () => { + it('should return all the nodes after the given offset', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemOffsetFetchWithNegativeEnables(paging: {limit: -1, offset: 2}) { + ${offsetPageInfoField} + ${graphqlNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + const { nodes, pageInfo }: OffsetConnectionType = + body.data.todoItemOffsetFetchWithNegativeEnables + expect(pageInfo).toEqual({ + hasNextPage: false, + hasPreviousPage: true + }) + expect(nodes).toHaveLength(98) + + expect(nodes).toEqual(todoItems.slice(2)) + })) + }) + }) + + afterAll(async () => { + await app.close() + }) +}) diff --git a/examples/fetch-all-with-negative/src/app.module.ts b/examples/fetch-all-with-negative/src/app.module.ts new file mode 100644 index 000000000..1d476bbb3 --- /dev/null +++ b/examples/fetch-all-with-negative/src/app.module.ts @@ -0,0 +1,20 @@ +import { ApolloDriver } from '@nestjs/apollo' +import { Module } from '@nestjs/common' +import { GraphQLModule } from '@nestjs/graphql' +import { TypeOrmModule } from '@nestjs/typeorm' + +import { formatGraphqlError, typeormOrmConfig } from '../../helpers' +import { TodoItemModule } from './todo-item/todo-item.module' + +@Module({ + imports: [ + TypeOrmModule.forRoot(typeormOrmConfig('fetch_all_with_negative')), + GraphQLModule.forRoot({ + driver: ApolloDriver, + autoSchemaFile: 'schema.gql', + formatError: formatGraphqlError + }), + TodoItemModule + ] +}) +export class AppModule {} diff --git a/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-disable.dto.ts b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-disable.dto.ts new file mode 100644 index 000000000..9db8b895b --- /dev/null +++ b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-disable.dto.ts @@ -0,0 +1,15 @@ +import { ObjectType } from '@nestjs/graphql' +import { FilterableField, QueryOptions } from '@ptc-org/nestjs-query-graphql' + +@ObjectType('TodoItemCursorFetchWithNegativeDisable') +@QueryOptions({ enableTotalCount: true }) +export class TodoItemCursorFetchWithNegativeDisableDTO { + @FilterableField() + id!: number + + @FilterableField() + title!: string + + @FilterableField() + completed: boolean +} diff --git a/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-enable.dto.ts b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-enable.dto.ts new file mode 100644 index 000000000..e8a0510a1 --- /dev/null +++ b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-cursor-fetch-all-enable.dto.ts @@ -0,0 +1,15 @@ +import { ObjectType } from '@nestjs/graphql' +import { FilterableField, QueryOptions } from '@ptc-org/nestjs-query-graphql' + +@ObjectType('TodoItemCursorFetchWithNegativeEnable') +@QueryOptions({ enableTotalCount: true, enableFetchAllWithNegative: true }) +export class TodoItemCursorFetchWithNegativeEnableDTO { + @FilterableField() + id!: number + + @FilterableField() + title!: string + + @FilterableField() + completed: boolean +} diff --git a/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-disable.dto.ts b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-disable.dto.ts new file mode 100644 index 000000000..6a6dd34f2 --- /dev/null +++ b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-disable.dto.ts @@ -0,0 +1,15 @@ +import { ObjectType } from '@nestjs/graphql' +import { FilterableField, PagingStrategies, QueryOptions } from '@ptc-org/nestjs-query-graphql' + +@ObjectType('TodoItemOffsetFetchWithNegativeDisable') +@QueryOptions({ enableTotalCount: true, pagingStrategy: PagingStrategies.OFFSET }) +export class TodoItemOffsetFetchWithNegativeDisableDTO { + @FilterableField() + id!: number + + @FilterableField() + title!: string + + @FilterableField() + completed: boolean +} diff --git a/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-enable.dto.ts b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-enable.dto.ts new file mode 100644 index 000000000..9606bfcc8 --- /dev/null +++ b/examples/fetch-all-with-negative/src/todo-item/dto/todo-item-offset-fetch-all-enable.dto.ts @@ -0,0 +1,15 @@ +import { ObjectType } from '@nestjs/graphql' +import { FilterableField, PagingStrategies, QueryOptions } from '@ptc-org/nestjs-query-graphql' + +@ObjectType('TodoItemOffsetFetchWithNegativeEnable') +@QueryOptions({ enableTotalCount: true, enableFetchAllWithNegative: true, pagingStrategy: PagingStrategies.OFFSET }) +export class TodoItemOffsetFetchWithNegativeEnableDTO { + @FilterableField() + id!: number + + @FilterableField() + title!: string + + @FilterableField() + completed: boolean +} diff --git a/examples/fetch-all-with-negative/src/todo-item/todo-item.entity.ts b/examples/fetch-all-with-negative/src/todo-item/todo-item.entity.ts new file mode 100644 index 000000000..610ff5538 --- /dev/null +++ b/examples/fetch-all-with-negative/src/todo-item/todo-item.entity.ts @@ -0,0 +1,13 @@ +import { Column, Entity, PrimaryGeneratedColumn } from 'typeorm' + +@Entity({ name: 'todo_item' }) +export class TodoItemEntity { + @PrimaryGeneratedColumn() + id!: number + + @Column() + title!: string + + @Column() + completed!: boolean +} diff --git a/examples/fetch-all-with-negative/src/todo-item/todo-item.module.ts b/examples/fetch-all-with-negative/src/todo-item/todo-item.module.ts new file mode 100644 index 000000000..eab8257d3 --- /dev/null +++ b/examples/fetch-all-with-negative/src/todo-item/todo-item.module.ts @@ -0,0 +1,36 @@ +import { Module } from '@nestjs/common' +import { NestjsQueryGraphQLModule } from '@ptc-org/nestjs-query-graphql' +import { NestjsQueryTypeOrmModule } from '@ptc-org/nestjs-query-typeorm' + +import { TodoItemCursorFetchWithNegativeDisableDTO } from './dto/todo-item-cursor-fetch-all-disable.dto' +import { TodoItemCursorFetchWithNegativeEnableDTO } from './dto/todo-item-cursor-fetch-all-enable.dto' +import { TodoItemOffsetFetchWithNegativeDisableDTO } from './dto/todo-item-offset-fetch-all-disable.dto' +import { TodoItemOffsetFetchWithNegativeEnableDTO } from './dto/todo-item-offset-fetch-all-enable.dto' +import { TodoItemEntity } from './todo-item.entity' + +@Module({ + imports: [ + NestjsQueryGraphQLModule.forFeature({ + imports: [NestjsQueryTypeOrmModule.forFeature([TodoItemEntity])], + resolvers: [ + { + DTOClass: TodoItemCursorFetchWithNegativeEnableDTO, + EntityClass: TodoItemEntity + }, + { + DTOClass: TodoItemCursorFetchWithNegativeDisableDTO, + EntityClass: TodoItemEntity + }, + { + DTOClass: TodoItemOffsetFetchWithNegativeEnableDTO, + EntityClass: TodoItemEntity + }, + { + DTOClass: TodoItemOffsetFetchWithNegativeDisableDTO, + EntityClass: TodoItemEntity + } + ] + }) + ] +}) +export class TodoItemModule {} diff --git a/examples/init-scripts/postgres/init-fetch-all-with-negative.sql b/examples/init-scripts/postgres/init-fetch-all-with-negative.sql new file mode 100644 index 000000000..2fd3e6cc1 --- /dev/null +++ b/examples/init-scripts/postgres/init-fetch-all-with-negative.sql @@ -0,0 +1,3 @@ +CREATE USER fetch_all_with_negative WITH SUPERUSER; +CREATE DATABASE fetch_all_with_negative; +GRANT ALL PRIVILEGES ON DATABASE fetch_all_with_negative TO fetch_all_with_negative; diff --git a/examples/nest-cli.json b/examples/nest-cli.json index 6f15dc45c..9dd25a733 100644 --- a/examples/nest-cli.json +++ b/examples/nest-cli.json @@ -92,6 +92,15 @@ "tsConfigPath": "./tsconfig.json" } }, + "fetch-all-with-negative": { + "type": "application", + "root": "fetch-all-with-negative", + "entryFile": "main", + "sourceRoot": "fetch-all-with-negative/src", + "compilerOptions": { + "tsConfigPath": "./tsconfig.json" + } + }, "hooks": { "type": "application", "root": "hooks", diff --git a/packages/query-graphql/__tests__/types/connection/cursor-connection.type.spec.ts b/packages/query-graphql/__tests__/types/connection/cursor-connection.type.spec.ts index 633a933c7..12f43816b 100644 --- a/packages/query-graphql/__tests__/types/connection/cursor-connection.type.spec.ts +++ b/packages/query-graphql/__tests__/types/connection/cursor-connection.type.spec.ts @@ -28,7 +28,7 @@ describe('CursorConnectionType', (): void => { stringField!: string } - const createPage = (paging: CursorPagingType): CursorPagingType => plainToClass(getOrCreateCursorPagingType(), paging) + const createPage = (paging: CursorPagingType): CursorPagingType => plainToClass(getOrCreateCursorPagingType({}), paging) const createTestDTO = (index: number): TestDto => ({ stringField: `foo${index}`, diff --git a/packages/query-graphql/__tests__/types/query/paging.type.spec.ts b/packages/query-graphql/__tests__/types/query/paging.type.spec.ts index 9ce02d4e7..cc51adade 100644 --- a/packages/query-graphql/__tests__/types/query/paging.type.spec.ts +++ b/packages/query-graphql/__tests__/types/query/paging.type.spec.ts @@ -6,7 +6,7 @@ import { getOrCreateCursorPagingType } from '../../../src/types/query/paging' import { generateSchema } from '../../__fixtures__' describe('PagingType', (): void => { - const CursorPaging = getOrCreateCursorPagingType() + const CursorPaging = getOrCreateCursorPagingType({}) it('should create the correct filter graphql schema', async () => { @InputType() class Paging extends CursorPaging {} From 7f8913f525d56a86c374b0a4aa787e568051974b Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Mon, 20 Feb 2023 21:35:33 +0800 Subject: [PATCH 3/7] fix: Fix isValidPaging --- .../e2e/todo-cursor-fetch-all-disable.resolver.spec.ts | 2 +- .../e2e/todo-cursor-fetch-all-enable.resolver.spec.ts | 2 +- .../init-scripts/mysql/init-fetch-all-with-negative.sql | 5 +++++ .../src/types/connection/cursor/pager/pager.ts | 9 +++++++-- 4 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 examples/init-scripts/mysql/init-fetch-all-with-negative.sql diff --git a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts index 03ee1e869..0ce89ccc0 100644 --- a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts +++ b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-disable.resolver.spec.ts @@ -9,7 +9,7 @@ import { TodoItemCursorFetchWithNegativeDisableDTO } from '../src/todo-item/dto/ import { refresh } from './fixtures' import { cursorPageInfoField, edgeNodes, todoItemFields } from './graphql-fragments' -describe('TodoItemResolver (cursor paginatino - fetch all with negative disabled)', () => { +describe('TodoItemResolver (cursor pagination - fetch all with negative disabled)', () => { let app: INestApplication beforeAll(async () => { diff --git a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts index 2a4561a7e..7bfc0e567 100644 --- a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts +++ b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts @@ -9,7 +9,7 @@ import { TodoItemCursorFetchWithNegativeEnableDTO } from '../src/todo-item/dto/t import { refresh, todoItems } from './fixtures' import { cursorPageInfoField, edgeNodes, todoItemFields } from './graphql-fragments' -describe('TodoItemResolver (cursor paginatino - fetch all with negative enabled)', () => { +describe('TodoItemResolver (cursor pagination - fetch all with negative enabled)', () => { let app: INestApplication beforeAll(async () => { diff --git a/examples/init-scripts/mysql/init-fetch-all-with-negative.sql b/examples/init-scripts/mysql/init-fetch-all-with-negative.sql new file mode 100644 index 000000000..a1e4a8259 --- /dev/null +++ b/examples/init-scripts/mysql/init-fetch-all-with-negative.sql @@ -0,0 +1,5 @@ +CREATE +USER fetch_all_with_negative; +CREATE +DATABASE fetch_all_with_negative; +GRANT ALL PRIVILEGES ON fetch_all_with_negative.* TO fetch_all_with_negative; diff --git a/packages/query-graphql/src/types/connection/cursor/pager/pager.ts b/packages/query-graphql/src/types/connection/cursor/pager/pager.ts index a52bfa8dc..a598c24ed 100644 --- a/packages/query-graphql/src/types/connection/cursor/pager/pager.ts +++ b/packages/query-graphql/src/types/connection/cursor/pager/pager.ts @@ -37,11 +37,16 @@ export class CursorPager implements Pager> { private isValidPaging(pagingMeta: PagingMeta>): boolean { const minimumLimit = this.enableFetchAllWithNegative ? -1 : 1 + const hasLimit = 'limit' in pagingMeta.opts && pagingMeta.opts.limit !== null const isValidLimit = pagingMeta.opts.limit >= minimumLimit + if (hasLimit && !isValidLimit) { + return false + } + if ('offset' in pagingMeta.opts) { - return pagingMeta.opts.offset > 0 && isValidLimit + return pagingMeta.opts.offset > 0 || hasLimit } - return isValidLimit + return hasLimit } private async runQuery>( From 9bfe29093b9fa2c9ebe23efa3906540c2efea2bf Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Thu, 4 Apr 2024 23:12:04 +0800 Subject: [PATCH 4/7] test: Fix the typeorm tests with negative fetch and my sql --- .../query-typeorm/src/services/typeorm-query.service.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/query-typeorm/src/services/typeorm-query.service.ts b/packages/query-typeorm/src/services/typeorm-query.service.ts index 685e49491..8e458a5b8 100644 --- a/packages/query-typeorm/src/services/typeorm-query.service.ts +++ b/packages/query-typeorm/src/services/typeorm-query.service.ts @@ -20,6 +20,7 @@ import { UpdateOneOptions } from '@ptc-org/nestjs-query-core' import { DeleteResult, FindOptionsWhere, Repository } from 'typeorm' +import { DriverUtils } from 'typeorm/driver/DriverUtils' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { UpdateResult } from 'typeorm/query-builder/result/UpdateResult' @@ -55,6 +56,8 @@ export class TypeOrmQueryService readonly useSoftDelete: boolean + private readonly isMySQL: boolean + constructor( readonly repo: Repository, opts?: TypeOrmQueryServiceOpts @@ -63,6 +66,8 @@ export class TypeOrmQueryService this.filterQueryBuilder = opts?.filterQueryBuilder ?? new FilterQueryBuilder(this.repo) this.useSoftDelete = opts?.useSoftDelete ?? false + + this.isMySQL = DriverUtils.isMySQLFamily(repo.manager.connection.driver) } // eslint-disable-next-line @typescript-eslint/naming-convention @@ -84,6 +89,9 @@ export class TypeOrmQueryService * @param query - The Query used to filter, page, and sort rows. */ public async query(query: Query, opts?: QueryOptions): Promise { + if (this.isMySQL && query.paging.offset && !query.paging?.limit) { + query = { ...query, paging: { ...query.paging, limit: Number.MAX_SAFE_INTEGER } } + } const qb = this.filterQueryBuilder.select(query) if (opts?.withDeleted) { From f02c81dcee2f946c7918a1a93f5bc55f678842ef Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Thu, 4 Apr 2024 23:18:21 +0800 Subject: [PATCH 5/7] chore: Fix linter --- .../query-graphql/src/types/connection/cursor/pager/pager.ts | 5 ++++- packages/query-typeorm/src/services/typeorm-query.service.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/query-graphql/src/types/connection/cursor/pager/pager.ts b/packages/query-graphql/src/types/connection/cursor/pager/pager.ts index a598c24ed..b985a06e0 100644 --- a/packages/query-graphql/src/types/connection/cursor/pager/pager.ts +++ b/packages/query-graphql/src/types/connection/cursor/pager/pager.ts @@ -17,7 +17,10 @@ const DEFAULT_PAGING_META = (query: Query): PagingMeta implements Pager> { - constructor(readonly strategy: PagerStrategy, private readonly enableFetchAllWithNegative?: boolean) {} + constructor( + readonly strategy: PagerStrategy, + private readonly enableFetchAllWithNegative?: boolean + ) {} async page>( queryMany: QueryMany, diff --git a/packages/query-typeorm/src/services/typeorm-query.service.ts b/packages/query-typeorm/src/services/typeorm-query.service.ts index 8e458a5b8..9e9651cfc 100644 --- a/packages/query-typeorm/src/services/typeorm-query.service.ts +++ b/packages/query-typeorm/src/services/typeorm-query.service.ts @@ -89,7 +89,7 @@ export class TypeOrmQueryService * @param query - The Query used to filter, page, and sort rows. */ public async query(query: Query, opts?: QueryOptions): Promise { - if (this.isMySQL && query.paging.offset && !query.paging?.limit) { + if (this.isMySQL && query.paging?.offset && !query.paging?.limit) { query = { ...query, paging: { ...query.paging, limit: Number.MAX_SAFE_INTEGER } } } const qb = this.filterQueryBuilder.select(query) From 066732fb048f97d951e55e13e900269cb62f9421 Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Fri, 5 Apr 2024 09:57:52 +0800 Subject: [PATCH 6/7] Revert "test: Fix the typeorm tests with negative fetch and my sql" This reverts commit 9bfe29093b9fa2c9ebe23efa3906540c2efea2bf. # Conflicts: # packages/query-typeorm/src/services/typeorm-query.service.ts --- .../query-typeorm/src/services/typeorm-query.service.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/query-typeorm/src/services/typeorm-query.service.ts b/packages/query-typeorm/src/services/typeorm-query.service.ts index 9e9651cfc..685e49491 100644 --- a/packages/query-typeorm/src/services/typeorm-query.service.ts +++ b/packages/query-typeorm/src/services/typeorm-query.service.ts @@ -20,7 +20,6 @@ import { UpdateOneOptions } from '@ptc-org/nestjs-query-core' import { DeleteResult, FindOptionsWhere, Repository } from 'typeorm' -import { DriverUtils } from 'typeorm/driver/DriverUtils' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { UpdateResult } from 'typeorm/query-builder/result/UpdateResult' @@ -56,8 +55,6 @@ export class TypeOrmQueryService readonly useSoftDelete: boolean - private readonly isMySQL: boolean - constructor( readonly repo: Repository, opts?: TypeOrmQueryServiceOpts @@ -66,8 +63,6 @@ export class TypeOrmQueryService this.filterQueryBuilder = opts?.filterQueryBuilder ?? new FilterQueryBuilder(this.repo) this.useSoftDelete = opts?.useSoftDelete ?? false - - this.isMySQL = DriverUtils.isMySQLFamily(repo.manager.connection.driver) } // eslint-disable-next-line @typescript-eslint/naming-convention @@ -89,9 +84,6 @@ export class TypeOrmQueryService * @param query - The Query used to filter, page, and sort rows. */ public async query(query: Query, opts?: QueryOptions): Promise { - if (this.isMySQL && query.paging?.offset && !query.paging?.limit) { - query = { ...query, paging: { ...query.paging, limit: Number.MAX_SAFE_INTEGER } } - } const qb = this.filterQueryBuilder.select(query) if (opts?.withDeleted) { From ff0e245cc81dce181de8a131610ca55caa7454a0 Mon Sep 17 00:00:00 2001 From: ValentinVignal Date: Fri, 5 Apr 2024 10:36:50 +0800 Subject: [PATCH 7/7] test: Update the tests for postgresql and mysql --- ...o-cursor-fetch-all-enable.resolver.spec.ts | 109 +++++++++++++----- ...o-offset-fetch-all-enable.resolver.spec.ts | 61 +++++++++- 2 files changed, 138 insertions(+), 32 deletions(-) diff --git a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts index 7bfc0e567..531193c09 100644 --- a/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts +++ b/examples/fetch-all-with-negative/e2e/todo-cursor-fetch-all-enable.resolver.spec.ts @@ -12,6 +12,8 @@ import { cursorPageInfoField, edgeNodes, todoItemFields } from './graphql-fragme describe('TodoItemResolver (cursor pagination - fetch all with negative enabled)', () => { let app: INestApplication + const describeIf = (condition: boolean) => (condition ? describe : describe.skip) + beforeAll(async () => { const moduleRef = await Test.createTestingModule({ imports: [AppModule] @@ -36,14 +38,14 @@ describe('TodoItemResolver (cursor pagination - fetch all with negative enabled) describe('query', () => { describe('paging', () => { - it('should return all the nodes after the given cursor', () => + it('should return all the nodes', () => request(app.getHttpServer()) .post('/graphql') .send({ operationName: null, variables: {}, query: `{ - todoItemCursorFetchWithNegativeEnables(paging: {first: -1, after: "YXJyYXljb25uZWN0aW9uOjE="}) { + todoItemCursorFetchWithNegativeEnables(paging: {first: -1}) { ${cursorPageInfoField} ${edgeNodes(todoItemFields)} } @@ -56,41 +58,94 @@ describe('TodoItemResolver (cursor pagination - fetch all with negative enabled) expect(pageInfo).toEqual({ endCursor: 'YXJyYXljb25uZWN0aW9uOjk5', hasNextPage: false, - hasPreviousPage: true, - startCursor: 'YXJyYXljb25uZWN0aW9uOjI=' + hasPreviousPage: false, + startCursor: 'YXJyYXljb25uZWN0aW9uOjA=' }) - expect(edges).toHaveLength(98) + expect(edges).toHaveLength(100) - expect(edges.map((e) => e.node)).toEqual(todoItems.slice(2)) + expect(edges.map((e) => e.node)).toEqual(todoItems) })) - it('should return all the nodes before the given cursor', () => - request(app.getHttpServer()) - .post('/graphql') - .send({ - operationName: null, - variables: {}, - query: `{ + + describeIf(process.env.NESTJS_QUERY_DB_TYPE == 'postgres')('postgres', () => { + it('should return all the nodes after the given cursor', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemCursorFetchWithNegativeEnables(paging: {first: -1, after: "YXJyYXljb25uZWN0aW9uOjE="}) { + ${cursorPageInfoField} + ${edgeNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + const { edges, pageInfo }: CursorConnectionType = + body.data.todoItemCursorFetchWithNegativeEnables + expect(pageInfo).toEqual({ + endCursor: 'YXJyYXljb25uZWN0aW9uOjk5', + hasNextPage: false, + hasPreviousPage: true, + startCursor: 'YXJyYXljb25uZWN0aW9uOjI=' + }) + expect(edges).toHaveLength(98) + + expect(edges.map((e) => e.node)).toEqual(todoItems.slice(2)) + })) + }) + + describeIf(process.env.NESTJS_QUERY_DB_TYPE == 'mysql')('mysql', () => { + it('should return an error when requesting all the nodes after the given cursor', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemCursorFetchWithNegativeEnables(paging: {first: -1, after: "YXJyYXljb25uZWN0aW9uOjE="}) { + ${cursorPageInfoField} + ${edgeNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + expect(body.errors).toBeDefined() + expect(body.errors).toHaveLength(1) + expect(body.errors[0].message).toContain('RDBMS does not support OFFSET without LIMIT in SELECT statements') + expect(body.data).toBeNull() + })) + }) + }) + it('should return an error when request all the nodes before the given cursor', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ todoItemCursorFetchWithNegativeEnables(paging: {last: -1, before: "YXJyYXljb25uZWN0aW9uOjk4"}) { ${cursorPageInfoField} ${edgeNodes(todoItemFields)} } }` + }) + .expect(200) + .then(({ body }) => { + const { edges, pageInfo }: CursorConnectionType = + body.data.todoItemCursorFetchWithNegativeEnables + expect(pageInfo).toEqual({ + endCursor: 'YXJyYXljb25uZWN0aW9uOjk3', + hasNextPage: true, + hasPreviousPage: false, + startCursor: 'YXJyYXljb25uZWN0aW9uOjA=' }) - .expect(200) - .then(({ body }) => { - const { edges, pageInfo }: CursorConnectionType = - body.data.todoItemCursorFetchWithNegativeEnables - expect(pageInfo).toEqual({ - endCursor: 'YXJyYXljb25uZWN0aW9uOjk3', - hasNextPage: true, - hasPreviousPage: false, - startCursor: 'YXJyYXljb25uZWN0aW9uOjA=' - }) - expect(edges).toHaveLength(98) + expect(edges).toHaveLength(98) - expect(edges.map((e) => e.node)).toEqual(todoItems.slice(0, 98)) - })) - }) + expect(edges.map((e) => e.node)).toEqual(todoItems.slice(0, 98)) + })) }) afterAll(async () => { diff --git a/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts b/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts index 9a0af59b6..0d229184e 100644 --- a/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts +++ b/examples/fetch-all-with-negative/e2e/todo-offset-fetch-all-enable.resolver.spec.ts @@ -12,6 +12,8 @@ import { nodes as graphqlNodes, offsetPageInfoField, todoItemFields } from './gr describe('TodoItemResolver (offset pagination - fetch all with negative enabled)', () => { let app: INestApplication + const describeIf = (condition: boolean) => (condition ? describe : describe.skip) + beforeAll(async () => { const moduleRef = await Test.createTestingModule({ imports: [AppModule] @@ -36,14 +38,14 @@ describe('TodoItemResolver (offset pagination - fetch all with negative enabled) describe('query', () => { describe('paging', () => { - it('should return all the nodes after the given offset', () => + it('should return all the nodes', () => request(app.getHttpServer()) .post('/graphql') .send({ operationName: null, variables: {}, query: `{ - todoItemOffsetFetchWithNegativeEnables(paging: {limit: -1, offset: 2}) { + todoItemOffsetFetchWithNegativeEnables(paging: {limit: -1}) { ${offsetPageInfoField} ${graphqlNodes(todoItemFields)} } @@ -55,12 +57,61 @@ describe('TodoItemResolver (offset pagination - fetch all with negative enabled) body.data.todoItemOffsetFetchWithNegativeEnables expect(pageInfo).toEqual({ hasNextPage: false, - hasPreviousPage: true + hasPreviousPage: false }) - expect(nodes).toHaveLength(98) + expect(nodes).toHaveLength(100) - expect(nodes).toEqual(todoItems.slice(2)) + expect(nodes).toEqual(todoItems) })) + describeIf(process.env.NESTJS_QUERY_DB_TYPE == 'postgres')('postgres', () => { + it('should return all the nodes after the given offset', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemOffsetFetchWithNegativeEnables(paging: {limit: -1, offset: 2}) { + ${offsetPageInfoField} + ${graphqlNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + const { nodes, pageInfo }: OffsetConnectionType = + body.data.todoItemOffsetFetchWithNegativeEnables + expect(pageInfo).toEqual({ + hasNextPage: false, + hasPreviousPage: true + }) + expect(nodes).toHaveLength(98) + + expect(nodes).toEqual(todoItems.slice(2)) + })) + }) + describeIf(process.env.NESTJS_QUERY_DB_TYPE == 'mysql')('mysql', () => { + it('should return an error when fetching all the nodes after the given offset', () => + request(app.getHttpServer()) + .post('/graphql') + .send({ + operationName: null, + variables: {}, + query: `{ + todoItemOffsetFetchWithNegativeEnables(paging: {limit: -1, offset: 2}) { + ${offsetPageInfoField} + ${graphqlNodes(todoItemFields)} + } + }` + }) + .expect(200) + .then(({ body }) => { + expect(body.errors).toBeDefined() + expect(body.errors).toHaveLength(1) + expect(body.errors[0].message).toContain('RDBMS does not support OFFSET without LIMIT in SELECT statements') + expect(body.data).toBeNull() + })) + }) }) })