From 26da8e6986fda569eff419bc04c05f61687beffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 5 Feb 2025 16:51:45 -0300 Subject: [PATCH 1/7] refactor: allow undefined operation for deleting file by persistent id --- src/core/infra/repositories/ApiRepository.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/core/infra/repositories/ApiRepository.ts b/src/core/infra/repositories/ApiRepository.ts index d9be61f4..8ac423db 100644 --- a/src/core/infra/repositories/ApiRepository.ts +++ b/src/core/infra/repositories/ApiRepository.ts @@ -47,14 +47,16 @@ export abstract class ApiRepository { protected buildApiEndpoint( resourceName: string, - operation: string, - resourceId: number | string | undefined = undefined + operation?: string, + resourceId?: number | string ) { + const operationSegment = operation ? `/${operation}` : '' + return typeof resourceId === 'number' - ? `/${resourceName}/${resourceId}/${operation}` + ? `/${resourceName}/${resourceId}${operationSegment}` : typeof resourceId === 'string' - ? `/${resourceName}/:persistentId/${operation}?persistentId=${resourceId}` - : `/${resourceName}/${operation}` + ? `/${resourceName}/:persistentId${operationSegment}?persistentId=${resourceId}` + : `/${resourceName}${operationSegment}` } // eslint-disable-next-line @typescript-eslint/no-explicit-any From 84c923c20072d4cea769b5d761105a0c7a2ed34b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 5 Feb 2025 17:27:34 -0300 Subject: [PATCH 2/7] feat: add use case --- .../domain/repositories/IFilesRepository.ts | 2 ++ src/files/domain/useCases/DeleteFile.ts | 17 +++++++++++++++++ src/files/index.ts | 5 ++++- src/files/infra/repositories/FilesRepository.ts | 8 ++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 src/files/domain/useCases/DeleteFile.ts diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index 0f8ab8b8..8365374e 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -59,4 +59,6 @@ export interface IFilesRepository { datasetId: number | string, uploadedFileDTOs: UploadedFileDTO[] ): Promise + + deleteFile(fileId: number | string): Promise } diff --git a/src/files/domain/useCases/DeleteFile.ts b/src/files/domain/useCases/DeleteFile.ts new file mode 100644 index 00000000..6c99ff9a --- /dev/null +++ b/src/files/domain/useCases/DeleteFile.ts @@ -0,0 +1,17 @@ +import { IFilesRepository } from '../repositories/IFilesRepository' +import { UseCase } from '../../../core/domain/useCases/UseCase' + +export class DeleteFile implements UseCase { + constructor(private readonly filesRepository: IFilesRepository) {} + + /** + * Deletes a file. + * More detailed information about the file deletion behavior can be found in https://guides.dataverse.org/en/latest/api/native-api.html#deleting-files + * + * @param {number | string} [fileId] - The File identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers). + * @returns {Promise} -This method does not return anything upon successful completion. + */ + async execute(fileId: number | string): Promise { + return await this.filesRepository.deleteFile(fileId) + } +} diff --git a/src/files/index.ts b/src/files/index.ts index f621b0c9..0d847852 100644 --- a/src/files/index.ts +++ b/src/files/index.ts @@ -11,6 +11,7 @@ import { GetFileAndDataset } from './domain/useCases/GetFileAndDataset' import { UploadFile } from './domain/useCases/UploadFile' import { DirectUploadClient } from './infra/clients/DirectUploadClient' import { AddUploadedFilesToDataset } from './domain/useCases/AddUploadedFilesToDataset' +import { DeleteFile } from './domain/useCases/DeleteFile' const filesRepository = new FilesRepository() const directUploadClient = new DirectUploadClient(filesRepository) @@ -26,6 +27,7 @@ const getFileAndDataset = new GetFileAndDataset(filesRepository) const getFileCitation = new GetFileCitation(filesRepository) const uploadFile = new UploadFile(directUploadClient) const addUploadedFilesToDataset = new AddUploadedFilesToDataset(filesRepository) +const deleteFile = new DeleteFile(filesRepository) export { getDatasetFiles, @@ -38,7 +40,8 @@ export { getFileAndDataset, getFileCitation, uploadFile, - addUploadedFilesToDataset + addUploadedFilesToDataset, + deleteFile } export { FileModel as File, FileEmbargo, FileChecksum } from './domain/models/FileModel' diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 8fe5adee..9aebe720 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -293,4 +293,12 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { queryParams.searchText = fileSearchCriteria.searchText } } + + public async deleteFile(fileId: number | string): Promise { + return this.doDelete(this.buildApiEndpoint(this.filesResourceName, undefined, fileId)) + .then(() => undefined) + .catch((error) => { + throw error + }) + } } From 524aedef80427e8283f24dd259ff0dab425efe9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 5 Feb 2025 17:27:57 -0300 Subject: [PATCH 3/7] docs: use case documentation --- docs/useCases.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/useCases.md b/docs/useCases.md index 26317d1c..6142a868 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -49,6 +49,7 @@ The different use cases currently available in the package are classified below, - [List Files in a Dataset](#list-files-in-a-dataset) - [Files write use cases](#files-write-use-cases) - [File Uploading Use Cases](#file-uploading-use-cases) + - [Delete a File](#delete-a-file) - [Metadata Blocks](#metadata-blocks) - [Metadata Blocks read use cases](#metadata-blocks-read-use-cases) - [Get All Facetable Metadata Fields](#get-all-facetable-metadata-fields) @@ -1203,6 +1204,26 @@ The following error might arise from the `AddUploadedFileToDataset` use case: - AddUploadedFileToDatasetError: This error indicates that there was an error while adding the uploaded file to the dataset. +#### Delete a File + +Deletes a File. + +##### Example call: + +```typescript +import { deleteFile } from '@iqss/dataverse-client-javascript' + +/* ... */ + +const fileId = 12345 + +deleteFile.execute(fileId) + +/* ... */ +``` + +_See [use case](../src/files/domain/useCases/DeleteFile.ts) implementation_. + ## Metadata Blocks ### Metadata Blocks read use cases From 53b1704298975634b7616f246c43a2cbf5898f97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Thu, 6 Feb 2025 16:26:26 -0300 Subject: [PATCH 4/7] test: add unit, integration and functional tests --- test/functional/files/DeleteFile.test.ts | 89 +++++++++++++++++++ .../integration/files/FilesRepository.test.ts | 81 ++++++++++++++++- test/unit/files/DeleteFile.test.ts | 25 ++++++ 3 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 test/functional/files/DeleteFile.test.ts create mode 100644 test/unit/files/DeleteFile.test.ts diff --git a/test/functional/files/DeleteFile.test.ts b/test/functional/files/DeleteFile.test.ts new file mode 100644 index 00000000..d4e19286 --- /dev/null +++ b/test/functional/files/DeleteFile.test.ts @@ -0,0 +1,89 @@ +import { + ApiConfig, + createDataset, + CreatedDatasetIdentifiers, + deleteFile, + getDatasetFileCounts, + getDatasetFiles, + WriteError +} from '../../../src' +import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig' +import { + createCollectionViaApi, + deleteCollectionViaApi +} from '../../testHelpers/collections/collectionHelper' +import { deleteUnpublishedDatasetViaApi } from '../../testHelpers/datasets/datasetHelper' +import { uploadFileViaApi } from '../../testHelpers/files/filesHelper' +import { TestConstants } from '../../testHelpers/TestConstants' + +describe('execute', () => { + const testCollectionAlias = 'deleteFileFunctionalTest' + let testDatasetIds: CreatedDatasetIdentifiers + const testTextFile1Name = 'test-file-1.txt' + + beforeAll(async () => { + ApiConfig.init( + TestConstants.TEST_API_URL, + DataverseApiAuthMechanism.API_KEY, + process.env.TEST_API_KEY + ) + await createCollectionViaApi(testCollectionAlias) + + try { + testDatasetIds = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + testCollectionAlias + ) + } catch (error) { + throw new Error('Tests beforeAll(): Error while creating test dataset') + } + await uploadFileViaApi(testDatasetIds.numericId, testTextFile1Name).catch(() => { + throw new Error(`Tests beforeAll(): Error while uploading file ${testTextFile1Name}`) + }) + }) + + afterAll(async () => { + try { + await deleteUnpublishedDatasetViaApi(testDatasetIds.numericId) + } catch (error) { + throw new Error('Tests afterAll(): Error while deleting test dataset') + } + try { + await deleteCollectionViaApi(testCollectionAlias) + } catch (error) { + throw new Error('Tests afterAll(): Error while deleting test collection') + } + }) + + test('should successfully delete a file', async () => { + try { + const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId) + console.log({ datasetFiles }) + await deleteFile.execute(datasetFiles.files[0].id) + } catch (error) { + throw new Error('File should be deleted') + } finally { + const datasetFileCounts = await getDatasetFileCounts.execute(testDatasetIds.numericId) + console.log({ datasetFileCounts }) + expect(datasetFileCounts.total).toEqual(0) + } + }) + + test('should throw an error when the file id does not exist', async () => { + expect.assertions(2) + let writeError: WriteError | undefined = undefined + const nonExistentFileId = 5 + + try { + await deleteFile.execute(nonExistentFileId) + throw new Error('Use case should throw an error') + } catch (error) { + writeError = error as WriteError + } finally { + expect(writeError).toBeInstanceOf(WriteError) + expect(writeError?.message).toEqual( + `There was an error when writing the resource. Reason was: [404] File with ID ${nonExistentFileId} not found.` + ) + } + }) +}) diff --git a/test/integration/files/FilesRepository.test.ts b/test/integration/files/FilesRepository.test.ts index 5d8b6821..7aff8c49 100644 --- a/test/integration/files/FilesRepository.test.ts +++ b/test/integration/files/FilesRepository.test.ts @@ -28,7 +28,7 @@ import { } from '../../../src/datasets' import { FileModel } from '../../../src/files/domain/models/FileModel' import { FileCounts } from '../../../src/files/domain/models/FileCounts' -import { FileDownloadSizeMode } from '../../../src' +import { FileDownloadSizeMode, WriteError } from '../../../src' import { deaccessionDatasetViaApi, publishDatasetViaApi, @@ -646,4 +646,83 @@ describe('FilesRepository', () => { ).rejects.toThrow(errorExpected) }) }) + + describe('deleteFile', () => { + let deleFileTestDatasetIds: CreatedDatasetIdentifiers + const testTextFile1Name = 'test-file-1.txt' + + beforeEach(async () => { + try { + deleFileTestDatasetIds = await createDataset.execute(TestConstants.TEST_NEW_DATASET_DTO) + } catch (error) { + throw new Error('Tests beforeEach(): Error while creating test dataset') + } + await uploadFileViaApi(deleFileTestDatasetIds.numericId, testTextFile1Name).catch(() => { + throw new Error(`Tests beforeEach(): Error while uploading file ${testTextFile1Name}`) + }) + }) + + test('should successfully delete a file', async () => { + const datasetFiles = await sut.getDatasetFiles( + deleFileTestDatasetIds.numericId, + latestDatasetVersionId, + false, + FileOrderCriteria.NAME_AZ + ) + await sut.deleteFile(datasetFiles.files[0].id) + + const datasetFileCounts = await sut.getDatasetFileCounts( + deleFileTestDatasetIds.numericId, + latestDatasetVersionId, + false + ) + expect(datasetFileCounts.total).toEqual(0) + + await deleteUnpublishedDatasetViaApi(deleFileTestDatasetIds.numericId) + }) + + test('should delete a file from the draft dataset but not from the published dataset', async () => { + await publishDatasetViaApi(deleFileTestDatasetIds.numericId).catch(() => { + throw new Error('Error while publishing test Dataset') + }) + + await waitForNoLocks(deleFileTestDatasetIds.numericId, 10).catch(() => { + throw new Error('Error while waiting for no locks') + }) + + const datasetFiles = await sut.getDatasetFiles( + deleFileTestDatasetIds.numericId, + latestDatasetVersionId, + false, + FileOrderCriteria.NAME_AZ + ) + await sut.deleteFile(datasetFiles.files[0].id) + + const datasetFileCounts = await sut.getDatasetFileCounts( + deleFileTestDatasetIds.numericId, + DatasetNotNumberedVersion.DRAFT, + false + ) + + expect(datasetFileCounts.total).toEqual(0) + + const publishedDatasetFileCounts = await sut.getDatasetFileCounts( + deleFileTestDatasetIds.numericId, + DatasetNotNumberedVersion.LATEST_PUBLISHED, + false + ) + + expect(publishedDatasetFileCounts.total).toBeGreaterThan(0) + + await deletePublishedDatasetViaApi(deleFileTestDatasetIds.persistentId).catch(() => { + throw new Error('Error while deleting published test Dataset') + }) + }) + + test('should return error when file does not exist', async () => { + const expectedError = new WriteError(`[404] File with ID ${nonExistentFiledId} not found.`) + + await expect(sut.deleteFile(nonExistentFiledId)).rejects.toThrow(expectedError) + }) + }) }) diff --git a/test/unit/files/DeleteFile.test.ts b/test/unit/files/DeleteFile.test.ts new file mode 100644 index 00000000..39b99d83 --- /dev/null +++ b/test/unit/files/DeleteFile.test.ts @@ -0,0 +1,25 @@ +import { IFilesRepository } from '../../../src/files/domain/repositories/IFilesRepository' +import { WriteError } from '../../../src' +import { DeleteFile } from '../../../src/files/domain/useCases/DeleteFile' + +describe('execute', () => { + test('should return undefined when repository call is successful', async () => { + const filesRepositoryStub: IFilesRepository = {} as IFilesRepository + filesRepositoryStub.deleteFile = jest.fn().mockResolvedValue(undefined) + + const sut = new DeleteFile(filesRepositoryStub) + + const actual = await sut.execute(1) + + expect(actual).toEqual(undefined) + }) + + test('should return error result on repository error', async () => { + const filesRepositoryStub: IFilesRepository = {} as IFilesRepository + filesRepositoryStub.deleteFile = jest.fn().mockRejectedValue(new WriteError()) + + const sut = new DeleteFile(filesRepositoryStub) + + await expect(sut.execute(1)).rejects.toThrow(WriteError) + }) +}) From f59f53b1d14685f4e8bd4ee27247af73c34999ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Mon, 10 Feb 2025 08:18:37 -0300 Subject: [PATCH 5/7] remove logs and add doc line --- docs/useCases.md | 2 ++ test/functional/files/DeleteFile.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/useCases.md b/docs/useCases.md index 6142a868..0808413d 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1224,6 +1224,8 @@ deleteFile.execute(fileId) _See [use case](../src/files/domain/useCases/DeleteFile.ts) implementation_. +The `fileId` parameter can be a string, for persistent identifiers, or a number, for numeric identifiers. + ## Metadata Blocks ### Metadata Blocks read use cases diff --git a/test/functional/files/DeleteFile.test.ts b/test/functional/files/DeleteFile.test.ts index d4e19286..c16ef476 100644 --- a/test/functional/files/DeleteFile.test.ts +++ b/test/functional/files/DeleteFile.test.ts @@ -58,13 +58,13 @@ describe('execute', () => { test('should successfully delete a file', async () => { try { const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId) - console.log({ datasetFiles }) + await deleteFile.execute(datasetFiles.files[0].id) } catch (error) { throw new Error('File should be deleted') } finally { const datasetFileCounts = await getDatasetFileCounts.execute(testDatasetIds.numericId) - console.log({ datasetFileCounts }) + expect(datasetFileCounts.total).toEqual(0) } }) From 2d585b0c60e2444be005713484ab5a927b738001 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Mon, 10 Feb 2025 08:26:13 -0300 Subject: [PATCH 6/7] doc: extra doc from native api guide --- docs/useCases.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/useCases.md b/docs/useCases.md index 0808413d..a00d9972 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1226,6 +1226,12 @@ _See [use case](../src/files/domain/useCases/DeleteFile.ts) implementation_. The `fileId` parameter can be a string, for persistent identifiers, or a number, for numeric identifiers. +Note that the behavior of deleting files depends on if the dataset has ever been published or not. + +- If the dataset has never been published, the file will be deleted forever. +- If the dataset has published, the file is deleted from the draft (and future published versions). +- If the dataset has published, the deleted file can still be downloaded because it was part of a published version. + ## Metadata Blocks ### Metadata Blocks read use cases From 9646936fc54fa57ae0607b7b2a743f5643cc2c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Mon, 10 Feb 2025 14:40:19 -0300 Subject: [PATCH 7/7] test: add filesRepository cases --- test/unit/files/FilesRepository.test.ts | 70 ++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/test/unit/files/FilesRepository.test.ts b/test/unit/files/FilesRepository.test.ts index 741e9500..b1879eaa 100644 --- a/test/unit/files/FilesRepository.test.ts +++ b/test/unit/files/FilesRepository.test.ts @@ -30,7 +30,7 @@ import { createFileCountsPayload } from '../../testHelpers/files/fileCountsHelper' import { createFilesTotalDownloadSizePayload } from '../../testHelpers/files/filesTotalDownloadSizeHelper' -import { FileDownloadSizeMode } from '../../../src' +import { FileDownloadSizeMode, WriteError } from '../../../src' import { createMultipartFileUploadDestinationModel, createMultipartFileUploadDestinationPayload, @@ -1159,4 +1159,72 @@ describe('FilesRepository', () => { ).rejects.toThrow(ReadError) }) }) + + describe('deleteFile', () => { + describe('by numeric id', () => { + test('should return undefined on success', async () => { + const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/files/${testFile.id}` + + jest.spyOn(axios, 'delete').mockResolvedValue(undefined) + + // API Key auth + const actual = await sut.deleteFile(testFile.id) + expect(actual).toBeUndefined() + expect(axios.delete).toHaveBeenCalledWith( + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY + ) + + // Session cookie auth + ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.SESSION_COOKIE) + const actual2 = await sut.deleteFile(testFile.id) + expect(actual2).toBeUndefined() + expect(axios.delete).toHaveBeenCalledWith( + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_SESSION_COOKIE + ) + }) + + test('should return error result on error response', async () => { + jest.spyOn(axios, 'delete').mockRejectedValue(TestConstants.TEST_ERROR_RESPONSE) + + await expect(sut.deleteFile(testFile.id)).rejects.toThrow(WriteError) + }) + }) + + describe('by persistent id', () => { + test('should return undefined on success', async () => { + const expectedApiEndpoint = `${TestConstants.TEST_API_URL}/files/:persistentId?persistentId=${TestConstants.TEST_DUMMY_PERSISTENT_ID}` + + jest.spyOn(axios, 'delete').mockResolvedValue(undefined) + + // API Key auth + const actual = await sut.deleteFile(TestConstants.TEST_DUMMY_PERSISTENT_ID) + expect(axios.delete).toHaveBeenCalledWith( + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_API_KEY + ) + expect(actual).toBeUndefined() + + // Session cookie auth + ApiConfig.init(TestConstants.TEST_API_URL, DataverseApiAuthMechanism.SESSION_COOKIE) + + const actual2 = await sut.deleteFile(TestConstants.TEST_DUMMY_PERSISTENT_ID) + expect(actual2).toBeUndefined() + + expect(axios.delete).toHaveBeenCalledWith( + expectedApiEndpoint, + TestConstants.TEST_EXPECTED_AUTHENTICATED_REQUEST_CONFIG_SESSION_COOKIE + ) + }) + + test('should return error result on error response', async () => { + jest.spyOn(axios, 'delete').mockRejectedValue(TestConstants.TEST_ERROR_RESPONSE) + + await expect(sut.deleteFile(TestConstants.TEST_DUMMY_PERSISTENT_ID)).rejects.toThrow( + WriteError + ) + }) + }) + }) })