Skip to content

Commit 65acf6a

Browse files
committed
feat: add configurable file upload options and related tests
1 parent a937365 commit 65acf6a

File tree

5 files changed

+199
-16
lines changed

5 files changed

+199
-16
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,15 @@ This changelog follows the principles of [Keep a Changelog](https://keepachangel
1313
- New Use Case: [Create a Dataset Template](./docs/useCases.md#create-a-dataset-template) under Collections.
1414

1515
- New Use Case: [Update Terms of Access](./docs/useCases.md#update-terms-of-access).
16+
- Files: Added `FilesConfig` class for configuring file upload behavior at runtime, including:
17+
- `useS3Tagging`: Option to disable S3 object tagging (`x-amz-tagging` header) for S3-compatible storage that doesn't support tagging. Default: `true`.
18+
- `maxMultipartRetries`: Configurable maximum retries for multipart upload parts. Default: `5`.
19+
- `fileUploadTimeoutMs`: Configurable timeout for file upload operations. Default: `60000`.
1620

1721
### Changed
1822

1923
- Add pagination query parameters to Dataset Version Summeries and File Version Summaries use cases
24+
- Files: `DirectUploadClient` constructor now accepts a `DirectUploadClientConfig` object instead of a plain number for `maxMultipartRetries`.
2025

2126
### Fixed
2227

src/files/index.ts

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { GetFile } from './domain/useCases/GetFile'
99
import { GetFileCitation } from './domain/useCases/GetFileCitation'
1010
import { GetFileAndDataset } from './domain/useCases/GetFileAndDataset'
1111
import { UploadFile } from './domain/useCases/UploadFile'
12-
import { DirectUploadClient } from './infra/clients/DirectUploadClient'
12+
import { DirectUploadClient, DirectUploadClientConfig } from './infra/clients/DirectUploadClient'
1313
import { AddUploadedFilesToDataset } from './domain/useCases/AddUploadedFilesToDataset'
1414
import { DeleteFile } from './domain/useCases/DeleteFile'
1515
import { ReplaceFile } from './domain/useCases/ReplaceFile'
@@ -20,8 +20,40 @@ import { UpdateFileCategories } from './domain/useCases/UpdateFileCategories'
2020
import { GetFileVersionSummaries } from './domain/useCases/GetFileVersionSummaries'
2121
import { IsFileDeleted } from './domain/useCases/IsFileDeleted'
2222

23+
/**
24+
* Configuration for file upload operations.
25+
* Use FilesConfig.init() to configure upload behavior before using uploadFile.
26+
*/
27+
class FilesConfig {
28+
private static uploadConfig: DirectUploadClientConfig = {}
29+
30+
/**
31+
* Initialize file upload configuration.
32+
* @param config - Configuration options for file uploads
33+
* @param config.useS3Tagging - Whether to include S3 tagging header (x-amz-tagging: dv-state=temp).
34+
* Set to false if your S3 implementation doesn't support object tagging. Default: true
35+
* @param config.maxMultipartRetries - Maximum number of retries for multipart upload parts. Default: 5
36+
* @param config.fileUploadTimeoutMs - Timeout in milliseconds for file upload operations. Default: 60000
37+
*/
38+
static init(config: DirectUploadClientConfig) {
39+
this.uploadConfig = config
40+
}
41+
42+
static getConfig(): DirectUploadClientConfig {
43+
return this.uploadConfig
44+
}
45+
}
46+
2347
const filesRepository = new FilesRepository()
24-
const directUploadClient = new DirectUploadClient(filesRepository)
48+
// DirectUploadClient is created lazily to allow configuration before first use
49+
let directUploadClientInstance: DirectUploadClient | null = null
50+
51+
const getDirectUploadClient = (): DirectUploadClient => {
52+
if (!directUploadClientInstance) {
53+
directUploadClientInstance = new DirectUploadClient(filesRepository, FilesConfig.getConfig())
54+
}
55+
return directUploadClientInstance
56+
}
2557

2658
const getDatasetFiles = new GetDatasetFiles(filesRepository)
2759
const getDatasetFileCounts = new GetDatasetFileCounts(filesRepository)
@@ -32,7 +64,6 @@ const getDatasetFilesTotalDownloadSize = new GetDatasetFilesTotalDownloadSize(fi
3264
const getFile = new GetFile(filesRepository)
3365
const getFileAndDataset = new GetFileAndDataset(filesRepository)
3466
const getFileCitation = new GetFileCitation(filesRepository)
35-
const uploadFile = new UploadFile(directUploadClient)
3667
const addUploadedFilesToDataset = new AddUploadedFilesToDataset(filesRepository)
3768
const deleteFile = new DeleteFile(filesRepository)
3869
const replaceFile = new ReplaceFile(filesRepository)
@@ -43,6 +74,27 @@ const updateFileCategories = new UpdateFileCategories(filesRepository)
4374
const getFileVersionSummaries = new GetFileVersionSummaries(filesRepository)
4475
const isFileDeleted = new IsFileDeleted(filesRepository)
4576

77+
// uploadFile is created lazily to respect FilesConfig settings
78+
let uploadFileInstance: UploadFile | null = null
79+
80+
/**
81+
* Uploads a file to remote storage and returns the storage identifier.
82+
* Respects FilesConfig settings (call FilesConfig.init() before first upload if you need custom config).
83+
*/
84+
const uploadFile = {
85+
execute: (
86+
datasetId: number | string,
87+
file: File,
88+
progress: (now: number) => void,
89+
abortController: AbortController
90+
): Promise<string> => {
91+
if (!uploadFileInstance) {
92+
uploadFileInstance = new UploadFile(getDirectUploadClient())
93+
}
94+
return uploadFileInstance.execute(datasetId, file, progress, abortController)
95+
}
96+
}
97+
4698
export {
4799
getDatasetFiles,
48100
getFileDownloadCount,
@@ -62,7 +114,8 @@ export {
62114
updateFileCategories,
63115
replaceFile,
64116
getFileVersionSummaries,
65-
isFileDeleted
117+
isFileDeleted,
118+
FilesConfig
66119
}
67120

68121
export { FileModel as File, FileEmbargo, FileChecksum } from './domain/models/FileModel'

src/files/infra/clients/DirectUploadClient.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,27 @@ import { MultipartAbortError } from './errors/MultipartAbortError'
1515
import { FileUploadCancelError } from './errors/FileUploadCancelError'
1616
import { ApiConstants } from '../../../core/infra/repositories/ApiConstants'
1717

18+
export interface DirectUploadClientConfig {
19+
/** Maximum number of retries for multipart upload parts. Default: 5 */
20+
maxMultipartRetries?: number
21+
/** Whether to include S3 tagging header (x-amz-tagging: dv-state=temp). Default: true
22+
* Set to false if your S3 implementation doesn't support object tagging. */
23+
useS3Tagging?: boolean
24+
/** Timeout in milliseconds for file upload operations. Default: 60000 */
25+
fileUploadTimeoutMs?: number
26+
}
27+
1828
export class DirectUploadClient implements IDirectUploadClient {
1929
private filesRepository: IFilesRepository
2030
private maxMultipartRetries: number
31+
private useS3Tagging: boolean
32+
private readonly fileUploadTimeoutMs: number
2133

22-
private readonly fileUploadTimeoutMs: number = 60_000
23-
24-
constructor(filesRepository: IFilesRepository, maxMultipartRetries = 5) {
34+
constructor(filesRepository: IFilesRepository, config: DirectUploadClientConfig = {}) {
2535
this.filesRepository = filesRepository
26-
this.maxMultipartRetries = maxMultipartRetries
36+
this.maxMultipartRetries = config.maxMultipartRetries ?? 5
37+
this.useS3Tagging = config.useS3Tagging ?? true
38+
this.fileUploadTimeoutMs = config.fileUploadTimeoutMs ?? 60_000
2739
}
2840

2941
public async uploadFile(
@@ -59,11 +71,15 @@ export class DirectUploadClient implements IDirectUploadClient {
5971
): Promise<void> {
6072
try {
6173
const arrayBuffer = await file.arrayBuffer()
74+
const headers: Record<string, string> = {
75+
'Content-Type': 'application/octet-stream'
76+
}
77+
// Only add S3 tagging header if enabled (some S3 implementations don't support it)
78+
if (this.useS3Tagging) {
79+
headers['x-amz-tagging'] = 'dv-state=temp'
80+
}
6281
await axios.put(destination.urls[0], arrayBuffer, {
63-
headers: {
64-
'Content-Type': 'application/octet-stream',
65-
'x-amz-tagging': 'dv-state=temp'
66-
},
82+
headers,
6783
timeout: this.fileUploadTimeoutMs,
6884
signal: abortController.signal,
6985
onUploadProgress: (progressEvent) =>

test/unit/files/DirectUploadClient.test.ts

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,56 @@ describe('uploadFile', () => {
8787

8888
expect(actual).toEqual(testDestination.storageId)
8989
})
90+
91+
test('should include S3 tagging header by default', async () => {
92+
const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
93+
const testDestination: FileUploadDestination = createSingleFileUploadDestinationModel()
94+
filesRepositoryStub.getFileUploadDestination = jest.fn().mockResolvedValue(testDestination)
95+
96+
const axiosPutSpy = jest.spyOn(axios, 'put').mockResolvedValue(undefined)
97+
98+
const sut = new DirectUploadClient(filesRepositoryStub)
99+
100+
const progressMock = jest.fn()
101+
const abortController = new AbortController()
102+
103+
await sut.uploadFile(1, testFile, progressMock, abortController)
104+
105+
expect(axiosPutSpy).toHaveBeenCalledWith(
106+
testDestination.urls[0],
107+
expect.anything(),
108+
expect.objectContaining({
109+
headers: expect.objectContaining({
110+
'x-amz-tagging': 'dv-state=temp'
111+
})
112+
})
113+
)
114+
})
115+
116+
test('should not include S3 tagging header when useS3Tagging is false', async () => {
117+
const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
118+
const testDestination: FileUploadDestination = createSingleFileUploadDestinationModel()
119+
filesRepositoryStub.getFileUploadDestination = jest.fn().mockResolvedValue(testDestination)
120+
121+
const axiosPutSpy = jest.spyOn(axios, 'put').mockResolvedValue(undefined)
122+
123+
const sut = new DirectUploadClient(filesRepositoryStub, { useS3Tagging: false })
124+
125+
const progressMock = jest.fn()
126+
const abortController = new AbortController()
127+
128+
await sut.uploadFile(1, testFile, progressMock, abortController)
129+
130+
expect(axiosPutSpy).toHaveBeenCalledWith(
131+
testDestination.urls[0],
132+
expect.anything(),
133+
expect.objectContaining({
134+
headers: expect.not.objectContaining({
135+
'x-amz-tagging': expect.anything()
136+
})
137+
})
138+
)
139+
})
90140
})
91141

92142
describe('Multiple parts file', () => {
@@ -113,7 +163,7 @@ describe('uploadFile', () => {
113163
jest.spyOn(axios, 'delete').mockResolvedValue(undefined)
114164
jest.spyOn(axios, 'put').mockRejectedValue('error')
115165

116-
const sut = new DirectUploadClient(filesRepositoryStub, 1)
166+
const sut = new DirectUploadClient(filesRepositoryStub, { maxMultipartRetries: 1 })
117167

118168
const progressMock = jest.fn()
119169
const abortController = new AbortController()
@@ -143,7 +193,7 @@ describe('uploadFile', () => {
143193
const progressMock = jest.fn()
144194
const abortController = new AbortController()
145195

146-
const sut = new DirectUploadClient(filesRepositoryStub, 1)
196+
const sut = new DirectUploadClient(filesRepositoryStub, { maxMultipartRetries: 1 })
147197

148198
await expect(sut.uploadFile(1, testFile, progressMock, abortController)).rejects.toThrow(
149199
MultipartAbortError
@@ -165,7 +215,7 @@ describe('uploadFile', () => {
165215
const progressMock = jest.fn()
166216
const abortController = new AbortController()
167217

168-
const sut = new DirectUploadClient(filesRepositoryStub, 1)
218+
const sut = new DirectUploadClient(filesRepositoryStub, { maxMultipartRetries: 1 })
169219
await expect(sut.uploadFile(1, testFile, progressMock, abortController)).rejects.toThrow(
170220
MultipartCompletionError
171221
)
@@ -187,7 +237,7 @@ describe('uploadFile', () => {
187237
.mockResolvedValueOnce(successfulPartResponse)
188238
.mockResolvedValueOnce(undefined)
189239

190-
const sut = new DirectUploadClient(filesRepositoryStub, 1)
240+
const sut = new DirectUploadClient(filesRepositoryStub, { maxMultipartRetries: 1 })
191241

192242
const progressMock = jest.fn()
193243
const abortController = new AbortController()
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { FilesConfig } from '../../../src/files'
2+
3+
describe('FilesConfig', () => {
4+
beforeEach(() => {
5+
// Reset config before each test
6+
FilesConfig.init({})
7+
})
8+
9+
describe('init', () => {
10+
test('should set useS3Tagging configuration', () => {
11+
FilesConfig.init({ useS3Tagging: false })
12+
13+
const config = FilesConfig.getConfig()
14+
expect(config.useS3Tagging).toBe(false)
15+
})
16+
17+
test('should set maxMultipartRetries configuration', () => {
18+
FilesConfig.init({ maxMultipartRetries: 10 })
19+
20+
const config = FilesConfig.getConfig()
21+
expect(config.maxMultipartRetries).toBe(10)
22+
})
23+
24+
test('should set fileUploadTimeoutMs configuration', () => {
25+
FilesConfig.init({ fileUploadTimeoutMs: 120000 })
26+
27+
const config = FilesConfig.getConfig()
28+
expect(config.fileUploadTimeoutMs).toBe(120000)
29+
})
30+
31+
test('should set multiple configuration options', () => {
32+
FilesConfig.init({
33+
useS3Tagging: false,
34+
maxMultipartRetries: 3,
35+
fileUploadTimeoutMs: 30000
36+
})
37+
38+
const config = FilesConfig.getConfig()
39+
expect(config.useS3Tagging).toBe(false)
40+
expect(config.maxMultipartRetries).toBe(3)
41+
expect(config.fileUploadTimeoutMs).toBe(30000)
42+
})
43+
})
44+
45+
describe('getConfig', () => {
46+
test('should return empty config by default', () => {
47+
const config = FilesConfig.getConfig()
48+
expect(config).toEqual({})
49+
})
50+
51+
test('should return previously set config', () => {
52+
const expectedConfig = { useS3Tagging: true, maxMultipartRetries: 5 }
53+
FilesConfig.init(expectedConfig)
54+
55+
const config = FilesConfig.getConfig()
56+
expect(config).toEqual(expectedConfig)
57+
})
58+
})
59+
})

0 commit comments

Comments
 (0)