Refactors FilesController in FilesRouter and FilesController

spec/FilesController.spec.js

@@ -0,0 +1,27 @@
+var FilesController = require('../src/Controllers/FilesController').FilesController;
+var Config = require("../src/Config");
+
+// Small additional tests to improve overall coverage
+describe("FilesController",()=>{
+  
+  it("should properly expand objects", (done) => {
+    var config = new Config(Parse.applicationId);
+    var filesController = new FilesController();
+    var result = filesController.expandFilesInObject(config, function(){});
+    
+    expect(result).toBeUndefined();
+    
+    var fullFile = {
+      type: '__type',
+      url: "http://an.url"
+    }
+    
+    var anObject = {
+      aFile: fullFile
+    }
+    filesController.expandFilesInObject(config, anObject);
+    expect(anObject.aFile.url).toEqual("http://an.url");
+    
+    done();
+  }) 
+})

src/Config.js

@@ -1,34 +1,37 @@
 // A Config object provides information about how a specific app is
 // configured.
 // mount is the URL for the root of the API; includes http, domain, etc.
-function Config(applicationId, mount) {
-  var cache = require('./cache');
-  var DatabaseAdapter = require('./DatabaseAdapter');
+export class Config {
 
-  var cacheInfo = cache.apps[applicationId];
-  this.valid = !!cacheInfo;
-  if (!this.valid) {
-    return;
+  constructor(applicationId, mount) {
+    var cache = require('./cache');
+    var DatabaseAdapter = require('./DatabaseAdapter');
+
+    var cacheInfo = cache.apps[applicationId];
+    this.valid = !!cacheInfo;
+    if (!this.valid) {
+      return;
+    }
+
+    this.applicationId = applicationId;
+    this.collectionPrefix = cacheInfo.collectionPrefix || '';
+    this.masterKey = cacheInfo.masterKey;
+    this.clientKey = cacheInfo.clientKey;
+    this.javascriptKey = cacheInfo.javascriptKey;
+    this.dotNetKey = cacheInfo.dotNetKey;
+    this.restAPIKey = cacheInfo.restAPIKey;
+    this.fileKey = cacheInfo.fileKey;
+    this.facebookAppIds = cacheInfo.facebookAppIds;
+    this.enableAnonymousUsers = cacheInfo.enableAnonymousUsers;
+
+    this.database = DatabaseAdapter.getDatabaseConnection(applicationId);
+    this.filesController = cacheInfo.filesController;
+    this.pushController = cacheInfo.pushController;   
+    this.oauth = cacheInfo.oauth;
+
+    this.mount = mount;
   }
+};
 
-  this.applicationId = applicationId;
-  this.collectionPrefix = cacheInfo.collectionPrefix || '';
-  this.masterKey = cacheInfo.masterKey;
-  this.clientKey = cacheInfo.clientKey;
-  this.javascriptKey = cacheInfo.javascriptKey;
-  this.dotNetKey = cacheInfo.dotNetKey;
-  this.restAPIKey = cacheInfo.restAPIKey;
-  this.fileKey = cacheInfo.fileKey;
-  this.facebookAppIds = cacheInfo.facebookAppIds;
-  this.enableAnonymousUsers = cacheInfo.enableAnonymousUsers;
-
-  this.database = DatabaseAdapter.getDatabaseConnection(applicationId);
-  this.filesController = cacheInfo.filesController;
-  this.pushController = cacheInfo.pushController;   
-  this.oauth = cacheInfo.oauth;
-
-  this.mount = mount;
-}
-
-
+export default Config;
 module.exports = Config;

src/Controllers/FilesController.js

@@ -1,11 +1,5 @@
 // FilesController.js
-
-import express from 'express';
-import mime from 'mime';
 import { Parse } from 'parse/node';
-import BodyParser from 'body-parser';
-import * as Middlewares from '../middlewares';
-import Config from '../Config';
 import { randomHexString } from '../cryptoUtils';
 
 export class FilesController {
@@ -13,98 +7,23 @@ export class FilesController {
     this._filesAdapter = filesAdapter;
   }
 
-  static getHandler() {
-     return (req, res) => {
-       let config = new Config(req.params.appId);
-       return config.filesController.getHandler()(req, res);
-     }
+  getFileData(config, filename) {
+    return this._filesAdapter.getFileData(config, filename);
   }
-  
-  getHandler() {
-    return (req, res) => {
-      let config = new Config(req.params.appId);
-      let filename = req.params.filename;
-      this._filesAdapter.getFileData(config, filename).then((data) => {
-        res.status(200);
-        var contentType = mime.lookup(filename);
-        res.set('Content-type', contentType);
-        res.end(data);
-      }).catch((error) => {
-        res.status(404);
-        res.set('Content-type', 'text/plain');
-        res.end('File not found.');
+
+  createFile(config, filename, data) {
+    filename = randomHexString(32) + '_' + filename;
+    var location = this._filesAdapter.getFileLocation(config, filename);
+    return this._filesAdapter.createFile(config, filename, data).then(() => {
+      return Promise.resolve({
+        url: location,
+        name: filename
       });
-    };
-  }
+    });
+  } 
 
-  static createHandler() {
-     return (req, res, next) => {
-       let config = req.config;
-       return config.filesController.createHandler()(req, res, next);
-     }
-  }
-
-  createHandler() {
-    return (req, res, next) => {
-      if (!req.body || !req.body.length) {
-        next(new Parse.Error(Parse.Error.FILE_SAVE_ERROR,
-          'Invalid file upload.'));
-        return;
-      }
-
-      if (req.params.filename.length > 128) {
-        next(new Parse.Error(Parse.Error.INVALID_FILE_NAME,
-          'Filename too long.'));
-        return;
-      }
-
-      if (!req.params.filename.match(/^[_a-zA-Z0-9][a-zA-Z0-9@\.\ ~_-]*$/)) {
-        next(new Parse.Error(Parse.Error.INVALID_FILE_NAME,
-          'Filename contains invalid characters.'));
-        return;
-      }
-
-      const filesController = req.config.filesController;
-      // If a content-type is included, we'll add an extension so we can
-      // return the same content-type.
-      let extension = '';
-      let hasExtension = req.params.filename.indexOf('.') > 0;
-      let contentType = req.get('Content-type');
-      if (!hasExtension && contentType && mime.extension(contentType)) {
-        extension = '.' + mime.extension(contentType);
-      }
-
-      let filename = randomHexString(32) + '_' + req.params.filename + extension;
-      filesController._filesAdapter.createFile(req.config, filename, req.body).then(() => {
-        res.status(201);
-        var location = filesController._filesAdapter.getFileLocation(req.config, filename);
-        res.set('Location', location);
-        res.json({ url: location, name: filename });
-      }).catch((error) => {
-        next(new Parse.Error(Parse.Error.FILE_SAVE_ERROR,
-          'Could not store file.'));
-      });
-    };
-  }
-
-  static deleteHandler() {
-     return (req, res, next) => {
-       let config = req.config;
-       return config.filesController.deleteHandler()(req, res, next);
-     }
-  }
-
-  deleteHandler() {
-    return (req, res, next) => {
-      this._filesAdapter.deleteFile(req.config, req.params.filename).then(() => {
-        res.status(200);
-        // TODO: return useful JSON here?
-        res.end();
-      }).catch((error) => {
-        next(new Parse.Error(Parse.Error.FILE_DELETE_ERROR,
-          'Could not delete file.'));
-      });
-    };
+  deleteFile(config, filename) {
+    return this._filesAdapter.deleteFile(config, filename);
   }
 
   /**
@@ -135,32 +54,6 @@ export class FilesController {
       }
     }
   }
-
-  static getExpressRouter() {
-    let router = express.Router();
-    router.get('/files/:appId/:filename', FilesController.getHandler());
-
-    router.post('/files', function(req, res, next) {
-      next(new Parse.Error(Parse.Error.INVALID_FILE_NAME,
-        'Filename not provided.'));
-    });
-
-    router.post('/files/:filename',
-      Middlewares.allowCrossDomain,
-      BodyParser.raw({type: '*/*', limit: '20mb'}),
-      Middlewares.handleParseHeaders,
-      FilesController.createHandler()
-    );
-
-    router.delete('/files/:filename',
-      Middlewares.allowCrossDomain,
-      Middlewares.handleParseHeaders,
-      Middlewares.enforceMasterKeyAccess,
-      FilesController.deleteHandler()
-    );
-
-    return router;
-  }
 }
 
 export default FilesController;

src/Routers/AnalyticsRouter.js

@@ -1,7 +1,4 @@
 // AnalyticsRouter.js
-
-var Parse = require('parse/node').Parse;
-
 import PromiseRouter from '../PromiseRouter';
 
 // Returns a promise that resolves to an empty object response

src/Routers/FilesRouter.js

@@ -0,0 +1,104 @@
+import PromiseRouter from '../PromiseRouter';
+import express from 'express';
+import BodyParser from 'body-parser';
+import * as Middlewares from '../middlewares';
+import { randomHexString } from '../cryptoUtils';
+import mime from 'mime';
+import Config from '../Config';
+
+export class FilesRouter {
+  
+  getExpressRouter() {
+    var router = express.Router();
+    router.get('/files/:appId/:filename', this.getHandler);
+
+    router.post('/files', function(req, res, next) {
+      next(new Parse.Error(Parse.Error.INVALID_FILE_NAME,
+        'Filename not provided.'));
+    });
+
+    router.post('/files/:filename',
+      Middlewares.allowCrossDomain,
+      BodyParser.raw({type: '*/*', limit: '20mb'}),
+      Middlewares.handleParseHeaders,
+      this.createHandler
+    );
+
+    router.delete('/files/:filename',
+      Middlewares.allowCrossDomain,
+      Middlewares.handleParseHeaders,
+      Middlewares.enforceMasterKeyAccess,
+      this.deleteHandler
+    );
+    return router;
+  }
+  
+  getHandler(req, res, next) {
+    const config = new Config(req.params.appId);
+    const filesController = config.filesController;
+    const filename = req.params.filename;
+    filesController.getFileData(config, filename).then((data) => {
+        res.status(200);
+        var contentType = mime.lookup(filename);
+        res.set('Content-type', contentType);
+        res.end(data);
+      }).catch((error) => {
+        res.status(404);
+        res.set('Content-type', 'text/plain');
+        res.end('File not found.');
+      });
+  }
+  
+  createHandler(req, res, next) {
+    if (!req.body || !req.body.length) {
+      next(new Parse.Error(Parse.Error.FILE_SAVE_ERROR,
+        'Invalid file upload.'));
+      return;
+    }
+
+    if (req.params.filename.length > 128) {
+      next(new Parse.Error(Parse.Error.INVALID_FILE_NAME,
+        'Filename too long.'));
+      return;
+    }
+
+    if (!req.params.filename.match(/^[_a-zA-Z0-9][a-zA-Z0-9@\.\ ~_-]*$/)) {
+      next(new Parse.Error(Parse.Error.INVALID_FILE_NAME,
+        'Filename contains invalid characters.'));
+      return;
+    }
+    let extension = '';
+    
+    // Not very safe there.
+    const hasExtension = req.params.filename.indexOf('.') > 0;
+    const contentType = req.get('Content-type');
+    if (!hasExtension && contentType && mime.extension(contentType)) {
+      extension = '.' + mime.extension(contentType);
+    }
+
+    const filename = req.params.filename + extension;
+    const config = req.config;
+    const filesController = config.filesController;
+
+    filesController.createFile(config, filename, req.body).then((result) => {
+      res.status(201);  
+      res.set('Location', result.url);
+      res.json(result);
+    }).catch((err) => {
+       next(new Parse.Error(Parse.Error.FILE_SAVE_ERROR,
+          'Could not store file.'));
+    });
+  }
+  
+  deleteHandler(req, res, next) {
+    const filesController = req.config.filesController;
+    filesController.deleteFile(req.config, req.params.filename).then(() => {
+      res.status(200);
+      // TODO: return useful JSON here?
+      res.end();
+    }).catch((error) => {
+      next(new Parse.Error(Parse.Error.FILE_DELETE_ERROR,
+        'Could not delete file.'));
+    });
+  }
+}

src/index.js

@@ -29,6 +29,7 @@ import { FunctionsRouter }     from './Routers/FunctionsRouter';
 import { SchemasRouter }       from './Routers/SchemasRouter';
 import { IAPValidationRouter } from './Routers/IAPValidationRouter';
 import { PushRouter }          from './Routers/PushRouter';
+import { FilesRouter }         from './Routers/FilesRouter';
 
 import { FileLoggerAdapter }   from './Adapters/Logger/FileLoggerAdapter';
 import { LoggerController }    from './Controllers/LoggerController';
@@ -111,8 +112,9 @@ function ParseServer({
     }
   }
 
-  let filesController = new FilesController(filesAdapter);
-  let pushController = new PushController(pushAdapter);
+  const filesController = new FilesController(filesAdapter);
+  const pushController = new PushController(pushAdapter);
+  const loggerController = new LoggerController(loggerAdapter);
   
   cache.apps[appId] = {
     masterKey: masterKey,
@@ -125,6 +127,7 @@ function ParseServer({
     facebookAppIds: facebookAppIds,
     filesController: filesController,
     pushController: pushController,
+    loggerController: loggerController,
     enableAnonymousUsers: enableAnonymousUsers,
     oauth: oauth,
 };
@@ -143,7 +146,7 @@ function ParseServer({
   var api = express();
 
   // File handling needs to be before default middlewares are applied
-  api.use('/', FilesController.getExpressRouter());
+  api.use('/', new FilesRouter().getExpressRouter());
 
   // TODO: separate this from the regular ParseServer object
   if (process.env.TESTING == 1) {