EAGLESIX-2699 Improved the expression to better match the c++ code.

Support the array and object forms of the expression.
Enhance the test cases to test for exactly 3 args in the array form.
Enhance the test cases to test for arguments being out of order in the object form.

lib/pipeline/expressions/CondExpression.js

@@ -28,14 +28,22 @@ proto.getOpName = function getOpName() {
     return klass.opName;
 };
 
+/**
+ *
+ * @param expr	- I expect this to be the RHS of $cond:{...} or $cond:[,,,]
+ * @param vps
+ * @returns {*}
+ */
 klass.parse = function parse(expr, vps) {
 	// There may only be one argument - an array of 3 items, or a hash containing 3 keys.
     //this.checkArgLimit(3);
 
     // if not an object, return;
+	// todo I don't understand why we'd do this.  shouldn't expr be {}, [], or wrong?
     if (typeof(expr) !== Object)
 		return Expression.parse(expr, vps);
 
+	// ...or expr could be the entirety of $cond:{...} or $cond:[,,,].
 	if(!(klass.opName in expr)) {
 		throw new Error("Invalid expression. Expected to see '"+klass.opName+"'");
 	}
@@ -44,19 +52,38 @@ klass.parse = function parse(expr, vps) {
 
 	// If this is an Object and not an array, verify all the bits are specified.
 	// If this is an Object that is an array, verify there are three bits.
-    var args = Expression.parseOperand(expr, vps);
-// what is args?  If it is an array of scalars (array mode) or an array of objects (object mode), we're ok.
-// In the latter case, I am very not ok with the assuming if, then, and else are in a known order which is
-// what the original code implies.  It looks like if we see an 'if' we set this.operands[0]. If we see 'then', we set
-// this.operands[1], and if we see 'else' we set operands[2].  Bugcheck on unknown values. Bugcheck on insufficent
-// numbers of values, AKA !== 3
-    if (args[0] !== "if")
-		throw new Error("Missing 'if' parameter to $cond");
-    if (args[1] !== "then")
-		throw new Error("Missing 'then' parameter to $cond");
-    if (args[2] !== "else")
-		throw new Error("Missing 'else' parameter to $cond");
+	// (My issue here is that we got to this parse function when we parsed the $cond:{...} item, and we're calling
+	// parseOperand (again) without altering the input.)
+//    var args = Expression.parseOperand(expr, vps);
+
+	var args = expr[getOpName()];
+
+	if (typeof args !== 'object') throw new Error("this should not happen");
+	if (args instanceof Array) {
+		// it's the array form. Convert it to the object form.
+		if (args.length !== 3) throw new Error("$cond requires exactly three arguments");
+		args = {if: args[0], then: args[1], else: args[2]};
+	}
+
+	// One way or the other, args is now in object form.
+	Object.keys(args).forEach(function(arg) {
+		if (arg === 'if') {
+			ret.operands[0] = Expression.parseOperand(args['if'], vps);
+		}
+		else if (arg === 'then') {
+			ret.operands[1] = Expression.parseOperand(args['then'], vps);
+		}
+		else if (arg === 'else') {
+			ret.operands[2] = Expression.parseOperand(args['else'], vps);
+		}
+		else {
+			throw new Error("Unrecognized parameter to $cond: '" + arg + "'; code 17083");
+		}
+	});
 
+    if (!ret.operands[0]) throw new Error("Missing 'if' parameter to $cond");
+    if (!ret.operands[1]) throw new Error("Missing 'then' parameter to $cond");
+    if (!ret.operands[2]) throw new Error("Missing 'else' parameter to $cond");
 
     return ret;
 };

test/lib/pipeline/expressions/CondExpression_test.js

@@ -39,7 +39,11 @@ module.exports = {
 						Expression.parseOperand({$cond:[1,2]}, {});
 					})
 				},
-
+				"should fail if there are too many arguments": function() {
+					assert.throws(function(){
+						Expression.parseOperand({$cond:[1, 2, 3, 4]}, {});
+					})
+				},
 				"should evaluate boolean expression as true, then return 1; [ true === true, 1, 0 ]": function () {
 					assert.strictEqual(Expression.parseOperand({$cond: [ true, 1, 0 ]}, {}).evaluateInternal({}), 1);
 				},
@@ -77,16 +81,24 @@ module.exports = {
 						Expression.parseOperand({$cond:{ if: $a, then: 1, else: 0}}, {}).evaluate({$a: 1}),
 						1);
 				},
+				"should evaluate true even with mixed up args": function(){
+					assert.strictEqual(
+						Expression.parseOperand({$cond:{ else: 0, then: 1, if: $a }}, {}).evaluate({$a: 1}),
+						1);
+				},
 				"should evaluate false": function(){
 					assert.strictEqual(
 						Expression.parseOperand({$cond:{ if: $a, then: 0, else: 1}}, {}).evaluate({$a: 0}),
 						1);
+				},
+				"should evaluate false even with mixed up args": function() {
+					assert.strictEqual(
+						Expression.parseOperand({$cond: { else: 1, then: 0, if: $a}}, {}).evaluate({$a: 0}),
+						1);
 				}
-
 			}
 		}
 	}
-
 };
 
 if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);