-
Notifications
You must be signed in to change notification settings - Fork 236
LargeFileTaskUpload- High-level design changes #392
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| ##### LargeFileUpload Task Design Changes | ||
|
|
||
| This document proposes high-level design modifications to the `LargeFileUploadTask` implementation. Reasons for change - | ||
|
|
||
| - Enhancement - Support Node.js Stream upload. Issue [#320](https://github.com/microsoftgraph/msgraph-sdk-javascript/issues/320). | ||
| - Bug Fix - Support large file uploads to Outlook API. Issue [#359](https://github.com/microsoftgraph/msgraph-sdk-javascript/issues/359). | ||
| - Enhancement- Support upload progress handler callback. Issue [#305](https://github.com/microsoftgraph/msgraph-sdk-javascript/issues/305). | ||
|
|
||
| Outline of the current implementation - | ||
|
|
||
| ```TypeScript | ||
| interface LargeFileUploadTaskOptions { | ||
| rangeSize?: number; | ||
| } | ||
|
|
||
| interface FileObject { | ||
| content: ArrayBuffer | File; | ||
| name: string; | ||
| size: number; | ||
| } | ||
|
|
||
| // Create a LargeFileUploadTask object with the file object | ||
| constructor(client: Client, file: FileObject, uploadSession: LargeFileUploadSession, options: LargeFileUploadTaskOptions = {}) | ||
|
|
||
| // Call the LargeFileUploadTask.upload() function to upload large file to the API in ranges. | ||
| upload() { | ||
| while (true) { | ||
| const fileSlice = this.sliceFile(nextRange); | ||
| const response = await this.uploadSlice(fileSlice, nextRange, this.file.size); | ||
| // Upon completion of upload process incase of onedrive, driveItem is returned, which contains id | ||
| if (response.id !== undefined) { | ||
| return response; | ||
| } else { | ||
| this.updateTaskStatus(response); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Function to slice the file as per the "Next Expected Ranges". | ||
| sliceFile(range: Range): ArrayBuffer | Blob { | ||
| const blob = this.file.content.slice(range.minValue, range.maxValue + 1); | ||
| return blob; | ||
| } | ||
| ``` | ||
|
|
||
| ###### 1. Support Node.js Stream upload | ||
|
|
||
| - The requirement is to allow a `FileObject` with content of type [Readable Stream](https://nodejs.org/api/stream.html#stream_class_stream_readable) | ||
| - The `file.slice()` function is not applicable to `Readable Stream` and logic for splitting and handling `Stream` varies. | ||
| - Note - Chunk uploading is not supported by the Graph API and the stream will be split into multiple ranges and each range will uploaded sequentially. | ||
|
|
||
| - Proposed changes in the current design - | ||
| - Move `sliceFlice()` in the FileObject Interface | ||
| ```TypeScript | ||
| interface FileObject{ | ||
| sliceFile(range: Range): ArrayBuffer | Blob | ||
| } | ||
| ``` | ||
| - Introduce new classes implementing the `FileObject` interface. Example - `StreamLargeFile.ts` will contain the `sliceFile` function and logic to handle stream classes. | ||
| - Change in the upload function will look like | ||
| ``` | ||
| upload() { | ||
| // current : const fileSlice = this.sliceFile(nextRange); | ||
| // proposed change is as follows - | ||
| const fileSlice = this.fileObject.sliceFile(Range); | ||
| } | ||
| ``` | ||
| - This change is based on idea of Dependency Inversion principle. The goal is to depend on abstractions so that we can easily add or modify support for different file formats in the future or allow customized `FileObject` implementations. | ||
| - For browser support, take a dependency on [stream-browserify](https://www.npmjs.com/package/stream-browserify) - the stream module from node core, for browsers and make necessary updates to the rollup or bundling process. | ||
|
|
||
| ###### 2. Support large file uploads to Outlook API | ||
|
|
||
| - Bug in the current implemenation - `response.id !== undefined` this condition to mark the completion of an upload does not work for Outlook API since the final response does not contain a response body. | ||
| - An upload task should be marked as completed if the response status is a 201. [SDK-design document](https://github.com/microsoftgraph/msgraph-sdk-design/blob/master/tasks/FileUploadTask.md). | ||
| - The LargeFileUploadTask should allow uploads to OneDrive API, Outlook API and PrintDocument API. | ||
| - Proposed changes- | ||
| - Add class `UploadResult` containing `location` and `responseBody` properties. | ||
| - `location` provides access to the `location` field in the response headers. | ||
| - `responseBody` provides access to the Graph API response body. | ||
| - The `upload` task should return the `UploadResult` object on successful completion of task. | ||
|
|
||
| ###### 3. Support upload progress handler callback | ||
| - Proposed changes - | ||
| - Add interface -> `interface UploadEventHandler{ | ||
| extraCallbackParam?: unknown; | ||
| progress(range: Range, extraCallbackParam?: unknown):void | ||
| }` | ||
| - Add uploadEventHandlers option to -> | ||
| ``` | ||
| interface LargeFileUploadTaskOptions { | ||
| rangeSize?: number; | ||
| uploadEventHandlers?: UploadEventHandler; | ||
| } | ||
| ``` | ||
| - In the `upload` function call the `uploadEventHandlers.progress()` function if defined. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.