From 0f55bf378b2a2256377b8ce203acbc1c94250ffd Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 1 Sep 2023 12:26:05 +0100 Subject: [PATCH 1/7] Stash: getDatasetFileCounts use case WIP. Pending data access logic --- src/files/domain/models/FileCounts.ts | 23 +++++++++++ .../domain/repositories/IFilesRepository.ts | 3 ++ .../domain/useCases/GetDatasetFileCounts.ts | 19 +++++++++ src/files/index.ts | 5 ++- .../infra/repositories/FilesRepository.ts | 5 +++ test/testHelpers/files/fileCountsHelper.ts | 30 ++++++++++++++ test/unit/files/GetDatasetFileCounts.test.ts | 40 +++++++++++++++++++ 7 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 src/files/domain/models/FileCounts.ts create mode 100644 src/files/domain/useCases/GetDatasetFileCounts.ts create mode 100644 test/testHelpers/files/fileCountsHelper.ts create mode 100644 test/unit/files/GetDatasetFileCounts.test.ts diff --git a/src/files/domain/models/FileCounts.ts b/src/files/domain/models/FileCounts.ts new file mode 100644 index 00000000..b73c05c4 --- /dev/null +++ b/src/files/domain/models/FileCounts.ts @@ -0,0 +1,23 @@ +import { FileAccessStatus } from './FileCriteria'; + +export interface FileCounts { + total: number; + perFileContentType: FileContentTypeCount[]; + perAccessStatus: FileAccessStatusCount[]; + perCategoryTag: FileCategoryCount[]; +} + +export interface FileContentTypeCount { + contentType: string; + count: number; +} + +export interface FileAccessStatusCount { + accessStatus: FileAccessStatus; + count: number; +} + +export interface FileCategoryCount { + category: string; + count: number; +} diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index 7a6c116f..85a24bec 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -2,6 +2,7 @@ import { File } from '../models/File'; import { FileDataTable } from '../models/FileDataTable'; import { FileUserPermissions } from '../models/FileUserPermissions'; import { FileCriteria } from '../models/FileCriteria'; +import { FileCounts } from '../models/FileCounts'; export interface IFilesRepository { getDatasetFiles( @@ -12,6 +13,8 @@ export interface IFilesRepository { fileCriteria?: FileCriteria, ): Promise; + getDatasetFileCounts(datasetId: number | string, datasetVersionId: string): Promise; + getFileDownloadCount(fileId: number | string): Promise; getFileUserPermissions(fileId: number | string): Promise; diff --git a/src/files/domain/useCases/GetDatasetFileCounts.ts b/src/files/domain/useCases/GetDatasetFileCounts.ts new file mode 100644 index 00000000..6e73a889 --- /dev/null +++ b/src/files/domain/useCases/GetDatasetFileCounts.ts @@ -0,0 +1,19 @@ +import { UseCase } from '../../../core/domain/useCases/UseCase'; +import { IFilesRepository } from '../repositories/IFilesRepository'; +import { DatasetNotNumberedVersion } from '../../../datasets'; +import { FileCounts } from '../models/FileCounts'; + +export class GetDatasetFileCounts implements UseCase { + private filesRepository: IFilesRepository; + + constructor(filesRepository: IFilesRepository) { + this.filesRepository = filesRepository; + } + + async execute( + datasetId: number | string, + datasetVersionId: string | DatasetNotNumberedVersion = DatasetNotNumberedVersion.LATEST, + ): Promise { + return await this.filesRepository.getDatasetFileCounts(datasetId, datasetVersionId); + } +} diff --git a/src/files/index.ts b/src/files/index.ts index 02fc8ebc..1635eea2 100644 --- a/src/files/index.ts +++ b/src/files/index.ts @@ -1,5 +1,6 @@ import { FilesRepository } from './infra/repositories/FilesRepository'; import { GetDatasetFiles } from './domain/useCases/GetDatasetFiles'; +import { GetDatasetFileCounts } from './domain/useCases/GetDatasetFileCounts'; import { GetFileDownloadCount } from './domain/useCases/GetFileDownloadCount'; import { GetFileUserPermissions } from './domain/useCases/GetFileUserPermissions'; import { GetFileDataTables } from './domain/useCases/GetFileDataTables'; @@ -7,15 +8,17 @@ import { GetFileDataTables } from './domain/useCases/GetFileDataTables'; const filesRepository = new FilesRepository(); const getDatasetFiles = new GetDatasetFiles(filesRepository); +const getDatasetFileCounts = new GetDatasetFileCounts(filesRepository); const getFileDownloadCount = new GetFileDownloadCount(filesRepository); const getFileUserPermissions = new GetFileUserPermissions(filesRepository); const getFileDataTables = new GetFileDataTables(filesRepository); -export { getDatasetFiles, getFileDownloadCount, getFileUserPermissions, getFileDataTables }; +export { getDatasetFiles, getFileDownloadCount, getFileUserPermissions, getFileDataTables, getDatasetFileCounts }; export { File, FileEmbargo, FileChecksum } from './domain/models/File'; export { FileUserPermissions } from './domain/models/FileUserPermissions'; export { FileCriteria, FileOrderCriteria, FileAccessStatus } from './domain/models/FileCriteria'; +export { FileCounts, FileContentTypeCount, FileAccessStatusCount, FileCategoryCount } from './domain/models/FileCounts'; export { FileDataTable, FileDataVariable, diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 44eec219..2fd82fbc 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -7,6 +7,7 @@ import { transformDataTablesResponseToDataTables } from './transformers/fileData import { FileUserPermissions } from '../../domain/models/FileUserPermissions'; import { transformFileUserPermissionsResponseToFileUserPermissions } from './transformers/fileUserPermissionsTransformers'; import { FileCriteria } from '../../domain/models/FileCriteria'; +import { FileCounts } from '../../domain/models/FileCounts'; export interface GetFilesQueryParams { limit?: number; @@ -49,6 +50,10 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { }); } + getDatasetFileCounts(datasetId: string | number, datasetVersionId: string): Promise { + throw new Error('Method not implemented. Params' + datasetId + ' ' + datasetVersionId); + } + public async getFileDownloadCount(fileId: number | string): Promise { let endpoint; if (typeof fileId === 'number') { diff --git a/test/testHelpers/files/fileCountsHelper.ts b/test/testHelpers/files/fileCountsHelper.ts new file mode 100644 index 00000000..5faab957 --- /dev/null +++ b/test/testHelpers/files/fileCountsHelper.ts @@ -0,0 +1,30 @@ +import { FileCounts } from '../../../src/files/domain/models/FileCounts'; +import { FileAccessStatus } from '../../../src/files/domain/models/FileCriteria'; + +export const createFileCountsModel = (): FileCounts => { + return { + total: 4, + perFileContentType: [ + { + contentType: 'text/plain', + count: 4, + }, + ], + perAccessStatus: [ + { + accessStatus: FileAccessStatus.PUBLIC, + count: 3, + }, + { + accessStatus: FileAccessStatus.RESTRICTED, + count: 1, + }, + ], + perCategoryTag: [ + { + category: 'testCategory', + count: 2, + }, + ], + }; +}; diff --git a/test/unit/files/GetDatasetFileCounts.test.ts b/test/unit/files/GetDatasetFileCounts.test.ts new file mode 100644 index 00000000..c6c9f856 --- /dev/null +++ b/test/unit/files/GetDatasetFileCounts.test.ts @@ -0,0 +1,40 @@ +import { GetDatasetFileCounts } from '../../../src/files/domain/useCases/GetDatasetFileCounts'; +import { IFilesRepository } from '../../../src/files/domain/repositories/IFilesRepository'; +import { assert, createSandbox, SinonSandbox } from 'sinon'; +import { ReadError } from '../../../src/core/domain/repositories/ReadError'; +import { DatasetNotNumberedVersion } from '../../../src/datasets'; +import { FileCounts } from '../../../src/files/domain/models/FileCounts'; +import { createFileCountsModel } from '../../testHelpers/files/fileCountsHelper'; + +describe('execute', () => { + const sandbox: SinonSandbox = createSandbox(); + + afterEach(() => { + sandbox.restore(); + }); + + test('should return files on repository success', async () => { + const testFileCounts: FileCounts = createFileCountsModel(); + const filesRepositoryStub = {}; + const getDatasetFileCountsStub = sandbox.stub().returns(testFileCounts); + filesRepositoryStub.getDatasetFileCounts = getDatasetFileCountsStub; + const sut = new GetDatasetFileCounts(filesRepositoryStub); + + const actual = await sut.execute(1); + + assert.match(actual, testFileCounts); + assert.calledWithExactly(getDatasetFileCountsStub, 1, DatasetNotNumberedVersion.LATEST); + }); + + test('should return error result on repository error', async () => { + const filesRepositoryStub = {}; + const testReadError = new ReadError(); + filesRepositoryStub.getDatasetFileCounts = sandbox.stub().throwsException(testReadError); + const sut = new GetDatasetFileCounts(filesRepositoryStub); + + let actualError: ReadError = undefined; + await sut.execute(1).catch((e: ReadError) => (actualError = e)); + + assert.match(actualError, testReadError); + }); +}); From 8b5a4298e17cb6f4eec0cf7b294c5f839d3c6777 Mon Sep 17 00:00:00 2001 From: GPortas Date: Fri, 1 Sep 2023 15:17:54 +0100 Subject: [PATCH 2/7] Added: GetDatasetFileCounts use case data access logic --- src/files/domain/models/FileCounts.ts | 8 +- src/files/index.ts | 7 +- .../infra/repositories/FilesRepository.ts | 15 +- .../transformers/fileCountsTransformers.ts | 57 ++++++++ test/integration/environment/.env | 2 +- .../integration/files/FilesRepository.test.ts | 49 ++++++- test/testHelpers/files/fileCountsHelper.ts | 22 ++- test/unit/files/FilesRepository.test.ts | 133 ++++++++++++++++-- 8 files changed, 266 insertions(+), 27 deletions(-) create mode 100644 src/files/infra/repositories/transformers/fileCountsTransformers.ts diff --git a/src/files/domain/models/FileCounts.ts b/src/files/domain/models/FileCounts.ts index b73c05c4..e27200c1 100644 --- a/src/files/domain/models/FileCounts.ts +++ b/src/files/domain/models/FileCounts.ts @@ -2,9 +2,9 @@ import { FileAccessStatus } from './FileCriteria'; export interface FileCounts { total: number; - perFileContentType: FileContentTypeCount[]; + perContentType: FileContentTypeCount[]; perAccessStatus: FileAccessStatusCount[]; - perCategoryTag: FileCategoryCount[]; + perCategoryName: FileCategoryNameCount[]; } export interface FileContentTypeCount { @@ -17,7 +17,7 @@ export interface FileAccessStatusCount { count: number; } -export interface FileCategoryCount { - category: string; +export interface FileCategoryNameCount { + categoryName: string; count: number; } diff --git a/src/files/index.ts b/src/files/index.ts index 1635eea2..6d410f86 100644 --- a/src/files/index.ts +++ b/src/files/index.ts @@ -18,7 +18,12 @@ export { getDatasetFiles, getFileDownloadCount, getFileUserPermissions, getFileD export { File, FileEmbargo, FileChecksum } from './domain/models/File'; export { FileUserPermissions } from './domain/models/FileUserPermissions'; export { FileCriteria, FileOrderCriteria, FileAccessStatus } from './domain/models/FileCriteria'; -export { FileCounts, FileContentTypeCount, FileAccessStatusCount, FileCategoryCount } from './domain/models/FileCounts'; +export { + FileCounts, + FileContentTypeCount, + FileAccessStatusCount, + FileCategoryNameCount, +} from './domain/models/FileCounts'; export { FileDataTable, FileDataVariable, diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 2fd82fbc..a0b3741b 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -8,6 +8,7 @@ import { FileUserPermissions } from '../../domain/models/FileUserPermissions'; import { transformFileUserPermissionsResponseToFileUserPermissions } from './transformers/fileUserPermissionsTransformers'; import { FileCriteria } from '../../domain/models/FileCriteria'; import { FileCounts } from '../../domain/models/FileCounts'; +import { transformFileCountsResponseToFileCounts } from './transformers/fileCountsTransformers'; export interface GetFilesQueryParams { limit?: number; @@ -50,8 +51,18 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { }); } - getDatasetFileCounts(datasetId: string | number, datasetVersionId: string): Promise { - throw new Error('Method not implemented. Params' + datasetId + ' ' + datasetVersionId); + public async getDatasetFileCounts(datasetId: string | number, datasetVersionId: string): Promise { + let endpoint; + if (typeof datasetId === 'number') { + endpoint = `/datasets/${datasetId}/versions/${datasetVersionId}/files/counts`; + } else { + endpoint = `/datasets/:persistentId/versions/${datasetVersionId}/files/counts?persistentId=${datasetId}`; + } + return this.doGet(endpoint, true) + .then((response) => transformFileCountsResponseToFileCounts(response)) + .catch((error) => { + throw error; + }); } public async getFileDownloadCount(fileId: number | string): Promise { diff --git a/src/files/infra/repositories/transformers/fileCountsTransformers.ts b/src/files/infra/repositories/transformers/fileCountsTransformers.ts new file mode 100644 index 00000000..fe4f7f60 --- /dev/null +++ b/src/files/infra/repositories/transformers/fileCountsTransformers.ts @@ -0,0 +1,57 @@ +import { AxiosResponse } from 'axios'; +import { + FileCounts, + FileContentTypeCount, + FileCategoryNameCount, + FileAccessStatusCount, +} from '../../../domain/models/FileCounts'; +import { FileAccessStatus } from '../../../domain/models/FileCriteria'; + +export const transformFileCountsResponseToFileCounts = (response: AxiosResponse): FileCounts => { + const fileCountsPayload = response.data.data; + return { + total: fileCountsPayload.total, + perContentType: transformCountsPerContentTypePayload(fileCountsPayload.perContentType), + perAccessStatus: transformCountsPerAccessStatusPayload(fileCountsPayload.perAccessStatus), + perCategoryName: transformCountsPerCategoryNamePayload(fileCountsPayload.perCategoryName), + }; +}; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export const transformCountsPerContentTypePayload = (countsPerContentTypePayload: any): FileContentTypeCount[] => { + const fileContentTypeCounts: FileContentTypeCount[] = []; + const fileContentTypeCountKeys = Object.keys(countsPerContentTypePayload); + for (const fileContentTypeCountKey of fileContentTypeCountKeys) { + fileContentTypeCounts.push({ + contentType: fileContentTypeCountKey, + count: countsPerContentTypePayload[fileContentTypeCountKey], + }); + } + return fileContentTypeCounts; +}; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export const transformCountsPerCategoryNamePayload = (countsPerCategoryNamePayload: any): FileCategoryNameCount[] => { + const fileCategoryNameCounts: FileCategoryNameCount[] = []; + const fileCategoryNameCountKeys = Object.keys(countsPerCategoryNamePayload); + for (const fileCategoryNameCountKey of fileCategoryNameCountKeys) { + fileCategoryNameCounts.push({ + categoryName: fileCategoryNameCountKey, + count: countsPerCategoryNamePayload[fileCategoryNameCountKey], + }); + } + return fileCategoryNameCounts; +}; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export const transformCountsPerAccessStatusPayload = (countsPerAccessStatusPayload: any): FileAccessStatusCount[] => { + const fileAccessStatusCounts: FileAccessStatusCount[] = []; + const fileAccessStatusCountKeys = Object.keys(countsPerAccessStatusPayload); + for (const fileAccessStatusCountKey of fileAccessStatusCountKeys) { + fileAccessStatusCounts.push({ + accessStatus: fileAccessStatusCountKey as FileAccessStatus, + count: countsPerAccessStatusPayload[fileAccessStatusCountKey], + }); + } + return fileAccessStatusCounts; +}; diff --git a/test/integration/environment/.env b/test/integration/environment/.env index 0ca2696d..5478afd6 100644 --- a/test/integration/environment/.env +++ b/test/integration/environment/.env @@ -2,5 +2,5 @@ POSTGRES_VERSION=13 DATAVERSE_DB_USER=dataverse SOLR_VERSION=8.11.1 DATAVERSE_IMAGE_REGISTRY=ghcr.io -DATAVERSE_IMAGE_TAG=9714-files-api-extension-filters +DATAVERSE_IMAGE_TAG=9834-files-api-extension-file-counts DATAVERSE_BOOTSTRAP_TIMEOUT=5m diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index 351dd546..b8fb275e 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -1,6 +1,7 @@ import { FilesRepository } from '../../../src/files/infra/repositories/FilesRepository'; import { ApiConfig, DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig'; import { assert } from 'sinon'; +import { expect } from 'chai'; import { TestConstants } from '../../testHelpers/TestConstants'; import { createDatasetViaApi } from '../../testHelpers/datasets/datasetHelper'; import { uploadFileViaApi } from '../../testHelpers/files/filesHelper'; @@ -8,6 +9,7 @@ import { DatasetsRepository } from '../../../src/datasets/infra/repositories/Dat import { ReadError } from '../../../src/core/domain/repositories/ReadError'; import { FileCriteria, FileAccessStatus, FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria'; import { DatasetNotNumberedVersion } from '../../../src/datasets'; +import { FileCounts } from '../../../src/files/domain/models/FileCounts'; describe('FilesRepository', () => { const sut: FilesRepository = new FilesRepository(); @@ -21,6 +23,8 @@ describe('FilesRepository', () => { const latestDatasetVersionId = DatasetNotNumberedVersion.LATEST; + const datasetRepository = new DatasetsRepository(); + beforeAll(async () => { ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.API_KEY, process.env.TEST_API_KEY); await createDatasetViaApi() @@ -114,8 +118,6 @@ describe('FilesRepository', () => { }); describe('by persistent id', () => { - const datasetRepository = new DatasetsRepository(); - test('should return all files filtering by persistent id and version id', async () => { const testDataset = await datasetRepository.getDataset( TestConstants.TEST_CREATED_DATASET_ID, @@ -171,6 +173,49 @@ describe('FilesRepository', () => { }); }); + describe('getDatasetFileCounts', () => { + const expectedFileCounts: FileCounts = { + total: 4, + perContentType: [ + { + contentType: 'text/plain', + count: 3, + }, + { + contentType: 'text/tab-separated-values', + count: 1, + }, + ], + perAccessStatus: [ + { + accessStatus: FileAccessStatus.PUBLIC, + count: 4, + }, + ], + perCategoryName: [], + }; + + test('should return file count filtering by numeric id', async () => { + const actual = await sut.getDatasetFileCounts(TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId); + assert.match(actual.total, expectedFileCounts.total); + expect(actual.perContentType).to.have.deep.members(expectedFileCounts.perContentType); + expect(actual.perAccessStatus).to.have.deep.members(expectedFileCounts.perAccessStatus); + expect(actual.perCategoryName).to.have.deep.members(expectedFileCounts.perCategoryName); + }); + + test('should return file count filtering by persistent id', async () => { + const testDataset = await datasetRepository.getDataset( + TestConstants.TEST_CREATED_DATASET_ID, + latestDatasetVersionId, + ); + const actual = await sut.getDatasetFileCounts(testDataset.persistentId, latestDatasetVersionId); + assert.match(actual.total, expectedFileCounts.total); + expect(actual.perContentType).to.have.deep.members(expectedFileCounts.perContentType); + expect(actual.perAccessStatus).to.have.deep.members(expectedFileCounts.perAccessStatus); + expect(actual.perCategoryName).to.have.deep.members(expectedFileCounts.perCategoryName); + }); + }); + describe('getFileDownloadCount', () => { test('should return count filtering by file id and version id', async () => { const currentTestFiles = await sut.getDatasetFiles(TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId); diff --git a/test/testHelpers/files/fileCountsHelper.ts b/test/testHelpers/files/fileCountsHelper.ts index 5faab957..c863ac2b 100644 --- a/test/testHelpers/files/fileCountsHelper.ts +++ b/test/testHelpers/files/fileCountsHelper.ts @@ -4,7 +4,7 @@ import { FileAccessStatus } from '../../../src/files/domain/models/FileCriteria' export const createFileCountsModel = (): FileCounts => { return { total: 4, - perFileContentType: [ + perContentType: [ { contentType: 'text/plain', count: 4, @@ -20,11 +20,27 @@ export const createFileCountsModel = (): FileCounts => { count: 1, }, ], - perCategoryTag: [ + perCategoryName: [ { - category: 'testCategory', + categoryName: 'testCategory', count: 2, }, ], }; }; + +export const createFileCountsPayload = (): any => { + return { + total: 4, + perContentType: { + 'text/plain': 4, + }, + perAccessStatus: { + Public: 3, + Restricted: 1, + }, + perCategoryName: { + testCategory: 2, + }, + }; +}; diff --git a/test/unit/files/FilesRepository.test.ts b/test/unit/files/FilesRepository.test.ts index 6c1b132b..d61a394c 100644 --- a/test/unit/files/FilesRepository.test.ts +++ b/test/unit/files/FilesRepository.test.ts @@ -10,17 +10,14 @@ import { createFileDataTablePayload, createFileDataTableModel } from '../../test import { createFileUserPermissionsModel } from '../../testHelpers/files/fileUserPermissionsHelper'; import { FileCriteria, FileAccessStatus, FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria'; import { DatasetNotNumberedVersion } from '../../../src/datasets'; +import { createFileCountsModel, createFileCountsPayload } from '../../testHelpers/files/fileCountsHelper'; describe('FilesRepository', () => { const sandbox: SinonSandbox = createSandbox(); const sut: FilesRepository = new FilesRepository(); - const testFilesSuccessfulResponse = { - data: { - status: 'OK', - data: [createFilePayload()], - }, - }; const testFile = createFileModel(); + const testDatasetVersionId = DatasetNotNumberedVersion.LATEST; + const testDatasetId = 1; beforeEach(() => { ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.API_KEY, TestConstants.TEST_DUMMY_API_KEY); @@ -31,7 +28,13 @@ describe('FilesRepository', () => { }); describe('getDatasetFiles', () => { - const testDatasetVersionId = DatasetNotNumberedVersion.LATEST; + const testFilesSuccessfulResponse = { + data: { + status: 'OK', + data: [createFilePayload()], + }, + }; + const testLimit = 10; const testOffset = 20; const testCategory = 'testCategory'; @@ -50,15 +53,12 @@ describe('FilesRepository', () => { contentType: testFileCriteria.contentType, accessStatus: testFileCriteria.accessStatus.toString(), }; + const expectedFiles = [testFile]; describe('by numeric id and version id', () => { - const testDatasetId = 1; - test('should return files when providing id, version id, and response is successful', async () => { const axiosGetStub = sandbox.stub(axios, 'get').resolves(testFilesSuccessfulResponse); - const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/datasets/${testDatasetId}/versions/${testDatasetVersionId}/files`; - const expectedFiles = [testFile]; // API Key auth let actual = await sut.getDatasetFiles(testDatasetId, testDatasetVersionId); @@ -86,7 +86,13 @@ describe('FilesRepository', () => { test('should return files when providing id, version id, optional params, and response is successful', async () => { const axiosGetStub = sandbox.stub(axios, 'get').resolves(testFilesSuccessfulResponse); - const actual = await sut.getDatasetFiles(testDatasetId, testDatasetVersionId, testLimit, testOffset, testFileCriteria); + const actual = await sut.getDatasetFiles( + testDatasetId, + testDatasetVersionId, + testLimit, + testOffset, + testFileCriteria, + ); const expectedRequestConfig = { params: expectedRequestParams, @@ -120,7 +126,6 @@ describe('FilesRepository', () => { test('should return files when providing persistent id, version id, and response is successful', async () => { const axiosGetStub = sandbox.stub(axios, 'get').resolves(testFilesSuccessfulResponse); const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/datasets/:persistentId/versions/${testDatasetVersionId}/files?persistentId=${TestConstants.TEST_DUMMY_PERSISTENT_ID}`; - const expectedFiles = [testFile]; // API Key auth let actual = await sut.getDatasetFiles(TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId); @@ -173,7 +178,9 @@ describe('FilesRepository', () => { const axiosGetStub = sandbox.stub(axios, 'get').rejects(TestConstants.TEST_ERROR_RESPONSE); let error: ReadError = undefined; - await sut.getDatasetFiles(TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId).catch((e) => (error = e)); + await sut + .getDatasetFiles(TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId) + .catch((e) => (error = e)); assert.calledWithExactly( axiosGetStub, @@ -185,6 +192,104 @@ describe('FilesRepository', () => { }); }); + describe('getDatasetFileCounts', () => { + const testFileCountsSuccessfulResponse = { + data: { + status: 'OK', + data: createFileCountsPayload(), + }, + }; + const expectedCount = createFileCountsModel(); + + describe('by numeric id and version id', () => { + test('should return file counts when providing id, version id, and response is successful', async () => { + const axiosGetStub = sandbox.stub(axios, 'get').resolves(testFileCountsSuccessfulResponse); + const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/datasets/${testDatasetId}/versions/${testDatasetVersionId}/files/counts`; + + // API Key auth + let actual = await sut.getDatasetFileCounts(testDatasetId, testDatasetVersionId); + + assert.calledWithExactly( + axiosGetStub, + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY, + ); + assert.match(actual, expectedCount); + + // Session cookie auth + ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.SESSION_COOKIE); + + actual = await sut.getDatasetFileCounts(testDatasetId, testDatasetVersionId); + + assert.calledWithExactly( + axiosGetStub, + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_SESSION_COOKIE, + ); + assert.match(actual, expectedCount); + }); + + test('should return error result on error response', async () => { + const axiosGetStub = sandbox.stub(axios, 'get').rejects(TestConstants.TEST_ERROR_RESPONSE); + + let error: ReadError = undefined; + await sut.getDatasetFileCounts(testDatasetId, testDatasetVersionId).catch((e) => (error = e)); + + assert.calledWithExactly( + axiosGetStub, + `${TestConstants.TEST_API_URL}/datasets/${testDatasetId}/versions/${testDatasetVersionId}/files/counts`, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY, + ); + expect(error).to.be.instanceOf(Error); + }); + }); + + describe('by persistent id', () => { + test('should return files when providing persistent id, version id, and response is successful', async () => { + const axiosGetStub = sandbox.stub(axios, 'get').resolves(testFileCountsSuccessfulResponse); + const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/datasets/:persistentId/versions/${testDatasetVersionId}/files/counts?persistentId=${TestConstants.TEST_DUMMY_PERSISTENT_ID}`; + + // API Key auth + let actual = await sut.getDatasetFileCounts(TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId); + + assert.calledWithExactly( + axiosGetStub, + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY, + ); + assert.match(actual, expectedCount); + + // Session cookie auth + ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.SESSION_COOKIE); + + actual = await sut.getDatasetFileCounts(TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId); + + assert.calledWithExactly( + axiosGetStub, + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_SESSION_COOKIE, + ); + assert.match(actual, expectedCount); + }); + + test('should return error result on error response', async () => { + const axiosGetStub = sandbox.stub(axios, 'get').rejects(TestConstants.TEST_ERROR_RESPONSE); + + let error: ReadError = undefined; + await sut + .getDatasetFileCounts(TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId) + .catch((e) => (error = e)); + + assert.calledWithExactly( + axiosGetStub, + `${TestConstants.TEST_API_URL}/datasets/:persistentId/versions/${testDatasetVersionId}/files/counts?persistentId=${TestConstants.TEST_DUMMY_PERSISTENT_ID}`, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY, + ); + expect(error).to.be.instanceOf(Error); + }); + }); + }); + describe('getFileDownloadCount', () => { const testCount = 1; const testFileDownloadCountResponse = { From 682e1390f91e87c8914f5b955d61e94a4f4b5e3a Mon Sep 17 00:00:00 2001 From: GPortas Date: Sun, 3 Sep 2023 17:58:52 +0100 Subject: [PATCH 3/7] Added: enhanced test coverage for file counts by adding a categorized test file --- .../integration/files/FilesRepository.test.ts | 19 +++++++++++++++++-- test/testHelpers/files/filesHelper.ts | 7 +++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index b8fb275e..a463cb0b 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -4,7 +4,7 @@ import { assert } from 'sinon'; import { expect } from 'chai'; import { TestConstants } from '../../testHelpers/TestConstants'; import { createDatasetViaApi } from '../../testHelpers/datasets/datasetHelper'; -import { uploadFileViaApi } from '../../testHelpers/files/filesHelper'; +import { uploadFileViaApi, setFileCategoriesViaApi } from '../../testHelpers/files/filesHelper'; import { DatasetsRepository } from '../../../src/datasets/infra/repositories/DatasetsRepository'; import { ReadError } from '../../../src/core/domain/repositories/ReadError'; import { FileCriteria, FileAccessStatus, FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria'; @@ -18,6 +18,7 @@ describe('FilesRepository', () => { const testTextFile2Name = 'test-file-2.txt'; const testTextFile3Name = 'test-file-3.txt'; const testTabFile4Name = 'test-file-4.tab'; + const testCategoryName = 'testCategory'; const nonExistentFiledId = 200; @@ -60,6 +61,15 @@ describe('FilesRepository', () => { console.log(e); fail(`Tests beforeAll(): Error while uploading file ${testTabFile4Name}`); }); + // Categorize one of the uploaded test files + const currentTestFiles = await sut.getDatasetFiles(TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId); + const testFile = currentTestFiles[0]; + setFileCategoriesViaApi(testFile.id, [testCategoryName]) + .then() + .catch((e) => { + console.log(e); + fail(`Tests beforeAll(): Error while setting file categories to ${testFile.name}`); + }); }); describe('getDatasetFiles', () => { @@ -192,7 +202,12 @@ describe('FilesRepository', () => { count: 4, }, ], - perCategoryName: [], + perCategoryName: [ + { + categoryName: testCategoryName, + count: 1, + }, + ], }; test('should return file count filtering by numeric id', async () => { diff --git a/test/testHelpers/files/filesHelper.ts b/test/testHelpers/files/filesHelper.ts index d67afdbd..2da10fe7 100644 --- a/test/testHelpers/files/filesHelper.ts +++ b/test/testHelpers/files/filesHelper.ts @@ -73,3 +73,10 @@ export const uploadFileViaApi = async (datasetId: number, fileName: string): Pro }, }); }; + +export const setFileCategoriesViaApi = async (fileId: number, fileCategoryNames: string[]): Promise => { + const data = { categories: fileCategoryNames }; + return await axios.post(`${TestConstants.TEST_API_URL}/files/${fileId}/metadata/categories`, JSON.stringify(data), { + headers: { 'Content-Type': 'application/json', 'X-Dataverse-Key': process.env.TEST_API_KEY }, + }); +}; From babe2d72702653980eee142fa665fed51cdb8447 Mon Sep 17 00:00:00 2001 From: GPortas Date: Sun, 3 Sep 2023 18:36:47 +0100 Subject: [PATCH 4/7] Refactor: ApiRepository method for building API endpoints --- src/core/infra/repositories/ApiRepository.ts | 12 +++++ .../infra/repositories/DatasetsRepository.ts | 21 ++++---- .../infra/repositories/FilesRepository.ts | 51 ++++++------------- 3 files changed, 38 insertions(+), 46 deletions(-) diff --git a/src/core/infra/repositories/ApiRepository.ts b/src/core/infra/repositories/ApiRepository.ts index a1d1e0e4..a58fe658 100644 --- a/src/core/infra/repositories/ApiRepository.ts +++ b/src/core/infra/repositories/ApiRepository.ts @@ -26,6 +26,18 @@ export abstract class ApiRepository { }); } + protected buildApiEndpoint(resourceName: string, operation: string, resourceId: number | string = undefined) { + let endpoint; + if (typeof resourceId === 'number') { + endpoint = `/${resourceName}/${resourceId}/${operation}`; + } else if (typeof resourceId === 'string') { + endpoint = `/${resourceName}/:persistentId/${operation}?persistentId=${resourceId}`; + } else { + endpoint = `/${resourceName}/${operation}`; + } + return endpoint; + } + private buildRequestConfig(authRequired: boolean, queryParams: object): AxiosRequestConfig { const requestConfig: AxiosRequestConfig = { params: queryParams, diff --git a/src/datasets/infra/repositories/DatasetsRepository.ts b/src/datasets/infra/repositories/DatasetsRepository.ts index 32af4b12..0f93808d 100644 --- a/src/datasets/infra/repositories/DatasetsRepository.ts +++ b/src/datasets/infra/repositories/DatasetsRepository.ts @@ -4,8 +4,10 @@ import { Dataset } from '../../domain/models/Dataset'; import { transformVersionResponseToDataset } from './transformers/datasetTransformers'; export class DatasetsRepository extends ApiRepository implements IDatasetsRepository { + private readonly resourceName: string = 'datasets'; + public async getDatasetSummaryFieldNames(): Promise { - return this.doGet('/datasets/summaryFieldNames') + return this.doGet(this.buildApiEndpoint(this.resourceName, 'summaryFieldNames')) .then((response) => response.data.data) .catch((error) => { throw error; @@ -13,7 +15,7 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi } public async getPrivateUrlDataset(token: string): Promise { - return this.doGet(`/datasets/privateUrlDatasetVersion/${token}`) + return this.doGet(this.buildApiEndpoint(this.resourceName, `privateUrlDatasetVersion/${token}`)) .then((response) => transformVersionResponseToDataset(response)) .catch((error) => { throw error; @@ -21,13 +23,7 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi } public async getDataset(datasetId: number | string, datasetVersionId: string): Promise { - let endpoint; - if (typeof datasetId === 'number') { - endpoint = `/datasets/${datasetId}/versions/${datasetVersionId}`; - } else { - endpoint = `/datasets/:persistentId/versions/${datasetVersionId}?persistentId=${datasetId}`; - } - return this.doGet(endpoint, true) + return this.doGet(this.buildApiEndpoint(this.resourceName, `versions/${datasetVersionId}`, datasetId), true) .then((response) => transformVersionResponseToDataset(response)) .catch((error) => { throw error; @@ -35,7 +31,10 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi } public async getDatasetCitation(datasetId: number, datasetVersionId: string): Promise { - return this.doGet(`/datasets/${datasetId}/versions/${datasetVersionId}/citation`, true) + return this.doGet( + this.buildApiEndpoint(this.resourceName, `versions/${datasetVersionId}/citation`, datasetId), + true, + ) .then((response) => response.data.data.message) .catch((error) => { throw error; @@ -43,7 +42,7 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi } public async getPrivateUrlDatasetCitation(token: string): Promise { - return this.doGet(`/datasets/privateUrlDatasetVersion/${token}/citation`) + return this.doGet(this.buildApiEndpoint(this.resourceName, `privateUrlDatasetVersion/${token}/citation`)) .then((response) => response.data.data.message) .catch((error) => { throw error; diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index a0b3741b..d9d2bf45 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -21,6 +21,10 @@ export interface GetFilesQueryParams { } export class FilesRepository extends ApiRepository implements IFilesRepository { + private readonly datasetsResourceName: string = 'datasets'; + private readonly filesResourceName: string = 'files'; + private readonly accessResourceName: string = 'access/datafile'; + public async getDatasetFiles( datasetId: number | string, datasetVersionId: string, @@ -28,12 +32,6 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { offset?: number, fileCriteria?: FileCriteria, ): Promise { - let endpoint; - if (typeof datasetId === 'number') { - endpoint = `/datasets/${datasetId}/versions/${datasetVersionId}/files`; - } else { - endpoint = `/datasets/:persistentId/versions/${datasetVersionId}/files?persistentId=${datasetId}`; - } const queryParams: GetFilesQueryParams = {}; if (limit !== undefined) { queryParams.limit = limit; @@ -44,7 +42,11 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { if (fileCriteria !== undefined) { this.applyFileCriteriaToQueryParams(queryParams, fileCriteria); } - return this.doGet(endpoint, true, queryParams) + return this.doGet( + this.buildApiEndpoint(this.datasetsResourceName, `versions/${datasetVersionId}/files`, datasetId), + true, + queryParams, + ) .then((response) => transformFilesResponseToFiles(response)) .catch((error) => { throw error; @@ -52,13 +54,10 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { } public async getDatasetFileCounts(datasetId: string | number, datasetVersionId: string): Promise { - let endpoint; - if (typeof datasetId === 'number') { - endpoint = `/datasets/${datasetId}/versions/${datasetVersionId}/files/counts`; - } else { - endpoint = `/datasets/:persistentId/versions/${datasetVersionId}/files/counts?persistentId=${datasetId}`; - } - return this.doGet(endpoint, true) + return this.doGet( + this.buildApiEndpoint(this.datasetsResourceName, `versions/${datasetVersionId}/files/counts`, datasetId), + true, + ) .then((response) => transformFileCountsResponseToFileCounts(response)) .catch((error) => { throw error; @@ -66,13 +65,7 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { } public async getFileDownloadCount(fileId: number | string): Promise { - let endpoint; - if (typeof fileId === 'number') { - endpoint = `/files/${fileId}/downloadCount`; - } else { - endpoint = `/files/:persistentId/downloadCount?persistentId=${fileId}`; - } - return this.doGet(endpoint, true) + return this.doGet(this.buildApiEndpoint(this.filesResourceName, `downloadCount`, fileId), true) .then((response) => response.data.data.message as number) .catch((error) => { throw error; @@ -80,13 +73,7 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { } public async getFileUserPermissions(fileId: number | string): Promise { - let endpoint; - if (typeof fileId === 'number') { - endpoint = `/access/datafile/${fileId}/userPermissions`; - } else { - endpoint = `/access/datafile/:persistentId/userPermissions?persistentId=${fileId}`; - } - return this.doGet(endpoint, true) + return this.doGet(this.buildApiEndpoint(this.accessResourceName, `userPermissions`, fileId), true) .then((response) => transformFileUserPermissionsResponseToFileUserPermissions(response)) .catch((error) => { throw error; @@ -94,13 +81,7 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { } public async getFileDataTables(fileId: string | number): Promise { - let endpoint; - if (typeof fileId === 'number') { - endpoint = `/files/${fileId}/dataTables`; - } else { - endpoint = `/files/:persistentId/dataTables?persistentId=${fileId}`; - } - return this.doGet(endpoint, true) + return this.doGet(this.buildApiEndpoint(this.filesResourceName, `dataTables`, fileId), true) .then((response) => transformDataTablesResponseToDataTables(response)) .catch((error) => { throw error; From 3fa47c4235aa1b94a4e723b0501dc2a93f49408f Mon Sep 17 00:00:00 2001 From: GPortas Date: Sun, 3 Sep 2023 18:39:44 +0100 Subject: [PATCH 5/7] Changed: test name --- test/unit/files/GetDatasetFileCounts.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/files/GetDatasetFileCounts.test.ts b/test/unit/files/GetDatasetFileCounts.test.ts index c6c9f856..3aa5096b 100644 --- a/test/unit/files/GetDatasetFileCounts.test.ts +++ b/test/unit/files/GetDatasetFileCounts.test.ts @@ -13,7 +13,7 @@ describe('execute', () => { sandbox.restore(); }); - test('should return files on repository success', async () => { + test('should return file counts on repository success', async () => { const testFileCounts: FileCounts = createFileCountsModel(); const filesRepositoryStub = {}; const getDatasetFileCountsStub = sandbox.stub().returns(testFileCounts); From 9d7ced67e546ff504ba4f6e588e335ed834226d9 Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 6 Sep 2023 10:27:24 +0100 Subject: [PATCH 6/7] Changed: run deploy_pr action on all pull requests --- .github/workflows/deploy_pr.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/deploy_pr.yml b/.github/workflows/deploy_pr.yml index 90f7cb6f..f073e30a 100644 --- a/.github/workflows/deploy_pr.yml +++ b/.github/workflows/deploy_pr.yml @@ -1,9 +1,6 @@ name: deploy_pr -on: - pull_request: - branches: - - develop +on: pull_request jobs: test: From 3ac5ec708d4e6c90148349cca69a37a9525b4023 Mon Sep 17 00:00:00 2001 From: GPortas Date: Wed, 6 Sep 2023 12:00:59 +0100 Subject: [PATCH 7/7] Refactor: variable renamed in DatasetsRepository --- .../infra/repositories/DatasetsRepository.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/datasets/infra/repositories/DatasetsRepository.ts b/src/datasets/infra/repositories/DatasetsRepository.ts index 0f93808d..53f1fdc2 100644 --- a/src/datasets/infra/repositories/DatasetsRepository.ts +++ b/src/datasets/infra/repositories/DatasetsRepository.ts @@ -4,10 +4,10 @@ import { Dataset } from '../../domain/models/Dataset'; import { transformVersionResponseToDataset } from './transformers/datasetTransformers'; export class DatasetsRepository extends ApiRepository implements IDatasetsRepository { - private readonly resourceName: string = 'datasets'; + private readonly datasetsResourceName: string = 'datasets'; public async getDatasetSummaryFieldNames(): Promise { - return this.doGet(this.buildApiEndpoint(this.resourceName, 'summaryFieldNames')) + return this.doGet(this.buildApiEndpoint(this.datasetsResourceName, 'summaryFieldNames')) .then((response) => response.data.data) .catch((error) => { throw error; @@ -15,7 +15,7 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi } public async getPrivateUrlDataset(token: string): Promise { - return this.doGet(this.buildApiEndpoint(this.resourceName, `privateUrlDatasetVersion/${token}`)) + return this.doGet(this.buildApiEndpoint(this.datasetsResourceName, `privateUrlDatasetVersion/${token}`)) .then((response) => transformVersionResponseToDataset(response)) .catch((error) => { throw error; @@ -23,7 +23,7 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi } public async getDataset(datasetId: number | string, datasetVersionId: string): Promise { - return this.doGet(this.buildApiEndpoint(this.resourceName, `versions/${datasetVersionId}`, datasetId), true) + return this.doGet(this.buildApiEndpoint(this.datasetsResourceName, `versions/${datasetVersionId}`, datasetId), true) .then((response) => transformVersionResponseToDataset(response)) .catch((error) => { throw error; @@ -32,7 +32,7 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi public async getDatasetCitation(datasetId: number, datasetVersionId: string): Promise { return this.doGet( - this.buildApiEndpoint(this.resourceName, `versions/${datasetVersionId}/citation`, datasetId), + this.buildApiEndpoint(this.datasetsResourceName, `versions/${datasetVersionId}/citation`, datasetId), true, ) .then((response) => response.data.data.message) @@ -42,7 +42,7 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi } public async getPrivateUrlDatasetCitation(token: string): Promise { - return this.doGet(this.buildApiEndpoint(this.resourceName, `privateUrlDatasetVersion/${token}/citation`)) + return this.doGet(this.buildApiEndpoint(this.datasetsResourceName, `privateUrlDatasetVersion/${token}/citation`)) .then((response) => response.data.data.message) .catch((error) => { throw error;