Support for Large file upload task #94

muthurathinam · 2018-08-06T10:59:12Z

Large file upload task

In uncertain network connectivity areas uploading a large file would take multiple attempts to get it done. In case of larger files, it would take even more attempts to achieve or even impossible. Large file upload task helps in uploading large files effortlessly.

Implementation is done with the reference of this onedrive's resumable upload.

To create session and start upload:

let options =  {
    path: "/Documents",
    fileName: file.name,
    rangeSize: (1024 * 1024)
};

MicrosoftGraph.OneDriveLargeFileUploadTask.create(client, file, options)
.then((uploadTask) => {
    uploadTask.upload()
    .then((res) => {
        console.log(res);
        console.log("File Uploaded Successfully.!!");
    })
    .catch((err) => {
        console.log(err);
    });
}).catch((err) => {
    console.log(err);
});

With async/await syntactic sugar in ECMAScript 2017

async function uploadFile(client, file) {
    try {
        let options = {
            path: "/Documents",
            fileName: file.name,
            rangeSize: (1024 * 1024)
        };
        const uploadTask = await MicrosoftGraph.OneDriveLargeFileUploadTask.create(client, file, options);
        const response = await uploadTask.upload();
        console.log(response);
    } catch (err) {
        console.log(err);
    }
}

We can just resume the broken upload:

Lets consider some break down happens in the middle of the uploading, with the uploadTask object in hand resuming comes easy.

uploadTask.resume();

Even you can control the whole upload process:

Just create the upload task, and play with it by using sliceFile and uploadSlice methods

let range = uploadTask.getNextRange();
let slicedFile = uploadTask.sliceFile(range);
uploadTask.uploadSlice(slicedFile, range, uploadTask.file.size);

…es to separate folder in spec\n 3. Add tests for LargeFileUploadUtil\n 3.Clean browserfiy action

MIchaelMainer

👍 Customers will love this feature.

MIchaelMainer · 2018-08-07T18:39:21Z

lib/src/LargeFileUploadTask.d.ts

@@ -0,0 +1,47 @@
+/// <reference types="node" />


Add jsdoc comments for interfaces, classes, and members. This should be generated from proper comments in the typescript.

MIchaelMainer · 2018-08-07T18:40:08Z

lib/src/LargeFileUploadUtil.d.ts

@@ -0,0 +1 @@
+export declare const getValidRangeSize: (rangeSize?: number) => number;


Add js doc comments. This applies to all .d.ts

MIchaelMainer · 2018-08-07T18:51:22Z

src/LargeFileUploadUtil.ts

+}
+
+export const getValidRangeSize = (rangeSize: number = DEFAULT_FILE_SIZE): number => {
+    const sixtyMB = 60 * 1024 * 1024;


What is the significance of 60MB? Please add a comment about why this size is not arbitrary.

MIchaelMainer · 2018-08-07T18:56:09Z

src/LargeFileUploadTask.ts

+    // This method should be called to create an LargeFileUploadTask
+    // To create this uploadTask in node user has to provide the file in the NodeFile format
+
+    // retuns the instance of LargeFileUploadTask by making request to the sessionCreationUrl that is provided


Nit: typo 'retuns' becomes 'returns'

MIchaelMainer · 2018-08-07T18:59:45Z

src/LargeFileUploadTask.ts

+        }
+    }
+
+    // Delets the upload current upload task in the server


Nit: 'Delets' becomes 'Deletes'. Actually, this should be a proper doc comment so that it is picked up for .d.ts

MIchaelMainer · 2018-08-07T20:30:52Z

src/LargeFileUploadTask.ts

+        return blob;
+    }
+
+    // Uploads the file by slicing and uploadin each slice in a sequential manner


Nit: 'uploadin' becomes 'uploading'

MIchaelMainer · 2018-08-07T20:55:21Z

src/LargeFileUploadTask.ts

+            _fileObject: FileObject = <FileObject>{};
+        switch(file.constructor.name) {
+            case "Blob":
+                _fileObject.name = self.getRandomFileName();


We should enable customers to name the file they are creating when they submit it as a blob. Perhaps add a member to LargeFileUploadTaskOptions named blobFileName and change line 59 to be = (options.blobFileName !== undefined) ? options.blobFileName : self.getRandomFileName();

MIchaelMainer · 2018-08-07T20:57:37Z

spec/tasks/LargeFileUploadTask.ts

+        rangeSize: 327680
+    };
+    let uploadTask = new LargeFileUploadTask(getClient(), fileObj, uploadSession, options);
+    it('Should return an exceptoion as trying to upload a slice after the session has expired', (done) => {


Nit: 'exceptoionbecomesexception`

MIchaelMainer · 2018-08-07T21:02:05Z

spec/tasks/LargeFileUploadTask.ts

+        done();
+    });
+
+    it('Should return next range as default(empty) range,this is for the upload task completed', (done) => {


Nit: space needed 'range, this'

MIchaelMainer · 2018-08-07T21:11:10Z

src/LargeFileUploadTask.ts

+            while (true) {
+                let nextRange = self.getNextRange();
+                if (nextRange.maxValue === -1) {
+                    throw new Error("Invalid session: Uploading completed");


This also aplies to line 181. Would it make sense to create the Error, and then give it a descriptive name so that customers can inspect the error.name and handle it? The error message should give some info on what happed advice on how the error can be handled.

This reverts commit a72448b.

…nts, Update error message and add name

muthurathinam · 2018-08-08T10:51:12Z

@MIchaelMainer Made the changes that you are requested...

MIchaelMainer · 2018-08-09T17:02:37Z

src/LargeFileUploadTask.ts

+
+    /**
+     * @async
+     * Resume the upload session and continue uploading the file from the place where it left


Resume the upload session and continue uploading the file from the last sent range.

MIchaelMainer · 2018-08-09T17:07:03Z

src/LargeFileUploadTask.ts

+    }
+
+    /**
+     * To update the exipiration date and the next range


nit: expiration

MIchaelMainer · 2018-08-09T17:09:00Z

src/LargeFileUploadTask.ts

+     * @async
+     * Creates a LargeFilUploadTask
+     * @param client - The GraphClient instance
+     * @param file - File representated as Blob, File or NodeFile object


Change 'NodeFile' to 'Buffer'

MIchaelMainer · 2018-08-09T17:09:14Z

src/LargeFileUploadTask.ts

+    /**
+     * @static
+     * @async
+     * Creates a LargeFilUploadTask


Change LargeFilUploadTask to LargeFileUploadTask

MIchaelMainer · 2018-08-09T17:24:42Z

src/LargeFileUploadTask.ts

+            case "File":
+                let _file = <File>file;
+                _fileObject.content = _file;
+                _fileObject.name = _fileObject.name = (options.fileName !== undefined) ? options.fileName : _file.name;


Extra '_fileObject.name =' or is this something I don't understand?

Can _file.name ever be undefined? If so, we should use the random name as a last resort.

That is extra.!

Can _file.name ever be undefined? If so, we should use the random name as a last resort.

_file.name will never be undefined.

MIchaelMainer · 2018-08-09T17:31:59Z

src/LargeFileUploadTask.ts

+
+/**
+ * User Options to create the upload task
+ */


Consider this comment applicable to all of the files in this PR. Let's add doc comments to all of these types and members. These are all intended to be used by customers so let's make it easier for them to use them. Our .d.ts should be picking these up as well. I'm not sure how we go from .ts to .d.ts. Is there a tool for that?

/**

Interface used to define options when creating an upload task.

@interface

@Property {string} sessionRequestUrl - The default values for parties.

@Property {string} fileName - Optional. Specifies the file name of the file to upload. A file name will be created for you if you do not specify a value.

@Property {number} rangeSize - Optional. Specifies the range chunk size. The default value of ??? is used if a value is not specified.

@Property {number} maxTries - Optional. Specifies the ??? The default value of ??? is used if a value is not specified.
*/

sure, will define interface in this format.

Typescript compiling does that, when we do compile .ts files using typescript compiler, it generates .js, .d.ts and .js.map files

will investigate and make the compiler take comments from .ts to .d.ts if possible.

2. Pull request changes This reverts commit a72448b.

muthurathinam · 2018-08-10T07:04:34Z

@MIchaelMainer
Updated the pull request with the changes:

Changed compilation options to take comments from the .ts to .d.ts and .js files
Corrected typos in the comments
Made file name in options mandatory

deepak2016

Please discuss the sesssionURL in Option and verify try catch mechanism while making upload call.
Changes look good.

deepak2016 · 2018-08-13T10:22:05Z

.gitignore

not required, remove.

…OneDriveLargeFileUploadTask, write corresponding unit tests

muthurathinam · 2018-08-21T10:29:27Z

@darrelmiller @deepak2016 Done changes related to extending LargeFileUploadTask to OneDriveLargeFileUploadTask.

deepak2016 · 2018-08-28T10:51:37Z

spec/secrets.sample.ts

@@ -0,0 +1 @@
+export const AccessToken = "";


put a comment

Commenting done..!!

deepak2016 · 2018-08-28T10:55:12Z

spec/tasks/OneDriveLargeFileUploadTask.ts

+import { assert } from "chai";
+import { OneDriveLargeFileUploadTask } from "../../lib/src/OneDriveLargeFileUploadTask";
+
+declare const describe, it;


Needed...

declare is used to tell TypeScript that the variable has been created elsewhere. If you use declare, nothing is added to the JavaScript that is generated - it is simply a hint to the compiler.

For example, if you use an external script that defines var externalModule, you would use declare var externalModule to hint to the TypeScript compiler that externalModule has already been set up

https://stackoverflow.com/a/35020317/1161237

deepak2016 · 2018-08-28T10:55:41Z

spec/tasks/OneDriveLargeFileUploadTaskUtil.ts

+import { assert } from "chai";
+import * as OneDriveLargeFileUploadTaskUtil from "../../lib/src/OneDriveLargeFileUploadTaskUtil";
+
+declare const describe, it;


deepak2016 · 2018-08-28T10:57:02Z

spec/types/OneNote.ts

@@ -1,6 +1,6 @@
 import { assert } from 'chai'

-import { getClient, randomString } from "./test-helper"
+import { getClient, randomString } from "../test-helper"
 import { Notebook, OnenoteSection, OnenotePage } from '@microsoft/microsoft-graph-types-beta'


pls check if we need to run test against beta or v1.0

These models are now available in v1.0 so have changed it to use v1.0

muthurathinam-m added 3 commits August 1, 2018 15:54

Implement LargeFileUploadTask

a72448b

1. Rearrange unit test files directory structure\n 2. Move sample fil…

632ba6b

…es to separate folder in spec\n 3. Add tests for LargeFileUploadUtil\n 3.Clean browserfiy action

Support upload from node and write unit tests

e6eb090

darrelmiller added the in progress label Aug 6, 2018

muthurathinam requested review from darrelmiller, deepak2016, bullsseye and MIchaelMainer August 6, 2018 10:59

MIchaelMainer suggested changes Aug 7, 2018

View reviewed changes

muthurathinam-m added 2 commits August 8, 2018 10:48

Revert "Implement LargeFileUploadTask"

8832d8a

This reverts commit a72448b.

Provide option to mention name of the content, Add jsDoc format comme…

d8cc80b

…nts, Update error message and add name

MIchaelMainer suggested changes Aug 9, 2018

View reviewed changes

1. Implementing large file upload task

86e38c5

2. Pull request changes This reverts commit a72448b.

MIchaelMainer previously approved these changes Aug 10, 2018

View reviewed changes

deepak2016 previously approved these changes Aug 13, 2018

View reviewed changes

darrelmiller previously approved these changes Aug 16, 2018

View reviewed changes

Add OneDriveLargeFileUploadTask, segregate onedrive specific code to …

de10426

…OneDriveLargeFileUploadTask, write corresponding unit tests

muthurathinam dismissed stale reviews from darrelmiller, deepak2016, and MIchaelMainer via de10426 August 21, 2018 09:50

muthurathinam and others added 2 commits August 21, 2018 15:53

Merge branch 'dev' into LargeFileUploadTask

b025e0c

Pushing generated files after merge with dev branch

964581c

darrelmiller previously approved these changes Aug 23, 2018

View reviewed changes

deepak2016 reviewed Aug 28, 2018

View reviewed changes

deepak2016 previously approved these changes Aug 28, 2018

View reviewed changes

Change with respect to the comments in the PR

bcec01f

muthurathinam dismissed deepak2016’s stale review via bcec01f August 29, 2018 05:37

muthurathinam dismissed darrelmiller’s stale review via bcec01f August 29, 2018 05:37

deepak2016 approved these changes Aug 29, 2018

View reviewed changes

muthurathinam merged commit 7621b02 into dev Aug 29, 2018

muthurathinam deleted the LargeFileUploadTask branch August 29, 2018 08:30

muthurathinam mentioned this pull request Sep 11, 2018

Uploading files larger than 4 MB #111

Closed

snyk-bot mentioned this pull request Mar 27, 2020

[Snyk] Upgrade tslib from 1.9.3 to 1.11.1 mboocannon/msgraph-sdk-javascript#8

Open

snyk-bot mentioned this pull request Jun 17, 2020

[Snyk] Upgrade tslib from 1.9.3 to 1.13.0 mboocannon/msgraph-sdk-javascript#13

Open

snyk-bot mentioned this pull request Nov 5, 2020

[Snyk] Upgrade tslib from 1.9.3 to 1.14.1 mboocannon/msgraph-sdk-javascript#25

Open

mboocannon mentioned this pull request Nov 23, 2023

[Snyk] Upgrade tslib from 1.9.3 to 1.14.1 mboocannon/msgraph-sdk-javascript#78

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Large file upload task #94

Support for Large file upload task #94

muthurathinam commented Aug 6, 2018 •

edited

MIchaelMainer left a comment

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

MIchaelMainer Aug 7, 2018

muthurathinam commented Aug 8, 2018

MIchaelMainer Aug 9, 2018

MIchaelMainer Aug 9, 2018

MIchaelMainer Aug 9, 2018

MIchaelMainer Aug 9, 2018

MIchaelMainer Aug 9, 2018

muthurathinam Aug 10, 2018

muthurathinam Aug 10, 2018

MIchaelMainer Aug 9, 2018

muthurathinam Aug 10, 2018

muthurathinam commented Aug 10, 2018

deepak2016 left a comment

deepak2016 Aug 13, 2018

muthurathinam Aug 29, 2018

muthurathinam commented Aug 21, 2018

deepak2016 Aug 28, 2018

muthurathinam Aug 29, 2018

deepak2016 Aug 28, 2018

muthurathinam Aug 29, 2018

deepak2016 Aug 28, 2018

deepak2016 Aug 28, 2018

muthurathinam Aug 29, 2018

		@@ -0,0 +1 @@
		export declare const getValidRangeSize: (rangeSize?: number) => number;

Support for Large file upload task #94

Support for Large file upload task #94

Conversation

muthurathinam commented Aug 6, 2018 • edited

Large file upload task

MIchaelMainer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muthurathinam commented Aug 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muthurathinam commented Aug 10, 2018

deepak2016 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muthurathinam commented Aug 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muthurathinam commented Aug 6, 2018 •

edited