From 8d8a0cdde4b1909d60b5f4182246d7d38c758d85 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Tue, 14 Nov 2023 11:55:19 +0100 Subject: [PATCH 1/7] feat: handle cursor pagination --- .../agent/src/utils/context-filter-factory.ts | 3 ++- .../utils/forest-schema/generator-collection.ts | 2 +- packages/agent/src/utils/query-string.ts | 15 +++++++++++++++ .../datasource-toolkit/src/base-collection.ts | 4 +++- .../src/decorators/collection-decorator.ts | 6 +++++- packages/datasource-toolkit/src/index.ts | 1 + .../src/interfaces/collection.ts | 4 +++- .../src/interfaces/query/cursor.ts | 13 +++++++++++++ .../src/interfaces/query/filter/paginated.ts | 7 +++++++ .../test/__factories__/collection.ts | 3 ++- packages/forestadmin-client/src/schema/types.ts | 4 ++-- 11 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 packages/datasource-toolkit/src/interfaces/query/cursor.ts diff --git a/packages/agent/src/utils/context-filter-factory.ts b/packages/agent/src/utils/context-filter-factory.ts index 34e9b02fb7..98869f358d 100644 --- a/packages/agent/src/utils/context-filter-factory.ts +++ b/packages/agent/src/utils/context-filter-factory.ts @@ -18,7 +18,8 @@ export default class ContextFilterFactory { ): PaginatedFilter { return new PaginatedFilter({ sort: QueryStringParser.parseSort(collection, context), - page: QueryStringParser.parsePagination(context), + page: collection.paginationType === 'page' && QueryStringParser.parsePagination(context), + cursor: collection.paginationType === 'cursor' && QueryStringParser.parseCursor(context), ...ContextFilterFactory.build(collection, context, scope), ...partialFilter, }); diff --git a/packages/agent/src/utils/forest-schema/generator-collection.ts b/packages/agent/src/utils/forest-schema/generator-collection.ts index c621b85f73..983d02fbc2 100644 --- a/packages/agent/src/utils/forest-schema/generator-collection.ts +++ b/packages/agent/src/utils/forest-schema/generator-collection.ts @@ -25,7 +25,7 @@ export default class SchemaGeneratorCollection { isVirtual: false, name: collection.name, onlyForRelationships: false, - paginationType: 'page', + paginationType: collection.paginationType, segments: this.buildSegments(collection), }; } diff --git a/packages/agent/src/utils/query-string.ts b/packages/agent/src/utils/query-string.ts index 2d148f3549..cca5720800 100644 --- a/packages/agent/src/utils/query-string.ts +++ b/packages/agent/src/utils/query-string.ts @@ -4,6 +4,7 @@ import { Collection, ConditionTree, ConditionTreeValidator, + Cursor, Page, Projection, ProjectionFactory, @@ -194,4 +195,18 @@ export default class QueryStringParser { throw new ValidationError(`Invalid sort: ${sortString}`); } } + + static parseCursor(context: Context): Cursor { + const { query, body } = context.request as any; + + const queryItemsPerPage = ( + body?.data?.attributes?.all_records_subset_query?.['page[size]'] ?? + query['page[size]'] ?? + DEFAULT_ITEMS_PER_PAGE + ).toString(); + + const itemsPerPage = Number.parseInt(queryItemsPerPage, 10); + + return new Cursor(query.starting_before, query.starting_after, itemsPerPage); + } } diff --git a/packages/datasource-toolkit/src/base-collection.ts b/packages/datasource-toolkit/src/base-collection.ts index 4e708d7d7c..4b24adf9f2 100644 --- a/packages/datasource-toolkit/src/base-collection.ts +++ b/packages/datasource-toolkit/src/base-collection.ts @@ -3,7 +3,7 @@ import { Caller } from './interfaces/caller'; import { Chart } from './interfaces/chart'; import { Collection, DataSource } from './interfaces/collection'; import Aggregation, { AggregateResult } from './interfaces/query/aggregation'; -import PaginatedFilter from './interfaces/query/filter/paginated'; +import PaginatedFilter, { PaginationType } from './interfaces/query/filter/paginated'; import Filter from './interfaces/query/filter/unpaginated'; import Projection from './interfaces/query/projection'; import { RecordData } from './interfaces/record'; @@ -15,6 +15,8 @@ export default abstract class BaseCollection implements Collection { readonly schema: CollectionSchema; readonly nativeDriver: unknown; + paginationType: PaginationType = 'page'; + constructor(name: string, datasource: DataSource, nativeDriver: unknown = null) { this.dataSource = datasource; this.name = name; diff --git a/packages/datasource-toolkit/src/decorators/collection-decorator.ts b/packages/datasource-toolkit/src/decorators/collection-decorator.ts index 191ba64231..882bcbcd3e 100644 --- a/packages/datasource-toolkit/src/decorators/collection-decorator.ts +++ b/packages/datasource-toolkit/src/decorators/collection-decorator.ts @@ -3,7 +3,7 @@ import { Caller } from '../interfaces/caller'; import { Chart } from '../interfaces/chart'; import { Collection, DataSource } from '../interfaces/collection'; import Aggregation, { AggregateResult } from '../interfaces/query/aggregation'; -import PaginatedFilter from '../interfaces/query/filter/paginated'; +import PaginatedFilter, { PaginationType } from '../interfaces/query/filter/paginated'; import Filter from '../interfaces/query/filter/unpaginated'; import Projection from '../interfaces/query/projection'; import { CompositeId, RecordData } from '../interfaces/record'; @@ -33,6 +33,10 @@ export default class CollectionDecorator implements Collection { return this.childCollection.name; } + get paginationType(): PaginationType { + return this.childCollection.paginationType; + } + constructor(childCollection: Collection, dataSource: DataSource) { this.childCollection = childCollection; this.dataSource = dataSource; diff --git a/packages/datasource-toolkit/src/index.ts b/packages/datasource-toolkit/src/index.ts index 412c3d8f58..d53ecc0e93 100644 --- a/packages/datasource-toolkit/src/index.ts +++ b/packages/datasource-toolkit/src/index.ts @@ -16,6 +16,7 @@ export { default as ConditionTreeBranch } from './interfaces/query/condition-tre export { default as ConditionTreeLeaf } from './interfaces/query/condition-tree/nodes/leaf'; export { default as Filter } from './interfaces/query/filter/unpaginated'; export { default as Page } from './interfaces/query/page'; +export { default as Cursor } from './interfaces/query/cursor'; export { default as PaginatedFilter } from './interfaces/query/filter/paginated'; export { default as Projection } from './interfaces/query/projection'; export { default as Sort } from './interfaces/query/sort'; diff --git a/packages/datasource-toolkit/src/interfaces/collection.ts b/packages/datasource-toolkit/src/interfaces/collection.ts index c98439885f..c94cf5b4d8 100644 --- a/packages/datasource-toolkit/src/interfaces/collection.ts +++ b/packages/datasource-toolkit/src/interfaces/collection.ts @@ -2,7 +2,7 @@ import { ActionField, ActionResult } from './action'; import { Caller } from './caller'; import { Chart } from './chart'; import Aggregation, { AggregateResult } from './query/aggregation'; -import PaginatedFilter from './query/filter/paginated'; +import PaginatedFilter, { PaginationType } from './query/filter/paginated'; import Filter from './query/filter/unpaginated'; import Projection from './query/projection'; import { CompositeId, RecordData } from './record'; @@ -23,6 +23,8 @@ export interface Collection { get name(): string; get schema(): CollectionSchema; + paginationType: PaginationType; + execute( caller: Caller, name: string, diff --git a/packages/datasource-toolkit/src/interfaces/query/cursor.ts b/packages/datasource-toolkit/src/interfaces/query/cursor.ts new file mode 100644 index 0000000000..86fcacfe27 --- /dev/null +++ b/packages/datasource-toolkit/src/interfaces/query/cursor.ts @@ -0,0 +1,13 @@ +export type PlainCursor = { cursor: string; limit: number }; + +export default class Cursor { + before: string; + after: string; + limit: number; + + constructor(before?: string, after?: string, limit?: number) { + this.after = after; + this.before = before; + this.limit = limit; + } +} diff --git a/packages/datasource-toolkit/src/interfaces/query/filter/paginated.ts b/packages/datasource-toolkit/src/interfaces/query/filter/paginated.ts index 7f8caced19..4c2f6a2d95 100644 --- a/packages/datasource-toolkit/src/interfaces/query/filter/paginated.ts +++ b/packages/datasource-toolkit/src/interfaces/query/filter/paginated.ts @@ -1,25 +1,32 @@ import Filter, { FilterComponents, PlainFilter } from './unpaginated'; +import Cursor, { PlainCursor } from '../cursor'; import Page, { PlainPage } from '../page'; import Sort, { PlainSortClause } from '../sort'; +export type PaginationType = 'page' | 'cursor'; + export type PaginatedFilterComponents = FilterComponents & { sort?: Sort; page?: Page; + cursor?: Cursor; }; export type PlainPaginatedFilter = PlainFilter & { sort?: Array; page?: PlainPage; + cursor?: PlainCursor; }; export default class PaginatedFilter extends Filter { sort?: Sort; page?: Page; + cursor?: Cursor; constructor(parts: PaginatedFilterComponents) { super(parts); this.sort = parts.sort; this.page = parts.page; + this.cursor = parts.cursor; } override override(fields: PaginatedFilterComponents): PaginatedFilter { diff --git a/packages/datasource-toolkit/test/__factories__/collection.ts b/packages/datasource-toolkit/test/__factories__/collection.ts index 2ff33bdc2e..1f3c32eebe 100644 --- a/packages/datasource-toolkit/test/__factories__/collection.ts +++ b/packages/datasource-toolkit/test/__factories__/collection.ts @@ -19,7 +19,7 @@ export class CollectionFactory extends Factory { } } -export default CollectionFactory.define(() => ({ +export default CollectionFactory.define(() => ({ nativeDriver: null, dataSource: null, name: 'a collection', @@ -32,4 +32,5 @@ export default CollectionFactory.define(() => ({ update: jest.fn(), delete: jest.fn(), aggregate: jest.fn(), + paginationType: 'page', })); diff --git a/packages/forestadmin-client/src/schema/types.ts b/packages/forestadmin-client/src/schema/types.ts index b503cbfa63..dab4fb2efb 100644 --- a/packages/forestadmin-client/src/schema/types.ts +++ b/packages/forestadmin-client/src/schema/types.ts @@ -1,4 +1,4 @@ -import type { PrimitiveTypes } from '@forestadmin/datasource-toolkit'; +import type { PaginationType, PrimitiveTypes } from '@forestadmin/datasource-toolkit'; export type ForestSchema = { collections: ForestServerCollection[]; @@ -26,7 +26,7 @@ export type ForestServerCollection = { isSearchable: boolean; isVirtual: false; onlyForRelationships: boolean; - paginationType: 'page'; + paginationType: PaginationType; actions: Array; fields: Array; segments: Array; From 73f382bc1c66bfb478c5928b7043ed72d8a532cd Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Tue, 14 Nov 2023 12:14:41 +0100 Subject: [PATCH 2/7] fix: tests --- packages/_example/src/forest/typings.ts | 16 ++++++++-------- .../agent/src/utils/context-filter-factory.ts | 8 ++++++-- .../test/__factories__/collection.ts | 5 +++-- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/packages/_example/src/forest/typings.ts b/packages/_example/src/forest/typings.ts index 8e67d1b310..d0f06d4448 100644 --- a/packages/_example/src/forest/typings.ts +++ b/packages/_example/src/forest/typings.ts @@ -342,25 +342,25 @@ export type Schema = { 'dvd:rentalPrice': number; 'dvd:storeId': number; 'dvd:numberOfRentals': number; - 'dvd:store:id': number; - 'dvd:store:name': string; - 'dvd:store:ownerId': number; - 'dvd:store:ownerFullName': string; - 'dvd:store:owner:id': number; - 'dvd:store:owner:firstName': string; - 'dvd:store:owner:lastName': string; - 'dvd:store:owner:fullName': string; 'rental:id': number; 'rental:startDate': string; 'rental:endDate': string; 'rental:customerId': number; 'rental:numberOfDays': number; + 'dvd:store:id': number; + 'dvd:store:name': string; + 'dvd:store:ownerId': number; + 'dvd:store:ownerFullName': string; 'rental:customer:id': number; 'rental:customer:name': string; 'rental:customer:firstName': string; 'rental:customer:createdAt': string; 'rental:customer:updatedAt': string; 'rental:customer:deletedAt': string; + 'dvd:store:owner:id': number; + 'dvd:store:owner:firstName': string; + 'dvd:store:owner:lastName': string; + 'dvd:store:owner:fullName': string; }; }; 'owner': { diff --git a/packages/agent/src/utils/context-filter-factory.ts b/packages/agent/src/utils/context-filter-factory.ts index 98869f358d..0da90af354 100644 --- a/packages/agent/src/utils/context-filter-factory.ts +++ b/packages/agent/src/utils/context-filter-factory.ts @@ -18,8 +18,12 @@ export default class ContextFilterFactory { ): PaginatedFilter { return new PaginatedFilter({ sort: QueryStringParser.parseSort(collection, context), - page: collection.paginationType === 'page' && QueryStringParser.parsePagination(context), - cursor: collection.paginationType === 'cursor' && QueryStringParser.parseCursor(context), + page: + collection.paginationType === 'page' + ? QueryStringParser.parsePagination(context) + : undefined, + cursor: + collection.paginationType === 'cursor' ? QueryStringParser.parseCursor(context) : undefined, ...ContextFilterFactory.build(collection, context, scope), ...partialFilter, }); diff --git a/packages/datasource-toolkit/test/__factories__/collection.ts b/packages/datasource-toolkit/test/__factories__/collection.ts index 1f3c32eebe..067a68908f 100644 --- a/packages/datasource-toolkit/test/__factories__/collection.ts +++ b/packages/datasource-toolkit/test/__factories__/collection.ts @@ -2,6 +2,7 @@ import { Factory } from 'fishery'; import collectionSchemaFactory from './schema/collection-schema'; +import { PaginationType } from '../../src'; import { ActionField } from '../../src/interfaces/action'; import { Collection } from '../../src/interfaces/collection'; import { ActionSchema } from '../../src/interfaces/schema'; @@ -19,7 +20,7 @@ export class CollectionFactory extends Factory { } } -export default CollectionFactory.define(() => ({ +export default CollectionFactory.define(() => ({ nativeDriver: null, dataSource: null, name: 'a collection', @@ -32,5 +33,5 @@ export default CollectionFactory.define(() => ({ update: jest.fn(), delete: jest.fn(), aggregate: jest.fn(), - paginationType: 'page', + paginationType: 'page' as PaginationType, })); From 8b1a467a219acb59fb6e2beba434eb4065716f4a Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Wed, 15 Nov 2023 09:57:51 +0100 Subject: [PATCH 3/7] test: add tests --- packages/agent/src/utils/query-string.ts | 12 ++++- .../test/utils/context-filter-factory.test.ts | 36 +++++++++++++ .../generator-collection.test.ts | 10 ++++ .../agent/test/utils/query-string.test.ts | 50 +++++++++++++++++++ .../decorators/collection-decorator.test.ts | 11 ++++ 5 files changed, 118 insertions(+), 1 deletion(-) diff --git a/packages/agent/src/utils/query-string.ts b/packages/agent/src/utils/query-string.ts index cca5720800..b93845a9ac 100644 --- a/packages/agent/src/utils/query-string.ts +++ b/packages/agent/src/utils/query-string.ts @@ -206,7 +206,17 @@ export default class QueryStringParser { ).toString(); const itemsPerPage = Number.parseInt(queryItemsPerPage, 10); + const before = query.starting_before; + const after = query.starting_after; - return new Cursor(query.starting_before, query.starting_after, itemsPerPage); + if (Number.isNaN(itemsPerPage) || itemsPerPage <= 0) + throw new ValidationError(`Invalid cursor pagination [limit: ${itemsPerPage}]`); + + if (!before && !after) + throw new ValidationError( + 'Invalid cursor pagination, you should have at least "starting_before" or "starting_after" cursor set.', + ); + + return new Cursor(before, after, itemsPerPage); } } diff --git a/packages/agent/test/utils/context-filter-factory.test.ts b/packages/agent/test/utils/context-filter-factory.test.ts index 6885ef524d..3b9b3e6fd5 100644 --- a/packages/agent/test/utils/context-filter-factory.test.ts +++ b/packages/agent/test/utils/context-filter-factory.test.ts @@ -1,6 +1,7 @@ import { ConditionTreeBranch, ConditionTreeLeaf, + Cursor, Page, PaginatedFilter, Sort, @@ -91,4 +92,39 @@ describe('FilterFactory', () => { ); }); }); + + describe('with pagination cursor', () => { + const setupContextWithAllFeatures = () => { + const collection = factories.collection.build({ + schema: factories.collectionSchema.build({ + fields: { + id: factories.columnSchema.uuidPrimaryKey().build(), + }, + }), + paginationType: 'cursor', + }); + + const context = createMockContext({ + customProperties: { + query: { + 'page[size]': 10, + starting_after: 1, + }, + }, + }); + + const scope = factories.conditionTreeLeaf.build(); + + return { context, collection, scope }; + }; + + test('should build a paginated filter from a given context', () => { + const { context, collection, scope } = setupContextWithAllFeatures(); + + const filter = ContextFilterFactory.buildPaginated(collection, context, scope); + + expect(filter.cursor).toEqual(new Cursor(undefined, '1', 10)); + expect(filter.page).toBeUndefined(); + }); + }); }); diff --git a/packages/agent/test/utils/forest-schema/generator-collection.test.ts b/packages/agent/test/utils/forest-schema/generator-collection.test.ts index 3e65bfbb65..cf93c64061 100644 --- a/packages/agent/test/utils/forest-schema/generator-collection.test.ts +++ b/packages/agent/test/utils/forest-schema/generator-collection.test.ts @@ -37,6 +37,10 @@ describe('SchemaGeneratorCollection', () => { }), getForm: jest.fn().mockReturnValue(Promise.resolve(null)), }), + factories.collection.build({ + name: 'author', + paginationType: 'cursor', + }), ]); test('books should not be readonly and skip foreign keys', async () => { @@ -86,4 +90,10 @@ describe('SchemaGeneratorCollection', () => { relationship: 'HasOne', }); }); + + test('author should have pagination type cursor', async () => { + const schema = await SchemaGeneratorCollection.buildSchema(dataSource.getCollection('author')); + + expect(schema.paginationType).toEqual('cursor'); + }); }); diff --git a/packages/agent/test/utils/query-string.test.ts b/packages/agent/test/utils/query-string.test.ts index bd4159e6ad..fe14fbd44a 100644 --- a/packages/agent/test/utils/query-string.test.ts +++ b/packages/agent/test/utils/query-string.test.ts @@ -488,6 +488,56 @@ describe('QueryStringParser', () => { }); }); + describe('parseCursor', () => { + test('should return the pagination parameters', () => { + const context = createMockContext({ + customProperties: { query: { 'page[size]': 10, starting_after: 3 } }, + }); + + const cursor = QueryStringParser.parseCursor(context); + + expect(cursor.limit).toEqual(10); + expect(cursor.after).toEqual('3'); + }); + + describe('when context does not provide the limit parameters', () => { + test('should return the default limit 15', () => { + const context = createMockContext({ + customProperties: { query: { starting_before: 2 } }, + }); + + const cursor = QueryStringParser.parseCursor(context); + + expect(cursor.limit).toEqual(15); + expect(cursor.before).toEqual('2'); + }); + }); + + describe('when context provides invalid values', () => { + test('should return a ValidationError error on bad pageSize', () => { + const context = createMockContext({ + customProperties: { query: { 'page[size]': -5, starting_after: 1 } }, + }); + + const fn = () => QueryStringParser.parseCursor(context); + + expect(fn).toThrow('Invalid cursor pagination [limit: -5]'); + }); + + test('should return a ValidationError error on no cursor present', () => { + const context = createMockContext({ + customProperties: { query: { 'page[size]': 5 } }, + }); + + const fn = () => QueryStringParser.parseCursor(context); + + expect(fn).toThrow( + 'Invalid cursor pagination, you should have at least "starting_before" or "starting_after" cursor set.', + ); + }); + }); + }); + describe('parseSort', () => { test('should sort by pk ascending when not sort is given', () => { const context = createMockContext({ diff --git a/packages/datasource-toolkit/test/decorators/collection-decorator.test.ts b/packages/datasource-toolkit/test/decorators/collection-decorator.test.ts index ca2aff0d76..706c80f55c 100644 --- a/packages/datasource-toolkit/test/decorators/collection-decorator.test.ts +++ b/packages/datasource-toolkit/test/decorators/collection-decorator.test.ts @@ -269,6 +269,17 @@ describe('CollectionDecorator', () => { }); }); + describe('paginationType', () => { + it('calls the child paginationType', async () => { + const decoratedCollection = new DecoratedCollection( + factories.collection.build({ name: 'a name', paginationType: 'cursor' }), + factories.dataSource.build(), + ); + + expect(decoratedCollection.paginationType).toStrictEqual('cursor'); + }); + }); + describe('refineFilter', () => { it('should be the identity function', async () => { const decoratedCollection = new DecoratedCollection( From 2407bce2f9af6e0f54f65a68791cea60a9c4b7fa Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Wed, 15 Nov 2023 10:10:01 +0100 Subject: [PATCH 4/7] fix: lint --- packages/agent/src/utils/query-string.ts | 1 + packages/agent/test/utils/query-string.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/agent/src/utils/query-string.ts b/packages/agent/src/utils/query-string.ts index b93845a9ac..d6c17fe2f9 100644 --- a/packages/agent/src/utils/query-string.ts +++ b/packages/agent/src/utils/query-string.ts @@ -214,6 +214,7 @@ export default class QueryStringParser { if (!before && !after) throw new ValidationError( + // eslint-disable-next-line max-len 'Invalid cursor pagination, you should have at least "starting_before" or "starting_after" cursor set.', ); diff --git a/packages/agent/test/utils/query-string.test.ts b/packages/agent/test/utils/query-string.test.ts index fe14fbd44a..3ef9bc85ac 100644 --- a/packages/agent/test/utils/query-string.test.ts +++ b/packages/agent/test/utils/query-string.test.ts @@ -532,6 +532,7 @@ describe('QueryStringParser', () => { const fn = () => QueryStringParser.parseCursor(context); expect(fn).toThrow( + // eslint-disable-next-line max-len 'Invalid cursor pagination, you should have at least "starting_before" or "starting_after" cursor set.', ); }); From 9d07391e2756dd63442c9827d0f3f182b8be6d26 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Wed, 15 Nov 2023 11:45:56 +0100 Subject: [PATCH 5/7] test: add another tests --- packages/agent/src/utils/query-string.ts | 8 +---- .../test/utils/context-filter-factory.test.ts | 2 +- .../agent/test/utils/query-string.test.ts | 15 +------- .../src/interfaces/query/cursor.ts | 11 +++--- .../test/interfaces/cursor.test.ts | 35 +++++++++++++++++++ 5 files changed, 45 insertions(+), 26 deletions(-) create mode 100644 packages/datasource-toolkit/test/interfaces/cursor.test.ts diff --git a/packages/agent/src/utils/query-string.ts b/packages/agent/src/utils/query-string.ts index d6c17fe2f9..1bbf8aff47 100644 --- a/packages/agent/src/utils/query-string.ts +++ b/packages/agent/src/utils/query-string.ts @@ -212,12 +212,6 @@ export default class QueryStringParser { if (Number.isNaN(itemsPerPage) || itemsPerPage <= 0) throw new ValidationError(`Invalid cursor pagination [limit: ${itemsPerPage}]`); - if (!before && !after) - throw new ValidationError( - // eslint-disable-next-line max-len - 'Invalid cursor pagination, you should have at least "starting_before" or "starting_after" cursor set.', - ); - - return new Cursor(before, after, itemsPerPage); + return new Cursor(itemsPerPage, { before, after }); } } diff --git a/packages/agent/test/utils/context-filter-factory.test.ts b/packages/agent/test/utils/context-filter-factory.test.ts index 3b9b3e6fd5..6e00ec7af4 100644 --- a/packages/agent/test/utils/context-filter-factory.test.ts +++ b/packages/agent/test/utils/context-filter-factory.test.ts @@ -123,7 +123,7 @@ describe('FilterFactory', () => { const filter = ContextFilterFactory.buildPaginated(collection, context, scope); - expect(filter.cursor).toEqual(new Cursor(undefined, '1', 10)); + expect(filter.cursor).toEqual(new Cursor(10, { after: '1' })); expect(filter.page).toBeUndefined(); }); }); diff --git a/packages/agent/test/utils/query-string.test.ts b/packages/agent/test/utils/query-string.test.ts index 3ef9bc85ac..b4b9b00614 100644 --- a/packages/agent/test/utils/query-string.test.ts +++ b/packages/agent/test/utils/query-string.test.ts @@ -514,7 +514,7 @@ describe('QueryStringParser', () => { }); describe('when context provides invalid values', () => { - test('should return a ValidationError error on bad pageSize', () => { + test('should return a ValidationError', () => { const context = createMockContext({ customProperties: { query: { 'page[size]': -5, starting_after: 1 } }, }); @@ -523,19 +523,6 @@ describe('QueryStringParser', () => { expect(fn).toThrow('Invalid cursor pagination [limit: -5]'); }); - - test('should return a ValidationError error on no cursor present', () => { - const context = createMockContext({ - customProperties: { query: { 'page[size]': 5 } }, - }); - - const fn = () => QueryStringParser.parseCursor(context); - - expect(fn).toThrow( - // eslint-disable-next-line max-len - 'Invalid cursor pagination, you should have at least "starting_before" or "starting_after" cursor set.', - ); - }); }); }); diff --git a/packages/datasource-toolkit/src/interfaces/query/cursor.ts b/packages/datasource-toolkit/src/interfaces/query/cursor.ts index 86fcacfe27..ed26255dca 100644 --- a/packages/datasource-toolkit/src/interfaces/query/cursor.ts +++ b/packages/datasource-toolkit/src/interfaces/query/cursor.ts @@ -1,13 +1,16 @@ -export type PlainCursor = { cursor: string; limit: number }; +export type PlainCursor = { limit: number; cursor: { before?: string; after?: string } }; export default class Cursor { before: string; after: string; limit: number; - constructor(before?: string, after?: string, limit?: number) { - this.after = after; - this.before = before; + constructor(limit: number, cursor: { before?: string; after?: string }) { + this.after = cursor.after || null; + this.before = cursor.before || null; this.limit = limit; + + if (this.after && this.before) + throw new Error(`Cursor can't have before and after at same time.`); } } diff --git a/packages/datasource-toolkit/test/interfaces/cursor.test.ts b/packages/datasource-toolkit/test/interfaces/cursor.test.ts new file mode 100644 index 0000000000..114239b47d --- /dev/null +++ b/packages/datasource-toolkit/test/interfaces/cursor.test.ts @@ -0,0 +1,35 @@ +import Cursor from '../../src/interfaces/query/cursor'; + +describe('Cursor', () => { + describe('simple declaration', () => { + describe('whith all cursors defined', () => { + test('should throw an error', () => { + expect(() => new Cursor(10, { after: 'abc', before: 'abc' })).toThrow( + `Cursor can't have before and after at same time.`, + ); + }); + }); + + describe('whith no cursors defined', () => { + test('should default to null', () => { + const cursor = new Cursor(10, {}); + + expect(cursor).toEqual(new Cursor(10, { after: null, before: null })); + }); + }); + + describe('whith one cursor not defined', () => { + test('should default to null after', () => { + const cursor = new Cursor(10, { before: 'abc' }); + + expect(cursor).toEqual(new Cursor(10, { after: null, before: 'abc' })); + }); + + test('should default to null before', () => { + const cursor = new Cursor(10, { after: 'abc' }); + + expect(cursor).toEqual(new Cursor(10, { after: 'abc', before: null })); + }); + }); + }); +}); From 11d666f46fbfdea81aaf3b0bc331732a2cd4d23c Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Wed, 15 Nov 2023 15:26:43 +0100 Subject: [PATCH 6/7] chore: update cursor usage --- packages/agent/src/utils/query-string.ts | 6 ++-- .../test/utils/context-filter-factory.test.ts | 16 +++++++-- .../agent/test/utils/query-string.test.ts | 6 ++-- .../src/interfaces/query/cursor.ts | 13 +++---- .../test/interfaces/cursor.test.ts | 34 +++++++++---------- 5 files changed, 42 insertions(+), 33 deletions(-) diff --git a/packages/agent/src/utils/query-string.ts b/packages/agent/src/utils/query-string.ts index 1bbf8aff47..28d6b0cfd0 100644 --- a/packages/agent/src/utils/query-string.ts +++ b/packages/agent/src/utils/query-string.ts @@ -206,12 +206,12 @@ export default class QueryStringParser { ).toString(); const itemsPerPage = Number.parseInt(queryItemsPerPage, 10); - const before = query.starting_before; - const after = query.starting_after; + const cursor = query.starting_before || query.starting_after; + const backward = !!query.starting_before; if (Number.isNaN(itemsPerPage) || itemsPerPage <= 0) throw new ValidationError(`Invalid cursor pagination [limit: ${itemsPerPage}]`); - return new Cursor(itemsPerPage, { before, after }); + return new Cursor(itemsPerPage, cursor, backward); } } diff --git a/packages/agent/test/utils/context-filter-factory.test.ts b/packages/agent/test/utils/context-filter-factory.test.ts index 6e00ec7af4..8bf5ba0069 100644 --- a/packages/agent/test/utils/context-filter-factory.test.ts +++ b/packages/agent/test/utils/context-filter-factory.test.ts @@ -94,7 +94,7 @@ describe('FilterFactory', () => { }); describe('with pagination cursor', () => { - const setupContextWithAllFeatures = () => { + const setupContextWithAllFeatures = (backward = false) => { const collection = factories.collection.build({ schema: factories.collectionSchema.build({ fields: { @@ -108,7 +108,8 @@ describe('FilterFactory', () => { customProperties: { query: { 'page[size]': 10, - starting_after: 1, + starting_after: backward ? null : 1, + starting_before: backward ? 1 : null, }, }, }); @@ -123,7 +124,16 @@ describe('FilterFactory', () => { const filter = ContextFilterFactory.buildPaginated(collection, context, scope); - expect(filter.cursor).toEqual(new Cursor(10, { after: '1' })); + expect(filter.cursor).toEqual(new Cursor(10, '1')); + expect(filter.page).toBeUndefined(); + }); + + test('should build a paginated filter from a given backward context', () => { + const { context, collection, scope } = setupContextWithAllFeatures(true); + + const filter = ContextFilterFactory.buildPaginated(collection, context, scope); + + expect(filter.cursor).toEqual(new Cursor(10, '1', true)); expect(filter.page).toBeUndefined(); }); }); diff --git a/packages/agent/test/utils/query-string.test.ts b/packages/agent/test/utils/query-string.test.ts index b4b9b00614..ea2522fcd8 100644 --- a/packages/agent/test/utils/query-string.test.ts +++ b/packages/agent/test/utils/query-string.test.ts @@ -497,7 +497,8 @@ describe('QueryStringParser', () => { const cursor = QueryStringParser.parseCursor(context); expect(cursor.limit).toEqual(10); - expect(cursor.after).toEqual('3'); + expect(cursor.cursor).toEqual('3'); + expect(cursor.backward).toEqual(false); }); describe('when context does not provide the limit parameters', () => { @@ -509,7 +510,8 @@ describe('QueryStringParser', () => { const cursor = QueryStringParser.parseCursor(context); expect(cursor.limit).toEqual(15); - expect(cursor.before).toEqual('2'); + expect(cursor.cursor).toEqual('2'); + expect(cursor.backward).toEqual(true); }); }); diff --git a/packages/datasource-toolkit/src/interfaces/query/cursor.ts b/packages/datasource-toolkit/src/interfaces/query/cursor.ts index ed26255dca..7961f130e4 100644 --- a/packages/datasource-toolkit/src/interfaces/query/cursor.ts +++ b/packages/datasource-toolkit/src/interfaces/query/cursor.ts @@ -1,16 +1,13 @@ export type PlainCursor = { limit: number; cursor: { before?: string; after?: string } }; export default class Cursor { - before: string; - after: string; + cursor: string; + backward: boolean; limit: number; - constructor(limit: number, cursor: { before?: string; after?: string }) { - this.after = cursor.after || null; - this.before = cursor.before || null; + constructor(limit: number, cursor?: string, backward?: boolean) { this.limit = limit; - - if (this.after && this.before) - throw new Error(`Cursor can't have before and after at same time.`); + this.cursor = cursor || null; + this.backward = backward ?? false; } } diff --git a/packages/datasource-toolkit/test/interfaces/cursor.test.ts b/packages/datasource-toolkit/test/interfaces/cursor.test.ts index 114239b47d..b56600c4bd 100644 --- a/packages/datasource-toolkit/test/interfaces/cursor.test.ts +++ b/packages/datasource-toolkit/test/interfaces/cursor.test.ts @@ -2,33 +2,33 @@ import Cursor from '../../src/interfaces/query/cursor'; describe('Cursor', () => { describe('simple declaration', () => { - describe('whith all cursors defined', () => { - test('should throw an error', () => { - expect(() => new Cursor(10, { after: 'abc', before: 'abc' })).toThrow( - `Cursor can't have before and after at same time.`, - ); - }); - }); - describe('whith no cursors defined', () => { - test('should default to null', () => { - const cursor = new Cursor(10, {}); + test('should default cursor to null', () => { + const cursor = new Cursor(10); - expect(cursor).toEqual(new Cursor(10, { after: null, before: null })); + expect(cursor.limit).toEqual(10); + expect(cursor.cursor).toEqual(null); + expect(cursor.backward).toEqual(false); }); }); - describe('whith one cursor not defined', () => { - test('should default to null after', () => { - const cursor = new Cursor(10, { before: 'abc' }); + describe('when it is backward', () => { + test('should define right cursor', () => { + const cursor = new Cursor(10, 'abc', true); - expect(cursor).toEqual(new Cursor(10, { after: null, before: 'abc' })); + expect(cursor.limit).toEqual(10); + expect(cursor.cursor).toEqual('abc'); + expect(cursor.backward).toEqual(true); }); + }); + describe('when it is forward', () => { test('should default to null before', () => { - const cursor = new Cursor(10, { after: 'abc' }); + const cursor = new Cursor(10, 'abc'); - expect(cursor).toEqual(new Cursor(10, { after: 'abc', before: null })); + expect(cursor.limit).toEqual(10); + expect(cursor.cursor).toEqual('abc'); + expect(cursor.backward).toEqual(false); }); }); }); From 6c6d63545fde35dd681a49c3019d14f09c328a04 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Mon, 20 Nov 2023 18:35:43 +0100 Subject: [PATCH 7/7] fix: cursor --- packages/agent/src/utils/query-string.ts | 4 ++-- packages/agent/test/utils/context-filter-factory.test.ts | 2 +- packages/agent/test/utils/query-string.test.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/agent/src/utils/query-string.ts b/packages/agent/src/utils/query-string.ts index 28d6b0cfd0..6720ac1b82 100644 --- a/packages/agent/src/utils/query-string.ts +++ b/packages/agent/src/utils/query-string.ts @@ -206,8 +206,8 @@ export default class QueryStringParser { ).toString(); const itemsPerPage = Number.parseInt(queryItemsPerPage, 10); - const cursor = query.starting_before || query.starting_after; - const backward = !!query.starting_before; + const cursor = query.ending_before || query.starting_after; + const backward = !!query.ending_before; if (Number.isNaN(itemsPerPage) || itemsPerPage <= 0) throw new ValidationError(`Invalid cursor pagination [limit: ${itemsPerPage}]`); diff --git a/packages/agent/test/utils/context-filter-factory.test.ts b/packages/agent/test/utils/context-filter-factory.test.ts index 8bf5ba0069..ef4cae6da7 100644 --- a/packages/agent/test/utils/context-filter-factory.test.ts +++ b/packages/agent/test/utils/context-filter-factory.test.ts @@ -109,7 +109,7 @@ describe('FilterFactory', () => { query: { 'page[size]': 10, starting_after: backward ? null : 1, - starting_before: backward ? 1 : null, + ending_before: backward ? 1 : null, }, }, }); diff --git a/packages/agent/test/utils/query-string.test.ts b/packages/agent/test/utils/query-string.test.ts index ea2522fcd8..f2d6b6a5b9 100644 --- a/packages/agent/test/utils/query-string.test.ts +++ b/packages/agent/test/utils/query-string.test.ts @@ -504,7 +504,7 @@ describe('QueryStringParser', () => { describe('when context does not provide the limit parameters', () => { test('should return the default limit 15', () => { const context = createMockContext({ - customProperties: { query: { starting_before: 2 } }, + customProperties: { query: { ending_before: 2 } }, }); const cursor = QueryStringParser.parseCursor(context);