From 461b9ffcb7a00f09e165586bc3b77d7cb8775f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 19 Feb 2025 17:17:56 -0300 Subject: [PATCH 1/9] test: fix due to receiving items in different order --- test/functional/collections/GetCollectionItems.test.ts | 4 ++-- .../collections/CollectionsRepository.test.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/functional/collections/GetCollectionItems.test.ts b/test/functional/collections/GetCollectionItems.test.ts index 15e2814f..96832fef 100644 --- a/test/functional/collections/GetCollectionItems.test.ts +++ b/test/functional/collections/GetCollectionItems.test.ts @@ -61,8 +61,8 @@ describe('execute', () => { try { const actual = await getCollectionItems.execute(testCollectionAlias) - const actualFilePreview = actual.items[0] as FilePreview - const actualDatasetPreview = actual.items[1] as DatasetPreview + const actualFilePreview = actual.items[1] as FilePreview + const actualDatasetPreview = actual.items[0] as DatasetPreview expect(actualFilePreview.name).toBe('test-file-1.txt') expect(actualDatasetPreview.title).toBe('Dataset created using the createDataset use case') diff --git a/test/integration/collections/CollectionsRepository.test.ts b/test/integration/collections/CollectionsRepository.test.ts index b8f43967..3fb13607 100644 --- a/test/integration/collections/CollectionsRepository.test.ts +++ b/test/integration/collections/CollectionsRepository.test.ts @@ -339,8 +339,8 @@ describe('CollectionsRepository', () => { await new Promise((resolve) => setTimeout(resolve, 5000)) let actual = await sut.getCollectionItems(testCollectionAlias) - const actualFilePreview = actual.items[0] as FilePreview - const actualDatasetPreview = actual.items[1] as DatasetPreview + const actualFilePreview = actual.items[1] as FilePreview + const actualDatasetPreview = actual.items[0] as DatasetPreview const actualCollectionPreview = actual.items[2] as CollectionPreview const expectedFileMd5 = '68b22040025784da775f55cfcb6dee2e' @@ -473,7 +473,7 @@ describe('CollectionsRepository', () => { // Test limit and offset actual = await sut.getCollectionItems(testCollectionAlias, 1, 1) - expect((actual.items[0] as DatasetPreview).persistentId).toBe(testDatasetIds.persistentId) + expect((actual.items[0] as FilePreview).name).toBe(expectedFileName) expect(actual.items.length).toBe(1) expect(actual.totalItemCount).toBe(3) @@ -683,8 +683,8 @@ describe('CollectionsRepository', () => { ) expect(actual.items.length).toBe(3) expect(actual.totalItemCount).toBe(3) - expect((actual.items[0] as FilePreview).type).toBe(CollectionItemType.FILE) - expect((actual.items[1] as DatasetPreview).type).toBe(CollectionItemType.DATASET) + expect((actual.items[0] as DatasetPreview).type).toBe(CollectionItemType.DATASET) + expect((actual.items[1] as FilePreview).type).toBe(CollectionItemType.FILE) expect((actual.items[2] as CollectionPreview).type).toBe(CollectionItemType.COLLECTION) expect(actual.countPerObjectType.dataverses).toBe(1) expect(actual.countPerObjectType.datasets).toBe(1) From 7da8da7d49a0f9d2ea3030ecfa9ac1a0010446cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Fri, 21 Feb 2025 09:35:49 -0300 Subject: [PATCH 2/9] feat: repositories methods --- src/files/domain/dtos/UploadedFileDTO.ts | 1 + .../domain/repositories/IFilesRepository.ts | 2 ++ src/files/index.ts | 5 +++- .../infra/repositories/FilesRepository.ts | 30 +++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/files/domain/dtos/UploadedFileDTO.ts b/src/files/domain/dtos/UploadedFileDTO.ts index 96be67aa..24d6461c 100644 --- a/src/files/domain/dtos/UploadedFileDTO.ts +++ b/src/files/domain/dtos/UploadedFileDTO.ts @@ -8,4 +8,5 @@ export interface UploadedFileDTO { checksumValue: string checksumType: string mimeType: string + forceReplace?: boolean // Only used in the ReplaceFile use case, whether to allow the mimetype to change } diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index 8365374e..7cb322e2 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -61,4 +61,6 @@ export interface IFilesRepository { ): Promise deleteFile(fileId: number | string): Promise + + replaceFile(fileId: number, uploadedFileDTO: UploadedFileDTO): Promise } diff --git a/src/files/index.ts b/src/files/index.ts index 0d847852..fb7833c4 100644 --- a/src/files/index.ts +++ b/src/files/index.ts @@ -12,6 +12,7 @@ import { UploadFile } from './domain/useCases/UploadFile' import { DirectUploadClient } from './infra/clients/DirectUploadClient' import { AddUploadedFilesToDataset } from './domain/useCases/AddUploadedFilesToDataset' import { DeleteFile } from './domain/useCases/DeleteFile' +import { ReplaceFile } from './domain/useCases/ReplaceFile' const filesRepository = new FilesRepository() const directUploadClient = new DirectUploadClient(filesRepository) @@ -28,6 +29,7 @@ const getFileCitation = new GetFileCitation(filesRepository) const uploadFile = new UploadFile(directUploadClient) const addUploadedFilesToDataset = new AddUploadedFilesToDataset(filesRepository) const deleteFile = new DeleteFile(filesRepository) +const replaceFile = new ReplaceFile(filesRepository) export { getDatasetFiles, @@ -41,7 +43,8 @@ export { getFileCitation, uploadFile, addUploadedFilesToDataset, - deleteFile + deleteFile, + replaceFile } 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 9aebe720..8267ce0b 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -301,4 +301,34 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { throw error }) } + + public async replaceFile(fileId: number, uploadedFileDTO: UploadedFileDTO): Promise { + const requestBody: UploadedFileRequestBody = { + fileName: uploadedFileDTO.fileName, + checksum: { + '@value': uploadedFileDTO.checksumValue, + '@type': uploadedFileDTO.checksumType.toUpperCase() + }, + mimeType: uploadedFileDTO.mimeType, + storageIdentifier: uploadedFileDTO.storageId, + ...(uploadedFileDTO.description && { description: uploadedFileDTO.description }), + ...(uploadedFileDTO.categories && { categories: uploadedFileDTO.categories }), + ...(uploadedFileDTO.restrict && { restrict: uploadedFileDTO.restrict }), + ...(uploadedFileDTO.directoryLabel && { directoryLabel: uploadedFileDTO.directoryLabel }) + } + + const formData = new FormData() + formData.append('jsonData', JSON.stringify(requestBody)) + + return this.doPost( + this.buildApiEndpoint(this.filesResourceName, 'replace', fileId), + requestBody, + {}, + ApiConstants.CONTENT_TYPE_MULTIPART_FORM_DATA + ) + .then(() => undefined) + .catch((error) => { + throw error + }) + } } From 224e23f59a73b3fb51ca98f407a0f167e1a85ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Fri, 21 Feb 2025 09:40:17 -0300 Subject: [PATCH 3/9] docs: use cases doc --- src/files/domain/useCases/ReplaceFile.ts | 28 ++++++++++++++++++++++++ src/files/domain/useCases/UploadFile.ts | 4 +++- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 src/files/domain/useCases/ReplaceFile.ts diff --git a/src/files/domain/useCases/ReplaceFile.ts b/src/files/domain/useCases/ReplaceFile.ts new file mode 100644 index 00000000..24c479ae --- /dev/null +++ b/src/files/domain/useCases/ReplaceFile.ts @@ -0,0 +1,28 @@ +import { UseCase } from '../../../core/domain/useCases/UseCase' +import { UploadedFileDTO } from '../dtos/UploadedFileDTO' +import { IFilesRepository } from '../repositories/IFilesRepository' + +export class ReplaceFile implements UseCase { + private filesRepository: IFilesRepository + + constructor(filesRepository: IFilesRepository) { + this.filesRepository = filesRepository + } + + /** + * Replaces an existing file. + * + * This method completes the flow initiated by the UploadFile use case, which involves replacing an existing file with a new one just uploaded. + * (https://guides.dataverse.org/en/latest/developers/s3-direct-upload-api.html#replacing-an-existing-file-in-the-dataset) + * + * Note: This use case can be used independently of the UploadFile use case, e.g., supporting scenarios in which the files already exist in S3 or have been uploaded via some out-of-band method. + * + * @param {number} [fileId] - The File identifier. + * @param {UploadedFileDTO} [uploadedFileDTO] - File DTO associated with the uploaded file. + * @returns {Promise} A promise that resolves when the file has been successfully replaced. + * @throws {WriteError} - If there are errors while writing data. + */ + async execute(fileId: number, uploadedFileDTO: UploadedFileDTO): Promise { + await this.filesRepository.replaceFile(fileId, uploadedFileDTO) + } +} diff --git a/src/files/domain/useCases/UploadFile.ts b/src/files/domain/useCases/UploadFile.ts index b8aba616..ad10ef95 100644 --- a/src/files/domain/useCases/UploadFile.ts +++ b/src/files/domain/useCases/UploadFile.ts @@ -12,7 +12,9 @@ export class UploadFile implements UseCase { * Uploads a file to remote storage and returns the storage identifier. * * This use case is based on the Direct Upload API, particularly the first part of the flow, "Requesting Direct Upload of a DataFile". - * To fulfill the flow, you will need to call the AddUploadedFileToDataset Use Case to add the uploaded file to the dataset. + * To fulfill the flow, you could: + * - Call the AddUploadedFilesToDataset Use Case to add the uploaded file to the dataset. + * - Call the ReplaceFile Use Case to replace an existing file with the uploaded one. * (https://guides.dataverse.org/en/latest/developers/s3-direct-upload-api.html#requesting-direct-upload-of-a-datafile) * * @param {number | string} [datasetId] - The dataset identifier, which can be a string (for persistent identifiers) or a number (for numeric identifiers). From 96fec17dac7dada879e59ab7d54ebd634c82bbc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Fri, 21 Feb 2025 09:51:23 -0300 Subject: [PATCH 4/9] docs --- docs/useCases.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/useCases.md b/docs/useCases.md index fa726a78..cfe8eff8 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -51,6 +51,7 @@ The different use cases currently available in the package are classified below, - [Files write use cases](#files-write-use-cases) - [File Uploading Use Cases](#file-uploading-use-cases) - [Delete a File](#delete-a-file) + - [Replace a File](#replace-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) @@ -1264,6 +1265,37 @@ Note that the behavior of deleting files depends on if the dataset has ever been - 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. +#### Replace a File + +Replaces a File. Currently working for already uploaded S3 bucket files. + +##### Example call: + +```typescript +import { replaceFile } from '@iqss/dataverse-client-javascript' + +/* ... */ + +const fileId = 12345 +const uploadedFileDTO: UploadedFileDTO = { + fileName: 'the-file-name', + storageId: 'localstack1://mybucket:19121faf7e7-2c40322ff54e', + checksumType: 'md5', + checksumValue: 'ede3d3b685b4e137ba4cb2521329a75e', + mimeType: 'text/plain' +} + +replaceFile.execute(fileId, uploadedFileDTO) + +/* ... */ +``` + +_See [use case](../src/files/domain/useCases/ReplaceFile.ts) implementation_. + +The `fileId` parameter should be a number, the numeric identifier. + +The `uploadedFileDTO` parameter is a [UploadedFileDTO](../src/files/domain/dtos/UploadedFileDTO.ts) and includes properties related to the uploaded files. Some of these properties should be calculated from the uploaded File Blob objects and the resulting storage identifiers from the Upload File use case. + ## Metadata Blocks ### Metadata Blocks read use cases From c833344b9962876275ca286532c393e05930eff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Fri, 21 Feb 2025 10:11:55 -0300 Subject: [PATCH 5/9] test: add unit cases --- test/unit/files/ReplaceFile.test.ts | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 test/unit/files/ReplaceFile.test.ts diff --git a/test/unit/files/ReplaceFile.test.ts b/test/unit/files/ReplaceFile.test.ts new file mode 100644 index 00000000..4cf574b7 --- /dev/null +++ b/test/unit/files/ReplaceFile.test.ts @@ -0,0 +1,34 @@ +import { IFilesRepository } from '../../../src/files/domain/repositories/IFilesRepository' +import { UploadedFileDTO } from '../../../src/files' +import { ReplaceFile } from '../../../src/files/domain/useCases/ReplaceFile' +import { WriteError } from '../../../src/core' + +describe('execute', () => { + const testUploadedFileDTO: UploadedFileDTO = { + fileName: 'testfile', + storageId: 'testStorageId', + checksumValue: 'testChecksumValue', + checksumType: 'md5', + mimeType: 'test/type' + } + + test('should return undefined on client success', async () => { + const filesRepositoryStub: IFilesRepository = {} as IFilesRepository + filesRepositoryStub.replaceFile = jest.fn().mockResolvedValue(undefined) + + const sut = new ReplaceFile(filesRepositoryStub) + + const actual = await sut.execute(1, testUploadedFileDTO) + + expect(actual).toEqual(undefined) + }) + + test('should return error on client error', async () => { + const filesRepositoryStub: IFilesRepository = {} as IFilesRepository + filesRepositoryStub.replaceFile = jest.fn().mockRejectedValue(new WriteError('Some error')) + + const sut = new ReplaceFile(filesRepositoryStub) + + await expect(sut.execute(1, testUploadedFileDTO)).rejects.toThrow(WriteError) + }) +}) From 034eddb46157004a8e8b5a39e02957012f971bdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Fri, 21 Feb 2025 13:13:28 -0300 Subject: [PATCH 6/9] test: integration cases --- .../infra/repositories/FilesRepository.ts | 2 +- test/integration/files/DirectUpload.test.ts | 114 ++++++++++++++++++ test/testHelpers/files/filesHelper.ts | 7 +- 3 files changed, 120 insertions(+), 3 deletions(-) diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 8267ce0b..131e5f30 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -322,7 +322,7 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { return this.doPost( this.buildApiEndpoint(this.filesResourceName, 'replace', fileId), - requestBody, + formData, {}, ApiConstants.CONTENT_TYPE_MULTIPART_FORM_DATA ) diff --git a/test/integration/files/DirectUpload.test.ts b/test/integration/files/DirectUpload.test.ts index 87583732..5967f6c0 100644 --- a/test/integration/files/DirectUpload.test.ts +++ b/test/integration/files/DirectUpload.test.ts @@ -3,6 +3,7 @@ import { CreatedDatasetIdentifiers, DatasetNotNumberedVersion, FileOrderCriteria, + UploadedFileDTO, createDataset } from '../../../src' import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig' @@ -27,6 +28,7 @@ describe('Direct Upload', () => { const testCollectionAlias = 'directUploadTestCollection' let testDataset1Ids: CreatedDatasetIdentifiers let testDataset2Ids: CreatedDatasetIdentifiers + let testDataset3Ids: CreatedDatasetIdentifiers const filesRepositorySut = new FilesRepository() const directUploadSut: DirectUploadClient = new DirectUploadClient(filesRepositorySut) @@ -57,6 +59,10 @@ describe('Direct Upload', () => { TestConstants.TEST_NEW_DATASET_DTO, testCollectionAlias ) + testDataset3Ids = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + testCollectionAlias + ) } catch (error) { throw new Error('Tests beforeAll(): Error while creating test dataset') } @@ -67,6 +73,7 @@ describe('Direct Upload', () => { afterAll(async () => { await deleteUnpublishedDatasetViaApi(testDataset1Ids.numericId) await deleteUnpublishedDatasetViaApi(testDataset2Ids.numericId) + await deleteUnpublishedDatasetViaApi(testDataset3Ids.numericId) await deleteCollectionViaApi(testCollectionAlias) }) @@ -213,6 +220,113 @@ describe('Direct Upload', () => { ).rejects.toThrow(FileUploadCancelError) }) + test('should upload file add it to the dataset, upload a new one and replace it', async () => { + // 1 - Upload first file and add it to the dataset + const destination = await createTestFileUploadDestination( + singlepartFile, + testDataset3Ids.numericId + ) + const singlepartFileUrl = destination.urls[0] + + const progressMock = jest.fn() + const abortController = new AbortController() + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(false) + + const actualStorageId = await directUploadSut.uploadFile( + testDataset3Ids.numericId, + singlepartFile, + progressMock, + abortController, + destination + ) + expect(actualStorageId).toBe(destination.storageId) + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(true) + + let datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset3Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(0) + + const fileArrayBuffer = await singlepartFile.arrayBuffer() + const fileBuffer = Buffer.from(fileArrayBuffer) + + const uploadedFileDTO = { + fileName: singlepartFile.name, + storageId: actualStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(fileBuffer), + mimeType: singlepartFile.type + } + + await filesRepositorySut.addUploadedFilesToDataset(testDataset3Ids.numericId, [uploadedFileDTO]) + + datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset3Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(1) + expect(datasetFiles.files[0].name).toBe('singlepart-file') + expect(datasetFiles.files[0].sizeBytes).toBe(singlepartFile.size) + expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:') + + // 2 - Upload a new file and get the new storage id + const newSinglepartFile = await createSinglepartFileBlob('new-singlepart-file', 1500) + const newDestination = await createTestFileUploadDestination( + newSinglepartFile, + testDataset3Ids.numericId + ) + const newSinglepartFileUrl = newDestination.urls[0] + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(false) + + const newFileStorageId = await directUploadSut.uploadFile( + testDataset3Ids.numericId, + newSinglepartFile, + progressMock, + abortController, + newDestination + ) + expect(newFileStorageId).toBe(newDestination.storageId) + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(true) + + // 3 - Replace the old file with the new file (must have different content) + const currentFileId = datasetFiles.files[0].id + const newFileArrayBuffer = await newSinglepartFile.arrayBuffer() + const newFileBuffer = Buffer.from(newFileArrayBuffer) + const newUploadedFileDTO: UploadedFileDTO = { + fileName: newSinglepartFile.name, + storageId: newFileStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(newFileBuffer), + mimeType: newSinglepartFile.type + } + + await filesRepositorySut.replaceFile(currentFileId, newUploadedFileDTO) + + // 4 - Verify that the new file is in the dataset and the old file is not + datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset3Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(1) + expect(datasetFiles.files[0].name).toBe('new-singlepart-file') + expect(datasetFiles.files[0].sizeBytes).toBe(newSinglepartFile.size) + expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:') + }) + const createTestFileUploadDestination = async (file: File, testDatasetId: number) => { const filesRepository = new FilesRepository() const destination = await filesRepository.getFileUploadDestination(testDatasetId, file) diff --git a/test/testHelpers/files/filesHelper.ts b/test/testHelpers/files/filesHelper.ts index 666551bf..8262dce1 100644 --- a/test/testHelpers/files/filesHelper.ts +++ b/test/testHelpers/files/filesHelper.ts @@ -186,9 +186,12 @@ const enableFilePIDs = async (): Promise => { ) } -export async function createSinglepartFileBlob(): Promise { +export async function createSinglepartFileBlob( + fileName = 'singlepart-file', + fileSizeInBytes = 1000 +): Promise { try { - return await createFileBlobWithSize(1000, 'singlepart-file') + return await createFileBlobWithSize(fileSizeInBytes, fileName) } catch (error) { throw new Error(`Error while creating test singlepart file`) } From f8756666df53825fa74a5f5788af0fab18caa90f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Fri, 21 Feb 2025 15:02:38 -0300 Subject: [PATCH 7/9] test: add more integration cases --- .../infra/repositories/FilesRepository.ts | 2 + test/integration/files/DirectUpload.test.ts | 339 +++++++++++++++++- test/testHelpers/files/filesHelper.ts | 13 +- 3 files changed, 349 insertions(+), 5 deletions(-) diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 131e5f30..8b791980 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -51,6 +51,7 @@ export interface UploadedFileRequestBody { directoryLabel?: string categories?: string[] restrict?: boolean + forceReplace?: boolean } export interface ChecksumRequestBody { @@ -311,6 +312,7 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { }, mimeType: uploadedFileDTO.mimeType, storageIdentifier: uploadedFileDTO.storageId, + forceReplace: uploadedFileDTO.forceReplace, ...(uploadedFileDTO.description && { description: uploadedFileDTO.description }), ...(uploadedFileDTO.categories && { categories: uploadedFileDTO.categories }), ...(uploadedFileDTO.restrict && { restrict: uploadedFileDTO.restrict }), diff --git a/test/integration/files/DirectUpload.test.ts b/test/integration/files/DirectUpload.test.ts index 5967f6c0..a3931ab8 100644 --- a/test/integration/files/DirectUpload.test.ts +++ b/test/integration/files/DirectUpload.test.ts @@ -4,6 +4,7 @@ import { DatasetNotNumberedVersion, FileOrderCriteria, UploadedFileDTO, + WriteError, createDataset } from '../../../src' import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig' @@ -29,6 +30,9 @@ describe('Direct Upload', () => { let testDataset1Ids: CreatedDatasetIdentifiers let testDataset2Ids: CreatedDatasetIdentifiers let testDataset3Ids: CreatedDatasetIdentifiers + let testDatset4Ids: CreatedDatasetIdentifiers + let testDataset5Ids: CreatedDatasetIdentifiers + let testDataset6Ids: CreatedDatasetIdentifiers const filesRepositorySut = new FilesRepository() const directUploadSut: DirectUploadClient = new DirectUploadClient(filesRepositorySut) @@ -63,6 +67,18 @@ describe('Direct Upload', () => { TestConstants.TEST_NEW_DATASET_DTO, testCollectionAlias ) + testDatset4Ids = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + testCollectionAlias + ) + testDataset5Ids = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + testCollectionAlias + ) + testDataset6Ids = await createDataset.execute( + TestConstants.TEST_NEW_DATASET_DTO, + testCollectionAlias + ) } catch (error) { throw new Error('Tests beforeAll(): Error while creating test dataset') } @@ -74,6 +90,9 @@ describe('Direct Upload', () => { await deleteUnpublishedDatasetViaApi(testDataset1Ids.numericId) await deleteUnpublishedDatasetViaApi(testDataset2Ids.numericId) await deleteUnpublishedDatasetViaApi(testDataset3Ids.numericId) + await deleteUnpublishedDatasetViaApi(testDatset4Ids.numericId) + await deleteUnpublishedDatasetViaApi(testDataset5Ids.numericId) + await deleteUnpublishedDatasetViaApi(testDataset6Ids.numericId) await deleteCollectionViaApi(testCollectionAlias) }) @@ -220,7 +239,7 @@ describe('Direct Upload', () => { ).rejects.toThrow(FileUploadCancelError) }) - test('should upload file add it to the dataset, upload a new one and replace it', async () => { + test('should replace a file succesfully', async () => { // 1 - Upload first file and add it to the dataset const destination = await createTestFileUploadDestination( singlepartFile, @@ -327,6 +346,324 @@ describe('Direct Upload', () => { expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:') }) + test('should fail to replace a file when mimetype is different and forceReplace is false', async () => { + // 1 - Upload first file and add it to the dataset + const destination = await createTestFileUploadDestination( + singlepartFile, + testDatset4Ids.numericId + ) + const singlepartFileUrl = destination.urls[0] + + const progressMock = jest.fn() + const abortController = new AbortController() + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(false) + + const actualStorageId = await directUploadSut.uploadFile( + testDatset4Ids.numericId, + singlepartFile, + progressMock, + abortController, + destination + ) + expect(actualStorageId).toBe(destination.storageId) + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(true) + + let datasetFiles = await filesRepositorySut.getDatasetFiles( + testDatset4Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(0) + + const fileArrayBuffer = await singlepartFile.arrayBuffer() + const fileBuffer = Buffer.from(fileArrayBuffer) + + const uploadedFileDTO = { + fileName: singlepartFile.name, + storageId: actualStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(fileBuffer), + mimeType: singlepartFile.type + } + + await filesRepositorySut.addUploadedFilesToDataset(testDatset4Ids.numericId, [uploadedFileDTO]) + + datasetFiles = await filesRepositorySut.getDatasetFiles( + testDatset4Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(1) + expect(datasetFiles.files[0].name).toBe('singlepart-file') + expect(datasetFiles.files[0].sizeBytes).toBe(singlepartFile.size) + expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:') + + // 2 - Upload a new file and get the new storage id + const newSinglepartFile = await createSinglepartFileBlob( + 'new-singlepart-file', + 1500, + 'text/csv' + ) + const newDestination = await createTestFileUploadDestination( + newSinglepartFile, + testDatset4Ids.numericId + ) + const newSinglepartFileUrl = newDestination.urls[0] + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(false) + + const newFileStorageId = await directUploadSut.uploadFile( + testDatset4Ids.numericId, + newSinglepartFile, + progressMock, + abortController, + newDestination + ) + expect(newFileStorageId).toBe(newDestination.storageId) + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(true) + + // 3 - Replace the old file with the new file (must have different content) + const currentFileId = datasetFiles.files[0].id + const newFileArrayBuffer = await newSinglepartFile.arrayBuffer() + const newFileBuffer = Buffer.from(newFileArrayBuffer) + const newUploadedFileDTO: UploadedFileDTO = { + fileName: newSinglepartFile.name, + storageId: newFileStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(newFileBuffer), + mimeType: newSinglepartFile.type, + forceReplace: false + } + + const expectedError = new WriteError( + '[400] The original file (Plain Text) and replacement file (Comma Separated Values) are different file types.' + ) + + await expect(filesRepositorySut.replaceFile(currentFileId, newUploadedFileDTO)).rejects.toThrow( + expectedError + ) + }) + + test('should replace a file succesfully when mimetype is different but forceReplace is true', async () => { + // 1 - Upload first file and add it to the dataset + const destination = await createTestFileUploadDestination( + singlepartFile, + testDataset5Ids.numericId + ) + const singlepartFileUrl = destination.urls[0] + + const progressMock = jest.fn() + const abortController = new AbortController() + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(false) + + const actualStorageId = await directUploadSut.uploadFile( + testDataset5Ids.numericId, + singlepartFile, + progressMock, + abortController, + destination + ) + expect(actualStorageId).toBe(destination.storageId) + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(true) + + let datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset5Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(0) + + const fileArrayBuffer = await singlepartFile.arrayBuffer() + const fileBuffer = Buffer.from(fileArrayBuffer) + + const uploadedFileDTO = { + fileName: singlepartFile.name, + storageId: actualStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(fileBuffer), + mimeType: singlepartFile.type + } + + await filesRepositorySut.addUploadedFilesToDataset(testDataset5Ids.numericId, [uploadedFileDTO]) + + datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset5Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(1) + expect(datasetFiles.files[0].name).toBe('singlepart-file') + expect(datasetFiles.files[0].sizeBytes).toBe(singlepartFile.size) + expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:') + + // 2 - Upload a new file and get the new storage id + const newSinglepartFile = await createSinglepartFileBlob( + 'new-singlepart-file-diff-mimetype', + 1500, + 'text/csv' + ) + const newDestination = await createTestFileUploadDestination( + newSinglepartFile, + testDataset5Ids.numericId + ) + const newSinglepartFileUrl = newDestination.urls[0] + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(false) + + const newFileStorageId = await directUploadSut.uploadFile( + testDataset5Ids.numericId, + newSinglepartFile, + progressMock, + abortController, + newDestination + ) + expect(newFileStorageId).toBe(newDestination.storageId) + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(true) + + // 3 - Replace the old file with the new file (must have different content), the new file has a different mimetype but forceReplace is true + const currentFileId = datasetFiles.files[0].id + const newFileArrayBuffer = await newSinglepartFile.arrayBuffer() + const newFileBuffer = Buffer.from(newFileArrayBuffer) + const newUploadedFileDTO: UploadedFileDTO = { + fileName: newSinglepartFile.name, + storageId: newFileStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(newFileBuffer), + mimeType: newSinglepartFile.type, + forceReplace: true + } + + await filesRepositorySut.replaceFile(currentFileId, newUploadedFileDTO) + + // 4 - Verify that the new file is in the dataset and the old file is not + datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset5Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(1) + expect(datasetFiles.files[0].name).toBe('new-singlepart-file-diff-mimetype') + expect(datasetFiles.files[0].contentType).toBe(newSinglepartFile.type) + expect(datasetFiles.files[0].sizeBytes).toBe(newSinglepartFile.size) + expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:') + }) + + test('should fail to replace a file when the new image has the same content as the old image', async () => { + // 1 - Upload first file and add it to the dataset + const destination = await createTestFileUploadDestination( + singlepartFile, + testDataset6Ids.numericId + ) + const singlepartFileUrl = destination.urls[0] + + const progressMock = jest.fn() + const abortController = new AbortController() + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(false) + + const actualStorageId = await directUploadSut.uploadFile( + testDataset6Ids.numericId, + singlepartFile, + progressMock, + abortController, + destination + ) + expect(actualStorageId).toBe(destination.storageId) + + expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(true) + + let datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset6Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(0) + + const fileArrayBuffer = await singlepartFile.arrayBuffer() + const fileBuffer = Buffer.from(fileArrayBuffer) + + const uploadedFileDTO = { + fileName: singlepartFile.name, + storageId: actualStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(fileBuffer), + mimeType: singlepartFile.type + } + + await filesRepositorySut.addUploadedFilesToDataset(testDataset6Ids.numericId, [uploadedFileDTO]) + + datasetFiles = await filesRepositorySut.getDatasetFiles( + testDataset6Ids.numericId, + DatasetNotNumberedVersion.LATEST, + true, + FileOrderCriteria.NAME_AZ + ) + + expect(datasetFiles.totalFilesCount).toBe(1) + expect(datasetFiles.files[0].name).toBe('singlepart-file') + expect(datasetFiles.files[0].sizeBytes).toBe(singlepartFile.size) + expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:') + + // 2 - Upload a new file with the same content and get the new storage id + const newSinglepartFile = await createSinglepartFileBlob() + const newDestination = await createTestFileUploadDestination( + newSinglepartFile, + testDataset6Ids.numericId + ) + const newSinglepartFileUrl = newDestination.urls[0] + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(false) + + const newFileStorageId = await directUploadSut.uploadFile( + testDataset6Ids.numericId, + newSinglepartFile, + progressMock, + abortController, + newDestination + ) + expect(newFileStorageId).toBe(newDestination.storageId) + + expect(await singlepartFileExistsInBucket(newSinglepartFileUrl)).toBe(true) + + // 3 - Replace the old file with the new file (must have different content), the new file has a different mimetype but forceReplace is true + const currentFileId = datasetFiles.files[0].id + const newFileArrayBuffer = await newSinglepartFile.arrayBuffer() + const newFileBuffer = Buffer.from(newFileArrayBuffer) + const newUploadedFileDTO: UploadedFileDTO = { + fileName: newSinglepartFile.name, + storageId: newFileStorageId, + checksumType: checksumAlgorithm, + checksumValue: calculateBlobChecksum(newFileBuffer), + mimeType: newSinglepartFile.type + } + + const expectedError = new WriteError( + '[400] Error! You may not replace a file with a file that has duplicate content.' + ) + + await expect(filesRepositorySut.replaceFile(currentFileId, newUploadedFileDTO)).rejects.toThrow( + expectedError + ) + }) + const createTestFileUploadDestination = async (file: File, testDatasetId: number) => { const filesRepository = new FilesRepository() const destination = await filesRepository.getFileUploadDestination(testDatasetId, file) diff --git a/test/testHelpers/files/filesHelper.ts b/test/testHelpers/files/filesHelper.ts index 8262dce1..a403b483 100644 --- a/test/testHelpers/files/filesHelper.ts +++ b/test/testHelpers/files/filesHelper.ts @@ -188,10 +188,11 @@ const enableFilePIDs = async (): Promise => { export async function createSinglepartFileBlob( fileName = 'singlepart-file', - fileSizeInBytes = 1000 + fileSizeInBytes = 1000, + fileType = 'text/plain' ): Promise { try { - return await createFileBlobWithSize(fileSizeInBytes, fileName) + return await createFileBlobWithSize(fileSizeInBytes, fileName, fileType) } catch (error) { throw new Error(`Error while creating test singlepart file`) } @@ -205,9 +206,13 @@ export async function createMultipartFileBlob(): Promise { } } -async function createFileBlobWithSize(fileSizeInBytes: number, fileName: string): Promise { +async function createFileBlobWithSize( + fileSizeInBytes: number, + fileName: string, + fileType = 'text/plain' +): Promise { const blob = await createBlobWithSize(fileSizeInBytes) - return new File([blob], fileName, { type: 'text/plain' }) + return new File([blob], fileName, { type: fileType }) } async function createBlobWithSize(size: number): Promise { From a2793215bb835e3152bf5635395f66ad0bcb3a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Tue, 25 Feb 2025 19:06:26 -0300 Subject: [PATCH 8/9] fix: accept also pid --- src/files/domain/useCases/ReplaceFile.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/files/domain/useCases/ReplaceFile.ts b/src/files/domain/useCases/ReplaceFile.ts index 24c479ae..07cef15b 100644 --- a/src/files/domain/useCases/ReplaceFile.ts +++ b/src/files/domain/useCases/ReplaceFile.ts @@ -17,7 +17,7 @@ export class ReplaceFile implements UseCase { * * Note: This use case can be used independently of the UploadFile use case, e.g., supporting scenarios in which the files already exist in S3 or have been uploaded via some out-of-band method. * - * @param {number} [fileId] - The File identifier. + * @param {number | string} [fileId] - The File identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers). * @param {UploadedFileDTO} [uploadedFileDTO] - File DTO associated with the uploaded file. * @returns {Promise} A promise that resolves when the file has been successfully replaced. * @throws {WriteError} - If there are errors while writing data. From b681b76866235b21de4f9bd7da4410f215a3b223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 26 Feb 2025 09:36:58 -0300 Subject: [PATCH 9/9] fix: add string type --- docs/useCases.md | 2 +- src/files/domain/repositories/IFilesRepository.ts | 2 +- src/files/domain/useCases/ReplaceFile.ts | 2 +- src/files/infra/repositories/FilesRepository.ts | 5 ++++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/useCases.md b/docs/useCases.md index 28a4c2ec..47e09f6c 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1312,7 +1312,7 @@ replaceFile.execute(fileId, uploadedFileDTO) _See [use case](../src/files/domain/useCases/ReplaceFile.ts) implementation_. -The `fileId` parameter should be a number, the numeric identifier. +The `fileId` parameter can be a string, for persistent identifiers, or a number, for numeric identifiers. The `uploadedFileDTO` parameter is a [UploadedFileDTO](../src/files/domain/dtos/UploadedFileDTO.ts) and includes properties related to the uploaded files. Some of these properties should be calculated from the uploaded File Blob objects and the resulting storage identifiers from the Upload File use case. diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index a520adb2..42609380 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -62,7 +62,7 @@ export interface IFilesRepository { deleteFile(fileId: number | string): Promise - replaceFile(fileId: number, uploadedFileDTO: UploadedFileDTO): Promise + replaceFile(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise restrictFile(fileId: number | string, restrict: boolean): Promise } diff --git a/src/files/domain/useCases/ReplaceFile.ts b/src/files/domain/useCases/ReplaceFile.ts index 07cef15b..4f1f06c8 100644 --- a/src/files/domain/useCases/ReplaceFile.ts +++ b/src/files/domain/useCases/ReplaceFile.ts @@ -22,7 +22,7 @@ export class ReplaceFile implements UseCase { * @returns {Promise} A promise that resolves when the file has been successfully replaced. * @throws {WriteError} - If there are errors while writing data. */ - async execute(fileId: number, uploadedFileDTO: UploadedFileDTO): Promise { + async execute(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise { await this.filesRepository.replaceFile(fileId, uploadedFileDTO) } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index c91841d6..1b712445 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -303,7 +303,10 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { }) } - public async replaceFile(fileId: number, uploadedFileDTO: UploadedFileDTO): Promise { + public async replaceFile( + fileId: number | string, + uploadedFileDTO: UploadedFileDTO + ): Promise { const requestBody: UploadedFileRequestBody = { fileName: uploadedFileDTO.fileName, checksum: {