EAGLESIX-2699 working through various test cases

lib/pipeline/expressions/CondExpression.js

@@ -8,13 +8,10 @@
  * @constructor
  **/
 var CondExpression = module.exports = function CondExpression(vars) {
-    this.pCond = this.evaluateInternal(vars);
-    this.idx = this.pCond.coerceToBool() ? 1 : 2;
-    if (arguments.length !== 3) throw new Error("three args expected");
+		if (arguments.length !== 0) throw new Error("zero args expected");
     base.call(this);
 }, klass = CondExpression,
-	FixedArityExpression = require("./FixedArityExpressionT")(klass, 3),
-	base = FixedArityExpression,
+	base = require("./FixedArityExpressionT")(klass, 3),
 	proto = klass.prototype = Object.create(base.prototype, {
 		constructor: {
 			value: klass
@@ -23,41 +20,80 @@ var CondExpression = module.exports = function CondExpression(vars) {
 
 // DEPENDENCIES
 var Value = require("../Value"),
-    Expression = require("./Expression");
+    Expression = require("./Expression"),
+	FixedArityExpressionT = require("./FixedArityExpressionT");
 
 // PROTOTYPE MEMBERS
+klass.opName = "$cond";
 proto.getOpName = function getOpName() {
-    return "$cond";
+    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) {
-    this.checkArgLimit(3);
+	// 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;
-    if (typeof(expr) !== Object)
-		return Expression.parse(expr, vps);
+	// todo I don't understand why we'd do this.  shouldn't expr be {}, [], or wrong?
+    if (typeof(expr) !== Object || )
+		return FixedArityExpressionT.parse(expr, vps);
 
-    // verify
-    if (Expression.parseOperand(expr) !== "$cond")
-		throw new Error("Invalid expression");
+	// ...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+"'");
+	}
 
     var ret = new CondExpression();
 
-    var ex = Expression.parseObject(expr);
-    var args = Expression.parseOperand(expr, vps);
-    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");
+	// 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[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; code 17080");
+    if (!ret.operands[1]) throw new Error("Missing 'then' parameter to $cond; code 17081");
+    if (!ret.operands[2]) throw new Error("Missing 'else' parameter to $cond; code 17082");
 
     return ret;
 };
 
 /**
- * Use the $cond operator with the following syntax:  { $cond: [ <boolean-expression>, <true-case>, <false-case> ] }
+ * Use the $cond operator with the following syntax:
+ * { $cond: { if: <boolean-expression>, then: <true-case>, else: <false-case-> } }
+ * -or-
+ * { $cond: [ <boolean-expression>, <true-case>, <false-case> ] }
  * @method evaluate
  **/
 proto.evaluateInternal = function evaluateInternal(vars) {
@@ -74,4 +110,4 @@ proto.evaluateInternal = function evaluateInternal(vars) {
 };
 
 /** Register Expression */
-Expression.registerExpression("$cond", klass.parse);
+Expression.registerExpression(klass.opName, klass.parse);

lib/pipeline/expressions/NotExpression.js

@@ -9,10 +9,10 @@
  * @constructor
  **/
 var NotExpression = module.exports = function NotExpression() {
+		if (arguments.length !== 0) throw new Error("zero args expected");
 	base.call(this);
 }, klass = NotExpression,
-	FixedArityExpression = require("./FixedArityExpressionT")(klass, 1),
-	base = FixedArityExpression,
+	base = require("./FixedArityExpressionT")(klass, 1),
 	proto = klass.prototype = Object.create(base.prototype, {
 		constructor: {
 			value: klass
@@ -24,8 +24,9 @@ var Value = require("../Value"),
 	Expression = require("./Expression");
 
 // PROTOTYPE MEMBERS
+klass.opName = "$not";
 proto.getOpName = function getOpName() {
-	return "$not";
+	return klass.opName;
 };
 
 /**
@@ -38,4 +39,4 @@ proto.evaluateInternal = function evaluateInternal(vars) {
 };
 
 /** Register Expression */
-Expression.registerExpression("$not", base.parse);
+Expression.registerExpression(klass.opName, base.parse);

test/lib/pipeline/expressions/CondExpression_test.js

@@ -0,0 +1,128 @@
+"use strict";
+var assert = require("assert"),
+	CondExpression = require("../../../../lib/pipeline/expressions/CondExpression"),
+	Expression = require("../../../../lib/pipeline/expressions/Expression");
+
+
+module.exports = {
+
+	"CondExpression": {
+
+		"constructor()": {
+
+			"should not throw an Error when constructing without args": function testConstructor(){
+				assert.doesNotThrow(function(){
+					new CondExpression();
+				});
+			},
+
+			"should throw Error when constructing with 1 arg": function testConstructor1(){
+				assert.throws(function(){
+					new CondExpression(1);
+				});
+			}
+		},
+
+		"#getOpName()": {
+
+			"should return the correct op name; $cond": function testOpName(){
+				assert.equal(new CondExpression().getOpName(), "$cond");
+			}
+
+		},
+
+		"#evaluateInternal()": {
+			"array style": {
+
+				"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);
+				},
+
+				"should evaluate boolean expression as false, then return 0; [ false === true, 1, 0 ]": function () {
+					assert.strictEqual(Expression.parseOperand({$cond: [ false, 1, 0 ]}, {}).evaluateInternal({}), 0);
+				},
+				"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]}, {});
+					})
+				}
+			},
+
+			"object style": {
+				beforeEach: function(){
+					this.shouldFail = function(expr) {
+						assert.throws(function(){
+							Expression.parseOperand(expr, {});
+						});
+					}
+				},
+				"should fail because the $cond is missing": function(){
+					this.shouldFail({$zoot:[true, 1, 0 ]}, {});
+				},
+				"should fail because of missing if": function(){
+					this.shouldFail({$cond:{xif:1, then:2, else:3}});
+				},
+				"should fail because of missing then": function(){
+					this.shouldFail({$cond:{if:1, xthen:2, else:3}});
+				},
+				"should fail because of missing else": function(){
+					this.shouldFail({$cond:{if:1, then:2, xelse:3}});
+				},
+				"should fail because of empty if": function(){
+					this.shouldFail({$cond:{if:undefined, then:2, else:3}});
+				},
+				"should fail because of empty then": function(){
+					this.shouldFail({$cond:{if:1, then:undefined, else:3}});
+				},
+				"should fail because of empty else": function(){
+					this.shouldFail({$cond:{if:1, then:2, else:undefined}});
+				},
+				"should fail because of mystery args": function(){
+					this.shouldFail({$cond:{if:1, then:2, else:3, zoot:4}});
+				},
+				"should evaluate true": function(){
+					assert.strictEqual(
+						Expression.parseOperand({$cond:{ if: true, then: 1, else: 0}}, {}).evaluate({}),
+						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);

test/lib/pipeline/expressions/FieldPathExpression.js

@@ -44,7 +44,7 @@ module.exports = {
 				results = fieldPath.evaluateInternal(vars);
 				assert.strictEqual(results, undefined);
 			},
-
+// belowMissing
 			"should return undefined if field path is nested below Number": function testNestedBelowInt(){
 			    var vars = new Variables(1,{a:2}),
 				fieldPath = FieldPathExpression.create('a.b'),

test/lib/pipeline/expressions/IfNullExpression_test.js

@@ -0,0 +1,64 @@
+"use strict";
+var assert = require("assert"),
+	IfNullExpression = require("../../../../lib/pipeline/expressions/IfNullExpression"),
+	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
+	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
+	Variables = require("../../../../lib/pipeline/expressions/Variables"),
+	Expression = require("../../../../lib/pipeline/expressions/Expression");
+
+
+module.exports = {
+
+	"IfNullExpression": {
+
+		"constructor()": {
+
+			"should not throw Error when constructing without args": function() {
+				assert.doesNotThrow(function () {
+					new IfNullExpression();
+				});
+			},
+			"should throw Error when constructing with args": function () {
+				assert.throws(function () {
+					new IfNullExpression(1);
+				});
+			}
+		},
+
+		"#getOpName()": {
+
+			"should return the correct op name; $ifNull": function() {
+				assert.equal(new IfNullExpression().getOpName(), "$ifNull");
+			}
+
+		},
+
+		"#evaluateInternal()": {
+			beforeEach: function () {
+				this.vps = new VariablesParseState(new VariablesIdGenerator());
+				this.parsed = Expression.parseExpression("$ifNull", ["$a", "$b"], this.vps);
+				this.vars = new Variables(2);
+				this.vars.setValue(0, "a");
+				this.vars.setValue(1, "b");
+				this.makeParsed = function(a, b) {
+					return Expression.parseExpression("$ifNull", [a, b], this.vps);
+				}
+			},
+
+			"should return the left hand side if the left hand side is not null or undefined": function() {
+				//assert.strictEqual(this.parsed.evaluate(this.vars), 1);
+				assert.strictEqual(this.makeParsed(1, 2).evaluate(this.vars), 1);
+			},
+			"should return the right hand side if the left hand side is null": function() {
+				//assert.strictEqual(this.parsed.evaluate({a: null, b: 2}), 2);
+				assert.strictEqual(this.makeParsed(null, 2).evaluate(this.vars), 2);
+			},
+			"should return the right hand side if the left hand side is undefined": function() {
+				//assert.strictEqual(this.parsed.evaluate({b: 2}), 2);
+				assert.strictEqual(this.makeParsed(undefined, 2).evaluate(this.vars), 2);
+			}
+		}
+	}
+};
+
+if (!module.parent)(new (require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);

test/lib/pipeline/expressions/NotExpression.js

@@ -26,14 +26,6 @@ module.exports = {
 
 		},
 
-		"#getFactory()": {
-
-			"should return the constructor for this class": function factoryIsConstructor(){
-				assert.strictEqual(new NotExpression().getFactory(), undefined);
-			}
-
-		},
-
 		"#evaluateInternal()": {
 
 			"should return false for a true input; false for true": function testStuff(){