EAGLESIX-2696 Improved CondExpression and the test cases.

lib/pipeline/expressions/CondExpression.js

@@ -30,11 +30,14 @@ proto.getOpName = function getOpName() {
 };
 
 /**
- * A utility class. Not in the c++ code.
- * @param v				a value that should be tested for being null-ish.
+ * A utility class. Not in the c++ code.  If an operand has a .value, or if it has a ._fieldPath, it is not nullish.
+ * I don't have 100% confidence these are the only choices.
+ * @param op				An operand that should be tested for being null-ish.
  * @returns {boolean}
  */
-klass.isNullish = function(op) {return (op.value == null || op.value == undefined) && !op._fieldPath};
+klass.isNullish = function isNullish(op) {
+	return (op.value === null || op.value === undefined) && !op._fieldPath;
+};
 
 /**
  *
@@ -51,46 +54,40 @@ klass.parse = function parse(expr, vps) {
     if (typeof(expr) !== 'object')
 		return FixedArityExpressionT.parse(expr, vps);
 
-//NOTE: Deviation from Mongo: The c++ code has a verify( $cond ) structure. The implementation below will never work.
-//	// ...or expr could be the entirety of $cond:{...} or $cond:[,,,].
+//NOTE: Deviation from Mongo: The c++ code has a verify( $cond ) structure. The implementation below will never work as
+// $cond has been removed before we get here.
 //	if(!(klass.opName in expr)) {
 //		throw new Error("Invalid expression. Expected to see '"+klass.opName+"'");
 //	}
 
     var ret = new CondExpression();
 
-	// 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.
-	// (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[klass.opName];
 	var args = expr;
 
-	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]};
+		for(var i=0; i<3; i++) ret.operands[i] = Expression.parseOperand(args[i], vps);
+	} else {
+		// This is the object form. Find the keys regardless of their order.
+		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");
+			}
+		});
 	}
 
-	// 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");
-		}
-	});
-
+	// The Operands array should have been loaded.  Make sure they are all reasonable.
+	// TODO I think isNullish is brittle.
     if (klass.isNullish(ret.operands[0])) throw new Error("Missing 'if' parameter to $cond; code 17080");
     if (klass.isNullish(ret.operands[1])) throw new Error("Missing 'then' parameter to $cond; code 17081");
     if (klass.isNullish(ret.operands[2])) throw new Error("Missing 'else' parameter to $cond; code 17082");

test/lib/pipeline/expressions/CondExpression_test.js

@@ -39,12 +39,12 @@ module.exports = {
 				"should fail if there aren't enough arguments": function() {
 					assert.throws(function(){
 						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);
@@ -56,17 +56,17 @@ module.exports = {
 				"should fail when the 'if' position is empty": function(){
 					assert.throws(function(){
 						Expression.parseOperand({$cond:[undefined, 2, 3]}, {});
-					})
+					});
 				},
 				"should fail when the 'then' position is empty": function(){
 					assert.throws(function(){
 						Expression.parseOperand({$cond:[1, undefined, 3]}, {});
-					})
+					});
 				},
 				"should fail when the 'else' position is empty": function(){
 					assert.throws(function(){
 						Expression.parseOperand({$cond:[1, 2, undefined]}, {});
-					})
+					});
 				}
 			},