Changed: separated adding uploaded file to dataset from file upload u…

…se case

src/files/domain/clients/IDirectUploadClient.ts

@@ -7,5 +7,7 @@ export interface IDirectUploadClient {
     progress: (now: number) => void,
     abortController: AbortController,
     destination?: FileUploadDestination
-  ): Promise<void>
+  ): Promise<string>
+
+  addUploadedFileToDataset(datasetId: number | string, file: File, storageId: string): Promise<void>
 }

src/files/domain/useCases/AddUploadedFileToDataset.ts

@@ -0,0 +1,18 @@
+import { UseCase } from '../../../core/domain/useCases/UseCase'
+import { IDirectUploadClient } from '../clients/IDirectUploadClient'
+
+export class AddUploadedFileToDataset implements UseCase<void> {
+  private directUploadClient: IDirectUploadClient
+
+  constructor(directUploadClient: IDirectUploadClient) {
+    this.directUploadClient = directUploadClient
+  }
+
+  /**
+   * TODO
+   * @returns {Promise<void>}
+   */
+  async execute(datasetId: number | string, file: File, storageId: string): Promise<void> {
+    await this.directUploadClient.addUploadedFileToDataset(datasetId, file, storageId)
+  }
+}

src/files/domain/useCases/UploadFile.ts

@@ -1,7 +1,7 @@
 import { UseCase } from '../../../core/domain/useCases/UseCase'
 import { IDirectUploadClient } from '../clients/IDirectUploadClient'
 
-export class UploadFile implements UseCase<void> {
+export class UploadFile implements UseCase<string> {
   private directUploadClient: IDirectUploadClient
 
   constructor(directUploadClient: IDirectUploadClient) {
@@ -10,14 +10,14 @@ export class UploadFile implements UseCase<void> {
 
   /**
    * TODO
-   * @returns {Promise<void>}
+   * @returns {Promise<string>}
    */
   async execute(
     datasetId: number | string,
     file: File,
     progress: (now: number) => void,
     abortController: AbortController
-  ): Promise<void> {
-    await this.directUploadClient.uploadFile(datasetId, file, progress, abortController)
+  ): Promise<string> {
+    return await this.directUploadClient.uploadFile(datasetId, file, progress, abortController)
   }
 }

src/files/index.ts

@@ -10,6 +10,7 @@ import { GetFileCitation } from './domain/useCases/GetFileCitation'
 import { GetFileAndDataset } from './domain/useCases/GetFileAndDataset'
 import { UploadFile } from './domain/useCases/UploadFile'
 import { DirectUploadClient } from './infra/clients/DirectUploadClient'
+import { AddUploadedFileToDataset } from './domain/useCases/AddUploadedFileToDataset'
 
 const filesRepository = new FilesRepository()
 const directUploadClient = new DirectUploadClient(filesRepository)
@@ -24,6 +25,7 @@ const getFile = new GetFile(filesRepository)
 const getFileAndDataset = new GetFileAndDataset(filesRepository)
 const getFileCitation = new GetFileCitation(filesRepository)
 const uploadFile = new UploadFile(directUploadClient)
+const addUploadedFileToDataset = new AddUploadedFileToDataset(directUploadClient)
 
 export {
   getDatasetFiles,
@@ -35,7 +37,8 @@ export {
   getFile,
   getFileAndDataset,
   getFileCitation,
-  uploadFile
+  uploadFile,
+  addUploadedFileToDataset
 }
 
 export { File, FileEmbargo, FileChecksum } from './domain/models/File'

src/files/infra/clients/DirectUploadClient.ts

@@ -37,7 +37,7 @@ export class DirectUploadClient implements IDirectUploadClient {
     progress: (now: number) => void,
     abortController: AbortController,
     destination?: FileUploadDestination
-  ): Promise<void> {
+  ): Promise<string> {
     if (destination == undefined) {
       destination = await this.filesRepository
         .getFileUploadDestination(datasetId, file)
@@ -49,10 +49,12 @@ export class DirectUploadClient implements IDirectUploadClient {
 
     if (destination.urls.length === 1) {
       await this.uploadSinglepartFile(datasetId, file, destination, abortController)
-      progress(this.progressAfterFileUpload)
     } else {
       await this.uploadMultipartFile(datasetId, file, destination, progress, abortController)
     }
+    progress(this.progressAfterFileUpload)
+
+    return destination.storageId
   }
 
   private async uploadSinglepartFile(
@@ -145,10 +147,7 @@ export class DirectUploadClient implements IDirectUploadClient {
       destination,
       eTags,
       abortController
-    ).then(() => {
-      progress(this.progressAfterFileUpload)
-      undefined
-    })
+    )
   }
 
   private async abortMultipartUpload(

test/integration/files/DirectUploadClient.test.ts

@@ -1,4 +1,10 @@
-import { ApiConfig, CreatedDatasetIdentifiers, createDataset } from '../../../src'
+import {
+  ApiConfig,
+  CreatedDatasetIdentifiers,
+  DatasetNotNumberedVersion,
+  FileOrderCriteria,
+  createDataset
+} from '../../../src'
 import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig'
 import { FilesRepository } from '../../../src/files/infra/repositories/FilesRepository'
 import { DirectUploadClient } from '../../../src/files/infra/clients/DirectUploadClient'
@@ -16,7 +22,7 @@ import {
 } from '../../testHelpers/files/filesHelper'
 import { FileUploadCancelError } from '../../../src/files/infra/clients/errors/FileUploadCancelError'
 
-describe('uploadFile', () => {
+describe('DirectUploadClient', () => {
   const testCollectionAlias = 'directUploadTestCollection'
   let testDataset1Ids: CreatedDatasetIdentifiers
   let testDataset2Ids: CreatedDatasetIdentifiers
@@ -61,7 +67,7 @@ describe('uploadFile', () => {
     await deleteCollectionViaApi(testCollectionAlias)
   })
 
-  test('should upload file to destination when there is only one destination URL', async () => {
+  test('should upload file and add it to the dataset when there is only one destination URL', async () => {
     const destination = await createTestFileUploadDestination(
       singlepartFile,
       testDataset1Ids.numericId
@@ -73,22 +79,50 @@ describe('uploadFile', () => {
 
     expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(false)
 
-    await sut.uploadFile(
+    // Test uploadFile method
+
+    const actualStorageId = await sut.uploadFile(
       testDataset1Ids.numericId,
       singlepartFile,
       progressMock,
       abortController,
       destination
     )
+    expect(actualStorageId).toBe(destination.storageId)
 
     expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(true)
 
     expect(progressMock).toHaveBeenCalledWith(10)
     expect(progressMock).toHaveBeenCalledWith(100)
     expect(progressMock).toHaveBeenCalledTimes(2)
+
+    // Test addUploadedFileToDataset method
+
+    let datasetFiles = await filesRepository.getDatasetFiles(
+      testDataset1Ids.numericId,
+      DatasetNotNumberedVersion.LATEST,
+      true,
+      FileOrderCriteria.NAME_AZ
+    )
+
+    expect(datasetFiles.totalFilesCount).toBe(0)
+
+    await sut.addUploadedFileToDataset(testDataset1Ids.numericId, singlepartFile, actualStorageId)
+
+    datasetFiles = await filesRepository.getDatasetFiles(
+      testDataset1Ids.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:')
   })
 
-  test('should upload file to destinations when there are multiple destination URLs', async () => {
+  test('should upload file and add it to the dataset when there are multiple destination URLs', async () => {
     const destination = await createTestFileUploadDestination(
       multipartFile,
       testDataset2Ids.numericId
@@ -97,19 +131,47 @@ describe('uploadFile', () => {
     const progressMock = jest.fn()
     const abortController = new AbortController()
 
-    await sut.uploadFile(
+    // Test uploadFile method
+
+    const actualStorageId = await sut.uploadFile(
       testDataset2Ids.numericId,
       multipartFile,
       progressMock,
       abortController,
       destination
     )
+    expect(actualStorageId).toBe(destination.storageId)
 
     expect(progressMock).toHaveBeenCalledWith(10)
     expect(progressMock).toHaveBeenCalledWith(50)
     expect(progressMock).toHaveBeenCalledWith(90)
     expect(progressMock).toHaveBeenCalledWith(100)
     expect(progressMock).toHaveBeenCalledTimes(4)
+
+    // Test addUploadedFileToDataset method
+
+    let datasetFiles = await filesRepository.getDatasetFiles(
+      testDataset2Ids.numericId,
+      DatasetNotNumberedVersion.LATEST,
+      true,
+      FileOrderCriteria.NAME_AZ
+    )
+
+    expect(datasetFiles.totalFilesCount).toBe(0)
+
+    await sut.addUploadedFileToDataset(testDataset2Ids.numericId, multipartFile, actualStorageId)
+
+    datasetFiles = await filesRepository.getDatasetFiles(
+      testDataset2Ids.numericId,
+      DatasetNotNumberedVersion.LATEST,
+      true,
+      FileOrderCriteria.NAME_AZ
+    )
+
+    expect(datasetFiles.totalFilesCount).toBe(1)
+    expect(datasetFiles.files[0].name).toBe('multipart-file')
+    expect(datasetFiles.files[0].sizeBytes).toBe(multipartFile.size)
+    expect(datasetFiles.files[0].storageIdentifier).toContain('localstack1://mybucket:')
   })
 
   test('should not finish uploading file to destinations when user cancels immediately and there are multiple destination urls', async () => {

test/unit/files/AddUploadedFileToDataset.test.ts

@@ -0,0 +1,35 @@
+import { createSinglepartFileBlob } from '../../testHelpers/files/filesHelper'
+import { IDirectUploadClient } from '../../../src/files/domain/clients/IDirectUploadClient'
+import { DirectUploadClientError } from '../../../src/files/domain/clients/DirectUploadClientError'
+import { AddUploadedFileToDataset } from '../../../src/files/domain/useCases/AddUploadedFileToDataset'
+
+describe('execute', () => {
+  let testFile: File
+  const testStorageId = 'test'
+
+  beforeAll(async () => {
+    testFile = await createSinglepartFileBlob()
+  })
+
+  test('should return undefined on client success', async () => {
+    const directUploadClientStub: IDirectUploadClient = {} as IDirectUploadClient
+    directUploadClientStub.addUploadedFileToDataset = jest.fn().mockResolvedValue(undefined)
+
+    const sut = new AddUploadedFileToDataset(directUploadClientStub)
+
+    const actual = await sut.execute(1, testFile, testStorageId)
+
+    expect(actual).toEqual(undefined)
+  })
+
+  test('should return error on client error', async () => {
+    const directUploadClientStub: IDirectUploadClient = {} as IDirectUploadClient
+    directUploadClientStub.addUploadedFileToDataset = jest
+      .fn()
+      .mockRejectedValue(new DirectUploadClientError('test', 'test', 'test'))
+
+    const sut = new AddUploadedFileToDataset(directUploadClientStub)
+
+    await expect(sut.execute(1, testFile, testStorageId)).rejects.toThrow(DirectUploadClientError)
+  })
+})

test/unit/files/DirectUploadClient.test.ts

@@ -17,6 +17,7 @@ import { FilePartUploadError } from '../../../src/files/infra/clients/errors/Fil
 import { MultipartAbortError } from '../../../src/files/infra/clients/errors/MultipartAbortError'
 import { TestConstants } from '../../testHelpers/TestConstants'
 import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig'
+import { FileUploadDestination } from '../../../src/files/domain/models/FileUploadDestination'
 
 describe('uploadFile', () => {
   beforeEach(() => {
@@ -74,11 +75,10 @@ describe('uploadFile', () => {
       expect(progressMock).toHaveBeenCalledTimes(1)
     })
 
-    test('should return undefined on operation success', async () => {
+    test('should storage identifier on operation success', async () => {
       const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
-      filesRepositoryStub.getFileUploadDestination = jest
-        .fn()
-        .mockResolvedValue(createSingleFileUploadDestinationModel())
+      const testDestination: FileUploadDestination = createSingleFileUploadDestinationModel()
+      filesRepositoryStub.getFileUploadDestination = jest.fn().mockResolvedValue(testDestination)
       filesRepositoryStub.addUploadedFileToDataset = jest.fn().mockResolvedValue(undefined)
 
       jest.spyOn(axios, 'put').mockResolvedValue(undefined)
@@ -94,7 +94,7 @@ describe('uploadFile', () => {
       expect(progressMock).toHaveBeenCalledWith(100)
       expect(progressMock).toHaveBeenCalledTimes(2)
 
-      expect(actual).toEqual(undefined)
+      expect(actual).toEqual(testDestination.storageId)
     })
   })
 
@@ -193,7 +193,7 @@ describe('uploadFile', () => {
       expect(progressMock).toHaveBeenCalledTimes(3)
     })
 
-    test('should return undefined on operation success', async () => {
+    test('should return storage identifier on operation success', async () => {
       const testMultipartDestination = createMultipartFileUploadDestinationModel()
       const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
       filesRepositoryStub.getFileUploadDestination = jest
@@ -214,7 +214,7 @@ describe('uploadFile', () => {
 
       const actual = await sut.uploadFile(1, testFile, progressMock, abortController)
 
-      expect(actual).toEqual(undefined)
+      expect(actual).toEqual(testMultipartDestination.storageId)
 
       expect(progressMock).toHaveBeenCalledWith(10)
       expect(progressMock).toHaveBeenCalledWith(50)

test/unit/files/UploadFile.test.ts

@@ -5,14 +5,15 @@ import { DirectUploadClientError } from '../../../src/files/domain/clients/Direc
 
 describe('execute', () => {
   let testFile: File
+  const testStorageId = 'test'
 
   beforeAll(async () => {
     testFile = await createSinglepartFileBlob()
   })
 
-  test('should return undefined on client success', async () => {
+  test('should return storage identifier on client success', async () => {
     const directUploadClientStub: IDirectUploadClient = {} as IDirectUploadClient
-    directUploadClientStub.uploadFile = jest.fn().mockResolvedValue(undefined)
+    directUploadClientStub.uploadFile = jest.fn().mockResolvedValue(testStorageId)
 
     const sut = new UploadFile(directUploadClientStub)
 
@@ -21,7 +22,7 @@ describe('execute', () => {
 
     const actual = await sut.execute(1, testFile, progressMock, abortController)
 
-    expect(actual).toEqual(undefined)
+    expect(actual).toEqual(testStorageId)
   })
 
   test('should return error on client error', async () => {