Browse Source

EAGLESIX-2651: Nary: reorganize code, minor bugs, test utils

* split NaryExpressionT into NaryExpression and NaryExpressionBaseT to more closely match the original source code
* split expr test utils into their own file
Kyle P Davis 11 years ago
parent
commit
e31f8315b1

+ 132 - 141
lib/pipeline/expressions/NaryExpression.js

@@ -1,5 +1,9 @@
 "use strict";
 
+var Expression = require("./Expression"),
+	Variables = require("./Variables"),
+	ConstantExpression = require("./ConstantExpression");
+
 /**
  * The base class for all n-ary `Expression`s
  * @class NaryExpression
@@ -8,155 +12,142 @@
  * @extends mungedb-aggregate.pipeline.expressions.Expression
  * @constructor
  **/
-var Expression = require("./Expression"),
-	Variables = require("./Variables");
-
-var NaryExpressionT = module.exports = function NaryExpressionT(SubClass) {
-
-	var NaryExpression = function NaryExpression(){
-		if (arguments.length !== 0) throw new Error("Zero args expected");
-		this.operands = [];
-		base.call(this);
-	}, klass = NaryExpression, base = Expression, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
-
-	klass.parseArguments = function(exprElement, vps) {
-		var out = [];
-		if(exprElement instanceof Array) {
-			for(var ii = 0; ii < exprElement.length; ii++) {
-				out.push(Expression.parseOperand(exprElement[ii], vps));
-			}
-		} else {
-			out.push(Expression.parseOperand(exprElement, vps));
+var NaryExpression = module.exports = function NaryExpression() {
+	if (arguments.length !== 0) throw new Error("Zero args expected");
+	this.operands = [];
+	base.call(this);
+}, klass = NaryExpression, base = Expression, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+
+proto.optimize = function optimize() {
+	var n = this.operands.length;
+
+	// optimize sub-expressions and count constants
+	var constCount = 0;
+	for (var i = 0; i < n; ++i) {
+		var optimized = this.operands[i].optimize();
+
+		// substitute the optimized expression
+		this.operands[i] = optimized;
+
+		// check to see if the result was a constant
+		if (optimized instanceof ConstantExpression) {
+			constCount++;
 		}
-		return out;
-	};
-
-	klass.parse = function(expr, vps) {
-		var outExpr = new SubClass(),
-			args = NaryExpression.parseArguments(expr, vps);
-		outExpr.validateArguments(args);
-		outExpr.operands = args;
-		return outExpr;
-	};
-
-	proto.optimize = function optimize(){
-		var n = this.operands.length,
-			constCount = 0;
-
-		// optimize sub-expressions and count constants
-		for(var ii = 0; ii < n; ii++) {
-			var optimized = this.operands[ii].optimize();
-
-			// substitute the optimized expression
-			this.operands[ii] = optimized;
-
-			// check to see if the result was a constant
-			if(optimized instanceof ConstantExpression) {
-				constCount++;
-			}
-		}
-
-		// If all the operands are constant, we can replace this expression with a constant. Using
-		// an empty Variables since it will never be accessed.
-		if(constCount === n) {
-			var emptyVars = new Variables(),
-				result = this.evaluateInternal(emptyVars),
-				replacement = new ConstantExpression(result);
-			return replacement;
-		}
-
-		// Remaining optimizations are only for associative and commutative expressions.
-		if(!this.isAssociativeAndCommutative()) {
-			return this;
-		}
-
-		// Process vpOperand to split it into constant and nonconstant vectors.
-		// This can leave vpOperand in an invalid state that is cleaned up after the loop.
-		var constExprs = [],
-			nonConstExprs = [];
-		for(var i=0; i<this.operands.length; i++) { // NOTE: vpOperand grows in loop
-			var expr = this.operands[i];
-			if(expr instanceof ConstantExpression) {
-				constExprs.push(expr);
+	}
+
+	// If all the operands are constant, we can replace this expression with a constant. Using
+	// an empty Variables since it will never be accessed.
+	if (constCount === n) {
+		var emptyVars = new Variables(),
+			result = this.evaluateInternal(emptyVars),
+			replacement = ConstantExpression.create(result);
+		return replacement;
+	}
+
+	// Remaining optimizations are only for associative and commutative expressions.
+	if(!this.isAssociativeAndCommutative()) {
+		return this;
+	}
+
+	// Process vpOperand to split it into constant and nonconstant vectors.
+	// This can leave vpOperand in an invalid state that is cleaned up after the loop.
+	var constExprs = [],
+		nonConstExprs = [];
+	for (i = 0; i < this.operands.length; ++i) { // NOTE: vpOperand grows in loop
+		var expr = this.operands[i];
+		if (expr instanceof ConstantExpression) {
+			constExprs.push(expr);
+		} else {
+			// If the child operand is the same type as this, then we can
+			// extract its operands and inline them here because we know
+			// this is commutative and associative.  We detect sameness of
+			// the child operator by checking for equality of the opNames
+			var nary = expr instanceof NaryExpression ? expr : undefined;
+			if (!nary || nary.getOpName() !== this.getOpName) {
+				nonConstExprs.push(expr);
 			} else {
-				// If the child operand is the same type as this, then we can
-				// extract its operands and inline them here because we know
-				// this is commutative and associative.  We detect sameness of
-				// the child operator by checking for equality of the opNames
-				var nary = expr;
-				if(!(nary instanceof NaryExpression) || nary.getOpName() !== this.getOpName) {
-					nonConstExprs.push(expr);
-				} else {
-					// same expression, so flatten by adding to vpOperand which
-					// will be processed later in this loop.
-					for(var j=0; j<nary.operands.length; j++) {
-						this.operands.push(nary.operands[j]);
-					}
-				}
+				// same expression, so flatten by adding to vpOperand which
+				// will be processed later in this loop.
+				Array.prototype.push.apply(this.operands, nary.operands);
 			}
 		}
+	}
+
+	// collapse all constant expressions (if any)
+	var constValue;
+	if (constExprs.length > 0) {
+		this.operands = constExprs;
+		var emptyVars2 = new Variables();
+		constValue = this.evaluateInternal(emptyVars2);
+	}
+
+	// now set the final expression list with constant (if any) at the end
+	this.operands = nonConstExprs;
+	if (constExprs.length > 0) {
+		this.operands.push(ConstantExpression.create(constValue));
+	}
+
+	return this;
+};
 
-		// collapse all constant expressions (if any)
-		var constValue;
-		if(constExprs.length > 0) {
-			this.operands = constExprs;
-			var emptyVars = new Variables();
-			constValue = this.evaluateInternal(emptyVars);
-		}
-
-		// now set the final expression list with constant (if any) at the end
-		this.operands = nonConstExprs;
-		if (constExprs.length > 0) {
-			this.operands.push(new ConstantExpression(constValue));
-		}
+proto.addDependencies = function addDependencies(deps, path) {
+	for (var i = 0, l = this.operands.length; i < l; ++i) {
+		this.operands[i].addDependencies(deps);
+	}
+};
 
-		return this;
-	};
-
-	// DEPENDENCIES
-	var ConstantExpression = require("./ConstantExpression");
-
-	proto.getOpName = function getOpName(doc){
-		throw new Error("NOT IMPLEMENTED BY INHERITOR");
-	};
-
-	proto.addDependencies = function addDependencies(deps, path){
-		for(var i = 0, l = this.operands.length; i < l; ++i)
-			this.operands[i].addDependencies(deps);
-	};
-
-	/**
-	 * Add an operand to the n-ary expression.
-	 * @method addOperand
-	 * @param pExpression the expression to add
-	 **/
-	proto.addOperand = function addOperand(expr) {
-		this.operands.push(expr);
-	};
-
-	proto.isAssociativeAndCommutative = function isAssociativeAndCommutative() {
-		return false;
-	};
-
-	proto.serialize = function serialize(explain) {
-		var nOperand = this.operands.length,
-			array = [];
-
-		for(var i=0; i<nOperand; i++){
-			array.push(this.operands[i].serialize(explain));
-		}
+/**
+ * Add an operand to the n-ary expression.
+ * @method addOperand
+ * @param expr the expression to add
+ */
+proto.addOperand = function addOperand(expr) {
+	this.operands.push(expr);
+};
 
-		var obj = {};
-		obj[this.getOpName()] = array;
-		return obj;
-	};
+proto.serialize = function serialize(explain) {
+	var nOperand = this.operands.length,
+		array = [];
+	// build up the array
+	for (var i = 0; i < nOperand; i++) {
+		array.push(this.operands[i].serialize(explain));
+	}
+
+	var obj = {};
+	obj[this.getOpName()] = array;
+	return obj;
+};
 
-	proto.validateArguments = function(args) {
-		if(this.nargs !== args.length) {
-			throw new Error("Expression " + this.getOpName() + " takes exactly " + this.nargs + " arguments. " + args.length + " were passed in.");
-		}
-	};
+proto.isAssociativeAndCommutative = function isAssociativeAndCommutative() {
+	return false;
+};
 
-	return NaryExpression;
+/**
+ * Get the name of the operator.
+ * @method getOpName
+ * @returns the name of the operator; this string belongs to the class
+ *  implementation, and should not be deleted
+ *  and should not
+ */
+proto.getOpName = function getOpName() {
+	throw new Error("NOT IMPLEMENTED BY INHERITOR");
 };
 
+/**
+ * Allow subclasses the opportunity to validate arguments at parse time.
+ * @method validateArguments
+ * @param {[type]} args [description]
+ */
+proto.validateArguments = function(args) {};
+
+klass.parseArguments = function(exprElement, vps) {
+	var out = [];
+	if (exprElement instanceof Array) {
+		for (var ii = 0; ii < exprElement.length; ii++) {
+			out.push(Expression.parseOperand(exprElement[ii], vps));
+		}
+	} else {
+		out.push(Expression.parseOperand(exprElement, vps));
+	}
+	return out;
+};

+ 29 - 0
lib/pipeline/expressions/NaryExpressionBaseT.js

@@ -0,0 +1,29 @@
+"use strict";
+
+var NaryExpression = require("./NaryExpression");
+
+/**
+* Inherit from ExpressionVariadic or ExpressionFixedArity instead of directly from this class.
+* @class NaryExpressionBaseT
+* @namespace mungedb-aggregate.pipeline.expressions
+* @module mungedb-aggregate
+* @extends mungedb-aggregate.pipeline.expressions.Expression
+* @constructor
+**/
+var NaryExpressionBaseT = module.exports = function NaryExpressionBaseT(SubClass) {
+
+	var NaryExpressionBase = function NaryExpressionBase() {
+		if (arguments.length !== 0) throw new Error("Zero args expected");
+		base.call(this);
+	}, klass = NaryExpressionBase, base = NaryExpression, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+
+	klass.parse = function(objExpr, vps) {
+		var expr = new SubClass(),
+			args = NaryExpression.parseArguments(objExpr, vps);
+		expr.validateArguments(args);
+		expr.operands = args;
+		return expr;
+	};
+
+	return NaryExpressionBase;
+};

+ 176 - 269
test/lib/pipeline/expressions/NaryExpression_test.js

@@ -1,59 +1,26 @@
 "use strict";
+
 var assert = require("assert"),
 	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
 	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
-	NaryExpressionT = require("../../../../lib/pipeline/expressions/NaryExpressionT"),
+	NaryExpression = require("../../../../lib/pipeline/expressions/NaryExpression"),
 	ConstantExpression = require("../../../../lib/pipeline/expressions/ConstantExpression"),
 	FieldPathExpression = require("../../../../lib/pipeline/expressions/FieldPathExpression"),
-	Expression = require("../../../../lib/pipeline/expressions/Expression");
-
-function constify(obj, parentIsArray) {
-	parentIsArray = !parentIsArray ? false : true;
-	var bob = parentIsArray ? [] : {};
-	Object.keys(obj).forEach(function(key) {
-		var elem = obj[key];
-		if(elem.constructor === Object) {
-			bob[key] = constify(elem, false);
-		}
-		else if(elem.constructor === Array && !parentIsArray) {
-			bob[key] = constify(elem, true);
-		}
-		else if(key === "$const" ||
-			elem.constructor === String && elem[0] === '$') {
-			bob[key] = obj[key];
-		}
-		else {
-			bob[key] = {$const:obj[key]}
-		}
-	});
-	return bob;
-};
-
-function expressionToJson(expr) {
-	return expr.serialize(false);
-};
+	Expression = require("../../../../lib/pipeline/expressions/Expression"),
+	utils = require("./utils");
 
-function assertDependencies(expectedDeps, expr) {
-	var deps = new DepsTracker(),
-		depsJson = [];
-	expr.addDependencies(deps);
-	deps.forEach(function(dep) {
-		depsJson.push(dep);
-	});
-	assert.deepEqual(depsJson, expectedDeps);
-	assert.equal(deps.needWholeDocument, false);
-	assert.equal(deps.needTextScore, false);
-};
+// 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));
 
 // A dummy child of NaryExpression used for testing
-var TestableExpression = (function(){
-		// CONSTRUCTOR
-	var klass = function TestableExpression(isAssociativeAndCommutative){
+var Testable = (function(){
+	// CONSTRUCTOR
+	var klass = function Testable(isAssociativeAndCommutative){
 		this._isAssociativeAndCommutative = isAssociativeAndCommutative;
 		base.call(this);
-	}, base = NaryExpressionT(TestableExpression), proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+	}, base = NaryExpression, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
-	// PROTOTYPE MEMBERS
+	// MEMBERS
 	proto.evaluateInternal = function evaluateInternal(vars) {
 		// Just put all the values in a list.  This is not associative/commutative so
 		// the results will change if a factory is provided and operations are reordered.
@@ -62,272 +29,212 @@ var TestableExpression = (function(){
 		});
 	};
 
+	proto.getOpName = function getOpName() {
+		return "$testable";
+	};
+
 	proto.isAssociativeAndCommutative = function isAssociativeAndCommutative(){
 		return this._isAssociativeAndCommutative;
 	};
 
 	klass.create = function create(associativeAndCommutative) {
-		associativeAndCommutative = !associativeAndCommutative ? false : true; //NOTE: coercing to bool -- defaults to false
-		return new TestableExpression(associativeAndCommutative);
+		return new Testable(!!associativeAndCommutative);
 	};
 
 	klass.factory = function factory() {
-		return new TestableExpression(true);
-	};
-
-	proto.getOpName = function getOpName() {
-		return "$testable";
-	};
-
-	proto.assertContents = function assertContents(expectedContents) {
-		assert.deepEqual(constify({$testable:expectedContents}), expressionToJson(this));
+		return new Testable(true);
 	};
 
 	klass.createFromOperands = function(operands, haveFactory) {
-		haveFactory = !haveFactory ? false : true; //NOTE: coercing to bool -- defaults to false
-		var vps = new VariablesParseState(new VariablesIdGenerator()),
-			testable = new TestableExpression(haveFactory);
-		operands.forEach(function(x) {
-			testable.addOperand(Expression.parseOperand(x, vps));
+		if (haveFactory === undefined) haveFactory = false;
+		var idGenerator = new VariablesIdGenerator(),
+			vps = new VariablesParseState(idGenerator),
+			testable = Testable.create(haveFactory);
+		operands.forEach(function(element) {
+			testable.addOperand(Expression.parseOperand(element, vps));
 		});
 		return testable;
 	};
 
+	proto.assertContents = function assertContents(expectedContents) {
+		debugger;
+		assert.deepEqual(utils.constify({$testable:expectedContents}), utils.expressionToJson(this));
+	};
+
 	return klass;
 })();
 
+exports.NaryExpression = {
 
-module.exports = {
-
-	"NaryExpressionT": {
+	".parseArguments()": {
 
-		"generator": {
-
-			"can generate a NaryExpression class": function() {
-				assert.doesNotThrow(function() {
-					var NaryExpressionClass = NaryExpressionT(String),
-						naryEpressionIntance = new NaryExpressionClass();
-				});
-			}
+		"should parse a fieldPathExpression": function() {
+			var vps = new VariablesParseState(new VariablesIdGenerator()),
+				parsedArguments = NaryExpression.parseArguments("$field.path.expression", vps);
+			assert.equal(parsedArguments.length, 1);
+			assert(parsedArguments[0] instanceof FieldPathExpression);
+		},
 
-		}
+		"should parse an array of fieldPathExpressions": function() {
+			var vps = new VariablesParseState(new VariablesIdGenerator()),
+				parsedArguments = NaryExpression.parseArguments(["$field.path.expression", "$another.FPE"], vps);
+			assert.equal(parsedArguments.length, 2);
+			assert(parsedArguments[0] instanceof FieldPathExpression);
+			assert(parsedArguments[1] instanceof FieldPathExpression);
+		},
 
 	},
 
-	"NaryExpression": {
-
-		"statics": {
-
-			"parseArguments":{
-
-				"should parse a fieldPathExpression": function parsesFieldPathExpression() {
-					var NaryExpressionClass = NaryExpressionT(String),
-						vps = new VariablesParseState(new VariablesIdGenerator()),
-						parsedArguments = NaryExpressionClass.parseArguments("$field.path.expression", vps);
-						assert.equal(parsedArguments.length, 1);
-						assert(parsedArguments[0] instanceof FieldPathExpression);
-				},
+	/** Adding operands to the expression. */
+	"AddOperand": function testAddOperand() {
+		var testable = Testable.create();
+		testable.addOperand(new ConstantExpression(9));
+		testable.assertContents([9]);
+		testable.addOperand(new FieldPathExpression("ab.c"));
+		testable.assertContents([9, "$ab.c"]); //NOTE: Broken, not sure if problem with assertConents or FPE serialize
+	},
 
-				"should parse an array of fieldPathExpressions": function parsesFieldPathExpression() {
-					var NaryExpressionClass = NaryExpressionT(String),
-						vps = new VariablesParseState(new VariablesIdGenerator()),
-						parsedArguments = NaryExpressionClass.parseArguments(["$field.path.expression", "$another.FPE"], vps);
-						assert.equal(parsedArguments.length, 2);
-						assert(parsedArguments[0] instanceof FieldPathExpression);
-						assert(parsedArguments[1] instanceof FieldPathExpression);
-				}
-			}
+	/** Dependencies of the expression. */
+	"Dependencies": function testDependencies() {
+		var testable = Testable.create();
+
+		var assertDependencies = function assertDependencies(expectedDeps, expr) {
+			var deps = new DepsTracker(),
+				depsJson = [];
+			expr.addDependencies(deps);
+			deps.forEach(function(dep) {
+				depsJson.push(dep);
+			});
+			assert.deepEqual(depsJson, expectedDeps);
+			assert.equal(deps.needWholeDocument, false);
+			assert.equal(deps.needTextScore, false);
+		};
+
+		// No arguments.
+		assertDependencies([], testable);
+
+		// Add a constant argument.
+		testable.addOperand(new ConstantExpression(1));
+		assertDependencies([], testable);
+
+		// Add a field path argument.
+		testable.addOperand(new FieldPathExpression("ab.c"));
+		assertDependencies(["ab.c"], testable);
+
+		// Add an object expression.
+		var spec = {a:"$x", q:"$r"},
+			specElement = spec,
+			ctx = new Expression.ObjectCtx({isDocumentOk:true}),
+			vps = new VariablesParseState(new VariablesIdGenerator());
+		testable.addOperand(Expression.parseObject(specElement, ctx, vps));
+		assertDependencies(["ab.c", "r", "x"]);
+	},
 
-		},
+	/** Serialize to an object. */
+	"AddToJsonObj": function testAddToJsonObj() {
+		var testable = Testable.create();
+		testable.addOperand(new ConstantExpression(5));
+		assert.deepEqual(
+			{foo:{$testable:[{$const:5}]}},
+			{foo:testable.serialize(false)}
+		);
+	},
 
-		"addOperand": {
-			"run" : function run() {
-				var testable = new TestableExpression.create();
-				testable.addOperand(new ConstantExpression(9));
-				debugger;
-				testable.assertContents([9]);
-				testable.addOperand(new FieldPathExpression("ab.c"));
-				testable.assertContents([9, "$ab.c"]); //NOTE: Broken, not sure if problem with assertConents or FPE serialize
-			}
-		},
+	/** Serialize to an array. */
+	"AddToJsonArray": function testAddToJsonArray() {
+		var testable = Testable.create();
+		testable.addOperand(new ConstantExpression(5));
+		assert.deepEqual(
+			[{$testable:[{$const:5}]}],
+			[testable.serialize(false)]
+		);
+	},
 
-		"Dependencies": {
-			"run": function run() {
-				var testable = new TestableExpression.create();
-
-				// No arguments.
-				assertDependencies([], testable);
-
-				// Add a constant argument.
-				testable.addOperand(new ConstantExpression(1));
-				assertDependencies([], testable);
-
-				// Add a field path argument.
-				testable.addOperand(new FieldPathExpression("ab.c"));
-				assertDependencies(["ab.c"], testable);
-
-				// Add an object expression.
-				var spec = {a:"$x", q:"$r"},
-					specElement = spec,
-					ctx = new Expression.ObjectCtx({isDocumentOk:true}),
-					vps = new VariablesParseState(new VariablesIdGenerator());
-				testable.addOperand(Expression.parseObject(specElement, ctx, vps));
-				assertDependencies(["ab.c", "r", "x"]);
-			}
-		},
+	/** One operand is optimized to a constant, while another is left as is. */
+	"OptimizeOneOperand": function testOptimizeOneOperand() {
+		var spec = [{$and:[]},"$abc"],
+			testable = Testable.createFromOperands(spec);
+		testable.assertContents(spec);
+		assert.deepEqual(testable.serialize(), testable.optimize().serialize());
+		testable.assertContents([true, "$abc"]);
+	},
 
-		"AddToJsonObj": {
-			"run": function run() {
-				var testable = new TestableExpression.create();
-				testable.addOperand(new ConstantExpression(5));
-				assert.deepEqual(
-						{foo:{$testable:[{$const:5}]}},
-						{foo:testable.serialize(false)}
-					);
-			}
-		},
+	/** All operands are constants, and the operator is evaluated with them. */
+	"EvaluateAllConstantOperands": function testEvaluateAllConstantOperands() {
+		var spec = [1,2],
+			testable = Testable.createFromOperands(spec);
+		testable.assertContents(spec);
+		var optimized = testable.optimize();
+		assert.notDeepEqual(testable.serialize(), optimized.serialize());
+		assert.deepEqual({$const:[1,2]}, utils.expressionToJson(optimized));
+	},
 
-		"AddToJsonArray": {
-			"run": function run() {
-				var testable = new TestableExpression.create();
-				testable.addOperand(new ConstantExpression(5));
-				assert.deepEqual(
-						[{$testable:[{$const:5}]}],
-						[testable.serialize(false)]
-					);
-			}
-		},
+	"NoFactoryOptimize": {
+		// Without factory optimization, optimization will not produce a new expression.
 
-		"OptimizeOneOperand": {
-			"run": function run() {
-				var spec = [{$and:[]},"$abc"],
-					testable = TestableExpression.createFromOperands(spec);
-				testable.assertContents(spec);
-				assert.deepEqual(testable.serialize(), testable.optimize().serialize());
-				assertContents([true, "$abc"])
-			}
+		/** A string constant prevents factory optimization. */
+		"StringConstant": function testStringConstant() {
+			var testable = Testable.createFromOperands(["abc","def","$path"], true);
+			assert.deepEqual(testable.serialize(), testable.optimize().serialize());
 		},
 
-		"EvaluateAllConstantOperands": {
-			"run": function run() {
-				var spec = [1,2],
-					testable = TestableExpression.createFromOperands(spec);
-				testable.assertContents(spec);
-				var optimized = testable.optimize();
-				assert.notDeepEqual(testable.serialize(), optimized.serialize());
-				assert.deepEqual({$const:[1,2]}, expressionToJson(optimized));
-			}
+		/** A single (instead of multiple) constant prevents optimization.  SERVER-6192 */
+		"SingleConstant": function testSingleConstant() {
+			var testable = Testable.createFromOperands([55,"$path"], true);
+			assert.deepEqual(testable.serialize(), testable.optimize().serialize());
 		},
 
-		"NoFactoryOptimize": {
-			// Without factory optimization, optimization will not produce a new expression.
-
-			/** A string constant prevents factory optimization. */
-			"StringConstant": function run() {
-				var testable = TestableExpression.createFromOperands(["abc","def","$path"], true);
-				assert.deepEqual(testable.serialize(), testable.optimize().serialize());
-			},
-
-			/** A single (instead of multiple) constant prevents optimization.  SERVER-6192 */
-			"SingleConstant": function run() {
-				var testable = TestableExpression.createFromOperands([55,"$path"], true);
-				assert.deepEqual(testable.serialize(), testable.optimize().serialize());
-			},
-
-			/** Factory optimization is not used without a factory. */
-			"NoFactory": function run() {
-				var testable = TestableExpression.createFromOperands([55,66,"$path"], false);
-				assert.deepEqual(testable.serialize(), testable.optimize().serialize());
-			}
+		/** Factory optimization is not used without a factory. */
+		"NoFactory": function testNoFactory() {
+			var testable = Testable.createFromOperands([55,66,"$path"], false);
+			assert.deepEqual(testable.serialize(), testable.optimize().serialize());
 		},
 
-		/** Factory optimization separates constant from non constant expressions. */
-		"FactoryOptimize": {
+	},
 
-			// The constant expressions are evaluated separately and placed at the end.
-			"run": function run() {
-				var testable = TestableExpression.createFromOperands([55,66,"$path"], false),
-					optimized = testable.optimize();	
-				assert.deepEqual({$testable:["$path", [55,66]]}, expressionToJson(optimized));
-			}
-		},
+	/** Factory optimization separates constant from non constant expressions. */
+	"FactoryOptimize": function testFactoryOptimize() {
+		// The constant expressions are evaluated separately and placed at the end.
+		var testable = Testable.createFromOperands([55,66,"$path"], false),
+			optimized = testable.optimize();
+		assert.deepEqual({$testable:["$path", [55,66]]}, utils.expressionToJson(optimized));
+	},
 
-		/** Factory optimization flattens nested operators of the same type. */
-		"FlattenOptimize": {
-			"run": function run() {
-				var testable = TestableExpression.createFromOperands(
-						[55,"$path",{$add:[5,6,"$q"]},66],
-					true);
-				testable.addOperand(Testable.createFromOperands(
-						[99,100,"$another_path"],
-					true));
-				var optimized = testable.optimize();
-				assert.deepEqual(
-					constify({$testable:[
-							"$path",
-							{$add:["$q", 11]},
-							"$another_path",
-							[55, 66, [99, 100]]
-						]}),
-					expressionToJson(optimized));
-			}
-		},
+	/** Factory optimization flattens nested operators of the same type. */
+	"FlattenOptimize": function testFlattenOptimize() {
+		var testable = Testable.createFromOperands(
+				[55,"$path",{$add:[5,6,"$q"]},66],
+			true);
+		testable.addOperand(Testable.createFromOperands(
+				[99,100,"$another_path"],
+			true));
+		var optimized = testable.optimize();
+		assert.deepEqual(
+			utils.constify({$testable:[
+					"$path",
+					{$add:["$q", 11]},
+					"$another_path",
+					[55, 66, [99, 100]]
+				]}),
+			utils.expressionToJson(optimized));
+	},
 
-		/** Three layers of factory optimization are flattened. */
-		"FlattenThreeLayers": {
-			"run": function run() {
-				var top = TestableExpression.createFromOperands([1,2,"$a"], true),
-					nested = TestableExpression.createFromOperands([3,4,"$b"], true);
-				nested.addOperand(TestableExpression.createFromOperands([5,6,"$c"],true));	
-				top.addOperand(nested);
-				var optimized = top.optimize();
-				assert.deepEqual(
-					constify({$testable:[
-						"$a",
-						"$b",
-						"$c",
-						[1,2,[3,4,[5,6]]]]}),
-					expressionToJson(optimized));
-			}
-		},
+	/** Three layers of factory optimization are flattened. */
+	"FlattenThreeLayers": function testFlattenThreeLayers() {
+		var top = Testable.createFromOperands([1,2,"$a"], true),
+			nested = Testable.createFromOperands([3,4,"$b"], true);
+		nested.addOperand(Testable.createFromOperands([5,6,"$c"],true));
+		top.addOperand(nested);
+		var optimized = top.optimize();
+		assert.deepEqual(
+			utils.constify({$testable:[
+				"$a",
+				"$b",
+				"$c",
+				[1,2,[3,4,[5,6]]]]}),
+			utils.expressionToJson(optimized));
+	},
 
-		"constify": {
-			"simple": function simple() {
-				var obj = {a:'s'},
-					constified = constify(obj);
-				assert.deepEqual(constified, { a: { '$const': 's' } });
-			},
-			"array": function array() {
-				var obj = {a:['s']},
-					constified = constify(obj);
-				assert.deepEqual(constified, { a: [ { '$const': 's' } ] });
-			},
-			"array2": function array2() {
-				var obj = {a:['s', [5], {a:5}]},
-					constified = constify(obj);
-				assert.deepEqual(constified,
-					{ a: 
-						[{ '$const': 's' },
-						 { '$const': [ 5 ] },
-						 { a: { '$const': 5 } }]
-					});
-			},
-			"object": function object() {
-				var obj = {a:{b:{c:5}, d:'hi'}},
-					constified = constify(obj);
-				assert.deepEqual(constified, 
-					{ a: 
-						{ b: { c: { '$const': 5 } },
-							d: { '$const': 'hi' } } });
-			},
-			"fieldPathExpression": function fieldPathExpression() {
-				var obj = {a:"$field.path"},
-					constified = constify(obj);
-				assert.deepEqual(constified, obj);
-			}
-		}
-
-	}
 };
 
 if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);

+ 46 - 0
test/lib/pipeline/expressions/utils.js

@@ -0,0 +1,46 @@
+"use strict";
+
+var utils = module.exports = {
+
+	/**
+	 * Convert BSONObj to a BSONObj with our $const wrappings.
+	 * @method constify
+	 */
+	constify: function constify(obj, parentIsArray) {
+		if (parentIsArray === undefined) parentIsArray = false;
+		var bob = parentIsArray ? [] : {};
+		for (var key in obj) {
+			if (!obj.hasOwnProperty(key)) continue;
+			var elem = obj[key];
+			if (elem instanceof Object && elem.constructor === Object) {
+				bob[key] = utils.constify(elem, false);
+			} else if (Array.isArray(elem) && !parentIsArray) {
+				// arrays within arrays are treated as constant values by the real parser
+				bob[key] = utils.constify(elem, true);
+			} else if (key == "$const" ||
+					(typeof elem == "string" && elem[0] == "$")) {
+				bob[key] = obj[key];
+			} else {
+				bob[key] = {$const: obj[key]};
+			}
+		}
+		return bob;
+	},
+
+	//SKIPPED: assertBinaryEqual
+
+	//SKIPPED: toJson
+
+    /**
+     * Convert Expression to BSON.
+     * @method expressionToJson
+     */
+	expressionToJson: function expressionToJson(expr) {
+		return expr.serialize(false);
+	},
+
+	//SKIPPED: fromJson
+
+	//SKIPPED: valueFromJson
+
+};

+ 102 - 0
test/lib/pipeline/expressions/utils_test.js

@@ -0,0 +1,102 @@
+"use strict";
+
+var assert = require("assert"),
+	utils = require("./utils");
+
+// 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));
+
+exports.utils = {
+
+	".constify()": {
+
+		"simple": function() {
+			var original = {
+					a: 1,
+					b: "s"
+				},
+				expected = {
+					a: {
+						$const: 1
+					},
+					b: {
+						$const: "s"
+					}
+				};
+			assert.deepEqual(utils.constify(original), expected);
+		},
+
+		"array": function() {
+			var original = {
+					a: ["s"]
+				},
+				expected = {
+					a: [
+						{
+							$const: "s"
+						}
+					]
+				};
+			assert.deepEqual(utils.constify(original), expected);
+		},
+
+		"array2": function() {
+			var original = {
+					a: [
+						"s",
+						[5],
+						{
+							a: 5
+						}
+					]
+				},
+				expected = {
+					a: [{
+							$const: "s"
+					},
+						{
+							$const: [5]
+					},
+						{
+							a: {
+								$const: 5
+							}
+					}]
+				};
+			assert.deepEqual(utils.constify(original), expected);
+		},
+
+		"object": function() {
+			var original = {
+					a: {
+						b: {
+							c: 5
+						},
+						d: "hi"
+					}
+				},
+				expected = {
+					a: {
+						b: {
+							c: {
+								"$const": 5
+							}
+						},
+						d: {
+							"$const": "hi"
+						}
+					}
+				};
+			assert.deepEqual(utils.constify(original), expected);
+		},
+
+		"fieldPathExpression": function() {
+			var original = {
+				a: "$field.path"
+			};
+			assert.deepEqual(utils.constify(original), original);
+		},
+
+	},
+
+};