From 9fc4174889d99b13c21f21c27309fa8f776d15d2 Mon Sep 17 00:00:00 2001 From: Clinton Lunn Date: Wed, 31 Dec 2025 00:46:14 -0700 Subject: [PATCH] feat: add limit/offset pagination to all array-returning graphql queries --- src/db/AreaTypes.ts | 2 + src/db/ChangeLogType.ts | 6 ++ src/db/TickTypes.ts | 9 ++ src/graphql/area/AreaQueries.ts | 8 +- src/graphql/history/HistoryQueries.ts | 23 +++-- .../organization/OrganizationQueries.ts | 8 +- src/graphql/resolvers.ts | 8 +- src/graphql/schema/Area.gql | 4 +- src/graphql/schema/History.gql | 6 ++ src/graphql/schema/Organization.gql | 2 +- src/graphql/schema/Tick.gql | 4 +- src/graphql/tick/TickQueries.ts | 18 +++- src/model/AreaDataSource.ts | 25 +++-- src/model/AreaHistoryDatasource.ts | 14 ++- src/model/ChangeLogDataSource.ts | 20 ++-- src/model/OrganizationHistoryDatasource.ts | 14 ++- src/model/TickDataSource.ts | 14 ++- src/model/__tests__/Pagination.test.ts | 96 +++++++++++++++++++ 18 files changed, 240 insertions(+), 41 deletions(-) create mode 100644 src/model/__tests__/Pagination.test.ts diff --git a/src/db/AreaTypes.ts b/src/db/AreaTypes.ts index a9884eb8..4a919919 100644 --- a/src/db/AreaTypes.ts +++ b/src/db/AreaTypes.ts @@ -262,4 +262,6 @@ export enum OperationType { export interface BulkAreasGQLQueryInput { ancestors: string[] + limit?: number + offset?: number } diff --git a/src/db/ChangeLogType.ts b/src/db/ChangeLogType.ts index 8a57b894..31678220 100644 --- a/src/db/ChangeLogType.ts +++ b/src/db/ChangeLogType.ts @@ -71,12 +71,18 @@ export interface GetHistoryInputFilterType { userUuid: string fromDate: Date toDate: Date + limit?: number + offset?: number } export interface GetAreaHistoryInputFilterType { areaId: string + limit?: number + offset?: number } export interface GetOrganizationHistoryInputFilterType { orgId: MUUID + limit?: number + offset?: number } diff --git a/src/db/TickTypes.ts b/src/db/TickTypes.ts index a08fdb1d..e1f33fa2 100644 --- a/src/db/TickTypes.ts +++ b/src/db/TickTypes.ts @@ -111,4 +111,13 @@ export interface TickEditFilterType { export interface TickUserSelectors { userId?: MUUID username?: string + limit?: number + offset?: number +} + +export interface TickByClimbSelectors { + climbId: string + userId?: string + limit?: number + offset?: number } diff --git a/src/graphql/area/AreaQueries.ts b/src/graphql/area/AreaQueries.ts index dbb69f7a..5472e69e 100644 --- a/src/graphql/area/AreaQueries.ts +++ b/src/graphql/area/AreaQueries.ts @@ -15,8 +15,12 @@ const AreaQueries = { bulkAreas: async (_: any, params, { dataSources }: GQLContext): Promise => { const { areas } = dataSources - const { ancestors } = params as BulkAreasGQLQueryInput - return await areas.bulkDownloadAreas(ancestors) + const { ancestors, limit, offset } = params as BulkAreasGQLQueryInput + const DEFAULT_LIMIT = 500 + const MAX_LIMIT = 2000 + const safeLimit = Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT) + const safeOffset = offset ?? 0 + return await areas.bulkDownloadAreas(ancestors, safeLimit, safeOffset) } } diff --git a/src/graphql/history/HistoryQueries.ts b/src/graphql/history/HistoryQueries.ts index c10fc4d8..53392dfa 100644 --- a/src/graphql/history/HistoryQueries.ts +++ b/src/graphql/history/HistoryQueries.ts @@ -7,29 +7,38 @@ import { } from '../../db/ChangeLogType.js' import { GQLContext } from '../../types.js' +const DEFAULT_LIMIT = 50 +const MAX_LIMIT = 500 + const HistoryQueries = { getChangeHistory: async (_, { filter }, { dataSources }: GQLContext): Promise => { const { history } = dataSources - const { uuidList }: GetHistoryInputFilterType = filter ?? {} + const { uuidList, limit, offset }: GetHistoryInputFilterType = filter ?? {} // Note: userUuid, fromDate, toDate filters don't currently work. // Note: though we pull uuidList, we don't use it either. // Convert array of uuid in string to UUID[] const muidList = uuidList?.map(entry => muid.from(entry)) ?? [] - return await history.getChangeSets(muidList) + const safeLimit = Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT) + const safeOffset = offset ?? 0 + return await history.getChangeSets(muidList, safeLimit, safeOffset) }, getAreaHistory: async (_, { filter }, { dataSources }: GQLContext): Promise => { const { history } = dataSources - const { areaId }: GetAreaHistoryInputFilterType = filter ?? {} - const id = muid.from(areaId) - return await history.getAreaChangeSets(id) + const { areaId, limit, offset }: GetAreaHistoryInputFilterType = filter ?? {} + const id = areaId != null ? muid.from(areaId) : undefined + const safeLimit = Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT) + const safeOffset = offset ?? 0 + return await history.getAreaChangeSets(id, safeLimit, safeOffset) }, getOrganizationHistory: async (_, { filter }, { dataSources }: GQLContext): Promise => { const { history } = dataSources - const { orgId }: GetOrganizationHistoryInputFilterType = filter ?? {} - return await history.getOrganizationChangeSets(orgId) + const { orgId, limit, offset }: GetOrganizationHistoryInputFilterType = filter ?? {} + const safeLimit = Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT) + const safeOffset = offset ?? 0 + return await history.getOrganizationChangeSets(orgId, safeLimit, safeOffset) } } diff --git a/src/graphql/organization/OrganizationQueries.ts b/src/graphql/organization/OrganizationQueries.ts index 63e59842..e8304b31 100644 --- a/src/graphql/organization/OrganizationQueries.ts +++ b/src/graphql/organization/OrganizationQueries.ts @@ -15,15 +15,17 @@ const OrganizationQueries = { organizations: async ( _, - { filter, sort, limit = 40 }: { filter?: OrganizationGQLFilter, sort?: Sort, limit?: number }, + { filter, sort, limit = 40, offset = 0 }: { filter?: OrganizationGQLFilter, sort?: Sort, limit?: number, offset?: number }, { dataSources }: GQLContext ) => { const { organizations }: { organizations: OrganizationDataSource } = dataSources + const MAX_LIMIT = 500 + const safeLimit = Math.min(limit, MAX_LIMIT) const filtered = await organizations.findOrganizationsByFilter(filter) if (sort != null) { - return await filtered.collation({ locale: 'en' }).sort(sort).limit(limit).toArray() + return await filtered.collation({ locale: 'en' }).sort(sort).skip(offset).limit(safeLimit).toArray() } else { - return await filtered.limit(limit).toArray() + return await filtered.skip(offset).limit(safeLimit).toArray() } } } diff --git a/src/graphql/resolvers.ts b/src/graphql/resolvers.ts index bca1890e..7f1512ae 100644 --- a/src/graphql/resolvers.ts +++ b/src/graphql/resolvers.ts @@ -75,12 +75,16 @@ const resolvers = { areas: async ( _, - { filter, sort }: { filter?: GQLFilter, sort?: Sort }, + { filter, sort, limit, offset }: { filter?: GQLFilter, sort?: Sort, limit?: number, offset?: number }, { dataSources }: GQLContext ) => { const { areas } = dataSources + const DEFAULT_LIMIT = 50 + const MAX_LIMIT = 500 + const safeLimit = Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT) + const safeOffset = offset ?? 0 const filtered = await areas.findAreasByFilter(filter) - return filtered.collation({ locale: 'en' }).sort(sort).toArray() + return filtered.collation({ locale: 'en' }).sort(sort).skip(safeOffset).limit(safeLimit).toArray() }, area: async (_: any, diff --git a/src/graphql/schema/Area.gql b/src/graphql/schema/Area.gql index 126ad571..e39741eb 100644 --- a/src/graphql/schema/Area.gql +++ b/src/graphql/schema/Area.gql @@ -1,12 +1,12 @@ type Query { area(uuid: ID): Area - areas(filter: Filter, sort: Sort): [Area] + areas(filter: Filter, sort: Sort, limit: Int, offset: Int): [Area] """ Bulk download an area and its children starting from ancestors paths (inclusive). To keep payload at a reasonable size ancestors must have at least 2 elements. """ - bulkAreas(ancestors: [String!]!): [Area] + bulkAreas(ancestors: [String!]!, limit: Int, offset: Int): [Area] stats: Stats cragsNear( diff --git a/src/graphql/schema/History.gql b/src/graphql/schema/History.gql index 1940f789..279f0c44 100644 --- a/src/graphql/schema/History.gql +++ b/src/graphql/schema/History.gql @@ -3,14 +3,20 @@ input AllHistoryFilter { userUuid: ID fromDate: Date toDate: Date + limit: Int + offset: Int } input AreaHistoryFilter { areaId: ID + limit: Int + offset: Int } input OrganizationHistoryFilter { orgId: MUUID + limit: Int + offset: Int } type UpdateDescription { diff --git a/src/graphql/schema/Organization.gql b/src/graphql/schema/Organization.gql index 8bd5f82f..821e5e2d 100644 --- a/src/graphql/schema/Organization.gql +++ b/src/graphql/schema/Organization.gql @@ -1,6 +1,6 @@ type Query { organization(muuid: MUUID): Organization - organizations(filter: OrgFilter, sort: OrgSort, limit: Int): [Organization] + organizations(filter: OrgFilter, sort: OrgSort, limit: Int, offset: Int): [Organization] } "A climbing area, wall or crag" diff --git a/src/graphql/schema/Tick.gql b/src/graphql/schema/Tick.gql index cf72b048..534e5b2b 100644 --- a/src/graphql/schema/Tick.gql +++ b/src/graphql/schema/Tick.gql @@ -2,12 +2,12 @@ type Query { """ Gets all of the users current ticks by their Auth-0 userId or username """ - userTicks(userId: MUUID, username: String): [TickType] + userTicks(userId: MUUID, username: String, limit: Int, offset: Int): [TickType] """ Gets all of the users current ticks for a specific climb by their Auth-0 userId and Open-Beta ClimbId """ - userTicksByClimbId(userId: String, climbId: String): [TickType] + userTicksByClimbId(userId: String, climbId: String, limit: Int, offset: Int): [TickType] } type Mutation { diff --git a/src/graphql/tick/TickQueries.ts b/src/graphql/tick/TickQueries.ts index fa18d9e4..f88ee23c 100644 --- a/src/graphql/tick/TickQueries.ts +++ b/src/graphql/tick/TickQueries.ts @@ -1,15 +1,23 @@ -import { TickType, TickUserSelectors } from '../../db/TickTypes' +import { TickByClimbSelectors, TickType, TickUserSelectors } from '../../db/TickTypes' import type TickDataSource from '../../model/TickDataSource' +const DEFAULT_LIMIT = 50 +const MAX_LIMIT = 500 + const TickQueries = { userTicks: async (_, input: TickUserSelectors, { dataSources }): Promise => { const { ticks }: { ticks: TickDataSource } = dataSources - return await ticks.ticksByUser(input) + const { limit, offset, ...selectors } = input + const safeLimit = Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT) + const safeOffset = offset ?? 0 + return await ticks.ticksByUser(selectors, safeLimit, safeOffset) }, - userTicksByClimbId: async (_, input, { dataSources }): Promise => { + userTicksByClimbId: async (_, input: TickByClimbSelectors, { dataSources }): Promise => { const { ticks }: { ticks: TickDataSource } = dataSources - const { climbId, userId } = input - return await ticks.ticksByUserIdAndClimb(climbId, userId) + const { climbId, userId, limit, offset } = input + const safeLimit = Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT) + const safeOffset = offset ?? 0 + return await ticks.ticksByUserIdAndClimb(climbId, userId, safeLimit, safeOffset) } } diff --git a/src/model/AreaDataSource.ts b/src/model/AreaDataSource.ts index 1c27d5ec..1e6865a4 100644 --- a/src/model/AreaDataSource.ts +++ b/src/model/AreaDataSource.ts @@ -317,7 +317,7 @@ export default class AreaDataSource extends MongoDataSource { return await this.areaModel.find(filter).lean() } - async bulkDownloadAreas (ancestors: string[]): Promise { + async bulkDownloadAreas (ancestors: string[], limit: number = 500, offset: number = 0): Promise { if (ancestors.length < 2) { throw new GraphQLError('Must provide at least 2 ancestors.', { extensions: { @@ -326,10 +326,23 @@ export default class AreaDataSource extends MongoDataSource { }) } const ancestorsCSV = ancestors.join(',') - const [leafAreas, nonLeafAreas] = await Promise.all([ - this.findDescendantsByPath(ancestorsCSV, true), - this.findDescendantsByPath(ancestorsCSV, false) - ]) - return nonLeafAreas.concat(leafAreas) + return await this.findDescendantsByPathPaginated(ancestorsCSV, limit, offset) + } + + /** + * Find all descendants (inclusive) starting from path with pagination + * @param path comma-separated _id's of area + * @param limit maximum number of results + * @param offset number of results to skip + * @returns array of areas + */ + async findDescendantsByPathPaginated (path: string, limit: number = 500, offset: number = 0): Promise { + const regex = new RegExp(`^${path}`) + const filter: any = { ancestors: regex, _deleting: { $eq: null } } + return await this.collection + .find(filter) + .skip(offset) + .limit(limit) + .toArray() } } diff --git a/src/model/AreaHistoryDatasource.ts b/src/model/AreaHistoryDatasource.ts index 59248584..094aa41a 100644 --- a/src/model/AreaHistoryDatasource.ts +++ b/src/model/AreaHistoryDatasource.ts @@ -6,7 +6,7 @@ import { getChangeLogModel } from '../db/index.js' export class AreaHistoryDataSource extends MongoDataSource { changelogModel = getChangeLogModel() - async getChangeSetsByUuid (areaUuid?: MUUID): Promise { + async getChangeSetsByUuid (areaUuid?: MUUID, limit: number = 50, offset: number = 0): Promise { let rs if (areaUuid == null) { // No area id specified: return all changes @@ -22,6 +22,12 @@ export class AreaHistoryDataSource extends MongoDataSource { $sort: { createdAt: -1 } + }, + { + $skip: offset + }, + { + $limit: limit } ]) return rs as AreaChangeLogType[] @@ -53,6 +59,12 @@ export class AreaHistoryDataSource extends MongoDataSource { $sort: { createdAt: -1 } + }, + { + $skip: offset + }, + { + $limit: limit } ]) return rs2 diff --git a/src/model/ChangeLogDataSource.ts b/src/model/ChangeLogDataSource.ts index 48306570..62067d1e 100644 --- a/src/model/ChangeLogDataSource.ts +++ b/src/model/ChangeLogDataSource.ts @@ -63,27 +63,35 @@ export default class ChangeLogDataSource extends MongoDataSource return this } - async getAreaChangeSets (areaUuid?: MUUID): Promise { - return await AreaHistoryDataSource.getInstance().getChangeSetsByUuid(areaUuid) + async getAreaChangeSets (areaUuid?: MUUID, limit: number = 50, offset: number = 0): Promise { + return await AreaHistoryDataSource.getInstance().getChangeSetsByUuid(areaUuid, limit, offset) } - async getOrganizationChangeSets (orgId?: MUUID): Promise { - return await OrganizationHistoryDataSource.getInstance().getChangeSetsByOrgId(orgId) + async getOrganizationChangeSets (orgId?: MUUID, limit: number = 50, offset: number = 0): Promise { + return await OrganizationHistoryDataSource.getInstance().getChangeSetsByOrgId(orgId, limit, offset) } /** * Return all changes. For now just handle Area type. * @param uuidList optional filter + * @param limit maximum number of results (default 50) + * @param offset number of results to skip (default 0) * @returns change sets */ - async getChangeSets (uuidList: MUUID[]): Promise> { + async getChangeSets (uuidList: MUUID[], limit: number = 50, offset: number = 0): Promise> { return await this.changeLogModel.aggregate([ { $sort: { createdAt: -1 } + }, + { + $skip: offset + }, + { + $limit: limit } - ]).limit(500) + ]) } async _testRemoveAll (): Promise { diff --git a/src/model/OrganizationHistoryDatasource.ts b/src/model/OrganizationHistoryDatasource.ts index 1c934faa..2b37749b 100644 --- a/src/model/OrganizationHistoryDatasource.ts +++ b/src/model/OrganizationHistoryDatasource.ts @@ -6,7 +6,7 @@ import { getChangeLogModel } from '../db/index.js' export class OrganizationHistoryDataSource extends MongoDataSource { changelogModel = getChangeLogModel() - async getChangeSetsByOrgId (orgId?: MUUID): Promise { + async getChangeSetsByOrgId (orgId?: MUUID, limit: number = 50, offset: number = 0): Promise { let rs if (orgId == null) { // No orgId specified: return all changes @@ -22,6 +22,12 @@ export class OrganizationHistoryDataSource extends MongoDataSource { /** * Retrieve ticks of a user given their details * @param userSelectors Attributes that can be used to identify the user + * @param limit maximum number of results (default 50) + * @param offset number of results to skip (default 0) * @returns */ - async ticksByUser (userSelectors: TickUserSelectors): Promise { + async ticksByUser (userSelectors: Omit, limit: number = 50, offset: number = 0): Promise { const { userId: requestedUserId, username } = userSelectors if (requestedUserId == null && username == null) { throw new Error('Username or userId must be supplied') @@ -174,18 +176,24 @@ export default class TickDataSource extends MongoDataSource { return await this.tickModel .find({ userId: userIdObject._id.toUUID().toString() }) .sort({ dateClimbed: -1 }) + .skip(offset) + .limit(limit) .lean() } /** * Get all ticks by climb uuid and optional user uuid - * @param userId Optional user uuid * @param climbId climb uuid + * @param userId Optional user uuid + * @param limit maximum number of results (default 50) + * @param offset number of results to skip (default 0) */ - async ticksByUserIdAndClimb (climbId: string, userId?: string): Promise { + async ticksByUserIdAndClimb (climbId: string, userId?: string, limit: number = 50, offset: number = 0): Promise { return await this.tickModel .find({ ...(userId != null && { userId }), climbId }) .sort({ dateClimbed: -1 }) + .skip(offset) + .limit(limit) .lean() } diff --git a/src/model/__tests__/Pagination.test.ts b/src/model/__tests__/Pagination.test.ts new file mode 100644 index 00000000..e84e9d47 --- /dev/null +++ b/src/model/__tests__/Pagination.test.ts @@ -0,0 +1,96 @@ +import muuid from 'uuid-mongodb' +import { getAreaModel, getChangeLogModel, createIndexes } from '../../db/index.js' +import ChangeLogDataSource from '../ChangeLogDataSource.js' +import { OperationType } from '../../db/AreaTypes.js' +import MutableAreaDataSource from '../MutableAreaDataSource.js' +import AreaDataSource from '../AreaDataSource.js' +import inMemoryDB from '../../utils/inMemoryDB.js' + +describe('Pagination tests', () => { + let changeLog: ChangeLogDataSource + let areas: AreaDataSource + const testUser = muuid.v4() + + beforeAll(async () => { + await inMemoryDB.connect() + try { + await getAreaModel().collection.drop() + await getChangeLogModel().collection.drop() + } catch (e) { + // Collections may not exist yet + } + await createIndexes() + changeLog = ChangeLogDataSource.getInstance() + areas = MutableAreaDataSource.getInstance() + }) + + afterAll(async () => { + await inMemoryDB.close() + }) + + describe('History pagination', () => { + beforeAll(async () => { + // Create multiple change records for testing pagination + for (let i = 0; i < 10; i++) { + const session = await getChangeLogModel().startSession() + await changeLog.create(session, testUser, OperationType.addCountry) + await session.endSession() + } + }) + + it('should return limited results with limit parameter', async () => { + const results = await changeLog.getChangeSets([], 5, 0) + expect(results.length).toBe(5) + }) + + it('should skip results with offset parameter', async () => { + const allResults = await changeLog.getChangeSets([], 10, 0) + const offsetResults = await changeLog.getChangeSets([], 10, 5) + + expect(offsetResults.length).toBe(5) + // Verify offset worked - first item of offset results should match 6th item of all results + expect(offsetResults[0]._id.toString()).toBe(allResults[5]._id.toString()) + }) + + it('should handle limit + offset together', async () => { + const results = await changeLog.getChangeSets([], 3, 2) + expect(results.length).toBe(3) + }) + + it('should use default limit when not specified', async () => { + const results = await changeLog.getChangeSets([]) + expect(results.length).toBeLessThanOrEqual(50) // Default limit is 50 + }) + }) + + describe('Area pagination', () => { + beforeAll(async () => { + // Create a country (using valid ISO code) and multiple child areas + const country = await MutableAreaDataSource.getInstance().addCountry('USA') + for (let i = 0; i < 15; i++) { + await MutableAreaDataSource.getInstance().addArea( + testUser, + `TestArea${i}`, + country.metadata.area_id + ) + } + }) + + it('should paginate bulkDownloadAreas with limit', async () => { + // First get all areas to know what we're working with + const allAreas = await areas.findDescendantsByPathPaginated('', 100, 0) + + // Now test pagination + const limitedResults = await areas.findDescendantsByPathPaginated('', 5, 0) + expect(limitedResults.length).toBe(5) + }) + + it('should paginate bulkDownloadAreas with offset', async () => { + const page1 = await areas.findDescendantsByPathPaginated('', 5, 0) + const page2 = await areas.findDescendantsByPathPaginated('', 5, 5) + + // Ensure different results + expect(page1[0]._id.toString()).not.toBe(page2[0]._id.toString()) + }) + }) +})