From f5037f7288ba8b914a4b6fe73f98c5c01ba7a610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 12 Mar 2025 11:16:53 -0300 Subject: [PATCH 1/5] feat: return new file id after replacement --- src/files/domain/repositories/IFilesRepository.ts | 2 +- src/files/domain/useCases/ReplaceFile.ts | 8 ++++---- src/files/infra/repositories/FilesRepository.ts | 12 ++++++++++-- test/integration/files/DirectUpload.test.ts | 5 ++++- test/unit/files/ReplaceFile.test.ts | 6 +++--- 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/files/domain/repositories/IFilesRepository.ts b/src/files/domain/repositories/IFilesRepository.ts index eb7d7d67..476810e1 100644 --- a/src/files/domain/repositories/IFilesRepository.ts +++ b/src/files/domain/repositories/IFilesRepository.ts @@ -63,7 +63,7 @@ export interface IFilesRepository { deleteFile(fileId: number | string): Promise - replaceFile(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise + replaceFile(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise restrictFile(fileId: number | string, restrict: boolean): Promise updateFileMetadata( diff --git a/src/files/domain/useCases/ReplaceFile.ts b/src/files/domain/useCases/ReplaceFile.ts index 4f1f06c8..82528fea 100644 --- a/src/files/domain/useCases/ReplaceFile.ts +++ b/src/files/domain/useCases/ReplaceFile.ts @@ -2,7 +2,7 @@ import { UseCase } from '../../../core/domain/useCases/UseCase' import { UploadedFileDTO } from '../dtos/UploadedFileDTO' import { IFilesRepository } from '../repositories/IFilesRepository' -export class ReplaceFile implements UseCase { +export class ReplaceFile implements UseCase { private filesRepository: IFilesRepository constructor(filesRepository: IFilesRepository) { @@ -19,10 +19,10 @@ export class ReplaceFile implements UseCase { * * @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. + * @returns {Promise} A promise that resolves when the file has been successfully replaced and returns the new file identifier. * @throws {WriteError} - If there are errors while writing data. */ - async execute(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise { - await this.filesRepository.replaceFile(fileId, uploadedFileDTO) + async execute(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise { + return await this.filesRepository.replaceFile(fileId, uploadedFileDTO) } } diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index 87725b53..a6b72a7d 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -20,6 +20,7 @@ import { transformUploadDestinationsResponseToUploadDestination } from './transf import { UploadedFileDTO } from '../../domain/dtos/UploadedFileDTO' import { UpdateFileMetadataDTO } from '../../domain/dtos/UpdateFileMetadataDTO' import { ApiConstants } from '../../../core/infra/repositories/ApiConstants' +import { AxiosResponse } from 'axios' export interface GetFilesQueryParams { includeDeaccessioned: boolean @@ -60,6 +61,10 @@ export interface ChecksumRequestBody { '@type': string } +type ReplaceFileResponseMinimal = { + files: { dataFile: { id: number } }[] +} + export class FilesRepository extends ApiRepository implements IFilesRepository { private readonly datasetsResourceName: string = 'datasets' private readonly filesResourceName: string = 'files' @@ -307,7 +312,7 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { public async replaceFile( fileId: number | string, uploadedFileDTO: UploadedFileDTO - ): Promise { + ): Promise { const requestBody: UploadedFileRequestBody = { fileName: uploadedFileDTO.fileName, checksum: { @@ -332,7 +337,10 @@ export class FilesRepository extends ApiRepository implements IFilesRepository { {}, ApiConstants.CONTENT_TYPE_MULTIPART_FORM_DATA ) - .then(() => undefined) + .then((response: AxiosResponse<{ data: ReplaceFileResponseMinimal }>) => { + const fileNumber = response.data.data.files[0].dataFile.id + return fileNumber + }) .catch((error) => { throw error }) diff --git a/test/integration/files/DirectUpload.test.ts b/test/integration/files/DirectUpload.test.ts index a3931ab8..321d1256 100644 --- a/test/integration/files/DirectUpload.test.ts +++ b/test/integration/files/DirectUpload.test.ts @@ -330,7 +330,9 @@ describe('Direct Upload', () => { mimeType: newSinglepartFile.type } - await filesRepositorySut.replaceFile(currentFileId, newUploadedFileDTO) + const replaceResponse = await filesRepositorySut.replaceFile(currentFileId, newUploadedFileDTO) + + console.log({ replaceResponse }) // 4 - Verify that the new file is in the dataset and the old file is not datasetFiles = await filesRepositorySut.getDatasetFiles( @@ -341,6 +343,7 @@ describe('Direct Upload', () => { ) expect(datasetFiles.totalFilesCount).toBe(1) + expect(replaceResponse).toBe(currentFileId + 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:') diff --git a/test/unit/files/ReplaceFile.test.ts b/test/unit/files/ReplaceFile.test.ts index 4cf574b7..8a4c0304 100644 --- a/test/unit/files/ReplaceFile.test.ts +++ b/test/unit/files/ReplaceFile.test.ts @@ -12,15 +12,15 @@ describe('execute', () => { mimeType: 'test/type' } - test('should return undefined on client success', async () => { + test('should return file id on client success', async () => { const filesRepositoryStub: IFilesRepository = {} as IFilesRepository - filesRepositoryStub.replaceFile = jest.fn().mockResolvedValue(undefined) + filesRepositoryStub.replaceFile = jest.fn().mockResolvedValue(1) const sut = new ReplaceFile(filesRepositoryStub) const actual = await sut.execute(1, testUploadedFileDTO) - expect(actual).toEqual(undefined) + expect(actual).toEqual(1) }) test('should return error on client error', async () => { From 64dc16dd748c100a6cda0bf2b7944941b58f6e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 12 Mar 2025 11:40:30 -0300 Subject: [PATCH 2/5] fix: review tweaks --- docs/useCases.md | 6 +++++- src/files/infra/repositories/FilesRepository.ts | 2 +- test/integration/files/DirectUpload.test.ts | 2 -- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/useCases.md b/docs/useCases.md index 66312def..cd21b618 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1359,7 +1359,9 @@ const uploadedFileDTO: UploadedFileDTO = { mimeType: 'text/plain' } -replaceFile.execute(fileId, uploadedFileDTO) +replaceFile.execute(fileId, uploadedFileDTO).then((newFileId: number) => { + /* ... */ +}) /* ... */ ``` @@ -1370,6 +1372,8 @@ The `fileId` parameter can be a string, for persistent identifiers, or a number, 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. +The use case returns a number, which is the identifier of the new file. + #### Restrict or Unrestrict a File Restrict or unrestrict an existing file. diff --git a/src/files/infra/repositories/FilesRepository.ts b/src/files/infra/repositories/FilesRepository.ts index a6b72a7d..d30494f6 100644 --- a/src/files/infra/repositories/FilesRepository.ts +++ b/src/files/infra/repositories/FilesRepository.ts @@ -1,3 +1,4 @@ +import { AxiosResponse } from 'axios' import { ApiRepository } from '../../../core/infra/repositories/ApiRepository' import { IFilesRepository } from '../../domain/repositories/IFilesRepository' import { FileModel as FileModel } from '../../domain/models/FileModel' @@ -20,7 +21,6 @@ import { transformUploadDestinationsResponseToUploadDestination } from './transf import { UploadedFileDTO } from '../../domain/dtos/UploadedFileDTO' import { UpdateFileMetadataDTO } from '../../domain/dtos/UpdateFileMetadataDTO' import { ApiConstants } from '../../../core/infra/repositories/ApiConstants' -import { AxiosResponse } from 'axios' export interface GetFilesQueryParams { includeDeaccessioned: boolean diff --git a/test/integration/files/DirectUpload.test.ts b/test/integration/files/DirectUpload.test.ts index 321d1256..0ed5717a 100644 --- a/test/integration/files/DirectUpload.test.ts +++ b/test/integration/files/DirectUpload.test.ts @@ -332,8 +332,6 @@ describe('Direct Upload', () => { const replaceResponse = await filesRepositorySut.replaceFile(currentFileId, newUploadedFileDTO) - console.log({ replaceResponse }) - // 4 - Verify that the new file is in the dataset and the old file is not datasetFiles = await filesRepositorySut.getDatasetFiles( testDataset3Ids.numericId, From fbe4c7a14a33dae29d687b2b04f11bb11fbcd4f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 12 Mar 2025 11:47:42 -0300 Subject: [PATCH 3/5] remove dot to test push --- docs/useCases.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/useCases.md b/docs/useCases.md index cd21b618..a61d04ee 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1372,7 +1372,7 @@ The `fileId` parameter can be a string, for persistent identifiers, or a number, 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. -The use case returns a number, which is the identifier of the new file. +The use case returns a number, which is the identifier of the new file #### Restrict or Unrestrict a File From 8515a88cabfb2605c039456c5912a3f2c8ada570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Wed, 12 Mar 2025 11:48:21 -0300 Subject: [PATCH 4/5] revert: remove dot to test push --- docs/useCases.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/useCases.md b/docs/useCases.md index a61d04ee..cd21b618 100644 --- a/docs/useCases.md +++ b/docs/useCases.md @@ -1372,7 +1372,7 @@ The `fileId` parameter can be a string, for persistent identifiers, or a number, 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. -The use case returns a number, which is the identifier of the new file +The use case returns a number, which is the identifier of the new file. #### Restrict or Unrestrict a File From d6dad4b0f8802f30c8dff9da0da11536dead9dea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Thu, 13 Mar 2025 16:34:19 -0300 Subject: [PATCH 5/5] fix: directoryLabel not read from correct place --- src/files/infra/repositories/transformers/FilePayload.ts | 2 +- src/files/infra/repositories/transformers/fileTransformers.ts | 4 ++-- test/testHelpers/files/filesHelper.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/files/infra/repositories/transformers/FilePayload.ts b/src/files/infra/repositories/transformers/FilePayload.ts index 9f31a9ed..58afcc4a 100644 --- a/src/files/infra/repositories/transformers/FilePayload.ts +++ b/src/files/infra/repositories/transformers/FilePayload.ts @@ -10,7 +10,6 @@ export interface FilePayload { version: number description?: string restricted: boolean - directoryLabel?: string datasetVersionId?: number categories?: string[] contentType: string @@ -37,6 +36,7 @@ export interface FilePayload { } version: number restricted: boolean + directoryLabel?: string label: string datasetVersionId: number } diff --git a/src/files/infra/repositories/transformers/fileTransformers.ts b/src/files/infra/repositories/transformers/fileTransformers.ts index f0476de1..5350af01 100644 --- a/src/files/infra/repositories/transformers/fileTransformers.ts +++ b/src/files/infra/repositories/transformers/fileTransformers.ts @@ -44,8 +44,8 @@ const transformFilePayloadToFile = (filePayload: FilePayload): FileModel => { ...(filePayload.dataFile.description && { description: filePayload.dataFile.description }), restricted: filePayload.restricted, latestRestricted: filePayload.dataFile.restricted, - ...(filePayload.dataFile.directoryLabel && { - directoryLabel: filePayload.dataFile.directoryLabel + ...(filePayload.directoryLabel && { + directoryLabel: filePayload.directoryLabel }), ...(filePayload.dataFile.datasetVersionId && { datasetVersionId: filePayload.dataFile.datasetVersionId diff --git a/test/testHelpers/files/filesHelper.ts b/test/testHelpers/files/filesHelper.ts index 9de72830..1f0e6296 100644 --- a/test/testHelpers/files/filesHelper.ts +++ b/test/testHelpers/files/filesHelper.ts @@ -81,6 +81,7 @@ export const createFilePayload = (): FilePayload => { restricted: false, version: 1, datasetVersionId: 2, + directoryLabel: 'directoryLabel', dataFile: { id: 1, version: 1, @@ -116,7 +117,6 @@ export const createFilePayload = (): FilePayload => { isPartOf: { type: DvObjectType.DATAVERSE, identifier: 'root', displayName: 'Root' } }, description: 'description', - directoryLabel: 'directoryLabel', datasetVersionId: 1, originalFormat: 'originalFormat', originalSize: 127426,