From 498358cff9261fc3fd3e42b0aaa4b1eaadbe9a42 Mon Sep 17 00:00:00 2001 From: GPortas Date: Mon, 9 Oct 2023 15:32:54 +0100 Subject: [PATCH 1/4] Refactor: FileOrderCriteria decoupled from FileSearchCriteria --- src/files/domain/models/FileCriteria.ts | 23 ++++------ .../domain/repositories/IFilesRepository.ts | 5 ++- src/files/domain/useCases/GetDatasetFiles.ts | 8 ++-- src/files/index.ts | 2 +- .../infra/repositories/FilesRepository.ts | 34 ++++++++------- test/integration/environment/.env | 2 +- .../integration/files/FilesRepository.test.ts | 40 +++++++++++++---- test/unit/files/FilesRepository.test.ts | 43 ++++++++++++++----- test/unit/files/GetDatasetFiles.test.ts | 3 +- 9 files changed, 102 insertions(+), 58 deletions(-) diff --git a/src/files/domain/models/FileCriteria.ts b/src/files/domain/models/FileCriteria.ts index 98bea349..95de6acc 100644 --- a/src/files/domain/models/FileCriteria.ts +++ b/src/files/domain/models/FileCriteria.ts @@ -1,30 +1,25 @@ -export class FileCriteria { +export class FileSearchCriteria { constructor( - public readonly orderCriteria: FileOrderCriteria = FileOrderCriteria.NAME_AZ, public readonly contentType?: string, public readonly accessStatus?: FileAccessStatus, public readonly categoryName?: string, public readonly searchText?: string, ) {} - withOrderCriteria(orderCriteria: FileOrderCriteria): FileCriteria { - return new FileCriteria(orderCriteria, this.contentType, this.accessStatus, this.categoryName); + withContentType(contentType: string | undefined): FileSearchCriteria { + return new FileSearchCriteria(contentType, this.accessStatus, this.categoryName); } - withContentType(contentType: string | undefined): FileCriteria { - return new FileCriteria(this.orderCriteria, contentType, this.accessStatus, this.categoryName); + withAccessStatus(accessStatus: FileAccessStatus | undefined): FileSearchCriteria { + return new FileSearchCriteria(this.contentType, accessStatus, this.categoryName); } - withAccessStatus(accessStatus: FileAccessStatus | undefined): FileCriteria { - return new FileCriteria(this.orderCriteria, this.contentType, accessStatus, this.categoryName); + withCategoryName(categoryName: string | undefined): FileSearchCriteria { + return new FileSearchCriteria(this.contentType, this.accessStatus, categoryName); } - withCategoryName(categoryName: string | undefined): FileCriteria { - return new FileCriteria(this.orderCriteria, this.contentType, this.accessStatus, categoryName); - } - - withSearchText(searchText: string | undefined): FileCriteria { - return new FileCriteria(this.orderCriteria, this.contentType, this.accessStatus, this.categoryName, searchText); + withSearchText(searchText: string | undefined): FileSearchCriteria { + return new FileSearchCriteria(this.contentType, this.accessStatus, this.categoryName, searchText); } } diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index ce3f2a9e..a2354605 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -1,7 +1,7 @@ import { File } from '../models/File'; import { FileDataTable } from '../models/FileDataTable'; import { FileUserPermissions } from '../models/FileUserPermissions'; -import { FileCriteria } from '../models/FileCriteria'; +import { FileSearchCriteria, FileOrderCriteria } from '../models/FileCriteria'; import { FileCounts } from '../models/FileCounts'; import { FileDownloadSizeMode } from '../models/FileDownloadSizeMode'; @@ -10,9 +10,10 @@ export interface IFilesRepository { datasetId: number | string, datasetVersionId: string, includeDeaccessioned: boolean, + fileOrderCriteria: FileOrderCriteria, limit?: number, offset?: number, - fileCriteria?: FileCriteria, + fileSearchCriteria?: FileSearchCriteria, ): Promise; getDatasetFileCounts( diff --git a/src/files/domain/useCases/GetDatasetFiles.ts b/src/files/domain/useCases/GetDatasetFiles.ts index 4d2941bd..e6536aee 100644 --- a/src/files/domain/useCases/GetDatasetFiles.ts +++ b/src/files/domain/useCases/GetDatasetFiles.ts @@ -1,7 +1,7 @@ import { UseCase } from '../../../core/domain/useCases/UseCase'; import { IFilesRepository } from '../repositories/IFilesRepository'; import { File } from '../models/File'; -import { FileCriteria } from '../models/FileCriteria'; +import { FileSearchCriteria, FileOrderCriteria } from '../models/FileCriteria'; import { DatasetNotNumberedVersion } from '../../../datasets'; export class GetDatasetFiles implements UseCase { @@ -17,15 +17,17 @@ export class GetDatasetFiles implements UseCase { includeDeaccessioned: boolean = false, limit?: number, offset?: number, - fileCriteria?: FileCriteria, + fileSearchCriteria?: FileSearchCriteria, + fileOrderCriteria: FileOrderCriteria = FileOrderCriteria.NAME_AZ, ): Promise { return await this.filesRepository.getDatasetFiles( datasetId, datasetVersionId, includeDeaccessioned, + fileOrderCriteria, limit, offset, - fileCriteria, + fileSearchCriteria, ); } } diff --git a/src/files/index.ts b/src/files/index.ts index 8cc3eeea..ab8fd434 100644 --- a/src/files/index.ts +++ b/src/files/index.ts @@ -26,7 +26,7 @@ export { export { File, FileEmbargo, FileChecksum } from './domain/models/File'; export { FileUserPermissions } from './domain/models/FileUserPermissions'; -export { FileCriteria, FileOrderCriteria, FileAccessStatus } from './domain/models/FileCriteria'; +export { FileSearchCriteria, FileOrderCriteria, FileAccessStatus } from './domain/models/FileCriteria'; export { FileCounts, FileContentTypeCount, diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 7d2e71be..40c0c8be 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -6,7 +6,7 @@ import { FileDataTable } from '../../domain/models/FileDataTable'; import { transformDataTablesResponseToDataTables } from './transformers/fileDataTableTransformers'; import { FileUserPermissions } from '../../domain/models/FileUserPermissions'; import { transformFileUserPermissionsResponseToFileUserPermissions } from './transformers/fileUserPermissionsTransformers'; -import { FileCriteria } from '../../domain/models/FileCriteria'; +import { FileSearchCriteria, FileOrderCriteria } from '../../domain/models/FileCriteria'; import { FileCounts } from '../../domain/models/FileCounts'; import { transformFileCountsResponseToFileCounts } from './transformers/fileCountsTransformers'; import { FileDownloadSizeMode } from '../../domain/models/FileDownloadSizeMode'; @@ -31,12 +31,14 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { datasetId: number | string, datasetVersionId: string, includeDeaccessioned: boolean, + fileOrderCriteria: FileOrderCriteria, limit?: number, offset?: number, - fileCriteria?: FileCriteria, + fileSearchCriteria?: FileSearchCriteria, ): Promise { const queryParams: GetFilesQueryParams = { includeDeaccessioned: includeDeaccessioned, + orderCriteria: fileOrderCriteria.toString(), }; if (limit !== undefined) { queryParams.limit = limit; @@ -44,8 +46,8 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { if (offset !== undefined) { queryParams.offset = offset; } - if (fileCriteria !== undefined) { - this.applyFileCriteriaToQueryParams(queryParams, fileCriteria); + if (fileSearchCriteria !== undefined) { + this.applyFileSearchCriteriaToQueryParams(queryParams, fileSearchCriteria); } return this.doGet( this.buildApiEndpoint(this.datasetsResourceName, `versions/${datasetVersionId}/files`, datasetId), @@ -118,21 +120,21 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { }); } - private applyFileCriteriaToQueryParams(queryParams: GetFilesQueryParams, fileCriteria: FileCriteria) { - if (fileCriteria.accessStatus !== undefined) { - queryParams.accessStatus = fileCriteria.accessStatus.toString(); + private applyFileSearchCriteriaToQueryParams( + queryParams: GetFilesQueryParams, + fileSearchCriteria: FileSearchCriteria, + ) { + if (fileSearchCriteria.accessStatus !== undefined) { + queryParams.accessStatus = fileSearchCriteria.accessStatus.toString(); } - if (fileCriteria.categoryName !== undefined) { - queryParams.categoryName = fileCriteria.categoryName; + if (fileSearchCriteria.categoryName !== undefined) { + queryParams.categoryName = fileSearchCriteria.categoryName; } - if (fileCriteria.contentType !== undefined) { - queryParams.contentType = fileCriteria.contentType; + if (fileSearchCriteria.contentType !== undefined) { + queryParams.contentType = fileSearchCriteria.contentType; } - if (fileCriteria.searchText !== undefined) { - queryParams.searchText = fileCriteria.searchText; - } - if (fileCriteria.orderCriteria !== undefined) { - queryParams.orderCriteria = fileCriteria.orderCriteria.toString(); + if (fileSearchCriteria.searchText !== undefined) { + queryParams.searchText = fileSearchCriteria.searchText; } } } diff --git a/test/integration/environment/.env b/test/integration/environment/.env index 57062b3b..b0d68a17 100644 --- a/test/integration/environment/.env +++ b/test/integration/environment/.env @@ -2,5 +2,5 @@ POSTGRES_VERSION=13 DATAVERSE_DB_USER=dataverse SOLR_VERSION=9.3.0 DATAVERSE_IMAGE_REGISTRY=ghcr.io -DATAVERSE_IMAGE_TAG=9958-dataset-files-size +DATAVERSE_IMAGE_TAG=9907-file-counts-with-criteria DATAVERSE_BOOTSTRAP_TIMEOUT=5m diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index c88b8d3b..0f6ae4a6 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -7,7 +7,7 @@ import { createDatasetViaApi } from '../../testHelpers/datasets/datasetHelper'; 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'; +import { FileSearchCriteria, FileAccessStatus, FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria'; import { DatasetNotNumberedVersion } from '../../../src/datasets'; import { FileCounts } from '../../../src/files/domain/models/FileCounts'; import { FileDownloadSizeMode } from '../../../src'; @@ -67,6 +67,7 @@ describe('FilesRepository', () => { TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false, + FileOrderCriteria.NAME_AZ, ); const testFile = currentTestFiles[0]; setFileCategoriesViaApi(testFile.id, [testCategoryName]) @@ -78,14 +79,18 @@ describe('FilesRepository', () => { }); describe('getDatasetFiles', () => { - const testFileCriteria = new FileCriteria() - .withOrderCriteria(FileOrderCriteria.NEWEST) + const testFileCriteria = new FileSearchCriteria() .withContentType('text/plain') .withAccessStatus(FileAccessStatus.PUBLIC); describe('by numeric id', () => { test('should return all files filtering by dataset id and version id', async () => { - const actual = await sut.getDatasetFiles(TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false); + const actual = await sut.getDatasetFiles( + TestConstants.TEST_CREATED_DATASET_ID, + latestDatasetVersionId, + false, + FileOrderCriteria.NAME_AZ, + ); assert.match(actual.length, 4); assert.match(actual[0].name, testTextFile1Name); assert.match(actual[1].name, testTextFile2Name); @@ -98,6 +103,7 @@ describe('FilesRepository', () => { TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false, + FileOrderCriteria.NAME_AZ, 3, 3, undefined, @@ -106,11 +112,12 @@ describe('FilesRepository', () => { assert.match(actual[0].name, testTabFile4Name); }); - test('should return correct files filtering by dataset id, version id, and applying file criteria', async () => { + test('should return correct files filtering by dataset id, version id, and applying newest file criteria', async () => { let actual = await sut.getDatasetFiles( TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false, + FileOrderCriteria.NEWEST, undefined, undefined, testFileCriteria, @@ -125,7 +132,9 @@ describe('FilesRepository', () => { let error: ReadError = undefined; const nonExistentTestDatasetId = 100; - await sut.getDatasetFiles(nonExistentTestDatasetId, latestDatasetVersionId, false).catch((e) => (error = e)); + await sut + .getDatasetFiles(nonExistentTestDatasetId, latestDatasetVersionId, false, FileOrderCriteria.NAME_AZ) + .catch((e) => (error = e)); assert.match( error.message, @@ -140,7 +149,12 @@ describe('FilesRepository', () => { TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, ); - const actual = await sut.getDatasetFiles(testDataset.persistentId, latestDatasetVersionId, false); + const actual = await sut.getDatasetFiles( + testDataset.persistentId, + latestDatasetVersionId, + false, + FileOrderCriteria.NAME_AZ, + ); assert.match(actual.length, 4); assert.match(actual[0].name, testTextFile1Name); assert.match(actual[1].name, testTextFile2Name); @@ -157,6 +171,7 @@ describe('FilesRepository', () => { testDataset.persistentId, latestDatasetVersionId, false, + FileOrderCriteria.NAME_AZ, 3, 3, undefined, @@ -165,7 +180,7 @@ describe('FilesRepository', () => { assert.match(actual[0].name, testTabFile4Name); }); - test('should return correct files filtering by persistent id, version id, and applying file criteria', async () => { + test('should return correct files filtering by persistent id, version id, and applying newest file criteria', async () => { const testDataset = await datasetRepository.getDataset( TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, @@ -174,6 +189,7 @@ describe('FilesRepository', () => { testDataset.persistentId, latestDatasetVersionId, false, + FileOrderCriteria.NEWEST, undefined, undefined, testFileCriteria, @@ -188,7 +204,9 @@ describe('FilesRepository', () => { let error: ReadError = undefined; const testWrongPersistentId = 'wrongPersistentId'; - await sut.getDatasetFiles(testWrongPersistentId, latestDatasetVersionId, false).catch((e) => (error = e)); + await sut + .getDatasetFiles(testWrongPersistentId, latestDatasetVersionId, false, FileOrderCriteria.NAME_AZ) + .catch((e) => (error = e)); assert.match( error.message, @@ -282,6 +300,7 @@ describe('FilesRepository', () => { TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false, + FileOrderCriteria.NAME_AZ, ); const testFile = currentTestFiles[0]; const actual = await sut.getFileDownloadCount(testFile.id); @@ -306,6 +325,7 @@ describe('FilesRepository', () => { TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false, + FileOrderCriteria.NAME_AZ, ); const testFile = currentTestFiles[0]; const actual = await sut.getFileUserPermissions(testFile.id); @@ -331,6 +351,7 @@ describe('FilesRepository', () => { TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false, + FileOrderCriteria.NAME_AZ, ); const testFile = currentTestFiles[3]; const actual = await sut.getFileDataTables(testFile.id); @@ -342,6 +363,7 @@ describe('FilesRepository', () => { TestConstants.TEST_CREATED_DATASET_ID, latestDatasetVersionId, false, + FileOrderCriteria.NAME_AZ, ); const testFile = currentTestFiles[0]; diff --git a/test/unit/files/FilesRepository.test.ts b/test/unit/files/FilesRepository.test.ts index 81d12278..6049593c 100644 --- a/test/unit/files/FilesRepository.test.ts +++ b/test/unit/files/FilesRepository.test.ts @@ -8,7 +8,7 @@ import { TestConstants } from '../../testHelpers/TestConstants'; import { createFilePayload, createFileModel } from '../../testHelpers/files/filesHelper'; import { createFileDataTablePayload, createFileDataTableModel } from '../../testHelpers/files/fileDataTablesHelper'; import { createFileUserPermissionsModel } from '../../testHelpers/files/fileUserPermissionsHelper'; -import { FileCriteria, FileAccessStatus, FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria'; +import { FileSearchCriteria, FileAccessStatus, FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria'; import { DatasetNotNumberedVersion } from '../../../src/datasets'; import { createFileCountsModel, createFileCountsPayload } from '../../testHelpers/files/fileCountsHelper'; import { createFilesTotalDownloadSizePayload } from '../../testHelpers/files/filesTotalDownloadSizeHelper'; @@ -43,30 +43,32 @@ describe('FilesRepository', () => { const testOffset = 20; const testCategory = 'testCategory'; const testContentType = 'testContentType'; - const testFileCriteria = new FileCriteria() - .withOrderCriteria(FileOrderCriteria.NAME_ZA) + const testFileCriteria = new FileSearchCriteria() .withCategoryName(testCategory) .withContentType(testContentType) .withAccessStatus(FileAccessStatus.PUBLIC); + const testFileOrderCriteria = FileOrderCriteria.NAME_ZA; const expectedRequestConfigApiKey = { - params: { includeDeaccessioned: testIncludeDeaccessioned }, + params: { includeDeaccessioned: testIncludeDeaccessioned, orderCriteria: testFileOrderCriteria }, headers: TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY.headers, }; const expectedRequestConfigSessionCookie = { - params: { includeDeaccessioned: testIncludeDeaccessioned }, + params: { includeDeaccessioned: testIncludeDeaccessioned, orderCriteria: testFileOrderCriteria }, withCredentials: TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_SESSION_COOKIE.withCredentials, headers: TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_SESSION_COOKIE.headers, }; + const expectedRequestParamsWithOptional = { includeDeaccessioned: testIncludeDeaccessioned, limit: testLimit, offset: testOffset, - orderCriteria: testFileCriteria.orderCriteria.toString(), - categoryName: testFileCriteria.categoryName, + orderCriteria: testFileOrderCriteria, contentType: testFileCriteria.contentType, accessStatus: testFileCriteria.accessStatus.toString(), + categoryName: testFileCriteria.categoryName, }; + const expectedRequestConfigApiKeyWithOptional = { params: expectedRequestParamsWithOptional, headers: TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY.headers, @@ -80,7 +82,12 @@ describe('FilesRepository', () => { const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/datasets/${testDatasetId}/versions/${testDatasetVersionId}/files`; // API Key auth - let actual = await sut.getDatasetFiles(testDatasetId, testDatasetVersionId, testIncludeDeaccessioned); + let actual = await sut.getDatasetFiles( + testDatasetId, + testDatasetVersionId, + testIncludeDeaccessioned, + testFileOrderCriteria, + ); assert.calledWithExactly(axiosGetStub, expectedApiEndpoint, expectedRequestConfigApiKey); assert.match(actual, expectedFiles); @@ -88,7 +95,12 @@ describe('FilesRepository', () => { // Session cookie auth ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.SESSION_COOKIE); - actual = await sut.getDatasetFiles(testDatasetId, testDatasetVersionId, testIncludeDeaccessioned); + actual = await sut.getDatasetFiles( + testDatasetId, + testDatasetVersionId, + testIncludeDeaccessioned, + testFileOrderCriteria, + ); assert.calledWithExactly(axiosGetStub, expectedApiEndpoint, expectedRequestConfigSessionCookie); assert.match(actual, expectedFiles); @@ -101,6 +113,7 @@ describe('FilesRepository', () => { testDatasetId, testDatasetVersionId, testIncludeDeaccessioned, + testFileOrderCriteria, testLimit, testOffset, testFileCriteria, @@ -119,7 +132,7 @@ describe('FilesRepository', () => { let error: ReadError = undefined; await sut - .getDatasetFiles(testDatasetId, testDatasetVersionId, testIncludeDeaccessioned) + .getDatasetFiles(testDatasetId, testDatasetVersionId, testIncludeDeaccessioned, testFileOrderCriteria) .catch((e) => (error = e)); assert.calledWithExactly( @@ -141,6 +154,7 @@ describe('FilesRepository', () => { TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId, testIncludeDeaccessioned, + testFileOrderCriteria, ); assert.calledWithExactly(axiosGetStub, expectedApiEndpoint, expectedRequestConfigApiKey); @@ -153,6 +167,7 @@ describe('FilesRepository', () => { TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId, testIncludeDeaccessioned, + testFileOrderCriteria, ); assert.calledWithExactly(axiosGetStub, expectedApiEndpoint, expectedRequestConfigSessionCookie); @@ -166,6 +181,7 @@ describe('FilesRepository', () => { TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId, testIncludeDeaccessioned, + testFileOrderCriteria, testLimit, testOffset, testFileCriteria, @@ -184,7 +200,12 @@ describe('FilesRepository', () => { let error: ReadError = undefined; await sut - .getDatasetFiles(TestConstants.TEST_DUMMY_PERSISTENT_ID, testDatasetVersionId, testIncludeDeaccessioned) + .getDatasetFiles( + TestConstants.TEST_DUMMY_PERSISTENT_ID, + testDatasetVersionId, + testIncludeDeaccessioned, + testFileOrderCriteria, + ) .catch((e) => (error = e)); assert.calledWithExactly( diff --git a/test/unit/files/GetDatasetFiles.test.ts b/test/unit/files/GetDatasetFiles.test.ts index 840e4bdb..fb850a57 100644 --- a/test/unit/files/GetDatasetFiles.test.ts +++ b/test/unit/files/GetDatasetFiles.test.ts @@ -5,6 +5,7 @@ import { ReadError } from '../../../src/core/domain/repositories/ReadError'; import { File } from '../../../src/files/domain/models/File'; import { createFileModel } from '../../testHelpers/files/filesHelper'; import { DatasetNotNumberedVersion } from '../../../src/datasets'; +import { FileOrderCriteria } from '../../../src/files/domain/models/FileCriteria'; describe('execute', () => { const sandbox: SinonSandbox = createSandbox(); @@ -23,7 +24,7 @@ describe('execute', () => { const actual = await sut.execute(1); assert.match(actual, testFiles); - assert.calledWithExactly(getDatasetFilesStub, 1, DatasetNotNumberedVersion.LATEST, false, undefined, undefined, undefined); + assert.calledWithExactly(getDatasetFilesStub, 1, DatasetNotNumberedVersion.LATEST, false, FileOrderCriteria.NAME_AZ, undefined, undefined, undefined); }); test('should return error result on repository error', async () => { From 9929566bb57eaef7a4e2b80567d8d56bce854585 Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 10 Oct 2023 11:48:23 +0100 Subject: [PATCH 2/4] Added: FileSearchCriteria added to GetDatasetFileCounts use case --- .../domain/repositories/IFilesRepository.ts | 1 + .../domain/useCases/GetDatasetFileCounts.ts | 9 +++- .../infra/repositories/FilesRepository.ts | 11 +++-- .../integration/files/FilesRepository.test.ts | 35 +++++++++++++++ test/unit/files/FilesRepository.test.ts | 43 ++++++++++++++++--- test/unit/files/GetDatasetFileCounts.test.ts | 2 +- 6 files changed, 90 insertions(+), 11 deletions(-) diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index a2354605..b91cbf86 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -20,6 +20,7 @@ export interface IFilesRepository { datasetId: number | string, datasetVersionId: string, includeDeaccessioned: boolean, + fileSearchCriteria?: FileSearchCriteria, ): Promise; getDatasetFilesTotalDownloadSize( diff --git a/src/files/domain/useCases/GetDatasetFileCounts.ts b/src/files/domain/useCases/GetDatasetFileCounts.ts index 00803570..2ca43c03 100644 --- a/src/files/domain/useCases/GetDatasetFileCounts.ts +++ b/src/files/domain/useCases/GetDatasetFileCounts.ts @@ -2,6 +2,7 @@ import { UseCase } from '../../../core/domain/useCases/UseCase'; import { IFilesRepository } from '../repositories/IFilesRepository'; import { DatasetNotNumberedVersion } from '../../../datasets'; import { FileCounts } from '../models/FileCounts'; +import { FileSearchCriteria } from '../models/FileCriteria'; export class GetDatasetFileCounts implements UseCase { private filesRepository: IFilesRepository; @@ -14,7 +15,13 @@ export class GetDatasetFileCounts implements UseCase { datasetId: number | string, datasetVersionId: string | DatasetNotNumberedVersion = DatasetNotNumberedVersion.LATEST, includeDeaccessioned: boolean = false, + fileSearchCriteria?: FileSearchCriteria, ): Promise { - return await this.filesRepository.getDatasetFileCounts(datasetId, datasetVersionId, includeDeaccessioned); + return await this.filesRepository.getDatasetFileCounts( + datasetId, + datasetVersionId, + includeDeaccessioned, + fileSearchCriteria, + ); } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 40c0c8be..5af61ea2 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -64,13 +64,18 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { datasetId: string | number, datasetVersionId: string, includeDeaccessioned: boolean, + fileSearchCriteria?: FileSearchCriteria, ): Promise { + const queryParams: GetFilesQueryParams = { + includeDeaccessioned: includeDeaccessioned, + }; + if (fileSearchCriteria !== undefined) { + this.applyFileSearchCriteriaToQueryParams(queryParams, fileSearchCriteria); + } return this.doGet( this.buildApiEndpoint(this.datasetsResourceName, `versions/${datasetVersionId}/files/counts`, datasetId), true, - { - includeDeaccessioned: includeDeaccessioned, - }, + queryParams, ) .then((response) => transformFileCountsResponseToFileCounts(response)) .catch((error) => { diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index 0f6ae4a6..bd9e0576 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -255,6 +255,41 @@ describe('FilesRepository', () => { expect(actual.perCategoryName).to.have.deep.members(expectedFileCounts.perCategoryName); }); + test('should return file count filtering by numeric id and applying criteria', async () => { + const expectedFileCountsForCriteria: FileCounts = { + total: 1, + perContentType: [ + { + contentType: 'text/plain', + count: 1, + }, + ], + perAccessStatus: [ + { + accessStatus: FileAccessStatus.PUBLIC, + count: 1, + }, + ], + perCategoryName: [ + { + categoryName: testCategoryName, + count: 1, + }, + ], + }; + const testCriteria = new FileSearchCriteria().withCategoryName(testCategoryName); + const actual = await sut.getDatasetFileCounts( + TestConstants.TEST_CREATED_DATASET_ID, + latestDatasetVersionId, + false, + testCriteria, + ); + assert.match(actual.total, expectedFileCountsForCriteria.total); + expect(actual.perContentType).to.have.deep.members(expectedFileCountsForCriteria.perContentType); + expect(actual.perAccessStatus).to.have.deep.members(expectedFileCountsForCriteria.perAccessStatus); + expect(actual.perCategoryName).to.have.deep.members(expectedFileCountsForCriteria.perCategoryName); + }); + test('should return file count filtering by persistent id', async () => { const testDataset = await datasetRepository.getDataset( TestConstants.TEST_CREATED_DATASET_ID, diff --git a/test/unit/files/FilesRepository.test.ts b/test/unit/files/FilesRepository.test.ts index 6049593c..ebd24395 100644 --- a/test/unit/files/FilesRepository.test.ts +++ b/test/unit/files/FilesRepository.test.ts @@ -22,6 +22,12 @@ describe('FilesRepository', () => { const testDatasetVersionId = DatasetNotNumberedVersion.LATEST; const testDatasetId = 1; const testIncludeDeaccessioned = false; + const testCategory = 'testCategory'; + const testContentType = 'testContentType'; + const testFileCriteria = new FileSearchCriteria() + .withCategoryName(testCategory) + .withContentType(testContentType) + .withAccessStatus(FileAccessStatus.PUBLIC); beforeEach(() => { ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.API_KEY, TestConstants.TEST_DUMMY_API_KEY); @@ -41,12 +47,6 @@ describe('FilesRepository', () => { const testLimit = 10; const testOffset = 20; - const testCategory = 'testCategory'; - const testContentType = 'testContentType'; - const testFileCriteria = new FileSearchCriteria() - .withCategoryName(testCategory) - .withContentType(testContentType) - .withAccessStatus(FileAccessStatus.PUBLIC); const testFileOrderCriteria = FileOrderCriteria.NAME_ZA; const expectedRequestConfigApiKey = { @@ -236,6 +236,18 @@ describe('FilesRepository', () => { headers: TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_SESSION_COOKIE.headers, }; + const expectedRequestParamsWithOptional = { + includeDeaccessioned: testIncludeDeaccessioned, + contentType: testFileCriteria.contentType, + accessStatus: testFileCriteria.accessStatus.toString(), + categoryName: testFileCriteria.categoryName, + }; + + const expectedRequestConfigApiKeyWithOptional = { + params: expectedRequestParamsWithOptional, + headers: TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY.headers, + }; + const expectedCount = createFileCountsModel(); describe('by numeric id and version id', () => { @@ -258,6 +270,25 @@ describe('FilesRepository', () => { assert.match(actual, expectedCount); }); + test('should return file counts when providing id, version id, optional params, 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`; + + const actual = await sut.getDatasetFileCounts( + testDatasetId, + testDatasetVersionId, + testIncludeDeaccessioned, + testFileCriteria, + ); + + assert.calledWithExactly( + axiosGetStub, + expectedApiEndpoint, + expectedRequestConfigApiKeyWithOptional, + ); + assert.match(actual, expectedCount); + }); + test('should return error result on error response', async () => { const axiosGetStub = sandbox.stub(axios, 'get').rejects(TestConstants.TEST_ERROR_RESPONSE); diff --git a/test/unit/files/GetDatasetFileCounts.test.ts b/test/unit/files/GetDatasetFileCounts.test.ts index ce840967..4576290e 100644 --- a/test/unit/files/GetDatasetFileCounts.test.ts +++ b/test/unit/files/GetDatasetFileCounts.test.ts @@ -23,7 +23,7 @@ describe('execute', () => { const actual = await sut.execute(1); assert.match(actual, testFileCounts); - assert.calledWithExactly(getDatasetFileCountsStub, 1, DatasetNotNumberedVersion.LATEST, false); + assert.calledWithExactly(getDatasetFileCountsStub, 1, DatasetNotNumberedVersion.LATEST, false, undefined); }); test('should return error result on repository error', async () => { From ad136d77b621daafafb688fbf4e214ff8ddc5e6d Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 10 Oct 2023 12:12:20 +0100 Subject: [PATCH 3/4] Added: tabular tag name to FileSearchCriteria --- src/files/domain/models/FileCriteria.ts | 43 +++++++++++++++++-- .../infra/repositories/FilesRepository.ts | 4 ++ .../integration/files/FilesRepository.test.ts | 2 +- test/unit/files/FilesRepository.test.ts | 16 +++---- 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/files/domain/models/FileCriteria.ts b/src/files/domain/models/FileCriteria.ts index 95de6acc..3285624a 100644 --- a/src/files/domain/models/FileCriteria.ts +++ b/src/files/domain/models/FileCriteria.ts @@ -3,23 +3,58 @@ export class FileSearchCriteria { public readonly contentType?: string, public readonly accessStatus?: FileAccessStatus, public readonly categoryName?: string, + public readonly tabularTagName?: string, public readonly searchText?: string, ) {} withContentType(contentType: string | undefined): FileSearchCriteria { - return new FileSearchCriteria(contentType, this.accessStatus, this.categoryName); + return new FileSearchCriteria( + contentType, + this.accessStatus, + this.categoryName, + this.tabularTagName, + this.searchText, + ); } withAccessStatus(accessStatus: FileAccessStatus | undefined): FileSearchCriteria { - return new FileSearchCriteria(this.contentType, accessStatus, this.categoryName); + return new FileSearchCriteria( + this.contentType, + accessStatus, + this.categoryName, + this.tabularTagName, + this.searchText, + ); } withCategoryName(categoryName: string | undefined): FileSearchCriteria { - return new FileSearchCriteria(this.contentType, this.accessStatus, categoryName); + return new FileSearchCriteria( + this.contentType, + this.accessStatus, + categoryName, + this.tabularTagName, + this.searchText, + ); + } + + withTabularTagName(tabularTagName: string | undefined): FileSearchCriteria { + return new FileSearchCriteria( + this.contentType, + this.accessStatus, + this.categoryName, + tabularTagName, + this.searchText, + ); } withSearchText(searchText: string | undefined): FileSearchCriteria { - return new FileSearchCriteria(this.contentType, this.accessStatus, this.categoryName, searchText); + return new FileSearchCriteria( + this.contentType, + this.accessStatus, + this.categoryName, + this.tabularTagName, + searchText, + ); } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 5af61ea2..19a8f210 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -19,6 +19,7 @@ export interface GetFilesQueryParams { contentType?: string; accessStatus?: string; categoryName?: string; + tabularTagName?: string; searchText?: string; } @@ -135,6 +136,9 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { if (fileSearchCriteria.categoryName !== undefined) { queryParams.categoryName = fileSearchCriteria.categoryName; } + if (fileSearchCriteria.tabularTagName !== undefined) { + queryParams.tabularTagName = fileSearchCriteria.tabularTagName; + } if (fileSearchCriteria.contentType !== undefined) { queryParams.contentType = fileSearchCriteria.contentType; } diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index bd9e0576..8e5918ae 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -255,7 +255,7 @@ describe('FilesRepository', () => { expect(actual.perCategoryName).to.have.deep.members(expectedFileCounts.perCategoryName); }); - test('should return file count filtering by numeric id and applying criteria', async () => { + test('should return file count filtering by numeric id and applying category criteria', async () => { const expectedFileCountsForCriteria: FileCounts = { total: 1, perContentType: [ diff --git a/test/unit/files/FilesRepository.test.ts b/test/unit/files/FilesRepository.test.ts index ebd24395..e0ef6c3d 100644 --- a/test/unit/files/FilesRepository.test.ts +++ b/test/unit/files/FilesRepository.test.ts @@ -23,11 +23,13 @@ describe('FilesRepository', () => { const testDatasetId = 1; const testIncludeDeaccessioned = false; const testCategory = 'testCategory'; + const testTabularTagName = 'testTabularTagName'; const testContentType = 'testContentType'; const testFileCriteria = new FileSearchCriteria() - .withCategoryName(testCategory) - .withContentType(testContentType) - .withAccessStatus(FileAccessStatus.PUBLIC); + .withCategoryName(testCategory) + .withContentType(testContentType) + .withAccessStatus(FileAccessStatus.PUBLIC) + .withTabularTagName(testTabularTagName); beforeEach(() => { ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.API_KEY, TestConstants.TEST_DUMMY_API_KEY); @@ -67,6 +69,7 @@ describe('FilesRepository', () => { contentType: testFileCriteria.contentType, accessStatus: testFileCriteria.accessStatus.toString(), categoryName: testFileCriteria.categoryName, + tabularTagName: testFileCriteria.tabularTagName, }; const expectedRequestConfigApiKeyWithOptional = { @@ -241,6 +244,7 @@ describe('FilesRepository', () => { contentType: testFileCriteria.contentType, accessStatus: testFileCriteria.accessStatus.toString(), categoryName: testFileCriteria.categoryName, + tabularTagName: testFileCriteria.tabularTagName, }; const expectedRequestConfigApiKeyWithOptional = { @@ -281,11 +285,7 @@ describe('FilesRepository', () => { testFileCriteria, ); - assert.calledWithExactly( - axiosGetStub, - expectedApiEndpoint, - expectedRequestConfigApiKeyWithOptional, - ); + assert.calledWithExactly(axiosGetStub, expectedApiEndpoint, expectedRequestConfigApiKeyWithOptional); assert.match(actual, expectedCount); }); From 899638061b9558e598b7a92f3998be5ac74389fb Mon Sep 17 00:00:00 2001 From: GPortas Date: Tue, 10 Oct 2023 12:23:31 +0100 Subject: [PATCH 4/4] Changed: skipping getDatasetFilesTotalDownloadSize IT --- test/integration/files/FilesRepository.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index 8e5918ae..781f0c53 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -303,7 +303,8 @@ describe('FilesRepository', () => { }); }); - describe('getDatasetFilesTotalDownloadSize', () => { + // TODO: Remove skip once PR https://github.com/IQSS/dataverse/pull/9960 is merged + describe.skip('getDatasetFilesTotalDownloadSize', () => { const expectedTotalDownloadSize = 193; // 193 bytes test('should return total download size filtering by numeric id and ignoring original tabular size', async () => {