From dce208166337a8e47cc41301c9c5be0854199eaa Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Wed, 30 Nov 2022 22:17:38 -0500 Subject: [PATCH 1/8] fix: aligns mongoose PaginatedDocs type with actual lib type --- src/mongoose/types.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mongoose/types.ts b/src/mongoose/types.ts index 884e5e9f22..cc61754c58 100644 --- a/src/mongoose/types.ts +++ b/src/mongoose/types.ts @@ -3,10 +3,10 @@ export type PaginatedDocs = { totalDocs: number limit: number totalPages: number - page: number + page?: number pagingCounter: number hasPrevPage: boolean hasNextPage: boolean - prevPage: number | null - nextPage: number | null + prevPage?: number | null | undefined + nextPage?: number | null | undefined } From f7ce0c615d76035ee48ef32047613ab1415deb44 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Wed, 30 Nov 2022 22:18:37 -0500 Subject: [PATCH 2/8] feat: decouples limit from pagination, allows for no limit query --- src/collections/operations/find.ts | 11 +++-- src/collections/operations/findVersions.ts | 7 ++- .../operations/helpers/findDocs.ts | 44 +++++++++++++++++++ src/globals/operations/findVersions.ts | 7 ++- 4 files changed, 55 insertions(+), 14 deletions(-) create mode 100644 src/collections/operations/helpers/findDocs.ts diff --git a/src/collections/operations/find.ts b/src/collections/operations/find.ts index 7867d7e713..3ba04c4a7f 100644 --- a/src/collections/operations/find.ts +++ b/src/collections/operations/find.ts @@ -10,6 +10,7 @@ import { buildSortParam } from '../../mongoose/buildSortParam'; import replaceWithDraftIfAvailable from '../../versions/drafts/replaceWithDraftIfAvailable'; import { AccessResult } from '../../config/types'; import { afterRead } from '../../fields/hooks/afterRead'; +import { findDocs } from './helpers/findDocs'; export type Arguments = { collection: Collection @@ -136,7 +137,7 @@ async function find(incomingArgs: Arguments): Promis locale, }); - const optionsToExecute = { + const paginatedDocs = await findDocs(Model, query, { page: page || 1, limit: limit || 10, sort: { @@ -147,18 +148,16 @@ async function find(incomingArgs: Arguments): Promis useEstimatedCount, pagination, useCustomCountFn: pagination ? undefined : () => Promise.resolve(1), - }; + }); - const paginatedDocs = await Model.paginate(query, optionsToExecute); - - let result = { + let result: PaginatedDocs = { ...paginatedDocs, docs: paginatedDocs.docs.map((doc) => { const sanitizedDoc = JSON.parse(JSON.stringify(doc)); sanitizedDoc.id = sanitizedDoc._id; return sanitizeInternalFields(sanitizedDoc); }), - } as PaginatedDocs; + }; // ///////////////////////////////////// // Replace documents with drafts if available diff --git a/src/collections/operations/findVersions.ts b/src/collections/operations/findVersions.ts index 6b6ed8ac42..9005f49c23 100644 --- a/src/collections/operations/findVersions.ts +++ b/src/collections/operations/findVersions.ts @@ -10,6 +10,7 @@ import { PaginatedDocs } from '../../mongoose/types'; import { TypeWithVersion } from '../../versions/types'; import { afterRead } from '../../fields/hooks/afterRead'; import { buildVersionCollectionFields } from '../../versions/buildCollectionFields'; +import { findDocs } from './helpers/findDocs'; export type Arguments = { collection: Collection @@ -98,7 +99,7 @@ async function findVersions = any>(args: Arguments) locale, }); - const optionsToExecute = { + const paginatedDocs = await findDocs(VersionsModel, query, { page: page || 1, limit: limit || 10, sort: { @@ -107,9 +108,7 @@ async function findVersions = any>(args: Arguments) lean: true, leanWithId: true, useEstimatedCount, - }; - - const paginatedDocs = await VersionsModel.paginate(query, optionsToExecute); + }); // ///////////////////////////////////// // beforeRead - Collection diff --git a/src/collections/operations/helpers/findDocs.ts b/src/collections/operations/helpers/findDocs.ts new file mode 100644 index 0000000000..4bb0a19d64 --- /dev/null +++ b/src/collections/operations/helpers/findDocs.ts @@ -0,0 +1,44 @@ +import { PaginatedDocs } from '../../../mongoose/types'; +import { CollectionModel } from '../../config/types'; +import { Arguments } from '../find'; + +type FindDocsArguments = ({ + page: Arguments['page'] + limit: Arguments['limit'] + sort: { + [key: string]: string, + } +}) & ({ + pagination?: false +} | { + pagination: true + lean?: boolean + leanWithId?: boolean + useEstimatedCount?: boolean + useCustomCountFn?: (() => Promise) | undefined; +}) + +export async function findDocs(Model: CollectionModel, query: Record, args: FindDocsArguments): Promise> { + // ///////////////////////////////////// + // Model.paginate ignores limit when paginate is true + // pass limit=0 or pagination=false to skip + // ///////////////////////////////////// + if (args.limit !== 0 && args.pagination) { + return Model.paginate(query, args); + } + + const docs = await Model.find(query, undefined, args); + + return { + docs, + totalDocs: docs.length, + totalPages: 1, + page: undefined, + nextPage: null, + prevPage: null, + pagingCounter: 0, + hasNextPage: null, + hasPrevPage: null, + limit: args.limit || null, + }; +} diff --git a/src/globals/operations/findVersions.ts b/src/globals/operations/findVersions.ts index 3feb2bce28..96a883185e 100644 --- a/src/globals/operations/findVersions.ts +++ b/src/globals/operations/findVersions.ts @@ -10,6 +10,7 @@ import { TypeWithVersion } from '../../versions/types'; import { SanitizedGlobalConfig } from '../config/types'; import { afterRead } from '../../fields/hooks/afterRead'; import { buildVersionGlobalFields } from '../../versions/buildGlobalFields'; +import { findDocs } from '../../collections/operations/helpers/findDocs'; export type Arguments = { globalConfig: SanitizedGlobalConfig @@ -96,7 +97,7 @@ async function findVersions = any>(args: Arguments) locale, }); - const optionsToExecute = { + const paginatedDocs = await findDocs(VersionsModel, query, { page: page || 1, limit: limit || 10, sort: { @@ -105,9 +106,7 @@ async function findVersions = any>(args: Arguments) lean: true, leanWithId: true, useEstimatedCount, - }; - - const paginatedDocs = await VersionsModel.paginate(query, optionsToExecute); + }); // ///////////////////////////////////// // afterRead - Fields From dc4e0971a6d12a0e9706f33ea94c7e7da64b0567 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Thu, 1 Dec 2022 08:17:00 -0500 Subject: [PATCH 3/8] chore: simplifies logic to decouple pagination and limit --- src/collections/operations/find.ts | 9 ++-- src/collections/operations/findVersions.ts | 3 +- .../operations/helpers/findDocs.ts | 44 ------------------- src/globals/operations/findVersions.ts | 3 +- 4 files changed, 8 insertions(+), 51 deletions(-) delete mode 100644 src/collections/operations/helpers/findDocs.ts diff --git a/src/collections/operations/find.ts b/src/collections/operations/find.ts index 3ba04c4a7f..a7bbd79d4e 100644 --- a/src/collections/operations/find.ts +++ b/src/collections/operations/find.ts @@ -10,7 +10,6 @@ import { buildSortParam } from '../../mongoose/buildSortParam'; import replaceWithDraftIfAvailable from '../../versions/drafts/replaceWithDraftIfAvailable'; import { AccessResult } from '../../config/types'; import { afterRead } from '../../fields/hooks/afterRead'; -import { findDocs } from './helpers/findDocs'; export type Arguments = { collection: Collection @@ -137,9 +136,9 @@ async function find(incomingArgs: Arguments): Promis locale, }); - const paginatedDocs = await findDocs(Model, query, { + const fallbackLimit = pagination ? 10 : 0; + const paginatedDocs = await Model.paginate(query, { page: page || 1, - limit: limit || 10, sort: { [sortProperty]: sortOrder, }, @@ -148,6 +147,10 @@ async function find(incomingArgs: Arguments): Promis useEstimatedCount, pagination, useCustomCountFn: pagination ? undefined : () => Promise.resolve(1), + options: { + // limit must be set here to avoid being ignored when paginate is false + limit: typeof limit === 'number' ? limit : fallbackLimit, + }, }); let result: PaginatedDocs = { diff --git a/src/collections/operations/findVersions.ts b/src/collections/operations/findVersions.ts index 9005f49c23..ce4e279e3c 100644 --- a/src/collections/operations/findVersions.ts +++ b/src/collections/operations/findVersions.ts @@ -10,7 +10,6 @@ import { PaginatedDocs } from '../../mongoose/types'; import { TypeWithVersion } from '../../versions/types'; import { afterRead } from '../../fields/hooks/afterRead'; import { buildVersionCollectionFields } from '../../versions/buildCollectionFields'; -import { findDocs } from './helpers/findDocs'; export type Arguments = { collection: Collection @@ -99,7 +98,7 @@ async function findVersions = any>(args: Arguments) locale, }); - const paginatedDocs = await findDocs(VersionsModel, query, { + const paginatedDocs = await VersionsModel.paginate(query, { page: page || 1, limit: limit || 10, sort: { diff --git a/src/collections/operations/helpers/findDocs.ts b/src/collections/operations/helpers/findDocs.ts deleted file mode 100644 index 4bb0a19d64..0000000000 --- a/src/collections/operations/helpers/findDocs.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { PaginatedDocs } from '../../../mongoose/types'; -import { CollectionModel } from '../../config/types'; -import { Arguments } from '../find'; - -type FindDocsArguments = ({ - page: Arguments['page'] - limit: Arguments['limit'] - sort: { - [key: string]: string, - } -}) & ({ - pagination?: false -} | { - pagination: true - lean?: boolean - leanWithId?: boolean - useEstimatedCount?: boolean - useCustomCountFn?: (() => Promise) | undefined; -}) - -export async function findDocs(Model: CollectionModel, query: Record, args: FindDocsArguments): Promise> { - // ///////////////////////////////////// - // Model.paginate ignores limit when paginate is true - // pass limit=0 or pagination=false to skip - // ///////////////////////////////////// - if (args.limit !== 0 && args.pagination) { - return Model.paginate(query, args); - } - - const docs = await Model.find(query, undefined, args); - - return { - docs, - totalDocs: docs.length, - totalPages: 1, - page: undefined, - nextPage: null, - prevPage: null, - pagingCounter: 0, - hasNextPage: null, - hasPrevPage: null, - limit: args.limit || null, - }; -} diff --git a/src/globals/operations/findVersions.ts b/src/globals/operations/findVersions.ts index 96a883185e..6020c3dd09 100644 --- a/src/globals/operations/findVersions.ts +++ b/src/globals/operations/findVersions.ts @@ -10,7 +10,6 @@ import { TypeWithVersion } from '../../versions/types'; import { SanitizedGlobalConfig } from '../config/types'; import { afterRead } from '../../fields/hooks/afterRead'; import { buildVersionGlobalFields } from '../../versions/buildGlobalFields'; -import { findDocs } from '../../collections/operations/helpers/findDocs'; export type Arguments = { globalConfig: SanitizedGlobalConfig @@ -97,7 +96,7 @@ async function findVersions = any>(args: Arguments) locale, }); - const paginatedDocs = await findDocs(VersionsModel, query, { + const paginatedDocs = await VersionsModel.paginate(query, { page: page || 1, limit: limit || 10, sort: { From a71801006cbc4b989d5057a5f04e8e8e0a6dbeed Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Thu, 1 Dec 2022 09:10:56 -0500 Subject: [PATCH 4/8] fix: adjusts how limit is set, both in options and paginates limit --- src/collections/operations/find.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/collections/operations/find.ts b/src/collections/operations/find.ts index a7bbd79d4e..ed74909b68 100644 --- a/src/collections/operations/find.ts +++ b/src/collections/operations/find.ts @@ -136,20 +136,22 @@ async function find(incomingArgs: Arguments): Promis locale, }); - const fallbackLimit = pagination ? 10 : 0; + const usePagination = pagination && limit !== 0; + const limitToUse = limit ?? (usePagination ? 10 : 0); const paginatedDocs = await Model.paginate(query, { page: page || 1, sort: { [sortProperty]: sortOrder, }, + limit: limitToUse, lean: true, leanWithId: true, useEstimatedCount, - pagination, + pagination: usePagination, useCustomCountFn: pagination ? undefined : () => Promise.resolve(1), options: { - // limit must be set here to avoid being ignored when paginate is false - limit: typeof limit === 'number' ? limit : fallbackLimit, + // limit must also be set here, it's ignored when pagination is false + limit: limitToUse, }, }); From c8d1b9f88af62ad1ab927ca3d035fa4c031989f1 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Thu, 1 Dec 2022 10:06:22 -0500 Subject: [PATCH 5/8] fix: sanitize number query params before passing to find operation --- src/collections/requestHandlers/find.ts | 5 +++-- src/utilities/isNumber.ts | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 src/utilities/isNumber.ts diff --git a/src/collections/requestHandlers/find.ts b/src/collections/requestHandlers/find.ts index 660f77c998..9f94102a84 100644 --- a/src/collections/requestHandlers/find.ts +++ b/src/collections/requestHandlers/find.ts @@ -5,6 +5,7 @@ import { TypeWithID } from '../config/types'; import { PaginatedDocs } from '../../mongoose/types'; import find from '../operations/find'; import { Where } from '../../types'; +import { isNumber } from '../../utilities/isNumber'; // eslint-disable-next-line @typescript-eslint/no-explicit-any export default async function findHandler(req: PayloadRequest, res: Response, next: NextFunction): Promise> | void> { @@ -24,9 +25,9 @@ export default async function findHandler(req: Paylo collection: req.collection, where: req.query.where as Where, // This is a little shady page, - limit: Number(req.query.limit), + limit: isNumber(req.query.limit) ? Number(req.query.limit) : undefined, sort: req.query.sort as string, - depth: Number(req.query.depth), + depth: isNumber(req.query.depth) ? Number(req.query.depth) : undefined, draft: req.query.draft === 'true', }); diff --git a/src/utilities/isNumber.ts b/src/utilities/isNumber.ts new file mode 100644 index 0000000000..1604de7ff2 --- /dev/null +++ b/src/utilities/isNumber.ts @@ -0,0 +1,3 @@ +export function isNumber(value: unknown): boolean { + return !Number.isNaN(Number(value)); +} From 891f00d05cd57d9387dd25be81daa3de99e315ed Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Thu, 1 Dec 2022 10:44:11 -0500 Subject: [PATCH 6/8] fix: allows for limit bypass on version find operations --- src/collections/operations/findVersions.ts | 2 +- src/globals/operations/findVersions.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/collections/operations/findVersions.ts b/src/collections/operations/findVersions.ts index ce4e279e3c..4fb5842da6 100644 --- a/src/collections/operations/findVersions.ts +++ b/src/collections/operations/findVersions.ts @@ -100,7 +100,7 @@ async function findVersions = any>(args: Arguments) const paginatedDocs = await VersionsModel.paginate(query, { page: page || 1, - limit: limit || 10, + limit: limit ?? 10, sort: { [sortProperty]: sortOrder, }, diff --git a/src/globals/operations/findVersions.ts b/src/globals/operations/findVersions.ts index 6020c3dd09..f2bfce95f3 100644 --- a/src/globals/operations/findVersions.ts +++ b/src/globals/operations/findVersions.ts @@ -98,7 +98,7 @@ async function findVersions = any>(args: Arguments) const paginatedDocs = await VersionsModel.paginate(query, { page: page || 1, - limit: limit || 10, + limit: limit ?? 10, sort: { [sortProperty]: sortOrder, }, From 226c7d5da5bdecfa8abb258559275082f9e76d10 Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Thu, 1 Dec 2022 11:43:15 -0500 Subject: [PATCH 7/8] chore: adds REST test for http limitless query param --- test/collections-rest/int.spec.ts | 34 +++++++++++++++++++++++++++++++ test/helpers/rest.ts | 5 ++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/test/collections-rest/int.spec.ts b/test/collections-rest/int.spec.ts index 32ae8493ec..e029e56aa8 100644 --- a/test/collections-rest/int.spec.ts +++ b/test/collections-rest/int.spec.ts @@ -622,6 +622,40 @@ describe('collections-rest', () => { expect(result.totalDocs).toEqual(1); expect(result.docs).toEqual([post1]); }); + + describe('limit', () => { + beforeEach(async () => { + await mapAsync([...Array(50)], async (_, i) => createPost({ title: 'limit-test', number: i })); + }); + + it('should query a limited set of docs', async () => { + const { status, result } = await client.find({ + limit: 15, + query: { + title: { + equals: 'limit-test', + }, + }, + }); + + expect(status).toEqual(200); + expect(result.totalDocs).toEqual(15); + }); + + it('should query all docs when limit=0', async () => { + const { status, result } = await client.find({ + limit: 0, + query: { + title: { + equals: 'limit-test', + }, + }, + }); + + expect(status).toEqual(200); + expect(result.totalDocs).toEqual(50); + }); + }); }); }); }); diff --git a/test/helpers/rest.ts b/test/helpers/rest.ts index 59b0358568..41a2a53c50 100644 --- a/test/helpers/rest.ts +++ b/test/helpers/rest.ts @@ -154,7 +154,10 @@ export class RESTClient { }; const slug = args?.slug || this.defaultSlug; - const whereQuery = qs.stringify(args?.query ? { where: args.query } : {}, { + const whereQuery = qs.stringify({ + ...args, + ...(args?.query ? { where: args.query } : {}), + }, { addQueryPrefix: true, }); const fetchURL = `${this.serverURL}/api/${slug}${whereQuery}`; From e6bda625b44b3fb2da354535fd54ca0e02b96a8d Mon Sep 17 00:00:00 2001 From: Jarrod Flesch Date: Thu, 1 Dec 2022 12:17:47 -0500 Subject: [PATCH 8/8] chore: fixes test expectation --- test/collections-rest/int.spec.ts | 2 +- test/helpers/rest.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/collections-rest/int.spec.ts b/test/collections-rest/int.spec.ts index e029e56aa8..fffe890fbf 100644 --- a/test/collections-rest/int.spec.ts +++ b/test/collections-rest/int.spec.ts @@ -639,7 +639,7 @@ describe('collections-rest', () => { }); expect(status).toEqual(200); - expect(result.totalDocs).toEqual(15); + expect(result.docs).toHaveLength(15); }); it('should query all docs when limit=0', async () => { diff --git a/test/helpers/rest.ts b/test/helpers/rest.ts index 41a2a53c50..67d4c2e75b 100644 --- a/test/helpers/rest.ts +++ b/test/helpers/rest.ts @@ -155,8 +155,9 @@ export class RESTClient { const slug = args?.slug || this.defaultSlug; const whereQuery = qs.stringify({ - ...args, ...(args?.query ? { where: args.query } : {}), + limit: args?.limit, + page: args?.page, }, { addQueryPrefix: true, });