Style improvements (#2475)

* HooksRouter is enabled by default

* Adds middleswares on PromiseRouter, fixes #2410

* Move testing line to helper

* Modernize middlewares.js

* Moves DB uniqueness initialization to DBController, modernize

* Moves testing related code to spec folder

* remove unused _removeHook function

* Adds tests, docs for Analytics and improvements

* nit

* moves back TestUtils

spec/Analytics.spec.js

@@ -0,0 +1,61 @@
+const analyticsAdapter = {
+  appOpened: function(parameters, req) {}, 
+  trackEvent: function(eventName, parameters, req) {}
+}
+
+describe('AnalyticsController', () => {
+  it('should track a simple event', (done) => {
+    
+    spyOn(analyticsAdapter, 'trackEvent').and.callThrough();
+    reconfigureServer({
+      analyticsAdapter
+    }).then(() => {
+      return Parse.Analytics.track('MyEvent', {
+        key: 'value',
+        count: '0'
+      })
+    }).then(() => {
+      expect(analyticsAdapter.trackEvent).toHaveBeenCalled();
+      var lastCall = analyticsAdapter.trackEvent.calls.first();
+      let args = lastCall.args;
+      expect(args[0]).toEqual('MyEvent');
+      expect(args[1]).toEqual({
+        dimensions: {
+          key: 'value',
+          count: '0'
+        }
+      });
+      done();
+    }, (err) => {
+      fail(JSON.stringify(err));
+      done();
+    })
+  });
+  
+  it('should track a app opened event', (done) => {
+    
+    spyOn(analyticsAdapter, 'appOpened').and.callThrough();
+    reconfigureServer({
+      analyticsAdapter
+    }).then(() => {
+      return Parse.Analytics.track('AppOpened', {
+        key: 'value',
+        count: '0'
+      })
+    }).then(() => {
+      expect(analyticsAdapter.appOpened).toHaveBeenCalled();
+      var lastCall = analyticsAdapter.appOpened.calls.first();
+      let args = lastCall.args;
+      expect(args[0]).toEqual({
+        dimensions: {
+          key: 'value',
+          count: '0'
+        }
+      });
+      done();
+    }, (err) => {
+      fail(JSON.stringify(err));
+      done();
+    })
+  })
+})

spec/ParseAPI.spec.js

@@ -2,14 +2,13 @@
 // It would probably be better to refactor them into different files.
 'use strict';
 
-var DatabaseAdapter = require('../src/DatabaseAdapter');
 const MongoStorageAdapter = require('../src/Adapters/Storage/Mongo/MongoStorageAdapter');
 var request = require('request');
 const rp = require('request-promise');
 const Parse = require("parse/node");
 let Config = require('../src/Config');
 const SchemaController = require('../src/Controllers/SchemaController');
-var TestUtils = require('../src/index').TestUtils;
+var TestUtils = require('../src/TestUtils');
 const deepcopy = require('deepcopy');
 
 const userSchema = SchemaController.convertSchemaToAdapterSchema({ className: '_User', fields: Object.assign({}, SchemaController.defaultColumns._Default, SchemaController.defaultColumns._User) });

spec/helper.js

@@ -4,12 +4,11 @@
 jasmine.DEFAULT_TIMEOUT_INTERVAL = process.env.PARSE_SERVER_TEST_TIMEOUT || 5000;
 
 var cache = require('../src/cache').default;
-var DatabaseAdapter = require('../src/DatabaseAdapter');
 var express = require('express');
 var facebook = require('../src/authDataManager/facebook');
 var ParseServer = require('../src/index').ParseServer;
 var path = require('path');
-var TestUtils = require('../src/index').TestUtils;
+var TestUtils = require('../src/TestUtils');
 var MongoStorageAdapter = require('../src/Adapters/Storage/Mongo/MongoStorageAdapter');
 const GridStoreAdapter = require('../src/Adapters/Files/GridStoreAdapter').GridStoreAdapter;
 const PostgresStorageAdapter = require('../src/Adapters/Storage/Postgres/PostgresStorageAdapter');
@@ -87,6 +86,7 @@ const reconfigureServer = changedConfiguration => {
         cache.clear();
         app = express();
         api = new ParseServer(newConfiguration);
+        api.use(require('./testing-routes').router);
         app.use('/1', api);
 
         server = app.listen(port);

spec/testing-routes.js

@@ -1,11 +1,11 @@
 // testing-routes.js
-import AppCache         from './cache';
+import AppCache         from '../src/cache';
-import * as middlewares from './middlewares';
+import * as middlewares from '../src/middlewares';
-import { ParseServer }  from './index';
+import { ParseServer }  from '../src/index';
 import { Parse }        from 'parse/node';
 
 var express = require('express'),
-  cryptoUtils = require('./cryptoUtils');
+  cryptoUtils = require('../src/cryptoUtils');
 
 var router = express.Router();
 

src/Adapters/Analytics/AnalyticsAdapter.js

@@ -1,8 +1,18 @@
 export class AnalyticsAdapter {
+  
+  /*
+  @param parameters: the analytics request body, analytics info will be in the dimensions property
+  @param req: the original http request
+   */
   appOpened(parameters, req) {
     return Promise.resolve({});
   }
   
+  /*
+  @param eventName: the name of the custom eventName
+  @param parameters: the analytics request body, analytics info will be in the dimensions property
+  @param req: the original http request
+   */
   trackEvent(eventName, parameters, req) {
     return Promise.resolve({});
   }

src/Controllers/AnalyticsController.js

@@ -3,21 +3,23 @@ import { AnalyticsAdapter } from '../Adapters/Analytics/AnalyticsAdapter';
 
 export class AnalyticsController extends AdaptableController {
   appOpened(req) {
-    return this.adapter.appOpened(req.body, req).then(
+    return Promise.resolve().then(() => {
-      function(response) {
+      return this.adapter.appOpened(req.body, req);
-        return { response: response };
+    }).then((response) => {
-      }).catch((err) => {
+      return { response: response || {} };
-        return { response: {} };
+    }).catch((err) => {
-      });
+      return { response: {} };
+    });
   }
 
   trackEvent(req) {
-    return this.adapter.trackEvent(req.params.eventName, req.body, req).then(
+    return Promise.resolve().then(() => {
-      function(response) {
+      return this.adapter.trackEvent(req.params.eventName, req.body, req);
-        return { response: response };
+    }).then((response) => {
-      }).catch((err) => {
+      return { response: response || {} };
-        return { response: {} };
+    }).catch((err) => {
-      });
+      return { response: {} };
+    });
   }
 
   expectedAdapterType() {

src/Controllers/DatabaseController.js

@@ -1,15 +1,13 @@
 // A database adapter that works with data exported from the hosted
 // Parse database.
 
-import intersect from 'intersect';
+import { Parse }              from 'parse/node';
-import _         from 'lodash';
+import _                      from 'lodash';
-
+import mongdb                 from 'mongodb';
-var mongodb = require('mongodb');
+import intersect              from 'intersect';
-var Parse = require('parse/node').Parse;
+import deepcopy               from 'deepcopy';
-
+import logger                 from '../logger';
-var SchemaController = require('./SchemaController');
+import * as SchemaController  from './SchemaController';
-
-const deepcopy = require('deepcopy');
 
 function addWriteACL(query, acl) {
   let newQuery = _.cloneDeep(query);
@@ -880,6 +878,28 @@ DatabaseController.prototype.addPointerPermissions = function(schema, className,
   }
 }
 
+DatabaseController.prototype.performInitizalization = function() {
+  const requiredUserFields = { fields: { ...SchemaController.defaultColumns._Default, ...SchemaController.defaultColumns._User } };
+
+  let userClassPromise = this.loadSchema()
+    .then(schema => schema.enforceClassExists('_User'))
+
+  let usernameUniqueness = userClassPromise
+    .then(() => this.adapter.ensureUniqueness('_User', requiredUserFields, ['username']))
+    .catch(error => {
+      logger.warn('Unable to ensure uniqueness for usernames: ', error);
+      return Promise.reject(error);
+    });
+
+  let emailUniqueness = userClassPromise
+    .then(() => this.adapter.ensureUniqueness('_User', requiredUserFields, ['email']))
+    .catch(error => {
+      logger.warn('Unable to ensure uniqueness for user email addresses: ', error);
+      return Promise.reject(error);
+    });
+  return Promise.all([usernameUniqueness, emailUniqueness]);
+}
+
 function joinTableName(className, key) {
   return `_Join:${key}:${className}`;
 }

src/Controllers/HooksController.js

@@ -1,6 +1,5 @@
 /** @flow weak */
 
-import * as DatabaseAdapter from "../DatabaseAdapter";
 import * as triggers        from "../triggers";
 import * as Parse           from "parse/node";
 import * as request         from "request";

src/Controllers/SchemaController.js

@@ -268,7 +268,7 @@ const dbTypeMatchesObjectType = (dbType, objectType) => {
 
 // Stores the entire schema of the app in a weird hybrid format somewhere between
 // the mongo format and the Parse format. Soon, this will all be Parse format.
-class SchemaController {
+export default class SchemaController {
   _dbAdapter;
   data;
   perms;

src/Controllers/UserController.js

@@ -4,7 +4,6 @@ import AdaptableController from './AdaptableController';
 import MailAdapter         from '../Adapters/Email/MailAdapter';
 import rest                from '../rest';
 
-var DatabaseAdapter = require('../DatabaseAdapter');
 var RestWrite = require('../RestWrite');
 var RestQuery = require('../RestQuery');
 var hash = require('../password').hash;

src/DatabaseAdapter.js

@@ -1,21 +0,0 @@
-import AppCache from './cache';
-
-//Used by tests
-function destroyAllDataPermanently() {
-  if (process.env.TESTING) {
-    // This is super janky, but destroyAllDataPermanently is
-    // a janky interface, so we need to have some jankyness
-    // to support it
-    return Promise.all(Object.keys(AppCache.cache).map(appId => {
-      const app = AppCache.get(appId);
-      if (app.databaseController) {
-        return app.databaseController.deleteEverything();
-      } else {
-        return Promise.resolve();
-      }
-    }));
-  }
-  throw 'Only supported in test environment';
-}
-
-module.exports = { destroyAllDataPermanently };

src/ParseServer.js

@@ -2,7 +2,6 @@
 
 var batch = require('./batch'),
     bodyParser = require('body-parser'),
-    DatabaseAdapter = require('./DatabaseAdapter'),
     express = require('express'),
     middlewares = require('./middlewares'),
     multer = require('multer'),
@@ -56,16 +55,11 @@ import { PurgeRouter }          from './Routers/PurgeRouter';
 
 import DatabaseController       from './Controllers/DatabaseController';
 import SchemaCache              from './Controllers/SchemaCache';
-const SchemaController = require('./Controllers/SchemaController');
 import ParsePushAdapter         from 'parse-server-push-adapter';
 import MongoStorageAdapter      from './Adapters/Storage/Mongo/MongoStorageAdapter';
 // Mutate the Parse object to add the Cloud Code handlers
 addParseCloud();
 
-
-const requiredUserFields = { fields: { ...SchemaController.defaultColumns._Default, ...SchemaController.defaultColumns._User } };
-
-
 // ParseServer works like a constructor of an express app.
 // The args that we understand are:
 // "analyticsAdapter": an adapter class for analytics
@@ -205,22 +199,7 @@ class ParseServer {
 
     // TODO: create indexes on first creation of a _User object. Otherwise it's impossible to
     // have a Parse app without it having a _User collection.
-    let userClassPromise = databaseController.loadSchema()
+    const dbInitPromise = databaseController.performInitizalization();
-    .then(schema => schema.enforceClassExists('_User'))
-
-    let usernameUniqueness = userClassPromise
-    .then(() => databaseController.adapter.ensureUniqueness('_User', requiredUserFields, ['username']))
-    .catch(error => {
-      logger.warn('Unable to ensure uniqueness for usernames: ', error);
-      return Promise.reject(error);
-    });
-
-    let emailUniqueness = userClassPromise
-    .then(() => databaseController.adapter.ensureUniqueness('_User', requiredUserFields, ['email']))
-    .catch(error => {
-      logger.warn('Unable to ensure uniqueness for user email addresses: ', error);
-      return Promise.reject(error);
-    })
 
     AppCache.put(appId, {
       appId,
@@ -270,7 +249,7 @@ class ParseServer {
 
     // Note: Tests will start to fail if any validation happens after this is called.
     if (process.env.TESTING) {
-      __indexBuildCompletionCallbackForTests(Promise.all([usernameUniqueness, emailUniqueness]));
+      __indexBuildCompletionCallbackForTests(dbInitPromise);
     }
   }
 
@@ -284,21 +263,14 @@ class ParseServer {
     var api = express();
     //api.use("/apps", express.static(__dirname + "/public"));
     // File handling needs to be before default middlewares are applied
-    api.use('/', middlewares.allowCrossDomain, new FilesRouter().getExpressRouter({
+    api.use('/', middlewares.allowCrossDomain, new FilesRouter().expressRouter({
       maxUploadSize: maxUploadSize
     }));
 
-    api.use('/', bodyParser.urlencoded({extended: false}), new PublicAPIRouter().expressApp());
+    api.use('/', bodyParser.urlencoded({extended: false}), new PublicAPIRouter().expressRouter());
-
-    // TODO: separate this from the regular ParseServer object
-    if (process.env.TESTING == 1) {
-      api.use('/', require('./testing-routes').router);
-    }
 
     api.use(bodyParser.json({ 'type': '*/*' , limit: maxUploadSize }));
-    api.use(middlewares.allowCrossDomain);
     api.use(middlewares.allowMethodOverride);
-    api.use(middlewares.handleParseHeaders);
 
     let routers = [
       new ClassesRouter(),
@@ -315,21 +287,20 @@ class ParseServer {
       new FeaturesRouter(),
       new GlobalConfigRouter(),
       new PurgeRouter(),
+      new HooksRouter()
     ];
 
-    if (process.env.PARSE_EXPERIMENTAL_HOOKS_ENABLED || process.env.TESTING) {
-      routers.push(new HooksRouter());
-    }
-
     let routes = routers.reduce((memo, router) => {
       return memo.concat(router.routes);
     }, []);
 
     let appRouter = new PromiseRouter(routes, appId);
+    appRouter.use(middlewares.allowCrossDomain);
+    appRouter.use(middlewares.handleParseHeaders);
     
     batch.mountOnto(appRouter);
 
-    api.use(appRouter.expressApp());
+    api.use(appRouter.expressRouter());
 
     api.use(middlewares.handleParseErrors);
 

src/PromiseRouter.js

@@ -23,6 +23,7 @@ export default class PromiseRouter {
   //     location: optional. a location header
   constructor(routes = [], appId) {
     this.routes = routes;
+    this.middlewares = [];
     this.appId = appId;
     this.mountRoutes();
   }
@@ -38,6 +39,10 @@ export default class PromiseRouter {
     }
   };
 
+  use(middleware) {
+    this.middlewares.push(middleware);
+  }
+
   route(method, path, ...handlers) {
     switch(method) {
     case 'POST':
@@ -107,47 +112,17 @@ export default class PromiseRouter {
 
   // Mount the routes on this router onto an express app (or express router)
   mountOnto(expressApp) {
-    for (var route of this.routes) {
+    this.routes.forEach((route) => {
-      switch(route.method) {
+      let method = route.method.toLowerCase();
-      case 'POST':
+      let handler = makeExpressHandler(this.appId, route.handler);
-        expressApp.post(route.path, makeExpressHandler(this.appId, route.handler));
+      let args = [].concat(route.path, this.middlewares, handler);
-        break;
+      expressApp[method].apply(expressApp, args);
-      case 'GET':
+    });
-        expressApp.get(route.path, makeExpressHandler(this.appId, route.handler));
+    return expressApp;
-        break;
-      case 'PUT':
-        expressApp.put(route.path, makeExpressHandler(this.appId, route.handler));
-        break;
-      case 'DELETE':
-        expressApp.delete(route.path, makeExpressHandler(this.appId, route.handler));
-        break;
-      default:
-        throw 'unexpected code branch';
-      }
-    }
   };
 
-  expressApp() {
+  expressRouter() {
-    var expressApp = express();
+    return this.mountOnto(express.Router());
-    for (var route of this.routes) {
-      switch(route.method) {
-      case 'POST':
-        expressApp.post(route.path, makeExpressHandler(this.appId, route.handler));
-        break;
-      case 'GET':
-        expressApp.get(route.path, makeExpressHandler(this.appId, route.handler));
-        break;
-      case 'PUT':
-        expressApp.put(route.path, makeExpressHandler(this.appId, route.handler));
-        break;
-      case 'DELETE':
-        expressApp.delete(route.path, makeExpressHandler(this.appId, route.handler));
-        break;
-      default:
-        throw 'unexpected code branch';
-      }
-    }
-    return expressApp;
   }
 }
 

src/Routers/FilesRouter.js

@@ -7,7 +7,7 @@ import mime                from 'mime';
 
 export class FilesRouter {
 
-  getExpressRouter(options = {}) {
+  expressRouter(options = {}) {
     var router = express.Router();
     router.get('/files/:appId/:filename', this.getHandler);
 

src/Routers/PublicAPIRouter.js

@@ -152,10 +152,10 @@ export class PublicAPIRouter extends PromiseRouter {
       req => { return this.requestResetPassword(req); });
   }
 
-  expressApp() {
+  expressRouter() {
-    let router = express();
+    let router = express.Router();
     router.use("/apps", express.static(public_html));
-    router.use("/", super.expressApp());
+    router.use("/", super.expressRouter());
     return router;
   }
 }

src/TestUtils.js

@@ -1,15 +1,20 @@
-import { destroyAllDataPermanently } from './DatabaseAdapter';
+import AppCache from './cache';
 
-let unsupported = function() {
+//Used by tests
-  throw 'Only supported in test environment';
+function destroyAllDataPermanently() {
-};
+  if (!process.env.TESTING) {
-
+    throw 'Only supported in test environment';
-let _destroyAllDataPermanently;
+  }
-if (process.env.TESTING) {
+  return Promise.all(Object.keys(AppCache.cache).map(appId => {
-  _destroyAllDataPermanently = destroyAllDataPermanently;
+      const app = AppCache.get(appId);
-} else {
+      if (app.databaseController) {
-  _destroyAllDataPermanently = unsupported;
+        return app.databaseController.deleteEverything();
+      } else {
+        return Promise.resolve();
+      }
+    }));
 }
 
-export default {
+export {
-    destroyAllDataPermanently: _destroyAllDataPermanently};
+  destroyAllDataPermanently
+}

src/cloud-code/Parse.Cloud.js

@@ -41,11 +41,6 @@ ParseCloud.afterDelete = function(parseClass, handler) {
   triggers.addTrigger(triggers.Types.afterDelete, className, handler, Parse.applicationId);
 };
 
-ParseCloud._removeHook = function(category, name, type, applicationId) {
-  applicationId = applicationId || Parse.applicationId;
-  triggers._unregister(applicationId, category, name, type);
-};
-
 ParseCloud._removeAllHooks = () => {
   triggers._unregisterAll();
 }

src/middlewares.js

@@ -1,11 +1,9 @@
-import AppCache from './cache';
+import AppCache   from './cache';
-import log      from './logger';
+import log        from './logger';
-
+import Parse      from 'parse/node';
-var Parse = require('parse/node').Parse;
+import auth       from './Auth';
-
+import Config     from './Config';
-var auth = require('./Auth');
+import ClientSDK  from './ClientSDK';
-var Config = require('./Config');
-var ClientSDK = require('./ClientSDK');
 
 // Checks that the request is authorized for this app and checks user
 // auth too.
@@ -13,7 +11,7 @@ var ClientSDK = require('./ClientSDK');
 // Adds info to the request:
 // req.config - the Config for this app
 // req.auth - the Auth for this request
-function handleParseHeaders(req, res, next) {
+export function handleParseHeaders(req, res, next) {
   var mountPathLength = req.originalUrl.length - req.url.length;
   var mountPath = req.originalUrl.slice(0, mountPathLength);
   var mount = req.protocol + '://' + req.get('host') + mountPath;
@@ -205,7 +203,7 @@ function decodeBase64(str) {
   return new Buffer(str, 'base64').toString()
 }
 
-var allowCrossDomain = function(req, res, next) {
+export function allowCrossDomain(req, res, next) {
   res.header('Access-Control-Allow-Origin', '*');
   res.header('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE,OPTIONS');
   res.header('Access-Control-Allow-Headers', 'X-Parse-Master-Key, X-Parse-REST-API-Key, X-Parse-Javascript-Key, X-Parse-Application-Id, X-Parse-Client-Version, X-Parse-Session-Token, X-Requested-With, X-Parse-Revocable-Session, Content-Type');
@@ -219,7 +217,7 @@ var allowCrossDomain = function(req, res, next) {
   }
 };
 
-var allowMethodOverride = function(req, res, next) {
+export function allowMethodOverride(req, res, next) {
   if (req.method === 'POST' && req.body._method) {
     req.originalMethod = req.method;
     req.method = req.body._method;
@@ -228,7 +226,7 @@ var allowMethodOverride = function(req, res, next) {
   next();
 };
 
-var handleParseErrors = function(err, req, res, next) {
+export function handleParseErrors(err, req, res, next) {
   // TODO: Add logging as those errors won't make it to the PromiseRouter
   if (err instanceof Parse.Error) {
     var httpStatus;
@@ -259,7 +257,7 @@ var handleParseErrors = function(err, req, res, next) {
   next(err);
 };
 
-function enforceMasterKeyAccess(req, res, next) {
+export function enforceMasterKeyAccess(req, res, next) {
   if (!req.auth.isMaster) {
     res.status(403);
     res.end('{"error":"unauthorized: master key is required"}');
@@ -268,7 +266,7 @@ function enforceMasterKeyAccess(req, res, next) {
   next();
 }
 
-function promiseEnforceMasterKeyAccess(request) {
+export function promiseEnforceMasterKeyAccess(request) {
   if (!request.auth.isMaster) {
     let error = new Error();
     error.status = 403;
@@ -282,12 +280,3 @@ function invalidRequest(req, res) {
   res.status(403);
   res.end('{"error":"unauthorized"}');
 }
-
-module.exports = {
-  allowCrossDomain: allowCrossDomain,
-  allowMethodOverride: allowMethodOverride,
-  handleParseErrors: handleParseErrors,
-  handleParseHeaders: handleParseHeaders,
-  enforceMasterKeyAccess: enforceMasterKeyAccess,
-  promiseEnforceMasterKeyAccess,
-};