From 23e55e941e9f5f7809c082fc3054a74fc0d12e09 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 21 Feb 2016 23:47:07 -0500 Subject: [PATCH] Splits Adapter loading from AdaptableController - Adds dynamic prototype conformance check upon setting adapter - Throws when adapter is undefined, invalid in controller --- spec/AdaptableController.spec.js | 90 +++++++++----------------- spec/AdapterLoader.spec.js | 68 +++++++++++++++++++ spec/FilesController.spec.js | 4 +- spec/LoggerController.spec.js | 6 +- src/Adapters/AdapterLoader.js | 39 +++++++++++ src/Controllers/AdaptableController.js | 78 +++++++++++----------- src/Controllers/FilesController.js | 7 +- src/Controllers/LoggerController.js | 5 ++ src/Controllers/PushController.js | 12 +++- src/index.js | 17 ++--- 10 files changed, 210 insertions(+), 116 deletions(-) create mode 100644 spec/AdapterLoader.spec.js create mode 100644 src/Adapters/AdapterLoader.js diff --git a/spec/AdaptableController.spec.js b/spec/AdaptableController.spec.js index ce0ff20a..70a80dfa 100644 --- a/spec/AdaptableController.spec.js +++ b/spec/AdaptableController.spec.js @@ -1,70 +1,44 @@ var AdaptableController = require("../src/Controllers/AdaptableController").AdaptableController; var FilesAdapter = require("../src/Adapters/Files/FilesAdapter").default; +var FilesController = require("../src/Controllers/FilesController").FilesController; + +var MockController = function(options) { + AdaptableController.call(this, options); +} +MockController.prototype = Object.create(AdaptableController.prototype); +MockController.prototype.constructor = AdaptableController; describe("AdaptableController", ()=>{ - - it("should instantiate an adapter from string in object", (done) => { - var adapterPath = require('path').resolve("./spec/MockAdapter"); - var controller = new AdaptableController({ - adapter: adapterPath, - key: "value", - foo: "bar" - }); - - expect(controller.adapter instanceof Object).toBe(true); - expect(controller.options.key).toBe("value"); - expect(controller.options.foo).toBe("bar"); - expect(controller.adapter.options.key).toBe("value"); - expect(controller.adapter.options.foo).toBe("bar"); - done(); - }); - - it("should instantiate an adapter from string", (done) => { - var adapterPath = require('path').resolve("./spec/MockAdapter"); - var controller = new AdaptableController(adapterPath); - - expect(controller.adapter instanceof Object).toBe(true); - done(); - }); - - it("should instantiate an adapter from string that is module", (done) => { - var adapterPath = require('path').resolve("./src/Adapters/Files/FilesAdapter"); - var controller = new AdaptableController({ - adapter: adapterPath - }); - - expect(controller.adapter instanceof FilesAdapter).toBe(true); - done(); - }); - - it("should instantiate an adapter from function/Class", (done) => { - var controller = new AdaptableController({ - adapter: FilesAdapter - }); - expect(controller.adapter instanceof FilesAdapter).toBe(true); - done(); - }); - - it("should instantiate the default adapter from Class", (done) => { - AdaptableController.setDefaultAdapter(FilesAdapter); - var controller = new AdaptableController(); - expect(controller.adapter instanceof FilesAdapter).toBe(true); - done(); - }); - - it("should use the default adapter", (done) => { - var adapter = new FilesAdapter(); - AdaptableController.setDefaultAdapter(adapter); - var controller = new AdaptableController(); - expect(controller.adapter).toBe(adapter); - done(); - }); it("should use the provided adapter", (done) => { var adapter = new FilesAdapter(); - var controller = new AdaptableController(adapter); + var controller = new FilesController(adapter); expect(controller.adapter).toBe(adapter); done(); }); + + it("should throw when creating a new mock controller", (done) => { + var adapter = new FilesAdapter(); + expect(() => { + new MockController(adapter); + }).toThrow(); + done(); + }); + + it("should fail to instantiate a controller with wrong adapter", (done) => { + function WrongAdapter() {}; + var adapter = new WrongAdapter(); + expect(() => { + new FilesController(adapter); + }).toThrow(); + done(); + }); + + it("should fail to instantiate a controller without an adapter", (done) => { + expect(() => { + new FilesController(); + }).toThrow(); + done(); + }); }); \ No newline at end of file diff --git a/spec/AdapterLoader.spec.js b/spec/AdapterLoader.spec.js new file mode 100644 index 00000000..d5778969 --- /dev/null +++ b/spec/AdapterLoader.spec.js @@ -0,0 +1,68 @@ + +var AdapterLoader = require("../src/Adapters/AdapterLoader").AdapterLoader; +var FilesAdapter = require("../src/Adapters/Files/FilesAdapter").default; + +describe("AdaptableController", ()=>{ + + it("should instantiate an adapter from string in object", (done) => { + var adapterPath = require('path').resolve("./spec/MockAdapter"); + + var adapter = AdapterLoader.load({ + adapter: adapterPath, + key: "value", + foo: "bar" + }); + + expect(adapter instanceof Object).toBe(true); + expect(adapter.options.key).toBe("value"); + expect(adapter.options.foo).toBe("bar"); + done(); + }); + + it("should instantiate an adapter from string", (done) => { + var adapterPath = require('path').resolve("./spec/MockAdapter"); + var adapter = AdapterLoader.load(adapterPath); + + expect(adapter instanceof Object).toBe(true); + expect(adapter.options).toBe(adapterPath); + done(); + }); + + it("should instantiate an adapter from string that is module", (done) => { + var adapterPath = require('path').resolve("./src/Adapters/Files/FilesAdapter"); + var adapter = AdapterLoader.load({ + adapter: adapterPath + }); + + expect(adapter instanceof FilesAdapter).toBe(true); + done(); + }); + + it("should instantiate an adapter from function/Class", (done) => { + var adapter = AdapterLoader.load({ + adapter: FilesAdapter + }); + expect(adapter instanceof FilesAdapter).toBe(true); + done(); + }); + + it("should instantiate the default adapter from Class", (done) => { + var adapter = AdapterLoader.load(null, FilesAdapter); + expect(adapter instanceof FilesAdapter).toBe(true); + done(); + }); + + it("should use the default adapter", (done) => { + var defaultAdapter = new FilesAdapter(); + var adapter = AdapterLoader.load(null, defaultAdapter); + expect(adapter instanceof FilesAdapter).toBe(true); + done(); + }); + + it("should use the provided adapter", (done) => { + var originalAdapter = new FilesAdapter(); + var adapter = AdapterLoader.load(originalAdapter); + expect(adapter).toBe(originalAdapter); + done(); + }); +}); \ No newline at end of file diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index b0c6f568..67b36de9 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -1,4 +1,5 @@ var FilesController = require('../src/Controllers/FilesController').FilesController; +var GridStoreAdapter = require("../src/Adapters/Files/GridStoreAdapter").GridStoreAdapter; var Config = require("../src/Config"); // Small additional tests to improve overall coverage @@ -6,7 +7,8 @@ describe("FilesController",()=>{ it("should properly expand objects", (done) => { var config = new Config(Parse.applicationId); - var filesController = new FilesController(); + var adapter = new GridStoreAdapter(); + var filesController = new FilesController(adapter); var result = filesController.expandFilesInObject(config, function(){}); expect(result).toBeUndefined(); diff --git a/spec/LoggerController.spec.js b/spec/LoggerController.spec.js index 28c4bfbb..9372ed9d 100644 --- a/spec/LoggerController.spec.js +++ b/spec/LoggerController.spec.js @@ -76,13 +76,11 @@ describe('LoggerController', () => { }); it('should throw without an adapter', (done) => { - LoggerController.setDefaultAdapter(undefined); - var loggerController = new LoggerController(); + expect(() => { - loggerController.getLogs(); + var loggerController = new LoggerController(); }).toThrow(); - LoggerController.setDefaultAdapter(FileLoggerAdapter); done(); }); }); diff --git a/src/Adapters/AdapterLoader.js b/src/Adapters/AdapterLoader.js new file mode 100644 index 00000000..a0e0b877 --- /dev/null +++ b/src/Adapters/AdapterLoader.js @@ -0,0 +1,39 @@ + +export class AdapterLoader { + static load(options, defaultAdapter) { + let adapter; + + // We have options and options have adapter key + if (options) { + // Pass an adapter as a module name, a function or an instance + if (typeof options == "string" || typeof options == "function" || options.constructor != Object) { + adapter = options; + } + if (options.adapter) { + adapter = options.adapter; + } + } + + if (!adapter) { + adapter = defaultAdapter; + } + + // This is a string, require the module + if (typeof adapter === "string") { + adapter = require(adapter); + // If it's define as a module, get the default + if (adapter.default) { + adapter = adapter.default; + } + } + // From there it's either a function or an object + // if it's an function, instanciate and pass the options + if (typeof adapter === "function") { + var Adapter = adapter; + adapter = new Adapter(options); + } + return adapter; + } +} + +export default AdapterLoader; diff --git a/src/Controllers/AdaptableController.js b/src/Controllers/AdaptableController.js index 552253e0..fc014db1 100644 --- a/src/Controllers/AdaptableController.js +++ b/src/Controllers/AdaptableController.js @@ -8,8 +8,6 @@ based on the parameters passed */ -const DefaultAdapters = {}; - export class AdaptableController { /** * Check whether the api call has master key or not. @@ -22,51 +20,49 @@ export class AdaptableController { * - object: a plain javascript object (options.constructor === Object), if options.adapter is set, we'll try to load it with the same mechanics. * - function: we'll create a new instance from that function, and pass the options object */ - constructor(options) { - - let adapter; - - // We have options and options have adapter key - if (options) { - // Pass an adapter as a module name, a function or an instance - if (typeof options == "string" || typeof options == "function" || options.constructor != Object) { - adapter = options; - } - if (options.adapter) { - adapter = options.adapter; - } - } - - if (!adapter) { - adapter = this.defaultAdapter(); - } - - // This is a string, require the module - if (typeof adapter === "string") { - adapter = require(adapter); - // If it's define as a module, get the default - if (adapter.default) { - adapter = adapter.default; - } - } - // From there it's either a function or an object - // if it's an function, instanciate and pass the options - if (typeof adapter === "function") { - var Adapter = adapter; - adapter = new Adapter(options); - } - + constructor(adapter, options) { + this.setAdapter(adapter, options); + } + + setAdapter(adapter, options) { + this.validateAdapter(adapter); this.adapter = adapter; this.options = options; } - defaultAdapter() { - return DefaultAdapters[this.constructor.name]; + expectedAdapterType() { + throw new Error("Subclasses should implement expectedAdapterType()"); } - // Sets the default adapter for that Class - static setDefaultAdapter(defaultAdapter) { - DefaultAdapters[this.name] = defaultAdapter; + validateAdapter(adapter) { + + if (!adapter) { + throw new Error(this.constructor.name+" requires an adapter"); + } + + let Type = this.expectedAdapterType(); + // Allow skipping for testing + if (!Type) { + return; + } + + // Makes sure the prototype matches + let mismatches = Object.getOwnPropertyNames(Type.prototype).reduce( (obj, key) => { + const adapterType = typeof adapter[key]; + const expectedType = typeof Type.prototype[key]; + if (adapterType !== expectedType) { + obj[key] = { + expected: expectedType, + actual: adapterType + } + } + return obj; + }, {}); + + if (Object.keys(mismatches).length > 0) { + console.error(adapter, mismatches); + throw new Error("Adapter prototype don't match expected prototype"); + } } } diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index 285612ce..fd5cec8d 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -2,6 +2,7 @@ import { Parse } from 'parse/node'; import { randomHexString } from '../cryptoUtils'; import AdaptableController from './AdaptableController'; +import { FilesAdapter } from '../Adapters/Files/FilesAdapter'; export class FilesController extends AdaptableController { @@ -29,7 +30,7 @@ export class FilesController extends AdaptableController { * with the current mount point and app id. * Object may be a single object or list of REST-format objects. */ - expandFilesInObject(config, object) { + expandFilesInObject(config, object) { if (object instanceof Array) { object.map((obj) => this.expandFilesInObject(config, obj)); return; @@ -52,6 +53,10 @@ export class FilesController extends AdaptableController { } } } + + expectedAdapterType() { + return FilesAdapter; + } } export default FilesController; diff --git a/src/Controllers/LoggerController.js b/src/Controllers/LoggerController.js index f7f39a78..fb74aabd 100644 --- a/src/Controllers/LoggerController.js +++ b/src/Controllers/LoggerController.js @@ -1,6 +1,7 @@ import { Parse } from 'parse/node'; import PromiseRouter from '../PromiseRouter'; import AdaptableController from './AdaptableController'; +import { LoggerAdapter } from '../Adapters/Logger/LoggerAdapter'; const Promise = Parse.Promise; const MILLISECONDS_IN_A_DAY = 24 * 60 * 60 * 1000; @@ -70,6 +71,10 @@ export class LoggerController extends AdaptableController { }); return promise; } + + expectedAdapterType() { + return LoggerAdapter; + } } export default LoggerController; diff --git a/src/Controllers/PushController.js b/src/Controllers/PushController.js index feba11e9..22d9fe11 100644 --- a/src/Controllers/PushController.js +++ b/src/Controllers/PushController.js @@ -2,6 +2,7 @@ import { Parse } from 'parse/node'; import PromiseRouter from '../PromiseRouter'; import rest from '../rest'; import AdaptableController from './AdaptableController'; +import { PushAdapter } from '../Adapters/Push/PushAdapter'; export class PushController extends AdaptableController { @@ -25,7 +26,7 @@ export class PushController extends AdaptableController { deviceType + ' is not supported push type.'); } } - }; + } /** * Check whether the api call has master key or not. @@ -53,7 +54,8 @@ export class PushController extends AdaptableController { rest.find(config, auth, '_Installation', where).then(function(response) { return pushAdapter.send(body, response.results); }); - }; + } + /** * Get expiration time from the request body. * @param {Object} request A request object @@ -80,7 +82,11 @@ export class PushController extends AdaptableController { body['expiration_time'] + ' is not valid time.'); } return expirationTime.valueOf(); - }; + } + + expectedAdapterType() { + return PushAdapter; + } }; export default PushController; diff --git a/src/index.js b/src/index.js index e34f4c4a..92523ea8 100644 --- a/src/index.js +++ b/src/index.js @@ -33,14 +33,10 @@ import { PushRouter } from './Routers/PushRouter'; import { FilesRouter } from './Routers/FilesRouter'; import { LogsRouter } from './Routers/LogsRouter'; +import { AdapterLoader } from './Adapters/AdapterLoader'; import { FileLoggerAdapter } from './Adapters/Logger/FileLoggerAdapter'; import { LoggerController } from './Controllers/LoggerController'; - -FilesController.setDefaultAdapter(GridStoreAdapter); -PushController.setDefaultAdapter(ParsePushAdapter); -LoggerController.setDefaultAdapter(FileLoggerAdapter); - // Mutate the Parse object to add the Cloud Code handlers addParseCloud(); @@ -109,12 +105,17 @@ function ParseServer({ throw "argument 'cloud' must either be a string or a function"; } } + + + const filesControllerAdapter = AdapterLoader.load(filesAdapter, GridStoreAdapter); + const pushControllerAdapter = AdapterLoader.load(push, ParsePushAdapter); + const loggerControllerAdapter = AdapterLoader.load(loggerAdapter, FileLoggerAdapter); // We pass the options and the base class for the adatper, // Note that passing an instance would work too - const filesController = new FilesController(filesAdapter); - const pushController = new PushController(push); - const loggerController = new LoggerController(loggerAdapter); + const filesController = new FilesController(filesControllerAdapter); + const pushController = new PushController(pushControllerAdapter); + const loggerController = new LoggerController(loggerControllerAdapter); cache.apps[appId] = { masterKey: masterKey,