EAGLESIX-2651: Or: better sync w/ 2.6.5 code and tests (code was just reorganizing)

lib/pipeline/expressions/OrExpression.js

@@ -2,69 +2,84 @@
 
 /**
  * An $or pipeline expression.
- * @see evaluateInternal
  * @class OrExpression
+ * @extends mungedb-aggregate.pipeline.expressions.VariadicExpressionT
  * @namespace mungedb-aggregate.pipeline.expressions
  * @module mungedb-aggregate
  * @constructor
- **/
+ */
 var OrExpression = module.exports = function OrExpression(){
 	if (arguments.length !== 0) throw new Error("zero args expected");
 	base.call(this);
 }, klass = OrExpression, base = require("./VariadicExpressionT")(OrExpression), proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
-// DEPENDENCIES
 var Value = require("../Value"),
 	ConstantExpression = require("./ConstantExpression"),
 	CoerceToBoolExpression = require("./CoerceToBoolExpression"),
 	Expression = require("./Expression");
 
-// PROTOTYPE MEMBERS
-klass.opName = "$or";
-proto.getOpName = function getOpName(){
-	return klass.opName;
-};
-
-proto.isAssociativeAndCommutative = function isAssociativeAndCommutative() { return true; }
-
-/**
- * Takes an array of one or more values and returns true if any of the values in the array are true. Otherwise $or returns false.
- * @method evaluateInternal
- **/
 proto.evaluateInternal = function evaluateInternal(vars){
-	for(var i = 0, n = this.operands.length; i < n; ++i){
+	var n = this.operands.length;
+	for (var i = 0; i < n; ++i) {
 		var value = this.operands[i].evaluateInternal(vars);
-		if (Value.coerceToBool(value)) return true;
+		if (Value.coerceToBool(value))
+			return true;
 	}
 	return false;
 };
 
 proto.optimize = function optimize() {
-	var pE = base.prototype.optimize.call(this); // optimize the disjunction as much as possible
+	// optimize the disjunction as much as possible
+	var expr = base.prototype.optimize.call(this);
 
-	if (!(pE instanceof OrExpression)) return pE; // if the result isn't a disjunction, we can't do anything
-	var pOr = pE;
+	// if the result isn't a disjunction, we can't do anything
+	var orExp = expr instanceof OrExpression ? expr : undefined;
+	if (!orExp)
+		return expr;
 
-	// Check the last argument on the result; if it's not const (as promised
-	// by ExpressionNary::optimize(),) then there's nothing we can do.
-	var n = pOr.operands.length;
+	/*
+	 * Check the last argument on the result; if it's not constant (as
+	 * promised by ExpressionNary::optimize(),) then there's nothing
+	 * we can do.
+	 */
+	var n = orExp.operands.length;
 	// ExpressionNary::optimize() generates an ExpressionConstant for {$or:[]}.
-	if (!n) throw new Error("OrExpression must have operands!");
-	var pLast = pOr.operands[n - 1];
-	if (!(pLast instanceof ConstantExpression)) return pE;
+	if (n <= 0) throw new Error("Assertion failuer");
+	var lastExpr = orExp.operands[n - 1],
+		constExpr = lastExpr instanceof ConstantExpression ? lastExpr : undefined;
+	if (!constExpr)
+		return expr;
 
-	// Evaluate and coerce the last argument to a boolean.  If it's true, then we can replace this entire expression.
-	var last = Value.coerceToBool(pLast.evaluateInternal());
-	if (last) return new ConstantExpression(true);
+	/*
+	 * Evaluate and coerce the last argument to a boolean.  If it's true,
+	 * then we can replace this entire expression.
+	 */
+	var last = Value.coerceToBool(constExpr.evaluateInternal());
+	if (last)
+		return ConstantExpression.create(true);
 
-	// If we got here, the final operand was false, so we don't need it anymore.
-	// If there was only one other operand, we don't need the conjunction either.  Note we still need to keep the promise that the result will be a boolean.
-	if (n == 2) return new CoerceToBoolExpression(pOr.operands[0]);
+	/*
+	 * If we got here, the final operand was false, so we don't need it
+	 * anymore.  If there was only one other operand, we don't need the
+	 * conjunction either.  Note we still need to keep the promise that
+	 * the result will be a boolean.
+	 */
+	if (n === 2)
+		return CoerceToBoolExpression.create(orExp.operands[0]);
 
-	// Remove the final "false" value, and return the new expression.
-	pOr.operands.length = n - 1;
-	return pE;
+	/*
+	 * Remove the final "false" value, and return the new expression.
+	 */
+	orExp.operands.length = n - 1;
+	return expr;
 };
 
-/** Register Expression */
-Expression.registerExpression(klass.opName, base.parse);
+Expression.registerExpression("$or", base.parse);
+
+proto.getOpName = function getOpName() {
+	return "$or";
+};
+
+proto.isAssociativeAndCommutative = function isAssociativeAndCommutative() {
+	return true;
+};

test/lib/pipeline/expressions/OrExpression_test.js

@@ -1,184 +1,287 @@
 "use strict";
 var assert = require("assert"),
-	OrExpression = require("../../../../lib/pipeline/expressions/OrExpression"),
 	Expression = require("../../../../lib/pipeline/expressions/Expression"),
+	OrExpression = require("../../../../lib/pipeline/expressions/OrExpression"),
 	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
-	CoerceToBoolExpression = require("../../../../lib/pipeline/expressions/CoerceToBoolExpression"),
-	ConstantExpression = require("../../../../lib/pipeline/expressions/ConstantExpression"),
-	FieldPathExpression = require("../../../../lib/pipeline/expressions/FieldPathExpression"),
-	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator");
+	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
+	utils = require("./utils"),
+	constify = utils.constify,
+	expressionToJson = utils.expressionToJson;
+
+// Mocha one-liner to make these tests self-hosted
+if(!module.parent)return(require.cache[__filename]=null,(new(require("mocha"))({ui:"exports",reporter:"spec",grep:process.env.TEST_GREP})).addFile(__filename).run(process.exit));
+
+var TestBase = function TestBase(overrides) {
+		//NOTE: DEVIATION FROM MONGO: using this base class to make things easier to initialize
+		for (var key in overrides)
+			this[key] = overrides[key];
+	},
+	ExpectedResultBase = (function() {
+		var klass = function ExpectedResultBase() {
+			base.apply(this, arguments);
+		}, base = TestBase, proto = klass.prototype = Object.create(base.prototype);
+		proto.run = function() {
+			var specElement = this.spec instanceof Function ? this.spec() : this.spec,
+				idGenerator = new VariablesIdGenerator(),
+				vps = new VariablesParseState(idGenerator),
+				expr = Expression.parseOperand(specElement, vps);
+			assert.deepEqual(constify(specElement), expressionToJson(expr));
+			var expectedResult = this.expectedResult instanceof Function ? this.expectedResult() : this.expectedResult;
+			assert.strictEqual(expectedResult, expr.evaluate({a:1}));
+			var optimized = expr.optimize();
+			assert.strictEqual(expectedResult, optimized.evaluate({a:1}));
+		};
+		return klass;
+	})(),
+	OptimizeBase = (function() {
+		var klass = function OptimizeBase() {
+			base.apply(this, arguments);
+		}, base = TestBase, proto = klass.prototype = Object.create(base.prototype);
+		proto.run = function() {
+			var specElement = this.spec instanceof Function ? this.spec() : this.spec,
+				idGenerator = new VariablesIdGenerator(),
+				vps = new VariablesParseState(idGenerator),
+				expr = Expression.parseOperand(specElement, vps);
+			assert.deepEqual(constify(specElement), expressionToJson(expr));
+			var optimized = expr.optimize(),
+				expectedOptimized = this.expectedOptimized instanceof Function ? this.expectedOptimized() : this.expectedOptimized;
+			assert.deepEqual(expectedOptimized, expressionToJson(optimized));
+		};
+		return klass;
+	})(),
+	NoOptimizeBase = (function() {
+		var klass = function NoOptimizeBase() {
+			base.apply(this, arguments);
+		}, base = OptimizeBase, proto = klass.prototype = Object.create(base.prototype);
+		proto.expectedOptimized = function() {
+			return constify(this.spec instanceof Function ? this.spec() : this.spec);
+		};
+		return klass;
+	})();
+
+exports.OrExpression = {
+
+	"constructor()": {
+
+		"should construct instance": function() {
+			assert(new OrExpression() instanceof OrExpression);
+			assert(new OrExpression() instanceof Expression);
+		},
 
+		"should error if given args": function() {
+			assert.throws(function() {
+				new OrExpression("bad stuff");
+			});
+		},
+
+	},
 
-module.exports = {
+	"#getOpName()": {
 
-	"OrExpression": {
+		"should return the correct op name; $or": function(){
+			assert.equal(new OrExpression().getOpName(), "$or");
+		}
 
-		beforeEach: function() {
-			this.vps = new VariablesParseState(new VariablesIdGenerator());
+	},
+
+	"#evaluate()": {
+
+		"should return false if no operands": function testNoOperands(){
+			/** $or without operands. */
+			new ExpectedResultBase({
+				spec: {$or:[]},
+				expectedResult: false,
+			}).run();
 		},
 
-		"constructor()": {
+		"should return true if given true": function testTrue(){
+			/** $or passed 'true'. */
+			new ExpectedResultBase({
+				spec: {$or:[true]},
+				expectedResult: true,
+			}).run();
+		},
 
-			"should not throw Error when constructing without args": function testConstructor(){
-				assert.doesNotThrow(function(){
-					new OrExpression();
-				});
-			},
+		"should return false if given false": function testFalse(){
+			/** $or passed 'false'. */
+			new ExpectedResultBase({
+				spec: {$or:[false]},
+				expectedResult: false,
+			}).run();
+		},
 
-			"should throw Error when constructing with args": function testConstructor(){
-				assert.throws(function(){
-					new OrExpression(1);
-				});
-			}
+		"should return true if given true and true": function testTrueTrue(){
+			/** $or passed 'true', 'true'. */
+			new ExpectedResultBase({
+				spec: {$or:[true, true]},
+				expectedResult: true,
+			}).run();
+		},
 
+		"should return true if given true and false": function testTrueFalse(){
+			/** $or passed 'true', 'false'. */
+			new ExpectedResultBase({
+				spec: {$or:[true, false]},
+				expectedResult: true,
+			}).run();
 		},
 
-		"#getOpName()": {
+		"should return true if given false and true": function testFalseTrue(){
+			/** $or passed 'false', 'true'. */
+			new ExpectedResultBase({
+				spec: {$or:[false, true]},
+				expectedResult: true,
+			}).run();
+		},
 
-			"should return the correct op name; $or": function testOpName(){
-				assert.equal(new OrExpression().getOpName(), "$or");
-			}
+		"should return false if given false and false": function testFalseFalse(){
+			/** $or passed 'false', 'false'. */
+			new ExpectedResultBase({
+				spec: {$or:[false, false]},
+				expectedResult: false,
+			}).run();
+		},
 
+		"should return false if given false and false and false": function testFalseFalseFalse(){
+			/** $or passed 'false', 'false', 'false'. */
+			new ExpectedResultBase({
+				spec: {$or:[false, false, false]},
+				expectedResult: false,
+			}).run();
 		},
 
-		"#evaluateInternalInternal()": {
+		"should return true if given false and false and true": function testFalseFalseTrue(){
+			/** $or passed 'false', 'false', 'true'. */
+			new ExpectedResultBase({
+				spec: {$or:[false, false, true]},
+				expectedResult: true,
+			}).run();
+		},
 
-			"should return false if no operands were given; {$or:[]}": function testEmpty(){
-				assert.equal(Expression.parseOperand({$or:[]}, this.vps).evaluate(), false);
-			},
+		"should return true if given 0 and 1": function testZeroOne(){
+			/** $or passed '0', '1'. */
+			new ExpectedResultBase({
+				spec: {$or:[0, 1]},
+				expectedResult: true,
+			}).run();
+		},
 
-			"should return true if operands is one true; {$or:[true]}": function testTrue(){
-				assert.equal(Expression.parseOperand({$or:[true]}, this.vps).evaluate(), true);
-			},
+		"should return false if given 0 and false": function testZeroFalse(){
+			/** $or passed '0', 'false'. */
+			new ExpectedResultBase({
+				spec: {$or:[0, false]},
+				expectedResult: false,
+			}).run();
+		},
 
-			"should return false if operands is one false; {$or:[false]}": function testFalse(){
-				assert.equal(Expression.parseOperand({$or:[false]}, this.vps).evaluate(), false);
-			},
+		"should return true if given a field path to a truthy value": function testFieldPath(){
+			/** $or passed a field path. */
+			new ExpectedResultBase({
+				spec: {$or:["$a"]},
+				expectedResult: true,
+			}).run();
+		},
 
-			"should return true if operands are true or true; {$or:[true,true]}": function testTrueTrue(){
-				assert.equal(Expression.parseOperand({$or:[true,true]}, this.vps).evaluate(), true);
-			},
+	},
 
-			"should return true if operands are true or false; {$or:[true,false]}": function testTrueFalse(){
-				assert.equal(Expression.parseOperand({$or:[true,false]}, this.vps).evaluate(), true);
-			},
-
-			"should return true if operands are false or true; {$or:[false,true]}": function testFalseTrue(){
-				assert.equal(Expression.parseOperand({$or:[false,true]}, this.vps).evaluate(), true);
-			},
-
-			"should return false if operands are false or false; {$or:[false,false]}": function testFalseFalse(){
-				assert.equal(Expression.parseOperand({$or:[false,false]}, this.vps).evaluate(), false);
-			},
-
-			"should return false if operands are false, false, or false; {$or:[false,false,false]}": function testFalseFalseFalse(){
-				assert.equal(Expression.parseOperand({$or:[false,false,false]}, this.vps).evaluate(), false);
-			},
-
-			"should return false if operands are false, false, or false; {$or:[false,false,true]}": function testFalseFalseTrue(){
-				assert.equal(Expression.parseOperand({$or:[false,false,true]}, this.vps).evaluate(), true);
-			},
-
-			"should return true if operands are 0 or 1; {$or:[0,1]}": function testZeroOne(){
-				assert.equal(Expression.parseOperand({$or:[0,1]}, this.vps).evaluate(), true);
-			},
-
-			"should return false if operands are 0 or false; {$or:[0,false]}": function testZeroFalse(){
-				assert.equal(Expression.parseOperand({$or:[0,false]}, this.vps).evaluate(), false);
-			},
-
-			"should return true if operand is a path String to a truthy value; {$or:['$a']}": function testFieldPath(){
-				assert.equal(Expression.parseOperand({$or:['$a']}, this.vps).evaluate({a:1}), true);
-			}
-		},
-
-		"#optimize()": {
-
-			"should optimize a constant expression to a constant; {$or:[1]} == true": function testOptimizeConstantExpression(){
-				var a = Expression.parseOperand({$or:[1]}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			},
-
-			"should not optimize a non-constant expression; {$or:['$a']}; SERVER-6192": function testNonConstant(){
-				var a = Expression.parseOperand({$or:['$a']}, this.vps).optimize();
-				assert(a instanceof OrExpression);
-				assert.equal(a.operands.length, 1);
-			},
-
-			"should optimize an expression with a path or a '1' (is entirely constant); {$or:['$a',1]}": function testNonConstantOne(){
-				var a = Expression.parseOperand({$or:['$a',1]}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			},
-
-			"should optimize an expression with a field path or a '0'; {$or:['$a',0]}": function testNonConstantZero(){
-				var a = Expression.parseOperand({$or:['$a',0]}, this.vps).optimize();
-				assert(a instanceof CoerceToBoolExpression);
-				assert.equal(a.expression._fieldPath.fieldNames[1], "a");
-			},
-
-			"should optimize an expression with two field paths or '1' (is entirely constant); {$or:['$a','$b',1]}": function testNonConstantNonConstantOne(){
-				var a = Expression.parseOperand({$or:['$a','$b',1]}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			},
-
-			"should optimize an expression with two field paths or '0'; {$or:['$a','$b',0]}": function testNonConstantNonConstantZero(){
-				var a = Expression.parseOperand({$or:['$a','$b',0]}, this.vps).optimize();
-				assert(a instanceof OrExpression);
-				assert.equal(a.operands.length, 2);
-			},
-
-			"should optimize an expression with '0', '1', or a field path; {$or:[0,1,'$a']}": function testZeroOneNonConstant(){
-				var a = Expression.parseOperand({$or:[0,1,'$a']}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			},
-
-			"should optimize an expression with '0', '0', or a field path; {$or:[0,0,'$a']}": function testZeroZeroNonConstant(){
-				var a = Expression.parseOperand({$or:[0,0,'$a']}, this.vps).optimize();
-				assert(a instanceof CoerceToBoolExpression);
-				assert.equal(a.expression._fieldPath.fieldNames[1], "a");
-			},
-
-			"should optimize nested $or expressions properly or optimize out values evaluating to false; {$or:[0,{$or:[0]},'$a','$b']}": function testNested(){
-				var a = Expression.parseOperand({$or:[0,{$or:[0]},'$a','$b']}, this.vps).optimize();
-				assert(a instanceof OrExpression);
-				assert.equal(a.operands.length, 2);
-			},
-
-			"should optimize nested $or expressions containing a nested value evaluating to false; {$or:[0,{$or:[{$or:[1]}]},'$a','$b']}": function testNestedOne(){
-				var a = Expression.parseOperand({$or:[0,{$or:[{$or:[1]}]},'$a','$b']}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			},
-			// Tests below this line are extras I added.  Just 'cause.
-			"should handle a string of trues": function(){
-				var a = Expression.parseOperand({$or:[1,"x",1,1,true]}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			},
-			"should handle a string of falses": function(){
-				var a = Expression.parseOperand({$or:[0, false, 0, false, 0]}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), false);
-			},
-			"should handle true + false": function(){
-				var a = Expression.parseOperand({$or:[1, 0]}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			},
-			"should handle false + true": function(){
-				var a = Expression.parseOperand({$or:[0, 1]}, this.vps).optimize();
-				assert(a instanceof ConstantExpression);
-				assert.equal(a.evaluateInternal(), true);
-			}
+	"#optimize()": {
 
-		}
+		"should optimize a constant expression": function testOptimizeConstantExpression() {
+			/** A constant expression is optimized to a constant. */
+			new OptimizeBase({
+				spec: {$or:[1]},
+				expectedOptimized: {$const:true},
+			}).run();
+		},
 
-	}
+		"should not optimize a non constant": function testNonConstant() {
+			/** A non constant expression is not optimized. */
+			new NoOptimizeBase({
+				spec: {$or:["$a"]},
+			}).run();
+		},
 
-};
+		"should optimize truthy constant and truthy expression": function testConstantNonConstantTrue() {
+			/** An expression beginning with a single constant is optimized. */
+			new OptimizeBase({
+				spec: {$or:[1,"$a"]},
+				expectedOptimized: {$const:true},
+			}).run();
+		},
+
+		"should optimize falsy constant and truthy expression": function testConstantNonConstantFalse() {
+			/** An expression beginning with a single constant is optimized. */
+			new OptimizeBase({
+				spec: {$or:[0,"$a"]},
+				expectedOptimized: {$and:["$a"]},
+			}).run();
+			// note: using $and as serialization of ExpressionCoerceToBool rather than ExpressionAnd
+		},
+
+		"should optimize truthy expression and truthy constant": function testNonConstantOne() {
+			/** An expression with a field path and '1'. */
+			new OptimizeBase({
+				spec: {$or:["$a", 1]},
+				expectedOptimized: {$const:true},
+			}).run();
+		},
+
+		"should optimize truthy expression and falsy constant": function testNonConstantZero() {
+			/** An expression with a field path and '0'. */
+			new OptimizeBase({
+				spec: {$or:["$a", 0]},
+				expectedOptimized: {$and:["$a"]},
+			}).run();
+		},
+
+		"should optimize truthy expression, falsy expression, and truthy constant": function testNonConstantNonConstantOne() {
+			/** An expression with two field paths and '1'. */
+			new OptimizeBase({
+				spec: {$or:["$a","$b",1]},
+				expectedOptimized: {$const:true},
+			}).run();
+		},
+
+		"should optimize truthy expression, falsy expression, and falsy constant": function testNonConstantNonConstantZero() {
+			/** An expression with two field paths and '0'. */
+			new OptimizeBase({
+				spec: {$or:["$a","$b",0]},
+				expectedOptimized: {$or:["$a", "$b"]},
+			}).run();
+		},
 
-if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);
+		"should optimize to true if [0,1,'$a']": function testZeroOneNonConstant() {
+			/** An expression with '0', '1', and a field path. */
+			new OptimizeBase({
+				spec: {$or:[0,1,"$a"]},
+				expectedOptimized: {$const:true},
+			}).run();
+		},
+
+		"should optimize to {$and:'$a'} if [0,0,'$a']": function testZeroZeroNonConstant() {
+			/** An expression with '0', '0', and a field path. */
+			new OptimizeBase({
+				spec: {$or:[0,0,"$a"]},
+				expectedOptimized: {$and:["$a"]},
+			}).run();
+		},
+
+		"should optimize away nested falsey $or expressions": function testNested() {
+			/** Nested $or expressions. */
+			new OptimizeBase({
+				spec: {$or:[0, {$or:[0]}, "$a", "$b"]},
+				expectedOptimized: {$or: ["$a", "$b"]},
+			}).run();
+		},
+
+		"should optimize to tru if nested truthy $or expressions": function testNestedOne() {
+			/** Nested $or expressions containing a nested value evaluating to false. */
+			new OptimizeBase({
+				spec: {$or:[0, {$or:[ {$or:[1]} ]}, "$a", "$b"]},
+				expectedOptimized: {$const:true},
+			}).run();
+		},
+
+	},
+
+};