Finding areas that are untested and need love (#4131)

* Makes InstallationRouter like others

* Adds testing for Range file requests

- Fixes issue with small requests (0-2)

* Revert "Makes InstallationRouter like others"

This reverts commit e2d2a16ebf2757db6138c7b5b33c97c56c69ead6.

* Better handling of errors in FilesRouter

* Fix incorrectness in range requests

* Better/simpler logic

* Only on mongo at it requires Gridstore

* Open file streaming to all adapters supporting it

* Improves coverage of parsers

* Ensures depreciation warning is effective

* Removes unused function

* de-duplicate logic

* Removes necessity of overriding req.params.className on subclasses routers

* Use babel-preset-env to ensure min-version compatible code

* removes dead code

* Leverage indexes in order to infer which field is duplicated upon signup

- A note mentioned that it would be possible to leverage using the indexes on username/email to infer which is duplicated

* Small nit

* Better template to match column name

* Restores original implementation for safety

* nits

src/Adapters/Storage/Mongo/MongoCollection.js

@@ -79,10 +79,6 @@ export default class MongoCollection {
     return this._mongoCollection.updateMany(query, update);
   }
 
-  deleteOne(query) {
-    return this._mongoCollection.deleteOne(query);
-  }
-
   deleteMany(query) {
     return this._mongoCollection.deleteMany(query);
   }

src/Adapters/Storage/Mongo/MongoStorageAdapter.js

@@ -278,8 +278,15 @@ export class MongoStorageAdapter {
       .then(collection => collection.insertOne(mongoObject))
       .catch(error => {
         if (error.code === 11000) { // Duplicate value
-          throw new Parse.Error(Parse.Error.DUPLICATE_VALUE,
-            'A duplicate value for a field with unique values was provided');
+          const err = new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'A duplicate value for a field with unique values was provided');
+          err.underlyingError = error;
+          if (error.message) {
+            const matches = error.message.match(/index:[\sa-zA-Z0-9_\-\.]+\$?([a-zA-Z_-]+)_1/);
+            if (matches && Array.isArray(matches)) {
+              err.userInfo = { duplicated_field: matches[1] };
+            }
+          }
+          throw err;
         }
         throw error;
       });
@@ -375,9 +382,8 @@ export class MongoStorageAdapter {
       .catch(error => {
         if (error.code === 11000) {
           throw new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'Tried to ensure field uniqueness for a class that already has duplicates.');
-        } else {
-          throw error;
         }
+        throw error;
       });
   }
 

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

@@ -945,7 +945,15 @@ export class PostgresStorageAdapter {
       .then(() => ({ ops: [object] }))
       .catch(error => {
         if (error.code === PostgresUniqueIndexViolationError) {
-          throw new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'A duplicate value for a field with unique values was provided');
+          const err = new Parse.Error(Parse.Error.DUPLICATE_VALUE, 'A duplicate value for a field with unique values was provided');
+          err.underlyingError = error;
+          if (error.constraint) {
+            const matches = error.constraint.match(/unique_([a-zA-Z]+)/);
+            if (matches && Array.isArray(matches)) {
+              err.userInfo = { duplicated_field: matches[1] };
+            }
+          }
+          throw err;
         } else {
           throw error;
         }

src/Push/PushWorker.js

@@ -10,8 +10,6 @@ import * as utils             from './utils';
 import { ParseMessageQueue }  from '../ParseMessageQueue';
 import { PushQueue }          from './PushQueue';
 
-const UNSUPPORTED_BADGE_KEY = "unsupported";
-
 function groupByBadge(installations) {
   return installations.reduce((map, installation) => {
     const badge = installation.badge + '';
@@ -93,11 +91,7 @@ export class PushWorker {
     // Map the on the badges count and return the send result
     const promises = Object.keys(badgeInstallationsMap).map((badge) => {
       const payload = deepcopy(body);
-      if (badge == UNSUPPORTED_BADGE_KEY) {
-        delete payload.data.badge;
-      } else {
-        payload.data.badge = parseInt(badge);
-      }
+      payload.data.badge = parseInt(badge);
       const installations = badgeInstallationsMap[badge];
       return this.sendToAdapter(payload, installations, pushStatus, config);
     });

src/RestWrite.js

@@ -1011,9 +1011,19 @@ RestWrite.prototype.runDatabaseOperation = function() {
         if (this.className !== '_User' || error.code !== Parse.Error.DUPLICATE_VALUE) {
           throw error;
         }
+
+        // Quick check, if we were able to infer the duplicated field name
+        if (error && error.userInfo && error.userInfo.duplicated_field === 'username') {
+          throw new Parse.Error(Parse.Error.USERNAME_TAKEN, 'Account already exists for this username.');
+        }
+
+        if (error && error.userInfo && error.userInfo.duplicated_field === 'email') {
+          throw new Parse.Error(Parse.Error.EMAIL_TAKEN, 'Account already exists for this email address.');
+        }
+
         // If this was a failed user creation due to username or email already taken, we need to
         // check whether it was username or email and return the appropriate error.
-
+        // Fallback to the original method
         // TODO: See if we can later do this without additional queries by using named indexes.
         return this.config.database.find(
           this.className,

src/Routers/AudiencesRouter.js

@@ -3,25 +3,14 @@ import rest from '../rest';
 import * as middleware from '../middlewares';
 
 export class AudiencesRouter extends ClassesRouter {
+
+  className() {
+    return '_Audience';
+  }
+
   handleFind(req) {
     const body = Object.assign(req.body, ClassesRouter.JSONFromQuery(req.query));
-    var options = {};
-
-    if (body.skip) {
-      options.skip = Number(body.skip);
-    }
-    if (body.limit || body.limit === 0) {
-      options.limit = Number(body.limit);
-    }
-    if (body.order) {
-      options.order = String(body.order);
-    }
-    if (body.count) {
-      options.count = true;
-    }
-    if (body.include) {
-      options.include = String(body.include);
-    }
+    const options = ClassesRouter.optionsFromBody(body);
 
     return rest.find(req.config, req.auth, '_Audience', body.where, options, req.info.clientSDK)
       .then((response) => {
@@ -35,7 +24,6 @@ export class AudiencesRouter extends ClassesRouter {
   }
 
   handleGet(req) {
-    req.params.className = '_Audience';
     return super.handleGet(req)
       .then((data) => {
         data.response.query = JSON.parse(data.response.query);
@@ -44,21 +32,6 @@ export class AudiencesRouter extends ClassesRouter {
       });
   }
 
-  handleCreate(req) {
-    req.params.className = '_Audience';
-    return super.handleCreate(req);
-  }
-
-  handleUpdate(req) {
-    req.params.className = '_Audience';
-    return super.handleUpdate(req);
-  }
-
-  handleDelete(req) {
-    req.params.className = '_Audience';
-    return super.handleDelete(req);
-  }
-
   mountRoutes() {
     this.route('GET','/push_audiences', middleware.promiseEnforceMasterKeyAccess, req => { return this.handleFind(req); });
     this.route('GET','/push_audiences/:objectId', middleware.promiseEnforceMasterKeyAccess, req => { return this.handleGet(req); });

src/Routers/ClassesRouter.js

@@ -8,45 +8,20 @@ const ALLOWED_GET_QUERY_KEYS = ['keys', 'include'];
 
 export class ClassesRouter extends PromiseRouter {
 
+  className(req) {
+    return req.params.className;
+  }
+
   handleFind(req) {
     const body = Object.assign(req.body, ClassesRouter.JSONFromQuery(req.query));
-    const options = {};
-    const allowConstraints = ['skip', 'limit', 'order', 'count', 'keys',
-      'include', 'redirectClassNameForKey', 'where'];
-
-    for (const key of Object.keys(body)) {
-      if (allowConstraints.indexOf(key) === -1) {
-        throw new Parse.Error(Parse.Error.INVALID_QUERY, `Invalid parameter for query: ${key}`);
-      }
-    }
-
-    if (body.skip) {
-      options.skip = Number(body.skip);
-    }
-    if (body.limit || body.limit === 0) {
-      options.limit = Number(body.limit);
-    } else {
-      options.limit = Number(100);
-    }
-    if (body.order) {
-      options.order = String(body.order);
-    }
-    if (body.count) {
-      options.count = true;
-    }
-    if (typeof body.keys == 'string') {
-      options.keys = body.keys;
-    }
-    if (body.include) {
-      options.include = String(body.include);
-    }
+    const options = ClassesRouter.optionsFromBody(body);
     if (body.redirectClassNameForKey) {
       options.redirectClassNameForKey = String(body.redirectClassNameForKey);
     }
     if (typeof body.where === 'string') {
       body.where = JSON.parse(body.where);
     }
-    return rest.find(req.config, req.auth, req.params.className, body.where, options, req.info.clientSDK)
+    return rest.find(req.config, req.auth, this.className(req), body.where, options, req.info.clientSDK)
       .then((response) => {
         if (response && response.results) {
           for (const result of response.results) {
@@ -77,13 +52,13 @@ export class ClassesRouter extends PromiseRouter {
       options.include = String(body.include);
     }
 
-    return rest.get(req.config, req.auth, req.params.className, req.params.objectId, options, req.info.clientSDK)
+    return rest.get(req.config, req.auth, this.className(req), req.params.objectId, options, req.info.clientSDK)
       .then((response) => {
         if (!response.results || response.results.length == 0) {
           throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Object not found.');
         }
 
-        if (req.params.className === "_User") {
+        if (this.className(req) === "_User") {
 
           delete response.results[0].sessionToken;
 
@@ -99,16 +74,16 @@ export class ClassesRouter extends PromiseRouter {
   }
 
   handleCreate(req) {
-    return rest.create(req.config, req.auth, req.params.className, req.body, req.info.clientSDK);
+    return rest.create(req.config, req.auth, this.className(req), req.body, req.info.clientSDK);
   }
 
   handleUpdate(req) {
     const where = { objectId: req.params.objectId }
-    return rest.update(req.config, req.auth, req.params.className, where, req.body, req.info.clientSDK);
+    return rest.update(req.config, req.auth, this.className(req), where, req.body, req.info.clientSDK);
   }
 
   handleDelete(req) {
-    return rest.del(req.config, req.auth, req.params.className, req.params.objectId, req.info.clientSDK)
+    return rest.del(req.config, req.auth, this.className(req), req.params.objectId, req.info.clientSDK)
       .then(() => {
         return {response: {}};
       });
@@ -126,6 +101,39 @@ export class ClassesRouter extends PromiseRouter {
     return json
   }
 
+  static optionsFromBody(body) {
+    const allowConstraints = ['skip', 'limit', 'order', 'count', 'keys',
+      'include', 'redirectClassNameForKey', 'where'];
+
+    for (const key of Object.keys(body)) {
+      if (allowConstraints.indexOf(key) === -1) {
+        throw new Parse.Error(Parse.Error.INVALID_QUERY, `Invalid parameter for query: ${key}`);
+      }
+    }
+    const options = {};
+    if (body.skip) {
+      options.skip = Number(body.skip);
+    }
+    if (body.limit || body.limit === 0) {
+      options.limit = Number(body.limit);
+    } else {
+      options.limit = Number(100);
+    }
+    if (body.order) {
+      options.order = String(body.order);
+    }
+    if (body.count) {
+      options.count = true;
+    }
+    if (typeof body.keys == 'string') {
+      options.keys = body.keys;
+    }
+    if (body.include) {
+      options.include = String(body.include);
+    }
+    return options;
+  }
+
   mountRoutes() {
     this.route('GET', '/classes/:className', (req) => { return this.handleFind(req); });
     this.route('GET', '/classes/:className/:objectId', (req) => { return this.handleGet(req); });

src/Routers/FilesRouter.js

@@ -8,7 +8,7 @@ import logger              from '../logger';
 
 export class FilesRouter {
 
-  expressRouter(options = {}) {
+  expressRouter({ maxUploadSize = '20Mb' } = {}) {
     var router = express.Router();
     router.get('/files/:appId/:filename', this.getHandler);
 
@@ -19,7 +19,7 @@ export class FilesRouter {
 
     router.post('/files/:filename',
       Middlewares.allowCrossDomain,
-      BodyParser.raw({type: () => { return true; }, limit: options.maxUploadSize || '20mb'}), // Allow uploads without Content-Type, or with any Content-Type.
+      BodyParser.raw({type: () => { return true; }, limit: maxUploadSize }), // Allow uploads without Content-Type, or with any Content-Type.
       Middlewares.handleParseHeaders,
       this.createHandler
     );
@@ -108,87 +108,74 @@ export class FilesRouter {
 }
 
 function isFileStreamable(req, filesController){
-  if (req.get('Range')) {
-    if (!(typeof filesController.adapter.getFileStream === 'function')) {
-      return false;
-    }
-    if (typeof filesController.adapter.constructor.name !== 'undefined') {
-      if (filesController.adapter.constructor.name == 'GridStoreAdapter') {
-        return true;
-      }
-    }
-  }
-  return false;
+  return  req.get('Range') && typeof filesController.adapter.getFileStream === 'function';
+}
+
+function getRange(req) {
+  const parts = req.get('Range').replace(/bytes=/, "").split("-");
+  return { start: parseInt(parts[0], 10), end: parseInt(parts[1], 10) };
 }
 
 // handleFileStream is licenced under Creative Commons Attribution 4.0 International License (https://creativecommons.org/licenses/by/4.0/).
 // Author: LEROIB at weightingformypizza (https://weightingformypizza.wordpress.com/2015/06/24/stream-html5-media-content-like-video-audio-from-mongodb-using-express-and-gridstore/).
 function handleFileStream(stream, req, res, contentType) {
-  var buffer_size = 1024 * 1024;//1024Kb
+  const buffer_size = 1024 * 1024; //1024Kb
   // Range request, partiall stream the file
-  var parts = req.get('Range').replace(/bytes=/, "").split("-");
-  var partialstart = parts[0];
-  var partialend = parts[1];
-  var start = partialstart ? parseInt(partialstart, 10) : 0;
-  var end = partialend ? parseInt(partialend, 10) : stream.length - 1;
-  var chunksize = (end - start) + 1;
+  let {
+    start, end
+  } = getRange(req);
 
-  if (chunksize == 1) {
-    start = 0;
-    partialend = false;
+  const notEnded = (!end && end !== 0);
+  const notStarted = (!start && start !== 0);
+  // No end provided, we want all bytes
+  if (notEnded) {
+    end = stream.length - 1;
+  }
+  // No start provided, we're reading backwards
+  if (notStarted) {
+    start = stream.length - end;
+    end = start + end - 1;
   }
 
-  if (!partialend) {
-    if (((stream.length - 1) - start) < (buffer_size)) {
-      end = stream.length - 1;
-    }else{
-      end = start + (buffer_size);
-    }
-    chunksize = (end - start) + 1;
+  // Data exceeds the buffer_size, cap
+  if (end - start >= buffer_size) {
+    end = start + buffer_size - 1;
   }
 
-  if (start == 0 && end == 2) {
-    chunksize = 1;
-  }
+  const contentLength = (end - start) + 1;
 
   res.writeHead(206, {
     'Content-Range': 'bytes ' + start + '-' + end + '/' + stream.length,
     'Accept-Ranges': 'bytes',
-    'Content-Length': chunksize,
+    'Content-Length': contentLength,
     'Content-Type': contentType,
   });
 
   stream.seek(start, function () {
     // get gridFile stream
-    var gridFileStream = stream.stream(true);
-    var bufferAvail = 0;
-    var range = (end - start) + 1;
-    var totalbyteswanted = (end - start) + 1;
-    var totalbyteswritten = 0;
+    const gridFileStream = stream.stream(true);
+    let bufferAvail = 0;
+    let remainingBytesToWrite = contentLength;
+    let totalBytesWritten = 0;
     // write to response
-    gridFileStream.on('data', function (buff) {
-      bufferAvail += buff.length;
-      //Ok check if we have enough to cover our range
-      if (bufferAvail < range) {
-        //Not enough bytes to satisfy our full range
-        if (bufferAvail > 0) {
-          //Write full buffer
-          res.write(buff);
-          totalbyteswritten += buff.length;
-          range -= buff.length;
-          bufferAvail -= buff.length;
-        }
-      } else {
-        //Enough bytes to satisfy our full range!
-        if (bufferAvail > 0) {
-          const buffer = buff.slice(0,range);
-          res.write(buffer);
-          totalbyteswritten += buffer.length;
-          bufferAvail -= range;
-        }
+    gridFileStream.on('data', function (data) {
+      bufferAvail += data.length;
+      if (bufferAvail > 0) {
+        // slice returns the same buffer if overflowing
+        // safe to call in any case
+        const buffer = data.slice(0, remainingBytesToWrite);
+        // write the buffer
+        res.write(buffer);
+        // increment total
+        totalBytesWritten += buffer.length;
+        // decrement remaining
+        remainingBytesToWrite -= data.length;
+        // decrement the avaialbe buffer
+        bufferAvail -= buffer.length;
       }
-      if (totalbyteswritten >= totalbyteswanted) {
-        //totalbytes = 0;
+      // in case of small slices, all values will be good at that point
+      // we've written enough, end...
+      if (totalBytesWritten >= contentLength) {
         stream.close();
         res.end();
         this.destroy();

src/Routers/InstallationsRouter.js

@@ -4,26 +4,13 @@ import ClassesRouter from './ClassesRouter';
 import rest from '../rest';
 
 export class InstallationsRouter extends ClassesRouter {
+  className() {
+    return '_Installation';
+  }
+
   handleFind(req) {
     const body = Object.assign(req.body, ClassesRouter.JSONFromQuery(req.query));
-    var options = {};
-
-    if (body.skip) {
-      options.skip = Number(body.skip);
-    }
-    if (body.limit || body.limit === 0) {
-      options.limit = Number(body.limit);
-    }
-    if (body.order) {
-      options.order = String(body.order);
-    }
-    if (body.count) {
-      options.count = true;
-    }
-    if (body.include) {
-      options.include = String(body.include);
-    }
-
+    const options = ClassesRouter.optionsFromBody(body);
     return rest.find(req.config, req.auth,
       '_Installation', body.where, options, req.info.clientSDK)
       .then((response) => {
@@ -31,26 +18,6 @@ export class InstallationsRouter extends ClassesRouter {
       });
   }
 
-  handleGet(req) {
-    req.params.className = '_Installation';
-    return super.handleGet(req);
-  }
-
-  handleCreate(req) {
-    req.params.className = '_Installation';
-    return super.handleCreate(req);
-  }
-
-  handleUpdate(req) {
-    req.params.className = '_Installation';
-    return super.handleUpdate(req);
-  }
-
-  handleDelete(req) {
-    req.params.className = '_Installation';
-    return super.handleDelete(req);
-  }
-
   mountRoutes() {
     this.route('GET','/installations', req => { return this.handleFind(req); });
     this.route('GET','/installations/:objectId', req => { return this.handleGet(req); });

src/Routers/RolesRouter.js

@@ -2,29 +2,8 @@
 import ClassesRouter from './ClassesRouter';
 
 export class RolesRouter extends ClassesRouter {
-  handleFind(req) {
-    req.params.className = '_Role';
-    return super.handleFind(req);
-  }
-
-  handleGet(req) {
-    req.params.className = '_Role';
-    return super.handleGet(req);
-  }
-
-  handleCreate(req) {
-    req.params.className = '_Role';
-    return super.handleCreate(req);
-  }
-
-  handleUpdate(req) {
-    req.params.className = '_Role';
-    return super.handleUpdate(req);
-  }
-
-  handleDelete(req) {
-    req.params.className = '_Role';
-    return super.handleDelete(req);
+  className() {
+    return '_Role';
   }
 
   mountRoutes() {

src/Routers/SessionsRouter.js

@@ -7,29 +7,9 @@ import RestWrite     from '../RestWrite';
 import { newToken }  from '../cryptoUtils';
 
 export class SessionsRouter extends ClassesRouter {
-  handleFind(req) {
-    req.params.className = '_Session';
-    return super.handleFind(req);
-  }
 
-  handleGet(req) {
-    req.params.className = '_Session';
-    return super.handleGet(req);
-  }
-
-  handleCreate(req) {
-    req.params.className = '_Session';
-    return super.handleCreate(req);
-  }
-
-  handleUpdate(req) {
-    req.params.className = '_Session';
-    return super.handleUpdate(req);
-  }
-
-  handleDelete(req) {
-    req.params.className = '_Session';
-    return super.handleDelete(req);
+  className() {
+    return '_Session';
   }
 
   handleMe(req) {

src/Routers/UsersRouter.js

@@ -1,6 +1,5 @@
 // These methods handle the User-related routes.
 
-import deepcopy       from 'deepcopy';
 import Parse          from 'parse/node';
 import Config         from '../Config';
 import AccountLockout from '../AccountLockout';
@@ -12,32 +11,9 @@ import RestWrite      from '../RestWrite';
 const cryptoUtils = require('../cryptoUtils');
 
 export class UsersRouter extends ClassesRouter {
-  handleFind(req) {
-    req.params.className = '_User';
-    return super.handleFind(req);
-  }
 
-  handleGet(req) {
-    req.params.className = '_User';
-    return super.handleGet(req);
-  }
-
-  handleCreate(req) {
-    const data = deepcopy(req.body);
-    req.body = data;
-    req.params.className = '_User';
-
-    return super.handleCreate(req);
-  }
-
-  handleUpdate(req) {
-    req.params.className = '_User';
-    return super.handleUpdate(req);
-  }
-
-  handleDelete(req) {
-    req.params.className = '_User';
-    return super.handleDelete(req);
+  className() {
+    return '_User';
   }
 
   handleMe(req) {

src/triggers.js

@@ -54,25 +54,11 @@ export function removeFunction(functionName, applicationId) {
   delete _triggerStore[applicationId].Functions[functionName]
 }
 
-export function removeJob(jobName, applicationId) {
-  applicationId = applicationId || Parse.applicationId;
-  delete _triggerStore[applicationId].Jobs[jobName]
-}
-
 export function removeTrigger(type, className, applicationId) {
   applicationId = applicationId || Parse.applicationId;
   delete _triggerStore[applicationId].Triggers[type][className]
 }
 
-export function _unregister(appId,category,className,type) {
-  if (type) {
-    removeTrigger(className,type,appId);
-    delete _triggerStore[appId][category][className][type];
-  } else {
-    delete _triggerStore[appId][category][className];
-  }
-}
-
 export function _unregisterAll() {
   Object.keys(_triggerStore).forEach(appId => delete _triggerStore[appId]);
 }