From 305879a251ae84dcc80f02c07670b3cbbbb82a27 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sat, 20 Feb 2016 12:25:43 -0500 Subject: [PATCH] Refactors FilesController in FilesRouter and FilesController --- spec/FilesController.spec.js | 27 ++++++ src/Config.js | 57 ++++++------ src/Controllers/FilesController.js | 135 +++-------------------------- src/Routers/AnalyticsRouter.js | 3 - src/Routers/FilesRouter.js | 104 ++++++++++++++++++++++ src/index.js | 9 +- 6 files changed, 181 insertions(+), 154 deletions(-) create mode 100644 spec/FilesController.spec.js create mode 100644 src/Routers/FilesRouter.js diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js new file mode 100644 index 00000000..b0c6f568 --- /dev/null +++ b/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(); + }) +}) \ No newline at end of file diff --git a/src/Config.js b/src/Config.js index 3b9188a4..ed3d5aa7 100644 --- a/src/Config.js +++ b/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; diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index f1238694..e7d01763 100644 --- a/src/Controllers/FilesController.js +++ b/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; diff --git a/src/Routers/AnalyticsRouter.js b/src/Routers/AnalyticsRouter.js index 9e26e5f9..76be8d87 100644 --- a/src/Routers/AnalyticsRouter.js +++ b/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 diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js new file mode 100644 index 00000000..68bdd1e4 --- /dev/null +++ b/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.')); + }); + } +} \ No newline at end of file diff --git a/src/index.js b/src/index.js index 6512f322..dd671353 100644 --- a/src/index.js +++ b/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) {