Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix issue with allowMultipleUploadBatches #5400

Merged
merged 6 commits into from
Aug 15, 2024
Merged

fix issue with allowMultipleUploadBatches #5400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f3235f
fix issue with allowMultipleUploadBatches
mifi Aug 9, 2024
68f307a
attempt to fix test
mifi Aug 9, 2024
e973254
throw better error when all files have been canceled
mifi Aug 9, 2024
3456c23
wait for updateAssembly when restoring
mifi Aug 9, 2024
08e46ab
remove comment
aduh95 Aug 15, 2024
854812c
Merge branch 'main' into fix-transloadit-allowMultipleUploadBatches
aduh95 Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions e2e/cypress/integration/dashboard-transloadit.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ describe('Dashboard with Transloadit', () => {
cy.get('.uppy-StatusBar-actionBtn--upload').click()

cy.wait(['@createAssemblies', '@tusCreate']).then(() => {
const plugin = getPlugin(uppy)
const { assembly } = getPlugin(uppy)

expect(plugin.assembly.closed).to.be.false
expect(assembly.closed).to.be.false

uppy.cancelAll()

cy.wait(['@delete', '@tusDelete']).then(() => {
expect(plugin.assembly.closed).to.be.true
expect(assembly.closed).to.be.true
Comment on lines -68 to +75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those changes are purely aesthetic, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, because plugin.assembly is now set to undefined after an upload has finished, we need to get the assembly out first, or else the test was failing due to assembly being undefined on plugin

})
})
})
Expand Down
199 changes: 103 additions & 96 deletions packages/@uppy/transloadit/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,64 +400,64 @@ export default class Transloadit<
return newFile
}

#createAssembly(
async #createAssembly(
fileIDs: string[],
assemblyOptions: OptionsWithRestructuredFields,
) {
this.uppy.log('[Transloadit] Create Assembly')

return this.client
.createAssembly({
try {
const newAssembly = await this.client.createAssembly({
...assemblyOptions,
expectedFiles: fileIDs.length,
})
.then(async (newAssembly) => {
const files = this.uppy
.getFiles()
.filter(({ id }) => fileIDs.includes(id))
if (files.length === 0) {
// All files have been removed, cancelling.
await this.client.cancelAssembly(newAssembly)
return null
}

const assembly = new Assembly(newAssembly, this.#rateLimitedQueue)
const { status } = assembly
const assemblyID = status.assembly_id

const updatedFiles: Record<string, UppyFile<M, B>> = {}
files.forEach((file) => {
updatedFiles[file.id] = this.#attachAssemblyMetadata(file, status)
})
const files = this.uppy
.getFiles()
.filter(({ id }) => fileIDs.includes(id))

this.uppy.setState({
files: {
...this.uppy.getState().files,
...updatedFiles,
},
})
if (files.length === 0) {
// All files have been removed, cancelling.
await this.client.cancelAssembly(newAssembly)
return null
}

this.uppy.emit('transloadit:assembly-created', status, fileIDs)
const assembly = new Assembly(newAssembly, this.#rateLimitedQueue)
const { status } = assembly
const assemblyID = status.assembly_id

this.uppy.log(`[Transloadit] Created Assembly ${assemblyID}`)
return assembly
const updatedFiles: Record<string, UppyFile<M, B>> = {}
files.forEach((file) => {
updatedFiles[file.id] = this.#attachAssemblyMetadata(file, status)
})
.catch((err) => {
// TODO: use AssemblyError?
const wrapped = new ErrorWithCause(
`${this.i18n('creatingAssemblyFailed')}: ${err.message}`,
{ cause: err },
)
if ('details' in err) {
// @ts-expect-error details is not in the Error type
wrapped.details = err.details
}
if ('assembly' in err) {
// @ts-expect-error assembly is not in the Error type
wrapped.assembly = err.assembly
}
throw wrapped

this.uppy.setState({
files: {
...this.uppy.getState().files,
...updatedFiles,
},
})

this.uppy.emit('transloadit:assembly-created', status, fileIDs)

this.uppy.log(`[Transloadit] Created Assembly ${assemblyID}`)
return assembly
} catch (err) {
// TODO: use AssemblyError?
const wrapped = new ErrorWithCause(
`${this.i18n('creatingAssemblyFailed')}: ${err.message}`,
{ cause: err },
)
if ('details' in err) {
// @ts-expect-error details is not in the Error type
wrapped.details = err.details
}
if ('assembly' in err) {
// @ts-expect-error assembly is not in the Error type
wrapped.assembly = err.assembly
}
throw wrapped
}
}

#createAssemblyWatcher(idOrArrayOfIds: string | string[]) {
Expand Down Expand Up @@ -616,6 +616,7 @@ export default class Transloadit<
await this.client.cancelAssembly(assembly)
// TODO bubble this through AssemblyWatcher so its event handlers can clean up correctly
this.uppy.emit('transloadit:assembly-cancelled', assembly)
this.assembly = undefined
}

/**
Expand Down Expand Up @@ -702,20 +703,21 @@ export default class Transloadit<
this.#connectAssembly(this.assembly!)
}

// Force-update all Assemblies to check for missed events.
const updateAssemblies = () => {
// Force-update Assembly to check for missed events.
const updateAssembly = () => {
return this.assembly?.update()
}

// Restore all Assembly state.
this.restored = Promise.resolve().then(() => {
this.restored = (async () => {
restoreState()
restoreAssemblies()
updateAssemblies()
})

this.restored.then(() => {
await updateAssembly()
this.restored = null
})()

this.restored.catch((err) => {
this.uppy.log('Failed to restore', err)
})
}

Expand Down Expand Up @@ -800,8 +802,11 @@ export default class Transloadit<
try {
const assembly =
// this.assembly can already be defined if we recovered files with Golden Retriever (this.#onRestored)
(this.assembly ??
(await this.#createAssembly(fileIDs, assemblyOptions)))!
this.assembly ?? (await this.#createAssembly(fileIDs, assemblyOptions))

if (assembly == null)
throw new Error('All files were canceled after assembly was created')

if (this.opts.importFromUploadURLs) {
await this.#reserveFiles(assembly, fileIDs)
}
Expand All @@ -823,65 +828,67 @@ export default class Transloadit<
}
}

#afterUpload = (fileIDs: string[], uploadID: string): Promise<void> => {
const files = fileIDs.map((fileID) => this.uppy.getFile(fileID))
// Only use files without errors
const filteredFileIDs = files
.filter((file) => !file.error)
.map((file) => file.id)
#afterUpload = async (fileIDs: string[], uploadID: string): Promise<void> => {
try {
// If we're still restoring state, wait for that to be done.
await this.restored

// If we're still restoring state, wait for that to be done.
if (this.restored) {
return this.restored.then(() => {
return this.#afterUpload(filteredFileIDs, uploadID)
})
}
const files = fileIDs
.map((fileID) => this.uppy.getFile(fileID))
// Only use files without errors
.filter((file) => !file.error)

const assemblyID = this.assembly?.status.assembly_id
const assemblyID = this.assembly?.status.assembly_id

const closeSocketConnections = () => {
this.assembly?.close()
}
const closeSocketConnections = () => {
this.assembly?.close()
}

// If we don't have to wait for encoding metadata or results, we can close
// the socket immediately and finish the upload.
if (!this.#shouldWaitAfterUpload()) {
closeSocketConnections()
const status = this.assembly?.status
if (status != null) {
this.uppy.addResultData(uploadID, {
transloadit: [status],
})
// If we don't have to wait for encoding metadata or results, we can close
// the socket immediately and finish the upload.
if (!this.#shouldWaitAfterUpload()) {
closeSocketConnections()
const status = this.assembly?.status
if (status != null) {
this.uppy.addResultData(uploadID, {
transloadit: [status],
})
}
return
}
return Promise.resolve()
}

// If no Assemblies were created for this upload, we also do not have to wait.
// There's also no sockets or anything to close, so just return immediately.
if (!assemblyID) {
this.uppy.addResultData(uploadID, { transloadit: [] })
return Promise.resolve()
}
// If no Assemblies were created for this upload, we also do not have to wait.
// There's also no sockets or anything to close, so just return immediately.
if (!assemblyID) {
this.uppy.addResultData(uploadID, { transloadit: [] })
return
}

const incompleteFiles = files.filter(
(file) => !hasProperty(this.completedFiles, file.id),
)
incompleteFiles.forEach((file) => {
this.uppy.emit('postprocess-progress', file, {
mode: 'indeterminate',
message: this.i18n('encoding'),
const incompleteFiles = files.filter(
(file) => !hasProperty(this.completedFiles, file.id),
)
incompleteFiles.forEach((file) => {
this.uppy.emit('postprocess-progress', file, {
mode: 'indeterminate',
message: this.i18n('encoding'),
})
})
})

return this.#watcher.promise.then(() => {
await this.#watcher.promise
// assembly is now done processing!
closeSocketConnections()
const status = this.assembly?.status
if (status != null) {
this.uppy.addResultData(uploadID, {
transloadit: [status],
})
}
})
} finally {
// in case allowMultipleUploadBatches is true and the user wants to upload again,
// we need to allow a new assembly to be created.
// see https://github.com/transloadit/uppy/issues/5397
this.assembly = undefined
}
}

#closeAssemblyIfExists = () => {
Expand Down
Loading