Polish configurable upload PR

Author: ErykKul

.gitignore

@@ -9,10 +9,11 @@ node_modules
 
 # unit tests
 coverage
+test/environment/docker-dev-volumes
 
 # macOS
 .DS_Store
 
 # ignore npm lock
 package-json.lock
-.npmrc
+.npmrc

.prettierignore

@@ -1 +1,2 @@
-test/environment/docker-dev-volumes/
+test/environment/docker-dev-volumes
+test/environment/docker-dev-volumes/**

CHANGELOG.md

@@ -28,10 +28,8 @@ This changelog follows the principles of [Keep a Changelog](https://keepachangel
 - Templates: Added `setTemplateAsDefault` use case and repository method to support Dataverse endpoint `POST /dataverses/{id}/template/default/{templateId}`.
 - Templates: Added `unsetTemplateAsDefault` use case and repository method to support Dataverse endpoint `DELETE /dataverses/{id}/template/default`.
 - New Use Case: [Update Terms of Access](./docs/useCases.md#update-terms-of-access).
-- Files: Added `FilesConfig` class for configuring file upload behavior at runtime, including:
-  - `useS3Tagging`: Option to disable S3 object tagging (`x-amz-tagging` header) for S3-compatible storage that doesn't support tagging. Default: `true`.
-  - `maxMultipartRetries`: Configurable maximum retries for multipart upload parts. Default: `5`.
-  - `fileUploadTimeoutMs`: Configurable timeout for file upload operations. Default: `60000`.
+- Files: Direct uploads now forward the `tagging` value returned by the upload destination response as the `x-amz-tagging` header for single-part uploads.
+- Files: Added a `DirectUploadClientConfig` object for configuring multipart upload retries and upload timeout.
 - Guestbooks: Added use cases and repository support for guestbook creation, listing, and enabling/disabling.
 - Guestbooks: Added dataset-level guestbook assignment and removal support via `assignDatasetGuestbook` (`PUT /api/datasets/{identifier}/guestbook`) and `removeDatasetGuestbook` (`DELETE /api/datasets/{identifier}/guestbook`).
 - Datasets/Guestbooks: Added `guestbookId` in `getDataset` responses.

package.json

@@ -16,10 +16,10 @@
     "test:coverage": "jest --coverage -c jest.config.ts",
     "test:coverage:check": "jest --coverage --ci --config jest.config.ts",
     "lint": "npm run lint:eslint && npm run lint:prettier",
-    "lint:fix": "eslint --fix --ext .ts ./src --ignore-path .gitignore ./src",
-    "lint:eslint": "eslint --ignore-path .gitignore ./src",
-    "lint:prettier": "prettier --check '**/*.(yml|json|md)'",
-    "format": "prettier --write './src/**/*.{js,ts,md,json,yml,md}' --config ./.prettierrc",
+    "lint:fix": "eslint --fix --ext .ts --ignore-path .gitignore .",
+    "lint:eslint": "eslint --ext .ts --ignore-path .gitignore .",
+    "lint:prettier": "prettier --check '*.{yml,json,md}' 'docs/**/*.{yml,json,md}' 'test/environment/docker-compose.yml'",
+    "format": "prettier --write './src/**/*.{js,ts,md,json,yml}' '*.{yml,json,md}' 'docs/**/*.{yml,json,md}' 'test/environment/docker-compose.yml' --config ./.prettierrc",
     "typecheck": "tsc --noEmit",
     "prepare": "husky"
   },

src/files/infra/clients/DirectUploadClient.ts

@@ -99,12 +99,17 @@ export class DirectUploadClient implements IDirectUploadClient {
     const eTags: Record<number, string> = {}
     const maxRetries = this.maxMultipartRetries
     const limitConcurrency = pLimit(1)
+    let uploadFailed = false
 
     const uploadPart = async (
       destinationUrl: string,
       index: number,
       retries = 0
     ): Promise<void> => {
+      if (uploadFailed) {
+        return
+      }
+
       const offset = index * partMaxSize
       const partSize = Math.min(partMaxSize, file.size - offset)
       const fileSlice = file.slice(offset, offset + partSize)
@@ -125,6 +130,8 @@ export class DirectUploadClient implements IDirectUploadClient {
         eTags[`${index + 1}`] = eTag
       } catch (error) {
         if (axios.isCancel(error)) {
+          uploadFailed = true
+          limitConcurrency.clearQueue()
           await this.abortMultipartUpload(file.name, datasetId, destination.abortEndpoint as string)
           throw new FileUploadCancelError(file.name, datasetId)
         }
@@ -133,6 +140,8 @@ export class DirectUploadClient implements IDirectUploadClient {
           await new Promise((resolve) => setTimeout(resolve, backoffDelay))
           await uploadPart(destinationUrl, index, retries + 1)
         } else {
+          uploadFailed = true
+          limitConcurrency.clearQueue()
           await this.abortMultipartUpload(file.name, datasetId, destination.abortEndpoint as string)
 
           const errorMessage =

test/functional/collections/GetMyDataCollectionItems.test.ts

@@ -76,7 +76,7 @@ describe('execute', () => {
         [PublicationStatus.Deaccessioned],
         undefined,
         undefined,
-        undefined
+        testCollectionAlias
       )
       throw new Error('Use case should throw an error')
     } catch (error) {

test/functional/datasets/GetDatasetAvailableDatasetTypes.test.ts

@@ -25,7 +25,7 @@ describe('getDatasetAvailableDatasetTypes', () => {
           displayName: 'Dataset'
         }
       ]
-      expect(actualDatasetTypes).toEqual(expectedDatasetTypes)
+      expect(actualDatasetTypes).toEqual(expect.arrayContaining(expectedDatasetTypes))
     })
   })
 })

test/unit/files/DirectUploadClient.test.ts

@@ -88,9 +88,12 @@ describe('uploadFile', () => {
       expect(actual).toEqual(testDestination.storageId)
     })
 
-    test('should include S3 tagging header by default', async () => {
+    test('should include S3 tagging header when upload destination provides tagging', async () => {
       const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
-      const testDestination: FileUploadDestination = createSingleFileUploadDestinationModel()
+      const testDestination: FileUploadDestination = {
+        ...createSingleFileUploadDestinationModel(),
+        tagging: 'dv-state=temp'
+      }
       filesRepositoryStub.getFileUploadDestination = jest.fn().mockResolvedValue(testDestination)
 
       const axiosPutSpy = jest.spyOn(axios, 'put').mockResolvedValue(undefined)
@@ -113,14 +116,14 @@ describe('uploadFile', () => {
       )
     })
 
-    test('should not include S3 tagging header when useS3Tagging is false', async () => {
+    test('should not include S3 tagging header when upload destination omits tagging', async () => {
       const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
       const testDestination: FileUploadDestination = createSingleFileUploadDestinationModel()
       filesRepositoryStub.getFileUploadDestination = jest.fn().mockResolvedValue(testDestination)
 
       const axiosPutSpy = jest.spyOn(axios, 'put').mockResolvedValue(undefined)
 
-      const sut = new DirectUploadClient(filesRepositoryStub, { useS3Tagging: false })
+      const sut = new DirectUploadClient(filesRepositoryStub)
 
       const progressMock = jest.fn()
       const abortController = new AbortController()
@@ -137,6 +140,29 @@ describe('uploadFile', () => {
         })
       )
     })
+
+    test('should use configured file upload timeout', async () => {
+      const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
+      const testDestination: FileUploadDestination = createSingleFileUploadDestinationModel()
+      filesRepositoryStub.getFileUploadDestination = jest.fn().mockResolvedValue(testDestination)
+
+      const axiosPutSpy = jest.spyOn(axios, 'put').mockResolvedValue(undefined)
+
+      const sut = new DirectUploadClient(filesRepositoryStub, { fileUploadTimeoutMs: 30_000 })
+
+      const progressMock = jest.fn()
+      const abortController = new AbortController()
+
+      await sut.uploadFile(1, testFile, progressMock, abortController)
+
+      expect(axiosPutSpy).toHaveBeenCalledWith(
+        testDestination.urls[0],
+        expect.anything(),
+        expect.objectContaining({
+          timeout: 30_000
+        })
+      )
+    })
   })
 
   describe('Multiple parts file', () => {

test/unit/files/FilesConfig.test.ts

@@ -148,6 +148,48 @@ describe('FilesRepository', () => {
         expect(actual).toEqual(testMultipleFileUploadDestination)
       })
 
+      test('should return destination with tagging when single response includes tagging', async () => {
+        const tagging = 'dv-state=temp'
+        jest.spyOn(axios, 'get').mockResolvedValue({
+          data: {
+            status: 'OK',
+            data: {
+              ...createSingleFileUploadDestinationPayload(),
+              tagging
+            }
+          }
+        })
+        jest.spyOn(fs, 'statSync').mockReturnValue({ size: testFileSize } as fs.Stats)
+
+        const actual = await sut.getFileUploadDestination(testDatasetId, singlepartFile)
+
+        expect(actual).toEqual({
+          ...testSingleFileUploadDestination,
+          tagging
+        })
+      })
+
+      test('should return destination with tagging when multipart response includes tagging', async () => {
+        const tagging = 'dv-state=temp'
+        jest.spyOn(axios, 'get').mockResolvedValue({
+          data: {
+            status: 'OK',
+            data: {
+              ...createMultipartFileUploadDestinationPayload(),
+              tagging
+            }
+          }
+        })
+        jest.spyOn(fs, 'statSync').mockReturnValue({ size: testFileSize } as fs.Stats)
+
+        const actual = await sut.getFileUploadDestination(testDatasetId, multipartFile)
+
+        expect(actual).toEqual({
+          ...testMultipleFileUploadDestination,
+          tagging
+        })
+      })
+
       test('should return error on repository read error', async () => {
         jest.spyOn(axios, 'get').mockRejectedValue(TestConstants.TEST_ERROR_RESPONSE)
         jest.spyOn(fs, 'statSync').mockReturnValue({ size: testFileSize } as fs.Stats)