Repository: parse-community/Parse-SDK-JS
Pull request #1057 (2020-01-08 23:24:42)

Saving files with sessionToken
Closes: https://github.com/parse-community/Parse-SDK-JS/issues/1049

A small optimization with saving files in saveAll.
Clean up deferred promises.

@stevestencil Can you check and see if this works for you?

Let me know if anything else is needed.

codecov[bot]: # [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057?src=pr&el=h1) Report
> Merging [#1057](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057?src=pr&el=desc) into [master](https://codecov.io/gh/parse-community/Parse-SDK-JS/commit/77e406829ad776c3b08190d889a766dc86ccdd91?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057/graphs/tree.svg?width=650&token=8nCENM0QHj&height=150&src=pr)](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1057      +/-   ##
==========================================
- Coverage   92.19%   92.18%   -0.01%     
==========================================
  Files          54       54              
  Lines        5150     5146       -4     
  Branches     1145     1148       +3     
==========================================
- Hits         4748     4744       -4     
  Misses        402      402
```


| [Impacted Files](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/promiseUtils.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057/diff?src=pr&el=tree#diff-c3JjL3Byb21pc2VVdGlscy5qcw==) | `91.48% <ø> (ø)` | :arrow_up: |
| [src/RESTController.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057/diff?src=pr&el=tree#diff-c3JjL1JFU1RDb250cm9sbGVyLmpz) | `84% <100%> (-0.22%)` | :arrow_down: |
| [src/ParseObject.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057/diff?src=pr&el=tree#diff-c3JjL1BhcnNlT2JqZWN0Lmpz) | `89.38% <100%> (-0.06%)` | :arrow_down: |
| [src/TaskQueue.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057/diff?src=pr&el=tree#diff-c3JjL1Rhc2tRdWV1ZS5qcw==) | `87.5% <100%> (-1.79%)` | :arrow_down: |
| [src/ParseFile.js](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057/diff?src=pr&el=tree#diff-c3JjL1BhcnNlRmlsZS5qcw==) | `96.84% <100%> (+0.12%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057?src=pr&el=footer). Last update [77e4068...74fd982](https://codecov.io/gh/parse-community/Parse-SDK-JS/pull/1057?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

stevestencil: I noticed that when saving the files by using request() (instead of ajax()) it [sets the session token as "_SessionToken"](https://github.com/parse-community/Parse-SDK-JS/blob/74fd98224f1a3e5cac59b0a0784a25f84f117f04/src/RESTController.js#L262) in the body. With this change you're setting the session token in the header X-Parse-Session-Token. Unless I'm missing something, shouldn't these use one or the other?
dplewis: Looking [here](https://github.com/parse-community/parse-server/blob/985933955f502009da28fc4241ecd500ae6d44c4/src/middlewares.js) Both of these have existed since the beginning of parse. One represents the REST API with headers the other represents POST request with (headers included in data). In this case SaveFile is using REST style request so for consistency I'm using REST style header here.

I'm not really sure why they used `ajax()` instead of `request()` here, maybe there was a side-effect. We can try it in a separate PR.

For now does this work for you?

acinader: @dplewis this change looks good to me.  I'd prefer to keep what you've done with the session in the header where we now expect it and wouldn't make a requirement for adding this important capability cleaning up a log established inconsistency (unless you want to that is).

Patch:
diff --git a/src/ParseFile.js b/src/ParseFile.js
index 41f3539..e82fc1f 100644
--- a/src/ParseFile.js
+++ b/src/ParseFile.js
@@ -208,6 +208,8 @@ class ParseFile {
    *  * Valid options are:<ul>
    *   <li>useMasterKey: In Cloud Code and Node only, causes the Master Key to
    *     be used for this request.
+   *   <li>sessionToken: A valid session token, used for making a request on
+   *     behalf of a specific user.
    *   <li>progress: In Browser only, callback for upload progress
    * </ul>
    * @return {Promise} Promise that is resolved when the save finishes.
@@ -323,7 +325,7 @@ class ParseFile {
 }
 
 const DefaultController = {
-  saveFile: function(name: string, source: FileSource, options?: FullOptions) {
+  saveFile: async function(name: string, source: FileSource, options?: FullOptions) {
     if (source.format !== 'file') {
       throw new Error('saveFile can only be used with File-type sources.');
     }
@@ -336,6 +338,15 @@ const DefaultController = {
     if (jsKey) {
       headers['X-Parse-JavaScript-Key'] = jsKey;
     }
+    let sessionToken = options.sessionToken;
+    const userController = CoreManager.getUserController();
+    if (!sessionToken && userController) {
+      const currentUser = await userController.currentUserAsync();
+      sessionToken =  currentUser ? currentUser.getSessionToken() : undefined;
+    }
+    if (sessionToken) {
+      headers['X-Parse-Session-Token'] = sessionToken;
+    }
     let url = CoreManager.get('SERVER_URL');
     if (url[url.length - 1] !== '/') {
       url += '/';
diff --git a/src/ParseObject.js b/src/ParseObject.js
index 8d6d628..8486e88 100644
--- a/src/ParseObject.js
+++ b/src/ParseObject.js
@@ -19,7 +19,7 @@ import ParseACL from './ParseACL';
 import parseDate from './parseDate';
 import ParseError from './ParseError';
 import ParseFile from './ParseFile';
-import { when, continueWhile } from './promiseUtils';
+import { when, continueWhile, resolvingPromise } from './promiseUtils';
 import { DEFAULT_PIN, PIN_PREFIX } from './LocalDatastoreUtils';
 
 import {
@@ -2236,19 +2236,17 @@ const DefaultController = {
       }
       unsaved = unique(unsaved);
 
-      let filesSaved = Promise.resolve();
+      const filesSaved: Array<ParseFile> = [];
       let pending: Array<ParseObject> = [];
       unsaved.forEach((el) => {
         if (el instanceof ParseFile) {
-          filesSaved = filesSaved.then(() => {
-            return el.save();
-          });
+          filesSaved.push(el.save());
         } else if (el instanceof ParseObject) {
           pending.push(el);
         }
       });
 
-      return filesSaved.then(() => {
+      return Promise.all(filesSaved).then(() => {
         let objectError = null;
         return continueWhile(() => {
           return pending.length > 0;
@@ -2274,17 +2272,11 @@ const DefaultController = {
 
           // Queue up tasks for each object in the batch.
           // When every task is ready, the API request will execute
-          let res, rej;
-          const batchReturned = new Promise((resolve, reject) => { res = resolve; rej = reject; });
-          batchReturned.resolve = res;
-          batchReturned.reject = rej;
+          const batchReturned = new resolvingPromise();
           const batchReady = [];
           const batchTasks = [];
           batch.forEach((obj, index) => {
-            let res, rej;
-            const ready = new Promise((resolve, reject) => { res = resolve; rej = reject; });
-            ready.resolve = res;
-            ready.reject = rej;
+            const ready = new resolvingPromise();
             batchReady.push(ready);
             const task = function() {
               ready.resolve();
diff --git a/src/RESTController.js b/src/RESTController.js
index 385742c..4543398 100644
--- a/src/RESTController.js
+++ b/src/RESTController.js
@@ -11,6 +11,7 @@
 /* global XMLHttpRequest, XDomainRequest */
 import CoreManager from './CoreManager';
 import ParseError from './ParseError';
+import { resolvingPromise } from './promiseUtils';
 
 export type RequestOptions = {
   useMasterKey?: boolean;
@@ -90,11 +91,7 @@ const RESTController = {
     if (useXDomainRequest) {
       return ajaxIE9(method, url, data, headers, options);
     }
-
-    let res, rej;
-    const promise = new Promise((resolve, reject) => { res = resolve; rej = reject; });
-    promise.resolve = res;
-    promise.reject = rej;
+    const promise = resolvingPromise();
     let attempts = 0;
 
     const dispatch = function() {
diff --git a/src/TaskQueue.js b/src/TaskQueue.js
index e1aad3d..04b3a25 100644
--- a/src/TaskQueue.js
+++ b/src/TaskQueue.js
@@ -8,6 +8,7 @@
  *
  * @flow
  */
+import { resolvingPromise } from './promiseUtils';
 
 type Task = {
   task: () => Promise;
@@ -22,14 +23,7 @@ class TaskQueue {
   }
 
   enqueue(task: () => Promise): Promise {
-    let res;
-    let rej;
-    const taskComplete = new Promise((resolve, reject) => {
-      res = resolve;
-      rej = reject;
-    });
-    taskComplete.resolve = res;
-    taskComplete.reject = rej;
+    const taskComplete = new resolvingPromise();
     this.queue.push({
       task: task,
       _completion: taskComplete
diff --git a/src/promiseUtils.js b/src/promiseUtils.js
index 7950fef..dadca7c 100644
--- a/src/promiseUtils.js
+++ b/src/promiseUtils.js
@@ -1,3 +1,4 @@
+// Create Deferred Promise
 export function resolvingPromise() {
   let res;
   let rej;


Test patch:
diff --git a/src/__tests__/ParseFile-test.js b/src/__tests__/ParseFile-test.js
index ff419e7..9ff972d 100644
--- a/src/__tests__/ParseFile-test.js
+++ b/src/__tests__/ParseFile-test.js
@@ -546,4 +546,71 @@ describe('FileController', () => {
     expect(data).toBe('ParseA==');
     spy.mockRestore();
   });
+
+  it('saves files via ajax with sessionToken option', () => {
+    const request = function(method, path) {
+      const name = path.substr(path.indexOf('/') + 1);
+      return Promise.resolve({
+        name: name,
+        url: 'https://files.parsetfss.com/a/' + name
+      });
+    };
+    const ajax = function(method, path, data, headers) {
+      expect(headers['X-Parse-Session-Token']).toBe('testing_sessionToken')
+      const name = path.substr(path.indexOf('/') + 1);
+      return Promise.resolve({
+        response: {
+          name: name,
+          url: 'https://files.parsetfss.com/a/' + name
+        }
+      });
+    };
+    CoreManager.setRESTController({ request, ajax });
+    const file = new ParseFile('parse.txt', [61, 170, 236, 120]);
+    file._source.format = 'file';
+
+    return file.save({ sessionToken: 'testing_sessionToken' }).then(function(f) {
+      expect(f).toBe(file);
+      expect(f.name()).toBe('/api.parse.com/1/files/parse.txt');
+      expect(f.url()).toBe('https://files.parsetfss.com/a//api.parse.com/1/files/parse.txt');
+    });
+  });
+
+  it('saves files via ajax currentUser sessionToken', () => {
+    CoreManager.set('UserController', {
+      currentUserAsync() {
+        return Promise.resolve({
+          getSessionToken() {
+            return 'currentUserToken';
+          }
+        });
+      }
+    });
+    const request = function(method, path) {
+      const name = path.substr(path.indexOf('/') + 1);
+      return Promise.resolve({
+        name: name,
+        url: 'https://files.parsetfss.com/a/' + name
+      });
+    };
+    const ajax = function(method, path, data, headers) {
+      expect(headers['X-Parse-Session-Token']).toBe('currentUserToken')
+      const name = path.substr(path.indexOf('/') + 1);
+      return Promise.resolve({
+        response: {
+          name: name,
+          url: 'https://files.parsetfss.com/a/' + name
+        }
+      });
+    };
+    CoreManager.setRESTController({ request, ajax });
+    const file = new ParseFile('parse.txt', [61, 170, 236, 120]);
+    file._source.format = 'file';
+
+    return file.save().then(function(f) {
+      expect(f).toBe(file);
+      expect(f.name()).toBe('/api.parse.com/1/files/parse.txt');
+      expect(f.url()).toBe('https://files.parsetfss.com/a//api.parse.com/1/files/parse.txt');
+    });
+  });
 });
diff --git a/src/__tests__/ParseUser-test.js b/src/__tests__/ParseUser-test.js
index 78d6cf8..8837841 100644
--- a/src/__tests__/ParseUser-test.js
+++ b/src/__tests__/ParseUser-test.js
@@ -20,6 +20,7 @@ jest.dontMock('../ParseError');
 jest.dontMock('../ParseObject');
 jest.dontMock('../ParseOp');
 jest.dontMock('../ParseUser');
+jest.dontMock('../promiseUtils');
 jest.dontMock('../RESTController');
 jest.dontMock('../SingleInstanceStateController');
 jest.dontMock('../Storage');
diff --git a/src/__tests__/SingleInstanceStateController-test.js b/src/__tests__/SingleInstanceStateController-test.js
index 5883b26..0384b87 100644
--- a/src/__tests__/SingleInstanceStateController-test.js
+++ b/src/__tests__/SingleInstanceStateController-test.js
@@ -13,6 +13,7 @@ jest.dontMock('../ObjectStateMutations');
 jest.dontMock('../ParseFile');
 jest.dontMock('../ParseGeoPoint');
 jest.dontMock('../ParseOp');
+jest.dontMock('../promiseUtils');
 jest.dontMock('../SingleInstanceStateController');
 jest.dontMock('../TaskQueue');