-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
@uppy/aws-s3-multipart: fix Golden Retriever integration #4526
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,9 +60,12 @@ class MultipartUploader { | |
/** @type {() => void} */ | ||
#onSuccess | ||
|
||
/** @type {typeof import('../types/index').AwsS3MultipartOptions["shouldUseMultipart"]} */ | ||
/** @type {import('../types/index').AwsS3MultipartOptions["shouldUseMultipart"]} */ | ||
#shouldUseMultipart | ||
|
||
/** @type {boolean} */ | ||
#isRestoring | ||
|
||
#onReject = (err) => (err?.cause === pausingUploadReason ? null : this.#onError(err)) | ||
|
||
#maxMultipartParts = 10_000 | ||
|
@@ -83,6 +86,11 @@ class MultipartUploader { | |
this.#onError = this.options.onError | ||
this.#shouldUseMultipart = this.options.shouldUseMultipart | ||
|
||
// When we are restoring an upload, we already have an uploadId. Otherwise | ||
// we need to call `createMultipartUpload` to get an `uploadId`. | ||
// Non-multipart uploads are not restorable. | ||
this.#isRestoring = 'uploadId' in options | ||
|
||
this.#initChunks() | ||
} | ||
|
||
|
@@ -108,12 +116,12 @@ class MultipartUploader { | |
} | ||
this.#chunks = Array(arraySize) | ||
|
||
for (let i = 0, j = 0; i < fileSize; i += chunkSize, j++) { | ||
const end = Math.min(fileSize, i + chunkSize) | ||
for (let offset = 0, j = 0; offset < fileSize; offset += chunkSize, j++) { | ||
const end = Math.min(fileSize, offset + chunkSize) | ||
|
||
// Defer data fetching/slicing until we actually need the data, because it's slow if we have a lot of files | ||
const getData = () => { | ||
const i2 = i | ||
const i2 = offset | ||
return this.#data.slice(i2, end) | ||
} | ||
|
||
|
@@ -123,6 +131,14 @@ class MultipartUploader { | |
onComplete: this.#onPartComplete(j), | ||
shouldUseMultipart, | ||
} | ||
if (this.#isRestoring) { | ||
const size = offset + chunkSize > fileSize ? fileSize - offset : chunkSize | ||
// setAsUploaded is called by listPart, to keep up-to-date the | ||
// quantity of data that is left to actually upload. | ||
this.#chunks[j].setAsUploaded = () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why a function and not setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't know yet if it was already uploaded There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could use a comment how this is used later |
||
this.#chunkState[j].uploaded = size | ||
} | ||
} | ||
} | ||
} else { | ||
this.#chunks = [{ | ||
|
@@ -181,6 +197,9 @@ class MultipartUploader { | |
if (!this.#abortController.signal.aborted) this.#abortController.abort(pausingUploadReason) | ||
this.#abortController = new AbortController() | ||
this.#resumeUpload() | ||
} else if (this.#isRestoring) { | ||
Murderlon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this.options.companionComm.restoreUploadFile(this.#file, { uploadId: this.options.uploadId, key: this.options.key }) | ||
this.#resumeUpload() | ||
} else { | ||
this.#createUpload() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
options is passed by the user, why are we
uploadId
from there to know if we are restoring?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not passed by the user, it's passed by
index.js
uppy/packages/@uppy/aws-s3-multipart/src/index.js
Lines 611 to 627 in b6a6f83
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it the relationship between
uploadId
determining whether we are restoring more clear somehow?