Merge branch 'develop' into 178-publish-collection

Author: ekraffmiller

src/files/infra/clients/DirectUploadClient.ts

@@ -19,9 +19,6 @@ export class DirectUploadClient implements IDirectUploadClient {
   private filesRepository: IFilesRepository
   private maxMultipartRetries: number
 
-  private readonly progressAfterUrlGeneration: number = 10
-  private readonly progressAfterFileUpload: number = 100
-
   private readonly fileUploadTimeoutMs: number = 60_000
 
   constructor(filesRepository: IFilesRepository, maxMultipartRetries = 5) {
@@ -43,14 +40,12 @@ export class DirectUploadClient implements IDirectUploadClient {
           throw new UrlGenerationError(file.name, datasetId, error.message)
         })
     }
-    progress(this.progressAfterUrlGeneration)
 
     if (destination.urls.length === 1) {
-      await this.uploadSinglepartFile(datasetId, file, destination, abortController)
+      await this.uploadSinglepartFile(datasetId, file, destination, progress, abortController)
     } else {
       await this.uploadMultipartFile(datasetId, file, destination, progress, abortController)
     }
-    progress(this.progressAfterFileUpload)
 
     return destination.storageId
   }
@@ -59,18 +54,20 @@ export class DirectUploadClient implements IDirectUploadClient {
     datasetId: number | string,
     file: File,
     destination: FileUploadDestination,
+    progress: (now: number) => void,
     abortController: AbortController
   ): Promise<void> {
     try {
       const arrayBuffer = await file.arrayBuffer()
       await axios.put(destination.urls[0], arrayBuffer, {
         headers: {
           'Content-Type': 'application/octet-stream',
-          'Content-Length': file.size.toString(),
           'x-amz-tagging': 'dv-state=temp'
         },
         timeout: this.fileUploadTimeoutMs,
-        signal: abortController.signal
+        signal: abortController.signal,
+        onUploadProgress: (progressEvent) =>
+          progress(Math.round((progressEvent.loaded * 100) / file.size))
       })
     } catch (error) {
       if (axios.isCancel(error)) {
@@ -92,8 +89,6 @@ export class DirectUploadClient implements IDirectUploadClient {
     const maxRetries = this.maxMultipartRetries
     const limitConcurrency = pLimit(1)
 
-    const progressPartSize = 80 / destination.urls.length
-
     const uploadPart = async (
       destinationUrl: string,
       index: number,
@@ -106,17 +101,17 @@ export class DirectUploadClient implements IDirectUploadClient {
       try {
         const response = await axios.put(destinationUrl, fileSlice, {
           headers: {
-            'Content-Type': 'application/octet-stream',
-            'Content-Length': fileSlice.size
+            'Content-Type': 'application/octet-stream'
           },
           maxBodyLength: Infinity,
           maxContentLength: Infinity,
           timeout: this.fileUploadTimeoutMs,
-          signal: abortController.signal
+          signal: abortController.signal,
+          onUploadProgress: (progressEvent) =>
+            progress(Math.round(((offset + progressEvent.loaded) * 100) / file.size))
         })
         const eTag = response.headers['etag'].replace(/"/g, '')
         eTags[`${index + 1}`] = eTag
-        progress(Math.round(this.progressAfterUrlGeneration + progressPartSize * (index + 1)))
       } catch (error) {
         if (axios.isCancel(error)) {
           await this.abortMultipartUpload(file.name, datasetId, destination.abortEndpoint)

test/integration/files/DirectUpload.test.ts

@@ -95,10 +95,6 @@ describe('Direct Upload', () => {
 
     expect(await singlepartFileExistsInBucket(singlepartFileUrl)).toBe(true)
 
-    expect(progressMock).toHaveBeenCalledWith(10)
-    expect(progressMock).toHaveBeenCalledWith(100)
-    expect(progressMock).toHaveBeenCalledTimes(2)
-
     // Test FilesRepository.addUploadedFileToDataset method
 
     let datasetFiles = await filesRepositorySut.getDatasetFiles(
@@ -156,12 +152,6 @@ describe('Direct Upload', () => {
     )
     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 FilesRepository.addUploadedFileToDataset method
 
     let datasetFiles = await filesRepositorySut.getDatasetFiles(
@@ -221,10 +211,6 @@ describe('Direct Upload', () => {
         destination
       )
     ).rejects.toThrow(FileUploadCancelError)
-
-    expect(progressMock).not.toHaveBeenCalledWith(50)
-    expect(progressMock).not.toHaveBeenCalledWith(90)
-    expect(progressMock).not.toHaveBeenCalledWith(100)
   })
 
   const createTestFileUploadDestination = async (file: File, testDatasetId: number) => {

test/unit/files/DirectUploadClient.test.ts

@@ -50,8 +50,6 @@ describe('uploadFile', () => {
       await expect(sut.uploadFile(1, testFile, progressMock, abortController)).rejects.toThrow(
         UrlGenerationError
       )
-
-      expect(progressMock).not.toHaveBeenCalled()
     })
 
     test('should return FileUploadError when there is an error on single file upload', async () => {
@@ -70,9 +68,6 @@ describe('uploadFile', () => {
       await expect(sut.uploadFile(1, testFile, progressMock, abortController)).rejects.toThrow(
         FileUploadError
       )
-
-      expect(progressMock).toHaveBeenCalledWith(10)
-      expect(progressMock).toHaveBeenCalledTimes(1)
     })
 
     test('should storage identifier on operation success', async () => {
@@ -90,10 +85,6 @@ describe('uploadFile', () => {
 
       const actual = await sut.uploadFile(1, testFile, progressMock, abortController)
 
-      expect(progressMock).toHaveBeenCalledWith(10)
-      expect(progressMock).toHaveBeenCalledWith(100)
-      expect(progressMock).toHaveBeenCalledTimes(2)
-
       expect(actual).toEqual(testDestination.storageId)
     })
   })
@@ -138,9 +129,6 @@ describe('uploadFile', () => {
           params: {}
         }
       )
-
-      expect(progressMock).toHaveBeenCalledWith(10)
-      expect(progressMock).toHaveBeenCalledTimes(1)
     })
 
     test('should return MultipartAbortError when there is an error on multipart file upload and abort endpoint call fails', async () => {
@@ -160,9 +148,6 @@ describe('uploadFile', () => {
       await expect(sut.uploadFile(1, testFile, progressMock, abortController)).rejects.toThrow(
         MultipartAbortError
       )
-
-      expect(progressMock).toHaveBeenCalledWith(10)
-      expect(progressMock).toHaveBeenCalledTimes(1)
     })
 
     test('should return MultipartCompletionError when there is an error on multipart file completion', async () => {
@@ -186,11 +171,6 @@ describe('uploadFile', () => {
       )
 
       expect(axios.put).toHaveBeenCalledTimes(3)
-
-      expect(progressMock).toHaveBeenCalledWith(10)
-      expect(progressMock).toHaveBeenCalledWith(50)
-      expect(progressMock).toHaveBeenCalledWith(90)
-      expect(progressMock).toHaveBeenCalledTimes(3)
     })
 
     test('should return storage identifier on operation success', async () => {
@@ -215,12 +195,6 @@ describe('uploadFile', () => {
       const actual = await sut.uploadFile(1, testFile, progressMock, abortController)
 
       expect(actual).toEqual(testMultipartDestination.storageId)
-
-      expect(progressMock).toHaveBeenCalledWith(10)
-      expect(progressMock).toHaveBeenCalledWith(50)
-      expect(progressMock).toHaveBeenCalledWith(90)
-      expect(progressMock).toHaveBeenCalledWith(100)
-      expect(progressMock).toHaveBeenCalledTimes(4)
     })
   })
 })