From 4f806c7d9add60acde6aa47a9e239c6285b77a1c Mon Sep 17 00:00:00 2001 From: Andrew Brazzatti Date: Sun, 23 Nov 2025 22:13:23 +0000 Subject: [PATCH 1/6] Fix: Implement pagination for article file list retrieval in FigshareService --- typescript/api/services/FigshareService.ts | 52 ++++--- .../unit/services/FigshareService.test.ts | 135 ++++++++++++++++++ 2 files changed, 168 insertions(+), 19 deletions(-) create mode 100644 typescript/api/test/unit/services/FigshareService.test.ts diff --git a/typescript/api/services/FigshareService.ts b/typescript/api/services/FigshareService.ts index 39c25f0526..ac356e04be 100644 --- a/typescript/api/services/FigshareService.ts +++ b/typescript/api/services/FigshareService.ts @@ -695,15 +695,38 @@ export module Services { } private async getArticleFileList(articleId:string, logEnabled:boolean = true) { - let articleFileListConfig = this.getAxiosConfig('get', `/account/articles/${articleId}/files`, null); - if(logEnabled) { - sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - getArticleFileList - ${articleFileListConfig.method} - ${articleFileListConfig.url}`); + const defaultPageSize = 20; + const pageSizeConfig = _.get(sails.config, 'figshareAPI.mapping.upload.fileListPageSize', defaultPageSize); + const pageSize = _.isNumber(pageSizeConfig) && pageSizeConfig > 0 ? pageSizeConfig : defaultPageSize; + + let page = 1; + let articleFileList = []; + let hasMorePages = true; + + while(hasMorePages) { + let articleFileListConfig = this.getAxiosConfig('get', `/account/articles/${articleId}/files?page_size=${pageSize}&page=${page}`, null); + if(logEnabled) { + sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - getArticleFileList - page ${page} - ${articleFileListConfig.method} - ${articleFileListConfig.url}`); + } + let responseArticleList = await axios(articleFileListConfig); + if(logEnabled) { + sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - getArticleFileList - page ${page} status: ${responseArticleList.status} statusText: ${responseArticleList.statusText}`); + } + + let currentPage = responseArticleList.data; + if(_.isArray(currentPage) && currentPage.length > 0) { + articleFileList = articleFileList.concat(currentPage); + hasMorePages = currentPage.length === pageSize; + } else { + hasMorePages = false; + } + page++; } - let responseArticleList = await axios(articleFileListConfig); + if(logEnabled) { - sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - getArticleFileList - status: ${responseArticleList.status} statusText: ${responseArticleList.statusText}`); + sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - getArticleFileList - total files fetched ${articleFileList.length}`); } - let articleFileList = responseArticleList.data; + return articleFileList; } @@ -1401,11 +1424,7 @@ export module Services { //Try to upload files to article let that = this; - let articleFileListConfig = this.getAxiosConfig('get', `/account/articles/${articleId}/files`, null); - sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - checkUploadFilesPending - ${articleFileListConfig.method} - ${articleFileListConfig.url}`); - let responseArticleList = await axios(articleFileListConfig); - sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - checkUploadFilesPending - status: ${responseArticleList.status} statusText: ${responseArticleList.statusText}`); - let articleFileList = responseArticleList.data; + let articleFileList = await this.getArticleFileList(articleId); let filePath = sails.config.figshareAPI.attachmentsFigshareTempDir; sails.log[this.createUpdateFigshareArticleLogLevel]('FigService - checkUploadFilesPending - attachmentsFigshareTempDir '+filePath); @@ -1440,9 +1459,8 @@ export module Services { let attachId = attachmentFile.fileId; let fileName = attachmentFile.name; let fileSize = attachmentFile.size; - //check if the file has been uploaded already or not to the figshare article - responseArticleList = await axios(articleFileListConfig); - articleFileList = responseArticleList.data; + //check if the file has been uploaded already or not to the figshare article + articleFileList = await this.getArticleFileList(articleId, false); sails.log[this.createUpdateFigshareArticleLogLevel]('FigService - checkUploadFilesPending - article file list: '+JSON.stringify(articleFileList)); let filePendingToBeUploaded = _.find(articleFileList, ['name', fileName]); let fileFullPath = filePath + '/' +fileName; @@ -1899,11 +1917,7 @@ export module Services { if(articleId > 0) { - let articleFileListConfig = this.getAxiosConfig('get', `/account/articles/${articleId}/files`, null); - sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - deleteFilesAndUpdateDataLocationEntries - ${articleFileListConfig.method} - ${articleFileListConfig.url}`); - let responseArticleList = await axios(articleFileListConfig); - sails.log[this.createUpdateFigshareArticleLogLevel](`FigService - deleteFilesAndUpdateDataLocationEntries - status: ${responseArticleList.status} statusText: ${responseArticleList.statusText}`); - let articleFileList = responseArticleList.data; + let articleFileList = await this.getArticleFileList(articleId); let dataLocations = _.get(record,this.dataLocationsPathInRecord); let urlList = []; diff --git a/typescript/api/test/unit/services/FigshareService.test.ts b/typescript/api/test/unit/services/FigshareService.test.ts new file mode 100644 index 0000000000..df9ac02d47 --- /dev/null +++ b/typescript/api/test/unit/services/FigshareService.test.ts @@ -0,0 +1,135 @@ +const { describe, it, beforeEach } = require('mocha'); +const { expect } = require('chai'); + +// Minimal sails stub required by FigshareService +(global as any).sails = { + config: { + figshareAPIEnv: { + overrideArtifacts: { + mapping: { + artifacts: {} + } + } + }, + figshareAPI: { + APIToken: 'test-token', + baseURL: 'https://api.figshare.test', + frontEndURL: 'https://figshare.example', + mapping: { + upload: { + fileListPageSize: 2, + override: {} + }, + figshareOnlyPublishSelectedAttachmentFiles: false, + recordAllFilesUploaded: false, + targetState: {}, + response: { + article: {} + } + }, + diskSpaceThreshold: 0 + }, + figshareReDBoxFORMapping: { + FORMapping: [] + }, + record: { + createUpdateFigshareArticleLogLevel: 'verbose' + }, + queue: { + serviceName: '' + } + }, + log: { + verbose: () => {}, + info: () => {}, + error: () => {}, + warn: () => {}, + debug: () => {} + }, + services: {}, + on: () => {} +}; + +// axios is required at module load in FigshareService, so we must set the stub before importing it +const axiosCalls: any[] = []; +let axiosResponses: any[] = []; +const axiosStub = (config) => { + axiosCalls.push(config); + if (!axiosResponses.length) { + return Promise.reject(new Error('No axios mock response available')); + } + return Promise.resolve(axiosResponses.shift()); +}; +(require as any).cache[require.resolve('axios')] = { exports: axiosStub as any }; + +const { Services } = require('../../../services/FigshareService'); +const FigshareService = Services.FigshareService; + +describe('FigshareService - getArticleFileList pagination', () => { + let service; + + beforeEach(() => { + axiosCalls.length = 0; + axiosResponses = []; + service = new FigshareService(); + // Set required properties that are private in TS but accessible at runtime + (service as any).baseURL = 'https://api.figshare.test'; + (service as any).APIToken = 'test-token'; + (global as any).sails.config.figshareAPI.mapping.upload.fileListPageSize = 2; + }); + + it('aggregates multiple pages until a partial page is returned', async () => { + axiosResponses = [ + { status: 200, statusText: 'OK', data: [{ id: 1 }, { id: 2 }] }, + { status: 200, statusText: 'OK', data: [{ id: 3 }] } + ]; + + const files = await (service as any).getArticleFileList('123'); + + expect(files.map((f) => f.id)).to.deep.equal([1, 2, 3]); + expect(axiosCalls).to.have.length(2); + expect(axiosCalls[0].url).to.contain('/account/articles/123/files?page_size=2&page=1'); + expect(axiosCalls[1].url).to.contain('/account/articles/123/files?page_size=2&page=2'); + }); + + it('falls back to the default page size when the config value is invalid', async () => { + (global as any).sails.config.figshareAPI.mapping.upload.fileListPageSize = 'invalid'; + const expectedDefault = 100; + + axiosResponses = [ + { status: 200, statusText: 'OK', data: [{ id: 'a' }] } + ]; + + const files = await (service as any).getArticleFileList('abc'); + + expect(files.map((f) => f.id)).to.deep.equal(['a']); + expect(axiosCalls).to.have.length(1); + expect(axiosCalls[0].url).to.contain(`/account/articles/abc/files?page_size=${expectedDefault}&page=1`); + }); +}); + +describe('FigshareService - isFileUploadInProgress', () => { + let service; + + beforeEach(() => { + service = new FigshareService(); + (service as any).baseURL = 'https://api.figshare.test'; + (service as any).APIToken = 'test-token'; + }); + + it('returns true when any file has status "created"', async () => { + const mockFileList = [{ id: 1, status: 'available' }, { id: 2, status: 'created' }]; + + const inProgress = await (service as any).isFileUploadInProgress('article-1', mockFileList); + + expect(inProgress).to.equal(true); + }); + + it('returns false when no files are in progress', async () => { + const mockFileList = [{ id: 1, status: 'available' }]; + + const inProgress = await (service as any).isFileUploadInProgress('article-2', mockFileList); + + expect(inProgress).to.equal(false); + }); +}); From 08daeacae9f0c2e5c8c295610de55d04eeb5b8c7 Mon Sep 17 00:00:00 2001 From: Andrew Brazzatti Date: Sun, 23 Nov 2025 22:58:39 +0000 Subject: [PATCH 2/6] Fix: Update entrypoint in docker-compose files to use local nyc binary and set user to root for mocha service --- support/integration-testing/docker-compose.bruno.yml | 3 +-- support/integration-testing/docker-compose.mocha.yml | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/support/integration-testing/docker-compose.bruno.yml b/support/integration-testing/docker-compose.bruno.yml index 792ac308a1..a94ec1f41b 100755 --- a/support/integration-testing/docker-compose.bruno.yml +++ b/support/integration-testing/docker-compose.bruno.yml @@ -70,9 +70,8 @@ services: entrypoint: >- /bin/bash -c "cd /opt/redbox-portal && ./support/integration-testing/prepare-guest.sh /opt/redbox-portal && - npm install -g nyc && npm install && - nyc + ./node_modules/.bin/nyc --no-clean --report-dir coverage/bruno --reporter=lcov diff --git a/support/integration-testing/docker-compose.mocha.yml b/support/integration-testing/docker-compose.mocha.yml index 7b33a4d1e9..20d816a1c8 100755 --- a/support/integration-testing/docker-compose.mocha.yml +++ b/support/integration-testing/docker-compose.mocha.yml @@ -6,6 +6,7 @@ services: redboxportal: image: qcifengineering/redbox-portal:develop pull_policy: 'always' + user: "root" ports: - "1500:1500" # Debugging port @@ -46,9 +47,8 @@ services: entrypoint: >- /bin/bash -c "cd /opt/redbox-portal && ./support/integration-testing/prepare-guest.sh /opt/redbox-portal && - npm install -g nyc && npm install && - nyc + ./node_modules/.bin/nyc --no-clean --report-dir coverage/mocha --reporter=lcov From a6915c88a5b2c3bab8234f14071322f0e1489df4 Mon Sep 17 00:00:00 2001 From: Andrew Brazzatti Date: Mon, 24 Nov 2025 00:26:12 +0000 Subject: [PATCH 3/6] Fix: Update Docker images in integration testing configurations to use 'master' branch --- support/integration-testing/docker-compose.bruno.yml | 2 +- support/integration-testing/docker-compose.mocha.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/support/integration-testing/docker-compose.bruno.yml b/support/integration-testing/docker-compose.bruno.yml index a94ec1f41b..f9662eeab6 100755 --- a/support/integration-testing/docker-compose.bruno.yml +++ b/support/integration-testing/docker-compose.bruno.yml @@ -27,7 +27,7 @@ services: --output /opt/redbox-portal/.tmp/junit/backend-bruno/backend-bruno.xml --bail" redboxportal: - image: qcifengineering/redbox-portal:develop + image: qcifengineering/redbox-portal:master pull_policy: 'always' ports: - "1500:1500" diff --git a/support/integration-testing/docker-compose.mocha.yml b/support/integration-testing/docker-compose.mocha.yml index 20d816a1c8..5b6cd323ef 100755 --- a/support/integration-testing/docker-compose.mocha.yml +++ b/support/integration-testing/docker-compose.mocha.yml @@ -4,7 +4,7 @@ networks: services: redboxportal: - image: qcifengineering/redbox-portal:develop + image: qcifengineering/redbox-portal:master pull_policy: 'always' user: "root" ports: From 4f2da9e9405e5f10c0e4b56dae13ec800f9a7bcc Mon Sep 17 00:00:00 2001 From: Andrew Brazzatti Date: Mon, 24 Nov 2025 06:40:14 +0000 Subject: [PATCH 4/6] Fix: Improve pagination logic in FigshareService and update test cases for edge scenarios --- typescript/api/services/FigshareService.ts | 5 +-- .../unit/services/FigshareService.test.ts | 33 ++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/typescript/api/services/FigshareService.ts b/typescript/api/services/FigshareService.ts index ac356e04be..c4e24d13f1 100644 --- a/typescript/api/services/FigshareService.ts +++ b/typescript/api/services/FigshareService.ts @@ -715,8 +715,8 @@ export module Services { let currentPage = responseArticleList.data; if(_.isArray(currentPage) && currentPage.length > 0) { - articleFileList = articleFileList.concat(currentPage); - hasMorePages = currentPage.length === pageSize; + articleFileList.push(...currentPage); + hasMorePages = currentPage.length >= pageSize; } else { hasMorePages = false; } @@ -2261,3 +2261,4 @@ export module Services { } } module.exports = new Services.FigshareService().exports(); +module.exports.Services = Services; diff --git a/typescript/api/test/unit/services/FigshareService.test.ts b/typescript/api/test/unit/services/FigshareService.test.ts index df9ac02d47..1dcabb9068 100644 --- a/typescript/api/test/unit/services/FigshareService.test.ts +++ b/typescript/api/test/unit/services/FigshareService.test.ts @@ -1,5 +1,4 @@ -const { describe, it, beforeEach } = require('mocha'); -const { expect } = require('chai'); +const { describe, it, beforeEach, before } = require('mocha'); // Minimal sails stub required by FigshareService (global as any).sails = { @@ -13,8 +12,8 @@ const { expect } = require('chai'); }, figshareAPI: { APIToken: 'test-token', - baseURL: 'https://api.figshare.test', - frontEndURL: 'https://figshare.example', + baseURL: 'https://api.figshare.test.localhost', + frontEndURL: 'https://figshare.test.localhost', mapping: { upload: { fileListPageSize: 2, @@ -67,6 +66,12 @@ const FigshareService = Services.FigshareService; describe('FigshareService - getArticleFileList pagination', () => { let service; + let expect; + + before(async () => { + const chai = await import('chai'); + expect = chai.expect; + }); beforeEach(() => { axiosCalls.length = 0; @@ -94,7 +99,7 @@ describe('FigshareService - getArticleFileList pagination', () => { it('falls back to the default page size when the config value is invalid', async () => { (global as any).sails.config.figshareAPI.mapping.upload.fileListPageSize = 'invalid'; - const expectedDefault = 100; + const expectedDefault = 20; axiosResponses = [ { status: 200, statusText: 'OK', data: [{ id: 'a' }] } @@ -106,10 +111,28 @@ describe('FigshareService - getArticleFileList pagination', () => { expect(axiosCalls).to.have.length(1); expect(axiosCalls[0].url).to.contain(`/account/articles/abc/files?page_size=${expectedDefault}&page=1`); }); + + it('returns an empty list when no files are found', async () => { + axiosResponses = [ + { status: 200, statusText: 'OK', data: [] } + ]; + + const files = await (service as any).getArticleFileList('empty'); + + expect(files).to.deep.equal([]); + expect(axiosCalls).to.have.length(1); + expect(axiosCalls[0].url).to.contain('/account/articles/empty/files?page_size=2&page=1'); + }); }); describe('FigshareService - isFileUploadInProgress', () => { let service; + let expect; + + before(async () => { + const chai = await import('chai'); + expect = chai.expect; + }); beforeEach(() => { service = new FigshareService(); From 7c6b51bb537e16d92f6a88175ef32efb22697347 Mon Sep 17 00:00:00 2001 From: Andrew Brazzatti Date: Mon, 24 Nov 2025 07:16:29 +0000 Subject: [PATCH 5/6] Fix: Add TypeScript ignore comments for dynamic import of chai in FigshareService tests --- typescript/api/test/unit/services/FigshareService.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/typescript/api/test/unit/services/FigshareService.test.ts b/typescript/api/test/unit/services/FigshareService.test.ts index 1dcabb9068..6de0eca2eb 100644 --- a/typescript/api/test/unit/services/FigshareService.test.ts +++ b/typescript/api/test/unit/services/FigshareService.test.ts @@ -69,6 +69,7 @@ describe('FigshareService - getArticleFileList pagination', () => { let expect; before(async () => { + // @ts-ignore const chai = await import('chai'); expect = chai.expect; }); @@ -130,6 +131,7 @@ describe('FigshareService - isFileUploadInProgress', () => { let expect; before(async () => { + // @ts-ignore const chai = await import('chai'); expect = chai.expect; }); From 224cdc131739fc15d2782095872c51e6fa73c51a Mon Sep 17 00:00:00 2001 From: andrewbrazzatti Date: Tue, 25 Nov 2025 09:34:36 +1030 Subject: [PATCH 6/6] Change FigshareService baseURL for testing to include localhost --- typescript/api/test/unit/services/FigshareService.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/typescript/api/test/unit/services/FigshareService.test.ts b/typescript/api/test/unit/services/FigshareService.test.ts index 6de0eca2eb..20734c225d 100644 --- a/typescript/api/test/unit/services/FigshareService.test.ts +++ b/typescript/api/test/unit/services/FigshareService.test.ts @@ -79,7 +79,7 @@ describe('FigshareService - getArticleFileList pagination', () => { axiosResponses = []; service = new FigshareService(); // Set required properties that are private in TS but accessible at runtime - (service as any).baseURL = 'https://api.figshare.test'; + (service as any).baseURL = 'https://api.figshare.test.localhost'; (service as any).APIToken = 'test-token'; (global as any).sails.config.figshareAPI.mapping.upload.fileListPageSize = 2; }); @@ -138,7 +138,7 @@ describe('FigshareService - isFileUploadInProgress', () => { beforeEach(() => { service = new FigshareService(); - (service as any).baseURL = 'https://api.figshare.test'; + (service as any).baseURL = 'https://api.figshare.test.localhost'; (service as any).APIToken = 'test-token'; });