diff --git a/packages/agent/src/agent/routes/access/csv-related.ts b/packages/agent/src/agent/routes/access/csv-related.ts index ac38000863..aa6fd47761 100644 --- a/packages/agent/src/agent/routes/access/csv-related.ts +++ b/packages/agent/src/agent/routes/access/csv-related.ts @@ -33,7 +33,7 @@ export default class CsvRelatedRoute extends RelationRoute { const projection = QueryStringParser.parseProjection(this.foreignCollection, context); const scope = await this.services.permissions.getScope(this.foreignCollection, context); const caller = QueryStringParser.parseCaller(context); - const filter = ContextFilterFactory.buildPaginated(this.foreignCollection, context, scope); + const filter = ContextFilterFactory.build(this.foreignCollection, context, scope); const parentId = IdUtils.unpackId(this.collection.schema, context.params.parentId); const gen = CsvGenerator.generate( diff --git a/packages/agent/src/agent/routes/access/csv.ts b/packages/agent/src/agent/routes/access/csv.ts index 4cf6c24932..61e6933c2c 100644 --- a/packages/agent/src/agent/routes/access/csv.ts +++ b/packages/agent/src/agent/routes/access/csv.ts @@ -23,7 +23,7 @@ export default class CsvRoute extends CollectionRoute { const projection = QueryStringParser.parseProjection(this.collection, context); const scope = await this.services.permissions.getScope(this.collection, context); const caller = QueryStringParser.parseCaller(context); - const filter = ContextFilterFactory.buildPaginated(this.collection, context, scope); + const filter = ContextFilterFactory.build(this.collection, context, scope); const list = this.collection.list.bind(this.collection); const gen = CsvGenerator.generate(caller, projection, header, filter, this.collection, list); diff --git a/packages/agent/src/agent/utils/csv-generator.ts b/packages/agent/src/agent/utils/csv-generator.ts index 40e7660dbb..0eeb124c48 100644 --- a/packages/agent/src/agent/utils/csv-generator.ts +++ b/packages/agent/src/agent/utils/csv-generator.ts @@ -1,10 +1,10 @@ import { Caller, Collection, + Filter, Page, PaginatedFilter, Projection, - RecordData, RecordUtils, SortFactory, } from '@forestadmin/datasource-toolkit'; @@ -22,41 +22,28 @@ export default class CsvGenerator { caller: Caller, projection: Projection, header: string, - filter: PaginatedFilter, + baseFilter: Filter, collection: Collection, list: Collection['list'], ): AsyncGenerator { - yield writeToString([header.split(',')], { headers: true, includeEndRowDelimiter: true }); - - const limit = filter.page?.limit; - let skip = filter.page?.skip || 0; - - let areAllRecordsFetched = false; - const copiedFilter = { ...filter }; - - while (!areAllRecordsFetched) { - let currentPageSize = CHUNK_SIZE; - if (limit < skip) currentPageSize = skip - limit; + const sort = SortFactory.byPrimaryKeys(collection); + const filter = new PaginatedFilter({ ...baseFilter, sort }); + let skip = 0; - copiedFilter.page = new Page(skip, currentPageSize); - - if (!copiedFilter.sort || copiedFilter.sort.length === 0) { - copiedFilter.sort = SortFactory.byPrimaryKeys(collection); - } + yield writeToString([header.split(',')], { headers: true, includeEndRowDelimiter: true }); - // eslint-disable-next-line no-await-in-loop - const records = await list(caller, new PaginatedFilter(copiedFilter), projection); + while (true) { + const page = filter.override({ page: new Page(skip, CHUNK_SIZE) }); + const records = await list(caller, page, projection); // eslint-disable-line no-await-in-loop + yield writeToString( + records.map(record => projection.map(field => RecordUtils.getFieldValue(record, field))), + ); - yield CsvGenerator.convert(records, projection); + // If the page is not full, we are done. + if (records.length < CHUNK_SIZE) break; - areAllRecordsFetched = records.length < CHUNK_SIZE; - skip += currentPageSize; + // Update skip for next request. + skip += CHUNK_SIZE; } } - - private static convert(records: RecordData[], projection: Projection): Promise { - return writeToString( - records.map(record => projection.map(field => RecordUtils.getFieldValue(record, field))), - ); - } } diff --git a/packages/agent/test/agent/routes/access/csv-related.test.ts b/packages/agent/test/agent/routes/access/csv-related.test.ts index 91839701ba..227a21d3ba 100644 --- a/packages/agent/test/agent/routes/access/csv-related.test.ts +++ b/packages/agent/test/agent/routes/access/csv-related.test.ts @@ -106,16 +106,14 @@ describe('CsvRelatedRoute', () => { const listRelation = jest.spyOn(CollectionUtils, 'listRelation').mockResolvedValue([]); const csvGenerator = jest.spyOn(CsvGenerator, 'generate'); - const paginatedFilter = factories.filter.build(); - const buildPaginated = jest - .spyOn(ContextFilterFactory, 'buildPaginated') - .mockReturnValue(paginatedFilter); + const filter = factories.filter.build(); + const build = jest.spyOn(ContextFilterFactory, 'build').mockReturnValue(filter); // when await csvRoute.handleRelatedCsv(context); // then - expect(buildPaginated).toHaveBeenCalledWith(personsCollection, context, scopeCondition); + expect(build).toHaveBeenCalledWith(personsCollection, context, scopeCondition); expect(services.permissions.can).toHaveBeenCalledWith(context, 'browse:books'); expect(services.permissions.can).toHaveBeenCalledWith(context, 'export:books'); @@ -125,7 +123,7 @@ describe('CsvRelatedRoute', () => { { email: 'john.doe@domain.com', timezone: 'Europe/Paris' }, new Projection('id', 'name'), 'id,name', - paginatedFilter, + filter, personsCollection, expect.any(Function), ); diff --git a/packages/agent/test/agent/routes/access/csv.test.ts b/packages/agent/test/agent/routes/access/csv.test.ts index 60fc2420c7..2abe20e5ae 100644 --- a/packages/agent/test/agent/routes/access/csv.test.ts +++ b/packages/agent/test/agent/routes/access/csv.test.ts @@ -76,10 +76,8 @@ describe('CsvRoute', () => { booksCollection.list = jest.fn().mockReturnValue([]); const csvGenerator = jest.spyOn(CsvGenerator, 'generate'); - const paginatedFilter = factories.filter.build(); - const buildPaginated = jest - .spyOn(ContextFilterFactory, 'buildPaginated') - .mockReturnValue(paginatedFilter); + const filter = factories.filter.build(); + const build = jest.spyOn(ContextFilterFactory, 'build').mockReturnValue(filter); // when await csvRoute.handleCsv(context); @@ -87,14 +85,14 @@ describe('CsvRoute', () => { expect(services.permissions.can).toHaveBeenCalledWith(context, 'browse:books'); expect(services.permissions.can).toHaveBeenCalledWith(context, 'export:books'); - expect(buildPaginated).toHaveBeenCalledWith(booksCollection, context, scopeCondition); + expect(build).toHaveBeenCalledWith(booksCollection, context, scopeCondition); await readCsv(context.response.body as AsyncGenerator); expect(csvGenerator).toHaveBeenCalledWith( { email: 'john.doe@domain.com', timezone: 'Europe/Paris' }, ['id', 'name'], 'id,name', - paginatedFilter, + filter, booksCollection, expect.any(Function), ); diff --git a/packages/agent/test/agent/utils/csv-generator.test.ts b/packages/agent/test/agent/utils/csv-generator.test.ts index 27dd96ad8c..cd57d3d1a1 100644 --- a/packages/agent/test/agent/utils/csv-generator.test.ts +++ b/packages/agent/test/agent/utils/csv-generator.test.ts @@ -1,10 +1,4 @@ -import { - ConditionTreeLeaf, - Page, - PaginatedFilter, - Projection, - Sort, -} from '@forestadmin/datasource-toolkit'; +import { Filter, Page, Projection, Sort } from '@forestadmin/datasource-toolkit'; import * as factories from '../__factories__'; import CsvGenerator, { CHUNK_SIZE } from '../../../src/agent/utils/csv-generator'; @@ -19,20 +13,18 @@ describe('CsvGenerator', () => { { name: 'abd', id: 3 }, { name: 'abe', id: 4 }, ]; - const filter = new PaginatedFilter({ + const filter = new Filter({ conditionTree: factories.conditionTreeLeaf.build({ field: 'id', operator: 'Equal', value: '123e4567-e89b-12d3-a456-426614174000', }), - sort: new Sort(), - page: new Page(), }); const collection = factories.collection.build({ name: 'books', schema: factories.collectionSchema.build({ fields: { - id: factories.columnSchema.isPrimaryKey().build(), + id: factories.columnSchema.build({ columnType: 'Number', isPrimaryKey: true }), name: factories.columnSchema.build({ columnType: 'String' }), }, }), @@ -62,7 +54,7 @@ describe('CsvGenerator', () => { caller, factories.filter.build({ conditionTree: filter.conditionTree, - page: new Page(0, 1000), + page: new Page(0, CHUNK_SIZE), sort: new Sort({ ascending: true, field: 'id' }), }), projection, @@ -114,13 +106,20 @@ describe('CsvGenerator', () => { const setupWith2ChunkOfRecords = () => { const projection = new Projection('name', 'id'); - const records = Array.from({ length: CHUNK_SIZE }, (_, n: number) => [ - { name: 'ab', id: n }, - ]); - const filter = new PaginatedFilter({ - conditionTree: factories.conditionTreeLeaf.build(), + const records = Array.from({ length: CHUNK_SIZE * 2.5 }, (_, n: number) => ({ + id: n, + name: 'ab', + })); + + const filter = new Filter({}); + const collection = factories.collection.build({ + schema: { + fields: { + id: factories.columnSchema.build({ isPrimaryKey: true, columnType: 'Number' }), + name: factories.columnSchema.build({ columnType: 'String' }), + }, + }, }); - const collection = factories.collection.build(); return { records, filter, collection, projection }; }; @@ -130,9 +129,9 @@ describe('CsvGenerator', () => { collection.list = jest .fn() - .mockReturnValueOnce(records) - .mockReturnValueOnce(records) - .mockReturnValueOnce([]); + .mockReturnValueOnce(records.slice(0, CHUNK_SIZE)) + .mockReturnValueOnce(records.slice(CHUNK_SIZE, CHUNK_SIZE * 2)) + .mockReturnValueOnce(records.slice(CHUNK_SIZE * 2)); const caller = factories.caller.build(); const generator = CsvGenerator.generate( @@ -149,111 +148,31 @@ describe('CsvGenerator', () => { expect(collection.list).toHaveBeenNthCalledWith( 1, caller, - factories.filter.build({ - page: new Page(0, CHUNK_SIZE), - - conditionTree: expect.any(ConditionTreeLeaf), - sort: expect.any(Sort), - }), - expect.any(Projection), + { + page: { skip: 0, limit: CHUNK_SIZE }, + sort: [{ field: 'id', ascending: true }], + }, + ['name', 'id'], ); expect(collection.list).toHaveBeenNthCalledWith( 2, caller, - factories.filter.build({ - page: new Page(CHUNK_SIZE, CHUNK_SIZE), - - conditionTree: expect.any(ConditionTreeLeaf), - sort: expect.any(Sort), - }), - expect.any(Projection), + { + page: { skip: CHUNK_SIZE, limit: CHUNK_SIZE }, + sort: [{ field: 'id', ascending: true }], + }, + ['name', 'id'], ); expect(collection.list).toHaveBeenNthCalledWith( 3, caller, - factories.filter.build({ - page: new Page(CHUNK_SIZE * 2, CHUNK_SIZE), - - conditionTree: expect.any(ConditionTreeLeaf), - sort: expect.any(Sort), - }), - expect.any(Projection), + { + page: { skip: CHUNK_SIZE * 2, limit: CHUNK_SIZE }, + sort: [{ field: 'id', ascending: true }], + }, + ['name', 'id'], ); }); - - describe('when there is a page who ask a range of existing records', () => { - const setupWith2ChunkOfRecordsAndPageFilter = () => { - const projection = new Projection('name', 'id'); - - const records = Array.from({ length: CHUNK_SIZE }, (_, n: number) => [ - { name: 'ab', id: n }, - ]); - const filter = new PaginatedFilter({ - conditionTree: factories.conditionTreeLeaf.build(), - page: new Page(500, CHUNK_SIZE * 2), - }); - const collection = factories.collection.build(); - - return { records, filter, collection, projection }; - }; - - test('should export all the records asked by the page condition', async () => { - const { records, filter, collection, projection } = - setupWith2ChunkOfRecordsAndPageFilter(); - - collection.list = jest - .fn() - .mockReturnValueOnce(records) - .mockReturnValueOnce(records) - .mockReturnValueOnce(records.slice(0, 500)); - - const caller = factories.caller.build(); - const generator = CsvGenerator.generate( - caller, - projection, - 'name', - filter, - collection, - collection.list, - ); - await readCsv(generator); - - expect(collection.list).toHaveBeenCalledTimes(3); - - const startedSkipFromGivenPage = 500; - expect(collection.list).toHaveBeenNthCalledWith( - 1, - caller, - factories.filter.build({ - page: new Page(startedSkipFromGivenPage, CHUNK_SIZE), - conditionTree: expect.any(ConditionTreeLeaf), - sort: expect.any(Sort), - }), - expect.any(Projection), - ); - expect(collection.list).toHaveBeenNthCalledWith( - 2, - caller, - factories.filter.build({ - page: new Page(startedSkipFromGivenPage + CHUNK_SIZE, CHUNK_SIZE), - conditionTree: expect.any(ConditionTreeLeaf), - sort: expect.any(Sort), - }), - expect.any(Projection), - ); - expect(collection.list).toHaveBeenNthCalledWith( - 3, - caller, - factories.filter.build({ - page: new Page(startedSkipFromGivenPage + CHUNK_SIZE * 2, startedSkipFromGivenPage), - - conditionTree: expect.any(ConditionTreeLeaf), - sort: expect.any(Sort), - }), - expect.any(Projection), - ); - }); - }); }); }); });