Order of precedence inconsistent for falsey options when running from CLI

[steven-supersolid]

Steps to reproduce

Test 1
Set environment variable PARSE_CLUSTER_SIZE = 2
Run CLI from the following npm script in package.json

"start": "./node_modules/.bin/parse-server --cloud ./src/parse/main.js --cluster false src/parse/defaultConfig.js",

Expected: Cluster is not enabled, 1 process runs
Actual: Cluster is enabled, 2 processes run - FAIL, environment variable overrides CLI option

Test 2
Set environment variable PARSE_CLUSTER_SIZE = 2
Run CLI from the following npm script in package.json

"start": "./node_modules/.bin/parse-server --cloud ./src/parse/main.js --cluster 3 src/parse/defaultConfig.js",

Expected: Cluster is enabled, 3 processes runs
Actual: Cluster is enabled, 3 processes run - PASS, CLI option overrides environment variable

Environment Setup

• Server
  • parse-server version (Be specific! Don't say 'latest'.) : 2.2.25
  • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): Heroku

Logs/Trace

I added logging to numberOrBoolParser. It seems that this is called multiple times when cluster is false

parse-server --cloud src/main.js --cluster false src/parse/defaultConfig.js 
  Trace: numberOrBoolParser cluster false 
      at /app/node_modules/parse-server/lib/cli/utils/parsers.js:28:13 
      at Command.<anonymous> (/app/node_modules/commander/index.js:395:35) 
      at emitOne (events.js:96:13) 
      at Command.emit (events.js:188:7) 
      at Command.parseOptions (/app/node_modules/commander/index.js:699:14) 
      at Command.parse [as _parse] (/app/node_modules/commander/index.js:455:21) 
      at Command._commander.Command.parse (/app/node_modules/parse-server/lib/cli/utils/commander.js:126:8) 
      at exports.default (/app/node_modules/parse-server/lib/cli/utils/runner.js:22:23) 
      at Object.<anonymous> (/app/node_modules/parse-server/lib/cli/parse-server.js:101:22) 
      at Module._compile (module.js:570:32) 

  Trace: numberOrBoolParser cluster 2 
      at /app/node_modules/parse-server/lib/cli/utils/parsers.js:28:13 
      at /app/node_modules/parse-server/lib/cli/utils/commander.js:78:43 
      at Array.reduce (native) 
      at parseEnvironment (/app/node_modules/parse-server/lib/cli/utils/commander.js:69:43) 
      at Command._commander.Command.parse (/app/node_modules/parse-server/lib/cli/utils/commander.js:128:20) 
      at exports.default (/app/node_modules/parse-server/lib/cli/utils/runner.js:22:23) 
      at Object.<anonymous> (/app/node_modules/parse-server/lib/cli/parse-server.js:101:22) 
      at Module._compile (module.js:570:32) 
      at Object.Module._extensions..js (module.js:579:10) 
      at Module.load (module.js:487:32) 

  Configuration loaded from /app/src/parse/defaultConfig.js 

  Trace: numberOrBoolParser cluster false 
      at /app/node_modules/parse-server/lib/cli/utils/parsers.js:28:13 
      at Command.<anonymous> (/app/node_modules/commander/index.js:395:35) 
      at emitOne (events.js:96:13) 
      at Command.emit (events.js:188:7) 
      at Command.parseOptions (/app/node_modules/commander/index.js:699:14) 
      at Command.parse [as _parse] (/app/node_modules/commander/index.js:455:21) 
      at Command._commander.Command.parse (/app/node_modules/parse-server/lib/cli/utils/commander.js:126:8) 
      at exports.default (/app/node_modules/parse-server/lib/cli/utils/runner.js:22:23) 
      at Object.<anonymous> (/app/node_modules/parse-server/lib/cli/parse-server.js:101:22) 
      at Module._compile (module.js:570:32) 

  Trace: numberOrBoolParser cluster 2 
      at /app/node_modules/parse-server/lib/cli/utils/parsers.js:28:13 
      at /app/node_modules/parse-server/lib/cli/utils/commander.js:78:43 
      at Array.reduce (native) 
      at parseEnvironment (/app/node_modules/parse-server/lib/cli/utils/commander.js:69:43) 
      at Command._commander.Command.parse (/app/node_modules/parse-server/lib/cli/utils/commander.js:128:20) 
      at exports.default (/app/node_modules/parse-server/lib/cli/utils/runner.js:22:23) 
      at Object.<anonymous> (/app/node_modules/parse-server/lib/cli/parse-server.js:101:22) 
      at Module._compile (module.js:570:32) 
      at Object.Module._extensions..js (module.js:579:10) 
      at Module.load (module.js:487:32) 

  Configuration loaded from /app/src/parse/defaultConfig.js 

  Trace: numberOrBoolParser cluster false 
      at /app/node_modules/parse-server/lib/cli/utils/parsers.js:28:13 
      at Command.<anonymous> (/app/node_modules/commander/index.js:395:35) 
      at emitOne (events.js:96:13) 
      at Command.emit (events.js:188:7) 
      at Command.parseOptions (/app/node_modules/commander/index.js:699:14) 
      at Command.parse [as _parse] (/app/node_modules/commander/index.js:455:21) 
      at Command._commander.Command.parse (/app/node_modules/parse-server/lib/cli/utils/commander.js:126:8) 
      at exports.default (/app/node_modules/parse-server/lib/cli/utils/runner.js:22:23) 
      at Object.<anonymous> (/app/node_modules/parse-server/lib/cli/parse-server.js:101:22) 
      at Module._compile (module.js:570:32) 

  Trace: numberOrBoolParser cluster 2 
      at /app/node_modules/parse-server/lib/cli/utils/parsers.js:28:13 
      at /app/node_modules/parse-server/lib/cli/utils/commander.js:78:43 
      at Array.reduce (native) 
      at parseEnvironment (/app/node_modules/parse-server/lib/cli/utils/commander.js:69:43) 
      at Command._commander.Command.parse (/app/node_modules/parse-server/lib/cli/utils/commander.js:128:20) 
      at exports.default (/app/node_modules/parse-server/lib/cli/utils/runner.js:22:23) 
      at Object.<anonymous> (/app/node_modules/parse-server/lib/cli/parse-server.js:101:22) 
      at Module._compile (module.js:570:32) 
      at Object.Module._extensions..js (module.js:579:10) 
      at Module.load (module.js:487:32) 

  Configuration loaded from /app/src/parse/defaultConfig.js 

  [23] parse-server running on http://localhost:17398/parse 
  [29] parse-server running on http://localhost:17398/parse 

[steven-supersolid]

The problem seems to be in commander.js that falsey args will be overridden: https://github.com/ParsePlatform/parse-server/blob/master/src/cli/utils/commander.js#L111

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.setValuesIfNeeded = function(options) {
  Object.keys(options).forEach((key) => {
    if (!this[key]) {
      this[key] = options[key];
    }
  });
}

Couple of solutions:

1. Change falsey check in setValuesIfNeeded to use hasOwnProperty()
2. Set the values without checking if they exist and reorder code in to use the correct order of precedence

[flovilmart]

I like option 1, as it's what was intended :)

[steven-supersolid]

Cool, will put in a PR. Seems to solve the issue in my internal testing