Merge pull request #1363 from ParsePlatform/issue/1319

Improves config loading and tests

spec/CLI.spec.js

@@ -1,3 +1,4 @@
+'use strict';
 var commander = require("../src/cli/utils/commander").default;
 
 var definitions = {
@@ -11,7 +12,7 @@ var definitions = {
     action: function(value) {
       var value = parseInt(value);
       if (!Number.isInteger(value)) {
-        throw "port is invalid";
+        throw "arg2 is invalid";
       }
       return value;
     }
@@ -23,7 +24,7 @@ var definitions = {
 }
 
 describe("commander additions", () => {
-  
+
   afterEach((done) => {
     commander.options = [];
     delete commander.arg0;
@@ -33,7 +34,7 @@ describe("commander additions", () => {
     delete commander.arg4;
     done();
   })
-  
+
   it("should load properly definitions from args", (done) => {
     commander.loadDefinitions(definitions);
     commander.parse(["node","./CLI.spec.js","--arg0", "arg0Value", "--arg1", "arg1Value", "--arg2", "2", "--arg3", "some"]);
@@ -44,7 +45,7 @@ describe("commander additions", () => {
     expect(commander.arg4).toEqual("arg4Value");
     done();
   });
-  
+
   it("should load properly definitions from env", (done) => {
     commander.loadDefinitions(definitions);
     commander.parse([], {
@@ -58,7 +59,7 @@ describe("commander additions", () => {
     expect(commander.arg4).toEqual("arg4Value");
     done();
   });
-  
+
   it("should load properly use args over env", (done) => {
     commander.loadDefinitions(definitions);
     commander.parse(["node","./CLI.spec.js","--arg0", "arg0Value", "--arg4", "anotherArg4"], {
@@ -72,7 +73,7 @@ describe("commander additions", () => {
     expect(commander.arg4).toEqual("anotherArg4");
     done();
   });
-  
+
   it("should fail in action as port is invalid", (done) => {
     commander.loadDefinitions(definitions);
     expect(()=> {
@@ -81,7 +82,58 @@ describe("commander additions", () => {
         "PROGRAM_ARG_1": "arg1ENVValue",
         "PROGRAM_ARG_2": "hello",
       });
-    }).toThrow("port is invalid");
+    }).toThrow("arg2 is invalid");
     done();
   });
-});
+
+  it("should not override config.json", (done) => {
+    commander.loadDefinitions(definitions);
+    commander.parse(["node","./CLI.spec.js","--arg0", "arg0Value", "./spec/configs/CLIConfig.json"], {
+      "PROGRAM_ARG_0": "arg0ENVValue",
+      "PROGRAM_ARG_1": "arg1ENVValue",
+    });
+    let options = commander.getOptions();
+    expect(options.arg2).toBe(8888);
+    expect(options.arg3).toBe("hello"); //config value
+    expect(options.arg4).toBe('/1');
+    done();
+  });
+
+  it("should fail with invalid values in JSON", (done) => {
+    commander.loadDefinitions(definitions);
+    expect(() => {
+      commander.parse(["node","./CLI.spec.js","--arg0", "arg0Value", "./spec/configs/CLIConfigFail.json"], {
+        "PROGRAM_ARG_0": "arg0ENVValue",
+        "PROGRAM_ARG_1": "arg1ENVValue",
+      });
+    }).toThrow("arg2 is invalid")
+    done();
+  });
+
+  it("should fail when too many apps are set", (done) => {
+    commander.loadDefinitions(definitions);
+    expect(() => {
+      commander.parse(["node","./CLI.spec.js","./spec/configs/CLIConfigFailTooManyApps.json"]);
+    }).toThrow("Multiple apps are not supported")
+    done();
+  });
+
+  it("should load config from apps", (done) => {
+    commander.loadDefinitions(definitions);
+    commander.parse(["node", "./CLI.spec.js", "./spec/configs/CLIConfigApps.json"]);
+    let options = commander.getOptions();
+    expect(options.arg1).toBe("my_app");
+    expect(options.arg2).toBe(8888);
+    expect(options.arg3).toBe("hello"); //config value
+    expect(options.arg4).toBe('/1');
+    done();
+  });
+
+  it("should fail when passing an invalid arguement", (done) => {
+    commander.loadDefinitions(definitions);
+    expect(() => {
+      commander.parse(["node", "./CLI.spec.js", "./spec/configs/CLIConfigUnknownArg.json"]);
+    }).toThrow('error: unknown option myArg')
+    done();
+  });
+});

spec/configs/CLIConfig.json

@@ -0,0 +1,6 @@
+{
+  "arg1": "my_app",
+  "arg2": "8888",
+  "arg3": "hello",
+  "arg4": "/1"
+}

spec/configs/CLIConfigApps.json

@@ -0,0 +1,9 @@
+{
+  "apps": [
+  {
+    "arg1": "my_app",
+    "arg2": 8888,
+    "arg3": "hello",
+    "arg4": "/1"
+  }]
+}

spec/configs/CLIConfigFail.json

@@ -0,0 +1,6 @@
+{
+  "arg1": "my_app",
+  "arg2": "hello",
+  "arg3": "hello",
+  "arg4": "/1"
+}

spec/configs/CLIConfigFailTooManyApps.json

@@ -0,0 +1,16 @@
+{
+  "apps": [
+  {
+    "arg1": "my_app",
+    "arg2": "99999",
+    "arg3": "hello",
+    "arg4": "/1"
+  },
+  {
+    "arg1": "my_app2",
+    "arg2": "9999",
+    "arg3": "hello",
+    "arg4": "/1"
+  }
+  ]
+}

spec/configs/CLIConfigUnknownArg.json

@@ -0,0 +1,6 @@
+{
+  "arg1": "my_app",
+  "arg2": "8888",
+  "arg3": "hello",
+  "myArg": "/1"
+}

src/cli/parse-server.js

@@ -3,6 +3,7 @@ import express from 'express';
 import { ParseServer } from '../index';
 import definitions from './cli-definitions';
 import program from './utils/commander';
+import { mergeWithOptions } from './utils/commander';
 import colors from 'colors';
 
 program.loadDefinitions(definitions);
@@ -34,28 +35,7 @@ program.on('--help', function(){
 
 program.parse(process.argv, process.env);
 
-let options = {};
+let options = program.getOptions();
-if (program.args.length > 0 ) {
-  let jsonPath = program.args[0];
-  jsonPath = path.resolve(jsonPath);
-  let jsonConfig = require(jsonPath);
-  if (jsonConfig.apps) {
-    if (jsonConfig.apps.length > 1) {
-      throw 'Multiple apps are not supported';
-    }
-    options = jsonConfig.apps[0];
-  } else {
-    options = jsonConfig;
-  }
-  console.log(`Configuation loaded from ${jsonPath}`)
-}
-
-options = Object.keys(definitions).reduce(function (options, key) {
-  if (typeof program[key] !== 'undefined') {
-    options[key] = program[key];
-  }
-  return options;
-}, options);
 
 if (!options.serverURL) {
   options.serverURL = `http://localhost:${options.port}${options.mountPath}`;

src/cli/utils/commander.js

@@ -1,12 +1,12 @@
 import { Command } from 'commander';
-
+import path from 'path';
 let _definitions;
 let _reverseDefinitions;
 let _defaults;
 
 Command.prototype.loadDefinitions = function(definitions) {
   _definitions = definitions;
-  
+
   Object.keys(definitions).reduce((program, opt) => {
     if (typeof definitions[opt] == "object") {
       const additionalOptions = definitions[opt];
@@ -18,14 +18,14 @@ Command.prototype.loadDefinitions = function(definitions) {
     }
     return program.option(`--${opt} [${opt}]`);
   }, this);
-  
+
   _defaults = Object.keys(definitions).reduce((defs, opt) => {
     if(_definitions[opt].default) {
       defs[opt] = _definitions[opt].default;
     }
     return defs;
   }, {});
-  
+
   _reverseDefinitions = Object.keys(definitions).reduce((object, key) => {
       let value = definitions[key];
       if (typeof value == "object") {
@@ -36,7 +36,7 @@ Command.prototype.loadDefinitions = function(definitions) {
       }
       return object;
    }, {});
-   
+
    /* istanbul ignore next */
    this.on('--help', function(){
     console.log('  Configure From Environment:');
@@ -58,17 +58,46 @@ function parseEnvironment(env = {}) {
       }
       options[_reverseDefinitions[key]] = action(env[key]);
     }
-    return options; 
+    return options;
   }, {});
 }
 
+function parseConfigFile(program) {
+  let options = {};
+  if (program.args.length > 0) {
+    let jsonPath = program.args[0];
+    jsonPath = path.resolve(jsonPath);
+    let jsonConfig = require(jsonPath);
+    if (jsonConfig.apps) {
+      if (jsonConfig.apps.length > 1) {
+        throw 'Multiple apps are not supported';
+      }
+      options = jsonConfig.apps[0];
+    } else {
+      options = jsonConfig;
+    }
+    Object.keys(options).forEach((key) => {
+      let value = options[key];
+      if (!_definitions[key]) {
+        throw `error: unknown option ${key}`;
+      }
+      let action = _definitions[key].action;
+      if (action) {
+        options[key] = action(value);
+      }
+    })
+    console.log(`Configuation loaded from ${jsonPath}`)
+  }
+  return options;
+}
+
 Command.prototype.setValuesIfNeeded = function(options) {
   Object.keys(options).forEach((key) => {
    if (!this[key]) {
      this[key] = options[key];
-   } 
+   }
   });
-} 
+}
 
 Command.prototype._parse = Command.prototype.parse;
 
@@ -76,10 +105,22 @@ Command.prototype.parse = function(args, env) {
   this._parse(args);
   // Parse the environment first
   const envOptions = parseEnvironment(env);
-  
+  const fromFile = parseConfigFile(this);
   // Load the env if not passed from command line
   this.setValuesIfNeeded(envOptions);
+  // Load from file to override
+  this.setValuesIfNeeded(fromFile);
+  // Last set the defaults
   this.setValuesIfNeeded(_defaults);
 }
 
+Command.prototype.getOptions = function() {
+  return Object.keys(_definitions).reduce((options, key) => {
+    if (typeof this[key] !== 'undefined') {
+      options[key] = this[key];
+    }
+    return options;
+  }, {});
+}
+
 export default new Command();