mirror of
https://github.com/transloadit/uppy.git
synced 2026-07-17 16:50:22 +00:00
Fix allowMultipleUploadBatches by preventing file operations during upload (#6156)
## Implementation Plan for Transloadit allowMultipleUploadBatches Fix - [x] Explore the codebase to understand existing structure - [x] Review transloadit/src/index.ts - [x] Review existing event handlers (#onError, #onCancelAll) - [x] Review install() and uninstall() methods - [x] Understand test structure - [x] Implement the proper fix in packages/@uppy/transloadit/src/index.ts - [x] Set allowNewUpload: false at start of #prepareUpload (preprocessor) - [x] Reset allowNewUpload: true at end of #afterUpload (postprocessor) - [x] Reset allowNewUpload: true in #prepareUpload error handler - [x] Keep existing resets in #onError and #onCancelAll handlers - [x] Removed event listener approach (was causing race conditions) - [x] Update tests to match implementation - [x] Test allowNewUpload lifecycle through actual upload flow (preprocessor/postprocessor) - [x] Test allowNewUpload reset on preprocessor error - [x] Test allowNewUpload reset on error event - [x] Test allowNewUpload reset on cancel-all - [x] Remove obsolete event listener tests <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mifi <402547+mifi@users.noreply.github.com> Co-authored-by: Mikael Finstad <finstaden@gmail.com>
This commit is contained in:
Copilot
GitHub
parent
49db42d72a
commit
f6efd2ebcc
2 changed files with 214 additions and 29 deletions
|
|
@ -77,37 +77,99 @@ describe('Transloadit', () => {
|
|||
})
|
||||
|
||||
it('should complete when resuming after pause', async () => {
|
||||
let firstUploadCallCount = 0
|
||||
const assemblyStatusBase = {
|
||||
assembly_id: 'test-assembly-id',
|
||||
websocket_url: 'ws://localhost:8080',
|
||||
tus_url: 'http://localhost/resumable/files/',
|
||||
assembly_ssl_url:
|
||||
'https://api2.transloadit.com/assemblies/test-assembly-id',
|
||||
}
|
||||
|
||||
const tusUploads = new Map()
|
||||
let uploadIndex = 0
|
||||
const tusBaseUrl = 'http://localhost/resumable/files/'
|
||||
|
||||
const server = setupServer(
|
||||
http.post('*/assemblies', ({ request }) => {
|
||||
http.options('http://localhost/resumable/files*', () => {
|
||||
return new HttpResponse(null, {
|
||||
status: 204,
|
||||
headers: {
|
||||
'Tus-Resumable': '1.0.0',
|
||||
'Tus-Version': '1.0.0',
|
||||
'Tus-Extension': 'creation,creation-defer-length',
|
||||
},
|
||||
})
|
||||
}),
|
||||
http.post('http://localhost/resumable/files*', ({ request }) => {
|
||||
const uploadLengthHeader = request.headers.get('upload-length')
|
||||
const uploadLength = uploadLengthHeader ? Number(uploadLengthHeader) : 0
|
||||
const uploadId = `test-upload-${uploadIndex++}`
|
||||
tusUploads.set(uploadId, {
|
||||
length: Number.isNaN(uploadLength) ? 0 : uploadLength,
|
||||
offset: 0,
|
||||
})
|
||||
|
||||
return new HttpResponse(null, {
|
||||
status: 201,
|
||||
headers: {
|
||||
Location: `${tusBaseUrl}${uploadId}`,
|
||||
'Upload-Offset': '0',
|
||||
'Tus-Resumable': '1.0.0',
|
||||
},
|
||||
})
|
||||
}),
|
||||
http.head('http://localhost/resumable/files/:uploadId', ({ params }) => {
|
||||
const upload = tusUploads.get(params.uploadId)
|
||||
if (!upload) {
|
||||
return new HttpResponse(null, { status: 404 })
|
||||
}
|
||||
return new HttpResponse(null, {
|
||||
status: 200,
|
||||
headers: {
|
||||
'Upload-Offset': String(upload.offset),
|
||||
'Upload-Length': String(upload.length),
|
||||
'Tus-Resumable': '1.0.0',
|
||||
},
|
||||
})
|
||||
}),
|
||||
http.patch(
|
||||
'http://localhost/resumable/files/:uploadId',
|
||||
async ({ request, params }) => {
|
||||
const upload = tusUploads.get(params.uploadId)
|
||||
if (!upload) {
|
||||
return new HttpResponse(null, { status: 404 })
|
||||
}
|
||||
if (upload.offset === 0) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 200))
|
||||
}
|
||||
const body = await request.arrayBuffer()
|
||||
const offsetHeader = request.headers.get('upload-offset')
|
||||
const baseOffset = offsetHeader ? Number(offsetHeader) : upload.offset
|
||||
const nextOffset = baseOffset + body.byteLength
|
||||
upload.offset = nextOffset
|
||||
|
||||
return new HttpResponse(null, {
|
||||
status: 204,
|
||||
headers: {
|
||||
'Upload-Offset': String(nextOffset),
|
||||
'Tus-Resumable': '1.0.0',
|
||||
},
|
||||
})
|
||||
},
|
||||
),
|
||||
http.post('https://api2.transloadit.com/assemblies', ({ request }) => {
|
||||
return HttpResponse.json({
|
||||
assembly_id: 'test-assembly-id',
|
||||
websocket_url: 'ws://localhost:8080',
|
||||
tus_url: 'https://localhost/resumable/files/',
|
||||
assembly_ssl_url: 'https://localhost/assemblies/test-assembly-id',
|
||||
...assemblyStatusBase,
|
||||
ok: 'ASSEMBLY_EXECUTING',
|
||||
})
|
||||
}),
|
||||
http.get('*/assemblies/*', () => {
|
||||
http.get('https://api2.transloadit.com/assemblies/*', () => {
|
||||
return HttpResponse.json({
|
||||
assembly_id: 'test-assembly-id',
|
||||
...assemblyStatusBase,
|
||||
ok: 'ASSEMBLY_COMPLETED',
|
||||
results: {},
|
||||
})
|
||||
}),
|
||||
http.post('*/resumable/files/', () => {
|
||||
firstUploadCallCount++
|
||||
return HttpResponse.json({
|
||||
tus_enabled: true,
|
||||
resumable_file_id: `test-file-id-${firstUploadCallCount}`,
|
||||
})
|
||||
}),
|
||||
http.patch('*/resumable/files/*', () => {
|
||||
return HttpResponse.json({
|
||||
ok: 'RESUMABLE_FILE_UPLOADED',
|
||||
})
|
||||
}),
|
||||
http.post('https://transloaditstatus.com/client_error', () => {
|
||||
return HttpResponse.json({})
|
||||
}),
|
||||
|
|
@ -130,21 +192,129 @@ describe('Transloadit', () => {
|
|||
uppy.addFile({
|
||||
source: 'test',
|
||||
name: 'cat.jpg',
|
||||
data: Buffer.from('test file content'),
|
||||
data: new File([new Uint8Array([1, 2, 3, 4, 5])], 'cat.jpg', {
|
||||
type: 'image/jpeg',
|
||||
}),
|
||||
})
|
||||
uppy.addFile({
|
||||
source: 'test',
|
||||
name: 'traffic.jpg',
|
||||
data: Buffer.from('test file content 2'),
|
||||
data: new File([new Uint8Array([6, 7, 8, 9, 10, 11])], 'traffic.jpg', {
|
||||
type: 'image/jpeg',
|
||||
}),
|
||||
})
|
||||
|
||||
uppy.upload()
|
||||
// Initially should be true
|
||||
expect(uppy.getState().allowNewUpload).toBe(true)
|
||||
|
||||
const uploadPromise = uppy.upload()
|
||||
|
||||
// Should be set to false during upload
|
||||
expect(uppy.getState().allowNewUpload).toBe(false)
|
||||
|
||||
// Trying to add a new file during upload with Transloadit should not be possible
|
||||
expect(() =>
|
||||
uppy.addFile({
|
||||
source: 'test',
|
||||
name: 'additionalFile.jpg',
|
||||
data: new File([new Uint8Array([0])], 'additionalFile.jpg', {
|
||||
type: 'image/jpeg',
|
||||
}),
|
||||
}),
|
||||
).toThrowError('Cannot add more files')
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100))
|
||||
uppy.pauseAll()
|
||||
await uppy.upload()
|
||||
uppy.resumeAll()
|
||||
|
||||
await uploadPromise
|
||||
|
||||
expect(successSpy).toHaveBeenCalled()
|
||||
|
||||
// Should be reset to true after upload completes
|
||||
expect(uppy.getState().allowNewUpload).toBe(true)
|
||||
|
||||
server.close()
|
||||
})
|
||||
|
||||
it('resets allowNewUpload to true on preprocessor error', async () => {
|
||||
const uppy = new Core()
|
||||
uppy.use(Transloadit, {
|
||||
assemblyOptions: {
|
||||
params: {
|
||||
auth: { key: 'test-auth-key' },
|
||||
template_id: 'test-template-id',
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
// Mock createAssembly to throw an error
|
||||
uppy.getPlugin('Transloadit').client.createAssembly = () =>
|
||||
Promise.reject(new Error('Assembly creation failed'))
|
||||
|
||||
uppy.addFile({
|
||||
source: 'test',
|
||||
name: 'test.jpg',
|
||||
data: Buffer.from('test file content'),
|
||||
})
|
||||
|
||||
// Initially should be true
|
||||
expect(uppy.getState().allowNewUpload).toBe(true)
|
||||
|
||||
try {
|
||||
await uppy.upload()
|
||||
} catch {
|
||||
// Expected to fail
|
||||
}
|
||||
|
||||
// Should be reset to true after error
|
||||
expect(uppy.getState().allowNewUpload).toBe(true)
|
||||
})
|
||||
|
||||
it('resets allowNewUpload to true on cancel-all', async () => {
|
||||
const uppy = new Core()
|
||||
uppy.use(Transloadit, {
|
||||
assemblyOptions: {
|
||||
params: {
|
||||
auth: { key: 'test-auth-key' },
|
||||
template_id: 'test-template-id',
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
// Manually set allowNewUpload to false to simulate an upload in progress
|
||||
uppy.setState({ allowNewUpload: false })
|
||||
expect(uppy.getState().allowNewUpload).toBe(false)
|
||||
|
||||
// Simulate cancel-all
|
||||
uppy.cancelAll()
|
||||
|
||||
// Should be reset to true
|
||||
expect(uppy.getState().allowNewUpload).toBe(true)
|
||||
})
|
||||
|
||||
it('resets allowNewUpload to true on error event', () => {
|
||||
const uppy = new Core()
|
||||
uppy.use(Transloadit, {
|
||||
assemblyOptions: {
|
||||
params: {
|
||||
auth: { key: 'test-auth-key' },
|
||||
template_id: 'test-template-id',
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
// Manually set allowNewUpload to false to simulate an upload in progress
|
||||
uppy.setState({ allowNewUpload: false })
|
||||
expect(uppy.getState().allowNewUpload).toBe(false)
|
||||
|
||||
// Trigger error event
|
||||
uppy.emit('error', {
|
||||
name: 'TestError',
|
||||
message: 'Test error message',
|
||||
})
|
||||
|
||||
// Should be reset to true
|
||||
expect(uppy.getState().allowNewUpload).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -639,12 +639,15 @@ export default class Transloadit<
|
|||
* When all files are removed, cancel in-progress Assemblies.
|
||||
*/
|
||||
#onCancelAll = async () => {
|
||||
if (!this.assembly) return
|
||||
try {
|
||||
await this.#cancelAssembly(this.assembly.status)
|
||||
} catch (err) {
|
||||
this.uppy.log(err)
|
||||
if (this.assembly) {
|
||||
try {
|
||||
await this.#cancelAssembly(this.assembly.status)
|
||||
} catch (err) {
|
||||
this.uppy.log(err)
|
||||
}
|
||||
}
|
||||
// Reset allowNewUpload when upload is cancelled
|
||||
this.uppy.setState({ allowNewUpload: true })
|
||||
}
|
||||
|
||||
#onRestored = (pluginData: Record<string, PersistentState>) => {
|
||||
|
|
@ -813,6 +816,10 @@ export default class Transloadit<
|
|||
}
|
||||
|
||||
#prepareUpload = async (fileIDs: string[]) => {
|
||||
// Prevent adding/dropping files during upload to avoid creating multiple assemblies
|
||||
// TODO we should rewrite to instead infer allowNewUpload based on upload state
|
||||
this.uppy.setState({ allowNewUpload: false })
|
||||
|
||||
const assemblyOptions = (
|
||||
typeof this.opts.assemblyOptions === 'function'
|
||||
? await this.opts.assemblyOptions()
|
||||
|
|
@ -850,6 +857,8 @@ export default class Transloadit<
|
|||
this.uppy.emit('preprocess-complete', file)
|
||||
this.uppy.emit('upload-error', file, err)
|
||||
})
|
||||
// Reset allowNewUpload on error
|
||||
this.uppy.setState({ allowNewUpload: true })
|
||||
throw err
|
||||
}
|
||||
}
|
||||
|
|
@ -916,6 +925,8 @@ export default class Transloadit<
|
|||
// we need to allow a new assembly to be created.
|
||||
// see https://github.com/transloadit/uppy/issues/5397
|
||||
this.assembly = undefined
|
||||
// Allow new uploads now that this upload is complete
|
||||
this.uppy.setState({ allowNewUpload: true })
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -931,6 +942,9 @@ export default class Transloadit<
|
|||
.submitError(err)
|
||||
// if we can't report the error that sucks
|
||||
.catch(sendErrorToConsole(err))
|
||||
|
||||
// Reset allowNewUpload when upload encounters an error
|
||||
this.uppy.setState({ allowNewUpload: true })
|
||||
}
|
||||
|
||||
#onTusError = (_: UppyFile<M, B> | undefined, err: Error) => {
|
||||
|
|
@ -1008,6 +1022,7 @@ export default class Transloadit<
|
|||
this.uppy.removePreProcessor(this.#prepareUpload)
|
||||
this.uppy.removePostProcessor(this.#afterUpload)
|
||||
this.uppy.off('error', this.#onError)
|
||||
this.uppy.off('cancel-all', this.#onCancelAll)
|
||||
|
||||
if (this.opts.importFromUploadURLs) {
|
||||
this.uppy.off('upload-success', this.#onFileUploadURLAvailable)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue