소스 검색

Merge branch 'feature/mongo_2.6.5_expressions' into feature/mongo_2.6.5_expressions_SetDifference

Kyle P Davis 11 년 전
부모
커밋
804c37162a

+ 6 - 12
lib/pipeline/expressions/Expression.js

@@ -49,12 +49,7 @@ var ObjectCtx = Expression.ObjectCtx = (function() {
 		for (var k in opts) { // assign all given opts to self so long as they were part of klass.prototype as undefined properties
 			if (opts.hasOwnProperty(k) && proto.hasOwnProperty(k) && proto[k] === undefined) this[k] = opts[k];
 		}
-	}, base = Object,
-		proto = klass.prototype = Object.create(base.prototype, {
-			constructor: {
-				value: klass
-			}
-		});
+	}, proto = klass.prototype;
 
 	// PROTOTYPE MEMBERS
 	proto.isDocumentOk =
@@ -183,9 +178,8 @@ klass.expressionParserMap = {};
  * REGISTER_EXPRESSION("$foo", ExpressionFoo::parse);
  */
 klass.registerExpression = function registerExpression(key, parserFunc) {
-	if (key in klass.expressionParserMap) {
+	if (key in klass.expressionParserMap)
 		throw new Error("Duplicate expression (" + key + ") detected; massert code 17064");
-	}
 	klass.expressionParserMap[key] = parserFunc;
 	return 1;
 };
@@ -272,7 +266,7 @@ proto.optimize = function optimize() {
  *             where {a:1} inclusion objects aren't allowed, they get
  *             NULL.
  */
-proto.addDependencies = function addDependencies(deps, path) {
+proto.addDependencies = function addDependencies(deps, path) { //jshint ignore:line
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
 };
 
@@ -290,7 +284,7 @@ proto.isSimple = function isSimple() {
  * If explain is false, returns a Value parsable by parseOperand().
  * @method serialize
  */
-proto.serialize = function serialize(explain) {
+proto.serialize = function serialize(explain) { //jshint ignore:line
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
 };
 
@@ -304,7 +298,7 @@ proto.serialize = function serialize(explain) {
  * @param vars
  */
 proto.evaluate = function evaluate(vars) {
-	if (!(vars instanceof Variables)) vars = new Variables(0, vars); /// Evaluate expression with specified inputs and return result. (only used by tests)
+	if (vars instanceof Object && vars.constructor === Object) vars = new Variables(0, vars); /// Evaluate expression with specified inputs and return result. (only used by tests)
 	return this.evaluateInternal(vars);
 };
 
@@ -327,7 +321,7 @@ klass.removeFieldPrefix = function removeFieldPrefix(prefixedField) {
  * @method evaluate
  * @returns the computed value
  */
-proto.evaluateInternal = function evaluateInternal(vars) {
+proto.evaluateInternal = function evaluateInternal(vars) { //jshint ignore:line
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
 };
 

+ 5 - 4
lib/pipeline/expressions/FieldPathExpression.js

@@ -2,7 +2,6 @@
 
 var Expression = require("./Expression"),
     Variables = require("./Variables"),
-    Value = require("../Value"),
     FieldPath = require("../FieldPath");
 
 /**
@@ -53,11 +52,13 @@ klass.parse = function parse(raw, vps) {
     if (raw.length < 2) throw new Error("'$' by itself is not a valid FieldPath; uassert code 16872"); // need at least "$" and either "$" or a field name
     if (raw[1] === "$") {
         var fieldPath = raw.substr(2), // strip off $$
-            varName = fieldPath.substr(0, fieldPath.indexOf("."));
+            dotIndex = fieldPath.indexOf("."),
+            varName = fieldPath.substr(0, dotIndex !== -1 ? dotIndex : fieldPath.length);
         Variables.uassertValidNameForUserRead(varName);
-        return new FieldPathExpression(raw.slice(2), vps.getVariableName(varName));
+        return new FieldPathExpression(fieldPath, vps.getVariable(varName));
     } else {
-        return new FieldPathExpression("CURRENT." + raw.substr(1), vps.getVariable("CURRENT"));
+        return new FieldPathExpression("CURRENT." + raw.substr(1), // strip the "$" prefix
+            vps.getVariable("CURRENT"));
     }
 };
 

+ 16 - 16
lib/pipeline/expressions/Helpers.js

@@ -1,25 +1,25 @@
-module.exports = {
-	//Returns an object containing unique values. All keys are the same as the corresponding value.
-	arrayToSet : function arrayToSet(array){
+var Helpers = module.exports = { //jshint ignore:line
 
-		var set = {};
+	arrayToSet: function arrayToSet(val) { //NOTE: DEVIATION FROM MONGO: we return an Object of JSON-String to Values
+		if (!(val instanceof Array)) throw new Error("Assertion failure");
+		var array = val;
 
-		// This ensures no duplicates.
-		array.forEach(function (element) {
-			var elementString = JSON.stringify(element);
-			set[elementString] = element;
-		});
+		var set = {};
+		for (var i = 0, l = array.length; i < l; i++) {
+			var item = array[i],
+				itemHash = JSON.stringify(array[i]);
+			set[itemHash] = item;
+		}
 
 		return set;
 	},
 
-	setToArray: function setToArray(set){
+	setToArray: function setToArray(set) {	//TODO: used??
 		var array = [];
-
-		Object.keys(set).forEach(function (key) {
+		for (var key in set) {
 			array.push(set[key]);
-		});
-
+		}
 		return array;
-	}
-}
+	},
+
+};

+ 62 - 59
lib/pipeline/expressions/MapExpression.js

@@ -1,107 +1,110 @@
 "use strict";
 
 var MapExpression = module.exports = function MapExpression(varName, varId, input, each){
-	if (arguments.length !== 4) throw new Error("Four args expected");
+	if (arguments.length !== 4) throw new Error(klass.name + ": args expected: varName, varId, input, each");
 	this._varName = varName;
 	this._varId = varId;
 	this._input = input;
 	this._each = each;
 }, klass = MapExpression, Expression = require("./Expression"), base = Expression, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
-// DEPENDENCIES
-var Variables = require("./Variables"),
-	VariablesParseState = require("./VariablesParseState");
+var Value = require("../Value"),
+	Variables = require("./Variables");
 
-// PROTOTYPE MEMBERS
+klass.parse = function parse(expr, vpsIn) {
 
+	// if (!(exprFieldName)) throw new Error("Assertion failure"); //NOTE: DEVIATION FROM MONGO: we do not have exprFieldName here
 
-klass.parse = function parse(expr, vpsIn){
-	if(!("$map" in expr)) {
-		throw new Error("Tried to create a $let with something other than let. Looks like your parse map went all funny.");
+	if (Value.getType(expr) !== "Object") {
+		throw new Error("$map only supports an object as it's argument; uassert code 16878");
 	}
 
-	if(typeof(expr.$map) !== 'object' || (expr.$map instanceof Array)) {
-		throw new Error("$map only supports an object as it's argument:16878");
+	// "in" must be parsed after "as" regardless of BSON order
+	var inputElem,
+		asElem,
+		inElem,
+		args = expr;
+	for (var argFieldName in args) {
+		var arg = args[argFieldName];
+		if (argFieldName === "input") {
+			inputElem = arg;
+		} else if (argFieldName === "as") {
+			asElem = arg;
+		} else if (argFieldName === "in") {
+			inElem = arg;
+		} else {
+			throw new Error("Unrecognized parameter to $map: " + argFieldName + "; uassert code 16879");
+		}
 	}
 
-	var args = expr.$map,
-		inputElem = args.input,
-		inElem = args['in'],
-		asElem = args.as;
-
-	if(!inputElem) {
-		throw new Error("Missing 'input' parameter to $map: 16880");
-	}
-	if(!asElem) {
-		throw new Error("Missing 'as' parameter to $map: 16881");
-	}
-	if(!inElem) {
-		throw new Error("Missing 'in' parameter to $let: 16882");
-	}
+	if (!inputElem) throw new Error("Missing 'input' parameter to $map; uassert code 16880");
+	if (!asElem) throw new Error("Missing 'as' parameter to $map; uassert code 16881");
+	if (!inElem) throw new Error("Missing 'in' parameter to $map; uassert code 16882");
 
+	// parse "input"
+	var input = Expression.parseOperand(inputElem, vpsIn); // only has outer vars
 
-	if(Object.keys(args).length > 3) {
-		var bogus = Object.keys(args).filter(function(x) {return !(x === 'in' || x === 'as' || x === 'input');});
-		throw new Error("Unrecognized parameter to $map: " + bogus.join(",") + "- 16879");
-	}
-
-	var input = Expression.parseOperand(inputElem, vpsIn);
-
-	var vpsSub = new VariablesParseState(vpsIn),
+	// parse "as"
+	var vpsSub = vpsIn, // vpsSub gets our vars, vpsIn doesn't.
 		varName = asElem;
-
 	Variables.uassertValidNameForUserWrite(varName);
 	var varId = vpsSub.defineVariable(varName);
 
-	var invert = Expression.parseOperand(inElem, vpsSub);
+	// parse "in"
+	var inExpr = Expression.parseOperand(inElem, vpsSub); // has access to map variable
 
-	return new MapExpression(varName, varId, input, invert);
+	return new MapExpression(varName, varId, input, inExpr);
 };
 
-
 proto.optimize = function optimize() {
+	// TODO handle when _input is constant
 	this._input = this._input.optimize();
 	this._each = this._each.optimize();
 	return this;
 };
 
 proto.serialize = function serialize(explain) {
-	return {$map: {input:this._input.serialize(explain),
-				   as: this._varName,
-				   'in': this._each.serialize(explain)}};
+	return {
+		$map: {
+			input: this._input.serialize(explain),
+			as: this._varName,
+			in : this._each.serialize(explain)
+		}
+	};
 };
 
 proto.evaluateInternal = function evaluateInternal(vars) {
+	// guaranteed at parse time that this isn't using our _varId
 	var inputVal = this._input.evaluateInternal(vars);
-	if( inputVal === null) {
+	if (inputVal === null || inputVal === undefined)
 		return null;
-	}
 
-	if(!(inputVal instanceof Array)) {
-		throw new Error("Input to $map must be an Array, not a ____ 16883");
+	if (!(inputVal instanceof Array)){
+		throw new Error("input to $map must be an Array not " +
+			Value.getType(inputVal) + "; uassert code 16883");
 	}
 
-	if(inputVal.length === 0) {
-		return [];
-	}
+	if (inputVal.length === 0)
+		return inputVal;
+
+	var output = new Array(inputVal.length);
+	for (var i = 0, l = inputVal.length; i < l; i++) {
+		vars.setValue(this._varId, inputVal[i]);
 
-	// Diverge from Mongo source here, as Javascript has a builtin map operator.
-	return inputVal.map(function(x) {
-	   vars.setValue(this._varId, x);
-	   var toInsert = this._each.evaluateInternal(vars);
-	   if(toInsert === undefined) {
-		   toInsert = null;
-	   }
+		var toInsert = this._each.evaluateInternal(vars);
+		if (toInsert === undefined)
+			toInsert = null; // can't insert missing values into array
 
-	   return toInsert;
-   });
+		output[i] = toInsert;
+	}
+
+	return output;
 };
 
-proto.addDependencies = function addDependencies(deps, path){
-	this._input.addDependencies(deps, path);
-	this._each.addDependencies(deps, path);
+proto.addDependencies = function addDependencies(deps, path) { //jshint ignore:line
+	this._input.addDependencies(deps);
+	this._each.addDependencies(deps);
 	return deps;
 };
 
-
 Expression.registerExpression("$map", klass.parse);

+ 53 - 64
lib/pipeline/expressions/SetIsSubsetExpression.js

@@ -10,99 +10,88 @@
  **/
 
 var SetIsSubsetExpression = module.exports = function SetIsSubsetExpression() {
-	if (arguments.length !== 2) throw new Error("two args expected");
+	if (arguments.length !== 0) throw new Error(klass.name + ": no args expected");
 	base.call(this);
-}, klass = SetIsSubsetExpression,
-	FixedArityExpression = require("./FixedArityExpressionT")(klass, 2),
-	base = FixedArityExpression,
-	proto = klass.prototype = Object.create(base.prototype, {
-		constructor: {
-			value: klass
-		}
-	});
-
+}, klass = SetIsSubsetExpression, base = require("./FixedArityExpressionT")(SetIsSubsetExpression, 2), proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
-// lhs should be array, rhs should be set (object). See arrayToSet implementation.
-var setIsSubsetHelper = function setIsSubsetHelper(lhs, rhs){
+var Value = require("../Value"),
+	Expression = require("./Expression"),
+	NaryExpression = require("./NaryExpression"),
+	ConstantExpression = require("./ConstantExpression"),
+	Helpers = require("./Helpers");
 
-	var lset = Helpers.arrayToSet(lhs);
-		rkeys = Object.keys(rhs);
+var setIsSubsetHelper = function setIsSubsetHelper(lhs, rhs) { //NOTE: vector<Value> &lhs, ValueSet &rhs
 	// do not shortcircuit when lhs.size() > rhs.size()
 	// because lhs can have redundant entries
-	Object.keys(lset).forEach(function (lkey){
-		if (rkeys.indexOf(lkey) < 0){
+	for (var i = 0; i < lhs.length; i++) {
+		if (!(JSON.stringify(lhs[i]) in rhs)) {
 			return false;
 		}
-	});
-
+	}
 	return true;
 };
 
-var Optimized = function Optimized(cachedRhsSet, operands) {
-	this.operands = operands;
-	this._cachedRhsSet = cachedRhsSet;
-}
-
-Optimized.prototype = Object.create(SetIsSubsetExpression.prototype, {
-	constructor: {
-		value: Optimized
-	}
-});
+proto.evaluateInternal = function evaluateInternal(vars) {
+	var lhs = this.operands[0].evaluateInternal(vars),
+		rhs = this.operands[1].evaluateInternal(vars);
 
-Optimized.prototype.evaluateInternal = function evaluateInternal(vars){
-	lhs = this.operands[0].evaluateInternal(vars);
+	if (!(lhs instanceof Array))
+		throw new Error("both operands of " + this.getOpName() + ": must be arrays. First " +
+			"argument is of type " + Value.getType(lhs) + "; uassert code 17046");
+	if (!(rhs instanceof Array))
+		throw new Error("both operands of " + this.getOpName() + ": must be arrays. Second " +
+			"argument is of type " + Value.getType(rhs) + "; code 17042");
 
-	if (!(lhs instanceof Array)) throw new Error("uassert 17046: both operands of " + this.getOpName() + "  must be arrays. Second argument is of type " + typeof lhs);
-	
-	return setIsSubsetHelper(lhs, this._cachedRhsSet);
+	return setIsSubsetHelper(lhs, Helpers.arrayToSet(rhs));
 };
 
-// DEPENDENCIES
-var Value = require("../Value"),
-	Expression = require("./Expression"),
-	Helpers = require("./Helpers");
 
+/**
+ * This class handles the case where the RHS set is constant.
+ *
+ * Since it is constant we can construct the hashset once which makes the runtime performance
+ * effectively constant with respect to the size of RHS. Large, constant RHS is expected to be a
+ * major use case for $redact and this has been verified to improve performance significantly.
+ */
+function Optimized(cachedRhsSet, operands) {
+	this._cachedRhsSet = cachedRhsSet;
+	this.operands = operands;
+}
+Optimized.prototype = Object.create(SetIsSubsetExpression.prototype, {constructor:{value:Optimized}});
+Optimized.prototype.evaluateInternal = function evaluateInternal(vars){
+	var lhs = this.operands[0].evaluateInternal(vars);
+
+	if (!(lhs instanceof Array))
+		throw new Error("both operands of " + this.getOpName() + " must be arrays. First " +
+			"argument is of type: " + Value.getType(lhs) + "; uassert code 17310");
 
-// PROTOTYPE MEMBERS
-proto.getOpName = function getOpName() {
-	return "$setissubset";
+	return setIsSubsetHelper(lhs, this._cachedRhsSet);
 };
 
-proto.optimize = function optimize(cachedRhsSet, operands) {
 
+proto.optimize = function optimize(cachedRhsSet, operands) { //jshint ignore:line
 	// perform basic optimizations
-	var optimized = base.optimize.call(this);
+	var optimized = NaryExpression.optimize();
 
-	// if NaryExpression.optimize() created a new value, return it directly
-	if(optimized != this){
+	// if ExpressionNary::optimize() created a new value, return it directly
+	if(optimized !== this)
 		return optimized;
-	}
 
-	if (operands[1] instanceof ConstantExpression){
-		var ce = operands[1],
-			rhs = ce.getValue();
-
-		if (!(rhs instanceof Array)) throw new Error("uassert 17311: both operands of " + this.getOpName() + "  must be arrays. Second argument is of type " + typeof rhs);
+	var ce;
+	if ((ce = this.operands[1] instanceof ConstantExpression ? this.operands[1] : undefined)){
+		var rhs = ce.getValue();
+		if (!(rhs instanceof Array))
+			throw new Error("both operands of " + this.getOpName() + " must be arrays. Second " +
+				"argument is of type " + Value.getType(rhs) + "; uassert code 17311");
 
 		return new Optimized(Helpers.arrayToSet(rhs), this.operands);
 	}
 
 	return optimized;
-
 };
 
-/**
- * Takes 2 arrays. Returns true if the first is a subset of the second. Returns false otherwise.
- * @method evaluateInternal
- **/
-proto.evaluateInternal = function evaluateInternal(vars) {
-	var lhs = this.operands[0].evaluateInternal(vars),
-		rhs = this.operands[1].evaluateInternal(vars);
-	if (!(lhs instanceof Array)) throw new Error(this.getOpName() + ": object 1 must be an array. Got a(n) " + typeof lhs);
-	if (!(rhs instanceof Array)) throw new Error(this.getOpName() + ": object 2 must be an array. Got a(n) " + typeof rhs);
+Expression.registerExpression("$setIsSubset", base.parse);
 
-	return setIsSubsetHelper(lhs, Helpers.arrayToSet(rhs));
+proto.getOpName = function getOpName() {
+	return "$setIsSubset";
 };
-
-/** Register Expression */
-Expression.registerExpression("$setissubset", base.parse);

+ 94 - 74
lib/pipeline/expressions/Variables.js

@@ -1,57 +1,72 @@
 "use strict";
 
+// TODO: Look into merging with ExpressionContext and possibly ObjectCtx.
 /**
- * Class that stores/tracks variables
+ * The state used as input and working space for Expressions.
  * @class Variables
  * @namespace mungedb-aggregate.pipeline.expressions
  * @module mungedb-aggregate
  * @constructor
- **/
+ */
 var Variables = module.exports = function Variables(numVars, root){
-	if(numVars) {
-		if(typeof numVars !== 'number') {
-			throw new Error('numVars must be a number');
-		}
-	}
+	if (arguments.length === 0) numVars = 0; // This is only for expressions that use no variables (even ROOT).
+	if (typeof numVars !== "number") throw new Error("numVars must be a Number");
+
 	this._root = root || {};
-	this._rest = numVars ? [] : undefined; //An array of `Value`s
+	this._rest = numVars === 0 ? null : new Array(numVars);
 	this._numVars = numVars;
-}, klass = Variables,
-	base = Object,
-	proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+}, klass = Variables, proto = klass.prototype;
 
+klass.uassertValidNameForUserWrite = function uassertValidNameForUserWrite(varName) {
+	// System variables users allowed to write to (currently just one)
+	if (varName === "CURRENT") {
+		return;
+	}
 
-klass.ROOT_ID = -1;
+	if (!varName)
+		throw new Error("empty variable names are not allowed; uassert code 16866");
 
-// PROTOTYPE MEMBERS
+	var firstCharIsValid = (varName[0] >= "a" && varName[0] <= "z") ||
+		(varName[0] & "\x80"); // non-ascii
 
-/**
- * Sets the root variable
- * @method setRoot
- * @parameter root {Document} The root variable
- **/
-proto.setRoot = function setRoot(root){
-	if(!(root instanceof Object && root.constructor.name === 'Object')) { //NOTE: Type checking cause c++ does this for you
-		throw new Error('root must be an Object');
+	if (!firstCharIsValid)
+		throw new Error("'" + varName + "' starts with an invalid character for a user variable name; uassert code 16867");
+
+	for (var i = 1, l = varName.length; i < l; i++) {
+		var charIsValid = (varName[i] >= 'a' && varName[i] <= 'z') ||
+			(varName[i] >= 'A' && varName[i] <= 'Z') ||
+			(varName[i] >= '0' && varName[i] <= '9') ||
+			(varName[i] == '_') ||
+			(varName[i] & '\x80'); // non-ascii
+
+		if (!charIsValid)
+			throw new Error("'" + varName + "' contains an invalid character " +
+				"for a variable name: '" + varName[i] + "'; uassert code 16868");
 	}
-	this._root = root;
 };
 
-/**
- * Clears the root variable
- * @method clearRoot
- **/
-proto.clearRoot = function clearRoot(){
-	this._root = {};
-};
+klass.uassertValidNameForUserRead = function uassertValidNameForUserRead(varName) {
+	if (!varName)
+		throw new Error("empty variable names are not allowed; uassert code 16869");
 
-/**
- * Gets the root variable
- * @method getRoot
- * @return {Document} the root variable
- **/
-proto.getRoot = function getRoot(){
-	return this._root;
+	var firstCharIsValid = (varName[0] >= "a" && varName[0] <= "z") ||
+		(varName[0] >= "A" && varName[0] <= "Z") ||
+		(varName[0] & "\x80"); // non-ascii
+
+	if (!firstCharIsValid)
+		throw new Error("'" + varName + "' starts with an invalid character for a variable name; uassert code 16870");
+
+	for (var i = 1, l = varName.length; i < l; i++) {
+		var charIsValid = (varName[i] >= "a" && varName[i] <= "z") ||
+			(varName[i] >= "A" && varName[i] <= "Z") ||
+			(varName[i] >= "0" && varName[i] <= "9") ||
+			(varName[i] == "_") ||
+			(varName[i] & "\x80"); // non-ascii
+
+		if (!charIsValid)
+			throw new Error("'" + varName + "' contains an invalid character " +
+				"for a variable name: '" + varName[i] + "'; uassert code 16871");
+	}
 };
 
 /**
@@ -59,20 +74,11 @@ proto.getRoot = function getRoot(){
  * @method setValue
  * @param id {Number} The index where the value is stored in the _rest Array
  * @param value {Value} The value to store
- **/
+ */
 proto.setValue = function setValue(id, value) {
-	//NOTE: Some added type enforcement cause c++ does this for you
-	if(typeof id !== 'number') {
-		throw new Error('id must be a Number');
-	}
-
-	if(id === klass.ROOT_ID) {
-		throw new Error("mError 17199: can't use Variables#setValue to set ROOT");
-	}
-	if(id >= this._numVars) { // a > comparator would be off-by-one; i.e. if we have 5 vars, the max id would be 4
-		throw new Error("You have more variables than _numVars");
-	}
-
+	if (typeof id !== "number") throw new Error("id must be a Number");
+	if (id === klass.ROOT_ID) throw new Error("can't use Variables#setValue to set ROOT; massert code 17199");
+	if (id >= this._numVars) throw new Error("Assertion error");
 	this._rest[id] = value;
 };
 
@@ -81,46 +87,60 @@ proto.setValue = function setValue(id, value) {
  * @method getValue
  * @param id {Number} The index where the value was stored
  * @return {Value} The value
- **/
+ */
 proto.getValue = function getValue(id) {
-	//NOTE: Some added type enforcement cause c++ does this for you
-	if(typeof id !== 'number') {
-		throw new Error('id must be a Number');
-	}
-
-	if(id === klass.ROOT_ID) {
+	if (typeof id !== "number") throw new Error("id must be a Number");
+	if (id === klass.ROOT_ID)
 		return this._root;
-	}
-	if(id >= this._numVars) { // a > comparator would be off-by-one; i.e. if we have 5 vars, the max id would be 4
-		throw new Error("Cannot get value; id was greater than _numVars");
-	}
-
+	if (id >= this._numVars) throw new Error("Assertion error");
 	return this._rest[id];
 };
 
-
 /**
  * Get the value for id if it's a document
  * @method getDocument
  * @param id {Number} The index where the document was stored
  * @return {Object} The document
- **/
+ */
 proto.getDocument = function getDocument(id) {
-	//NOTE: Some added type enforcement cause c++ does this for you
-	if(typeof id !== 'number') {
-		throw new Error('id must be a Number');
-	}
+	if (typeof id !== "number") throw new Error("id must be a Number");
 
-	if(id === klass.ROOT_ID) {
+	if (id === klass.ROOT_ID)
 		return this._root;
-	}
-	if(id >= this._numVars) { // a > comparator would be off-by-one; i.e. if we have 5 vars, the max id would be 4
-		throw new Error("Cannot get value; id was greater than _numVars");
-	}
 
+	if (id >= this._numVars) throw new Error("Assertion error");
 	var value = this._rest[id];
-	if(typeof value === 'object' && value.constructor.name === 'Object') {
+	if (value instanceof Object && value.constructor === Object)
 		return value;
-	}
+
 	return {};
 };
+
+klass.ROOT_ID = -1;
+
+/**
+ * Use this instead of setValue for setting ROOT
+ * @method setRoot
+ * @parameter root {Document} The root variable
+ */
+proto.setRoot = function setRoot(root){
+	if (!(root instanceof Object && root.constructor === Object)) throw new Error("Assertion failure");
+	this._root = root;
+};
+
+/**
+ * Clears the root variable
+ * @method clearRoot
+ */
+proto.clearRoot = function clearRoot(){
+	this._root = {};
+};
+
+/**
+ * Gets the root variable
+ * @method getRoot
+ * @return {Document} the root variable
+ */
+proto.getRoot = function getRoot(){
+	return this._root;
+};

+ 8 - 8
lib/pipeline/expressions/VariablesIdGenerator.js

@@ -1,31 +1,31 @@
 "use strict";
 
-/** 
- * Class generates unused ids
+/**
+ * Generates Variables::Ids and keeps track of the number of Ids handed out.
  * @class VariablesIdGenerator
  * @namespace mungedb-aggregate.pipeline.expressions
  * @module mungedb-aggregate
  * @constructor
- **/
+ */
 var VariablesIdGenerator = module.exports = function VariablesIdGenerator(){
 	this._nextId = 0;
-}, klass = VariablesIdGenerator, base = Object, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+}, klass = VariablesIdGenerator, proto = klass.prototype;
 
 /**
  * Gets the next unused id
  * @method generateId
  * @return {Number} The unused id
- **/
+ */
 proto.generateId = function generateId() {
 	return this._nextId++;
 };
 
 /**
- * Gets the number of used ids
+ * Returns the number of Ids handed out by this Generator.
+ * Return value is intended to be passed to Variables constructor.
  * @method getIdCount
  * @return {Number} The number of used ids
- **/
+ */
 proto.getIdCount = function getIdCount() {
 	return this._nextId;
 };
-

+ 26 - 23
lib/pipeline/expressions/VariablesParseState.js

@@ -1,22 +1,25 @@
 "use strict";
 
-/** 
- * Class generates unused ids
+/**
+ * This class represents the Variables that are defined in an Expression tree.
+ *
+ * All copies from a given instance share enough information to ensure unique Ids are assigned
+ * and to propagate back to the original instance enough information to correctly construct a
+ * Variables instance.
+ *
  * @class VariablesParseState
  * @namespace mungedb-aggregate.pipeline.expressions
  * @module mungedb-aggregate
  * @constructor
- **/
-var Variables = require('./Variables'),
-	VariablesIdGenerator = require('./VariablesIdGenerator');
+ */
+var Variables = require("./Variables"),
+	VariablesIdGenerator = require("./VariablesIdGenerator");
 
 var VariablesParseState = module.exports = function VariablesParseState(idGenerator){
-	if(!idGenerator || idGenerator.constructor !== VariablesIdGenerator) {
-		throw new Error("idGenerator is required and must be of type VariablesIdGenerator");
-	}
+	if (!(idGenerator instanceof VariablesIdGenerator)) throw new Error("idGenerator is required and must be of type VariablesIdGenerator");
 	this._idGenerator = idGenerator;
-	this._variables = {}; //Note: The c++ type was StringMap<Variables::Id>
-}, klass = VariablesParseState, base = Object, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+	this._variables = {};
+}, klass = VariablesParseState, proto = klass.prototype;
 
 /**
  * Assigns a named variable a unique Id. This differs from all other variables, even
@@ -27,12 +30,12 @@ var VariablesParseState = module.exports = function VariablesParseState(idGenera
  * breaks that equivalence.
  *
  * NOTE: Name validation is responsibility of caller.
- **/
-proto.defineVariable = function generateId(name) {
+ */
+proto.defineVariable = function defineVariable(name) {
 	// caller should have validated before hand by using Variables::uassertValidNameForUserWrite
-	if(name === 'ROOT') {
-		throw new Error("mError 17275: Can't redefine ROOT");
-	}
+	if (name === "ROOT")
+		throw new Error("Can't redefine ROOT; massert code 17275");
+
 	var id = this._idGenerator.generateId();
 	this._variables[name] = id;
 	return id;
@@ -42,14 +45,14 @@ proto.defineVariable = function generateId(name) {
  * Returns the current Id for a variable. uasserts if the variable isn't defined.
  * @method getVariable
  * @param name {String} The name of the variable
- **/
-proto.getVariable = function getIdCount(name) {
-	var found = this._variables[name];
-	if(typeof found === 'number') return found;
-	if(name !== "ROOT" && name !== "CURRENT") {
-		throw new Error("uError 17276: Use of undefined variable " + name);
-	}
+ */
+proto.getVariable = function getVariable(name) {
+	var it = this._variables[name];
+	if (typeof it === "number")
+		return it;
+
+	if (name !== "ROOT" && name !== "CURRENT")
+		throw new Error("Use of undefined variable " + name + "; uassert code 17276");
 
 	return Variables.ROOT_ID;
 };
-

+ 115 - 0
test/lib/pipeline/expressions/MapExpression_test.js

@@ -0,0 +1,115 @@
+"use strict";
+var assert = require("assert"),
+	MapExpression = require("../../../../lib/pipeline/expressions/MapExpression"),
+	Expression = require("../../../../lib/pipeline/expressions/Expression"),
+	AddExpression = require("../../../../lib/pipeline/expressions/AddExpression"), // jshint ignore:line
+	IfNullExpression = require("../../../../lib/pipeline/expressions/IfNullExpression"), // jshint ignore:line
+	Variables = require("../../../../lib/pipeline/expressions/Variables"),
+	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
+	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
+	DepsTracker = require("../../../../lib/pipeline/DepsTracker"),
+	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));
+
+exports.MapExpression = {
+
+	"constructor()": {
+
+		"should accept 4 arguments": function () {
+			new MapExpression(1, 2, 3, 4);
+		},
+
+		"should accept only 4 arguments": function () {
+			assert.throws(function () { new MapExpression(); });
+			assert.throws(function () { new MapExpression(1); });
+			assert.throws(function () { new MapExpression(1, 2); });
+			assert.throws(function () { new MapExpression(1, 2, 3); });
+			assert.throws(function () { new MapExpression(1, 2, 3, 4, 5); });
+		},
+
+	},
+
+	"#optimize()": {
+
+		"should optimize both $map.input and $map.in": function() {
+			var spec = {$map:{
+					input: {$ifNull:[null, {$const:[1,2,3]}]},
+					as: "i",
+					in: {$add:["$$i","$$i",1,2]},
+				}},
+				idGenerator = new VariablesIdGenerator(),
+				vps = new VariablesParseState(idGenerator),
+				expr = Expression.parseOperand(spec, vps),
+				optimized = expr.optimize();
+			assert.strictEqual(optimized, expr, "should be same reference");
+			assert.deepEqual(expressionToJson(optimized._input), {$const:[1,2,3]});
+			assert.deepEqual(expressionToJson(optimized._each), constify({$add:["$$i","$$i",1,2]}));
+		},
+
+	},
+
+	"#serialize()": {
+
+		"should serialize to consistent order": function() {
+			var spec = {$map:{
+					as: "i",
+					in: {$add:["$$i","$$i"]},
+					input: {$const:[1,2,3]},
+				}},
+				expected = {$map:{
+					input: {$const:[1,2,3]},
+					as: "i",
+					in: {$add:["$$i","$$i"]},
+				}},
+				idGenerator = new VariablesIdGenerator(),
+				vps = new VariablesParseState(idGenerator),
+				expr = Expression.parseOperand(spec, vps);
+			assert.deepEqual(expressionToJson(expr), expected);
+		},
+
+	},
+
+	"#evaluate()": {
+
+		"should be able to map over a simple array": function() {
+			var spec = {$map:{
+					input: {$const:[1,2,3]},
+					as: "i",
+					in: {$add:["$$i","$$i"]},
+				}},
+				idGenerator = new VariablesIdGenerator(),
+				vps = new VariablesParseState(idGenerator),
+				expr = Expression.parseOperand(spec, vps),
+				vars = new Variables(1, {}); // must set numVars (usually handled by doc src)
+			assert.deepEqual(expr.evaluate(vars), [2, 4, 6]);
+		},
+
+	},
+
+	"#addDependencies()": {
+
+		"should add dependencies to both $map.input and $map.in": function () {
+			var spec = {$map:{
+					input: "$inputArray",
+					as: "i",
+					in: {$add:["$$i","$someConst"]},
+				}},
+				idGenerator = new VariablesIdGenerator(),
+				vps = new VariablesParseState(idGenerator),
+				expr = Expression.parseOperand(spec, vps),
+				deps = new DepsTracker();
+			expr.addDependencies(deps);
+			assert.strictEqual(Object.keys(deps.fields).length, 2);
+			assert.strictEqual("inputArray" in deps.fields, true);
+			assert.strictEqual("someConst" in deps.fields, true);
+			assert.strictEqual(deps.needWholeDocument, false);
+			assert.strictEqual(deps.needTextScore, false);
+		},
+
+	},
+
+};

+ 368 - 90
test/lib/pipeline/expressions/SetIsSubsetExpression.js

@@ -1,98 +1,376 @@
 "use strict";
 var assert = require("assert"),
-		SetIsSubsetExpression = require("../../../../lib/pipeline/expressions/SetIsSubsetExpression"),
-		Expression = require("../../../../lib/pipeline/expressions/Expression");
-
-
-module.exports = {
-
-		"SetIsSubsetExpression": {
-
-				"constructor()": {
-
-						"should throw Error when constructing without args": function testConstructor() {
-								assert.throws(function() {
-										new SetIsSubsetExpression();
-								});
-						}
-
-				},
-
-				"#getOpName()": {
-
-						"should return the correct op name; $setissubset": function testOpName() {
-								assert.equal(new SetIsSubsetExpression([1,2,3],[4,5,6]).getOpName(), "$setissubset");
-						}
-
-				},
-
-				"#evaluateInternal()": {
-
-						"Should fail if array1 is not an array": function testArg1() {
-								var array1 = "not an array",
-										array2 = [6, 7, 8, 9];
-								assert.throws(function() {
-										Expression.parseOperand({
-												$setissubset: ["$array1", "$array2"]
-										}).evaluateInternal({
-												array1: array1,
-												array2: array2
-										});
-								});
-						},
-
-						"Should fail if array2 is not an array": function testArg2() {
-								var array1 = [1, 2, 3, 4],
-										array2 = "not an array";
-								assert.throws(function() {
-										Expression.parseOperand({
-												$setissubset: ["$array1", "$array2"]
-										}).evaluateInternal({
-												array1: array1,
-												array2: array2
-										});
-								});
-						},
-
-						"Should fail if both are not an array": function testArg1andArg2() {
-								var array1 = "not an array",
-										array2 = "not an array";
-								assert.throws(function() {
-										Expression.parseOperand({
-												$setissubset: ["$array1", "$array2"]
-										}).evaluateInternal({
-												array1: array1,
-												array2: array2
-										});
-								});
-						},
-
-						"Should pass and return a true": function testBasicAssignment() {
-								var array1 = [1, 2, 3, 4, 5],
-										array2 = [2,3];
-								assert.strictEqual(Expression.parseOperand({
-										$setissubset: ["$array1", "$array2"]
-								}).evaluateInternal({
-										array1: array1,
-										array2: array2
-								}), true);
-						},
-
-						"Should pass and return false": function testBasicAssignment() {
-								var array1 = [1, 2, 3, 4, 5],
-										array2 = [7, 8, 9];
-								assert.strictEqual(Expression.parseOperand({
-										$setissubset: ["$array1", "$array2"]
-								}).evaluateInternal({
-										array1: array1,
-										array2: array2
-								}), true);
-						},
+	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
+	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
+	SetIsSubsetExpression = require("../../../../lib/pipeline/expressions/SetIsSubsetExpression"),
+	Expression = require("../../../../lib/pipeline/expressions/Expression");
 
+// 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 ExpectedResultBase = (function() {
+	var klass = function ExpectedResultBase(overrides) {
+		//NOTE: DEVIATION FROM MONGO: using this base class to make things easier to initialize
+		for (var key in overrides)
+			this[key] = overrides[key];
+	}, proto = klass.prototype;
+	proto.run = function() {
+		var spec = this.getSpec,
+			args = spec.input;
+		if (spec.expected !== undefined && spec.expected !== null) {
+			var fields = spec.expected;
+			for (var fieldFirst in fields) {
+				var fieldSecond = fields[fieldFirst],
+					expected = fieldSecond;
+					// obj = {<fieldFirst>: args}; //NOTE: DEVIATION FROM MONGO: see parseExpression below
+				var idGenerator = new VariablesIdGenerator(),
+					vps = new VariablesParseState(idGenerator),
+					expr = Expression.parseExpression(fieldFirst, args, vps),
+					result = expr.evaluate({});
+				if (result instanceof Array){
+					result.sort();
 				}
+				var errMsg = "for expression " + fieldFirst +
+					" with argument " + JSON.stringify(args) +
+					" full tree " + JSON.stringify(expr.serialize(false)) +
+					" expected: " + JSON.stringify(expected) +
+					" but got: " + JSON.stringify(result);
+				assert.deepEqual(result, expected, errMsg);
+				//TODO test optimize here
+			}
+		}
+		if (spec.error !== undefined && spec.error !== null) {
+			var asserters = spec.error,
+				n = asserters.length;
+			for (var i = 0; i < n; ++i) {
+				// var obj2 = {<asserters[i]>: args}; //NOTE: DEVIATION FROM MONGO: see parseExpression below
+				var idGenerator2 = new VariablesIdGenerator(),
+					vps2 = new VariablesParseState(idGenerator2);
+				assert.throws(function() {
+					// NOTE: parse and evaluatation failures are treated the same
+					expr = Expression.parseExpression(asserters[i], args, vps2);
+					expr.evaluate({});
+				}); // jshint ignore:line
+			}
+		}
+	};
+	return klass;
+})();
+
+exports.SetIsSubsetExpression = {
 
+	"constructor()": {
+
+		"should not throw Error when constructing without args": function() {
+			assert.doesNotThrow(function() {
+				new SetIsSubsetExpression();
+			});
+		},
+
+		"should throw Error when constructing with args": function() {
+			assert.throws(function() {
+				new SetIsSubsetExpression("someArg");
+			});
 		}
 
-};
+	},
+
+	"#getOpName()": {
+
+		"should return the correct op name; $setIsSubset": function() {
+			assert.equal(new SetIsSubsetExpression().getOpName(), "$setIsSubset");
+		}
+
+	},
+
+	"#evaluate()": {
+
+		"should handle when sets are the same": function Same(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], [1, 2]],
+					expected: {
+						$setIsSubset: true,
+						// $setEquals: true,
+						// $setIntersection: [1, 2],
+						// $setUnion: [1, 2],
+						// $setDifference: [],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the 2nd set has redundant items": function Redundant(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], [1, 2, 2]],
+					expected: {
+						$setIsSubset: true,
+						// $setEquals: true,
+						// $setIntersection: [1, 2],
+						// $setUnion: [1, 2],
+						// $setDifference: [],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the both sets have redundant items": function DoubleRedundant(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 1, 2], [1, 2, 2]],
+					expected: {
+						$setIsSubset: true,
+						// $setEquals: true,
+						// $setIntersection: [1, 2],
+						// $setUnion: [1, 2],
+						// $setDifference: [],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the 1st set is a superset": function Super(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], [1]],
+					expected: {
+						$setIsSubset: false,
+						// $setEquals: false,
+						// $setIntersection: [1],
+						// $setUnion: [1, 2],
+						// $setDifference: [2],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the 2nd set is a superset and has redundant items": function SuperWithRedundant(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2, 2], [1]],
+					expected: {
+						$setIsSubset: false,
+						// $setEquals: false,
+						// $setIntersection: [1],
+						// $setUnion: [1, 2],
+						// $setDifference: [2],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the 1st set is a subset": function Sub(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1], [1, 2]],
+					expected: {
+						$setIsSubset: true,
+						// $setEquals: false,
+						// $setIntersection: [1],
+						// $setUnion: [1, 2],
+						// $setDifference: [],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the sets are the same but backwards": function SameBackwards(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], [2, 1]],
+					expected: {
+						$setIsSubset: true,
+						// $setEquals: true,
+						// $setIntersection: [1, 2],
+						// $setUnion: [1, 2],
+						// $setDifference: [],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the sets do not overlap": function NoOverlap(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], [8, 4]],
+					expected: {
+						$setIsSubset: false,
+						// $setEquals: false,
+						// $setIntersection: [],
+						// $setUnion: [1, 2, 4, 8],
+						// $setDifference: [1, 2],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the sets do overlap": function Overlap(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], [8, 2, 4]],
+					expected: {
+						$setIsSubset: false,
+						// $setEquals: false,
+						// $setIntersection: [2],
+						// $setUnion: [1, 2, 4, 8],
+						// $setDifference: [1],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the 2nd set is null": function LastNull(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], null],
+					expected: {
+						// $setIntersection: null,
+						// $setUnion: null,
+						// $setDifference: null,
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+					],
+				},
+			}).run();
+		},
+
+		"should handle when the 1st set is null": function FirstNull(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [null, [1, 2]],
+					expected: {
+						// $setIntersection: null,
+						// $setUnion: null,
+						// $setDifference: null,
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+					],
+				},
+			}).run();
+		},
+
+		"should handle when the input has no args": function NoArg(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [],
+					expected: {
+						// $setIntersection: [],
+						// $setUnion: [],
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+						// "$setDifference"
+					],
+				},
+			}).run();
+		},
+
+		"should handle when the input has one arg": function OneArg(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2]],
+					expected: {
+						// $setIntersection: [1, 2],
+						// $setUnion: [1, 2],
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+						// "$setDifference"
+					],
+				},
+			}).run();
+		},
 
-if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);
+		"should handle when the input has empty arg": function EmptyArg(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2]],
+					expected: {
+						// $setIntersection: [1, 2],
+						// $setUnion: [1, 2],
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+						// "$setDifference"
+					],
+				},
+			}).run();
+		},
+
+		"should handle when the input has empty left arg": function LeftArgEmpty(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[]],
+					expected: {
+						// $setIntersection: [],
+						// $setUnion: [],
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+						// "$setDifference"
+					],
+				},
+			}).run();
+		},
+
+		"should handle when the input has empty right arg": function RightArgEmpty(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2], []],
+					expected: {
+						// $setIntersection: [],
+						// $setUnion: [1, 2],
+						$setIsSubset: false,
+						// $setEquals: false,
+						// $setDifference: [1, 2],
+					},
+				},
+			}).run();
+		},
+
+		"should handle when the input has many args": function ManyArgs(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[8, 3], ["asdf", "foo"], [80.3, 34], [], [80.3, "foo", 11, "yay"]],
+					expected: {
+						// $setIntersection: [],
+						// $setEquals: false,
+						// $setUnion: [3, 8, 11, 34, 80.3, "asdf", "foo", "yay"],
+					},
+					error: [
+						"$setIsSubset",
+						// "$setDifference",
+					],
+				},
+			}).run();
+		},
+
+		"should handle when the input has many args that are equal sets": function ManyArgsEqual(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[1, 2, 4], [1, 2, 2, 4], [4, 1, 2], [2, 1, 1, 4]],
+					expected: {
+						// $setIntersection: [1, 2, 4],
+						// $setEquals: false,
+						// $setUnion: [1, 2, 4],
+					},
+					error: [
+						"$setIsSubset",
+						// "$setDifference",
+					],
+				},
+			}).run();
+		},
+
+	},
+
+};

+ 242 - 235
test/lib/pipeline/expressions/Variables.js

@@ -2,251 +2,258 @@
 var assert = require("assert"),
 	Variables = require("../../../../lib/pipeline/expressions/Variables");
 
+// 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));
 
-module.exports = {
+exports.Variables = {
 
-	"Variables": {
+	"constructor": {
 
-		"constructor": {
+		"should be able to construct empty variables": function() {
+			new Variables();
+		},
+
+		"should be able to give number of variables": function() {
+			new Variables(5);
+		},
 
-			"Should be able to construct empty variables": function canConstructEmpty() {
+		"should throw if not given a number": function() {
+			assert.throws(function() {
+				new Variables('hi');
+			});
+			assert.throws(function() {
+				new Variables({});
+			});
+			assert.throws(function() {
+				new Variables([]);
+			});
+			assert.throws(function() {
+				new Variables(new Date());
+			});
+		},
+
+		"setValue throws if no args given": function() {
+			assert.throws(function() {
 				var variables = new Variables();
-			},
+				variables.setValue(1, 'hi');
+			});
+		},
 
-			"Should be able to give number of variables": function giveNumber() {
-				var variables = new Variables(5);
-			},
-
-			"Should throw if not given a number": function throwsOnInvalid() {
-				assert.throws(function() {
-					var variables = new Variables('hi');
-				});
-				assert.throws(function() {
-					var variables = new Variables({});
-				});
-				assert.throws(function() {
-					var variables = new Variables([]);
-				});
-				assert.throws(function() {
-					var variables = new Variables(new Date());
-				});
-			},
-
-			"setValue throws if no args given": function setValueThrows() {
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.setValue(1, 'hi');
-				});
-				
-			}
-
-		},
-
-		"#setRoot": {
-			"should set the _root variable to the passed value": function setsRoot() {
-				var variables = new Variables(),
-					root = {'hi':'hi'};
-				variables.setRoot(root);
-				assert.equal(root, variables._root);
-			},
-
-			"must be an object": function mustBeObject() {
-				var variables = new Variables(),
-					root = 'hi';
-				assert.throws(function() {
-					variables.setRoot(root);
-				});
-			}
-		},
-
-		"#clearRoot": {
-			"should set the _root variable to empty obj": function setsRootToEmpty() {
-				var variables = new Variables(),
-					root = {'hi':'hi'};
-				variables.setRoot(root);
-				variables.clearRoot();
-				assert.deepEqual({}, variables._root);
-			}
+	},
+
+	"#setRoot": {
+
+		"should set the _root variable to the passed value": function() {
+			var variables = new Variables(),
+				root = {'hi':'hi'};
+			variables.setRoot(root);
+			assert.equal(root, variables._root);
 		},
 
-		"#getRoot": {
-			"should return the _root variable": function returnsRoot() {
-				var variables = new Variables(),
-					root = {'hi':'hi'};
+		"must be an object": function mustBeObject() {
+			var variables = new Variables(),
+				root = 'hi';
+			assert.throws(function() {
 				variables.setRoot(root);
-				assert.equal(root, variables.getRoot());
-			}
-		},
-
-		"#setValue": {
-			"id must be number": function idMustBeNumber() {
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.setValue('hi', 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.setValue(null, 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.setValue(new Date(), 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.setValue([], 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.setValue({}, 5);
-				});
-				assert.doesNotThrow(function() {
-					var variables = new Variables(5);
-					variables.setValue(1, 5);
-				});
-			},
-
-			"cannot use root id": function cannotUseRootId() {
-				assert.throws(function() {
-					var variables = new Variables(5);
-					variables.setValue(Variables.ROOT_ID, 'hi');
-				});
-			},
-
-			"cannot use id larger than initial size": function idSizeIsCorrect() {
-				assert.throws(function() {
-					var variables = new Variables(5);
-					variables.setValue(5, 'hi'); //off by one check
-				});
-				assert.throws(function() {
-					var variables = new Variables(5);
-					variables.setValue(6, 'hi');
-				});
-			},
-
-			"sets the value": function setsTheValue() {
+			});
+		},
+
+	},
+
+	"#clearRoot": {
+
+		"should set the _root variable to empty obj": function() {
+			var variables = new Variables(),
+				root = {'hi':'hi'};
+			variables.setRoot(root);
+			variables.clearRoot();
+			assert.deepEqual({}, variables._root);
+		},
+
+	},
+
+	"#getRoot": {
+
+		"should return the _root variable": function() {
+			var variables = new Variables(),
+				root = {'hi':'hi'};
+			variables.setRoot(root);
+			assert.equal(root, variables.getRoot());
+		},
+
+	},
+
+	"#setValue": {
+
+		"id must be number": function() {
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.setValue('hi', 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.setValue(null, 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.setValue(new Date(), 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.setValue([], 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.setValue({}, 5);
+			});
+			assert.doesNotThrow(function() {
 				var variables = new Variables(5);
-				variables.setValue(1, 'hi'); //off by one check
-				assert.equal(variables._rest[1], 'hi');
-			}
-		},
-
-		"#getValue": {
-			"id must be number": function idMustBeNumber() {
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getValue('hi', 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getValue(null, 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getValue(new Date(), 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getValue([], 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getValue({}, 5);
-				});
-				assert.doesNotThrow(function() {
-					var variables = new Variables(5);
-					variables.getValue(1, 5);
-				});
-			},
-
-			"returns root when given root id": function returnsRoot() {
-				var variables = new Variables(5),
-					root = {hi:'hi'};
-				variables.setRoot(root);
-				variables.getValue(Variables.ROOT_ID, root);
-			},
-
-			"cannot use id larger than initial size": function idSizeIsCorrect() {
-				assert.throws(function() {
-					var variables = new Variables(5);
-					variables.getValue(5, 'hi'); //off by one check
-				});
-				assert.throws(function() {
-					var variables = new Variables(5);
-					variables.getValue(6, 'hi');
-				});
-			},
-
-			"gets the value": function getsTheValue() {
+				variables.setValue(1, 5);
+			});
+		},
+
+		"cannot use root id": function() {
+			assert.throws(function() {
 				var variables = new Variables(5);
-				variables.setValue(1, 'hi');
-				assert.equal(variables.getValue(1), 'hi');
-			}
-		},
-
-		"#getDocument": {
-			"id must be number": function idMustBeNumber() {
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getDocument('hi', 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getDocument(null, 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getDocument(new Date(), 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getDocument([], 5);
-				});
-				assert.throws(function() {
-					var variables = new Variables();
-					variables.getDocument({}, 5);
-				});
-				assert.doesNotThrow(function() {
-					var variables = new Variables(5);
-					variables.getDocument(1, 5);
-				});
-			},
-
-			"returns root when given root id": function returnsRoot() {
-				var variables = new Variables(5),
-					root = {hi:'hi'};
-				variables.setRoot(root);
-				variables.getDocument(Variables.ROOT_ID, root);
-			},
-
-			"cannot use id larger than initial size": function idSizeIsCorrect() {
-				assert.throws(function() {
-					var variables = new Variables(5);
-					variables.getDocument(5, 'hi'); //off by one check
-				});
-				assert.throws(function() {
-					var variables = new Variables(5);
-					variables.getDocument(6, 'hi');
-				});
-			},
-
-			"gets the value": function getsTheDocument() {
-				var variables = new Variables(5),
-					value = {hi:'hi'};
-				variables.setValue(1, value);
-				assert.equal(variables.getDocument(1), value);
-			},
-
-			"only returns documents": function returnsOnlyDocs() {
-				var variables = new Variables(5),
-					value = 'hi';
-				variables.setValue(1, value);
-				assert.deepEqual(variables.getDocument(1), {});
-			}
-		}
-
-	}
+				variables.setValue(Variables.ROOT_ID, 'hi');
+			});
+		},
 
-};
+		"cannot use id larger than initial size": function() {
+			assert.throws(function() {
+				var variables = new Variables(5);
+				variables.setValue(5, 'hi'); //off by one check
+			});
+			assert.throws(function() {
+				var variables = new Variables(5);
+				variables.setValue(6, 'hi');
+			});
+		},
+
+		"sets the value": function() {
+			var variables = new Variables(5);
+			variables.setValue(1, 'hi'); //off by one check
+			assert.equal(variables._rest[1], 'hi');
+		},
 
-if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);
+	},
+
+	"#getValue": {
+
+		"id must be number": function() {
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getValue('hi', 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getValue(null, 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getValue(new Date(), 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getValue([], 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getValue({}, 5);
+			});
+			assert.doesNotThrow(function() {
+				var variables = new Variables(5);
+				variables.getValue(1, 5);
+			});
+		},
+
+		"returns root when given root id": function() {
+			var variables = new Variables(5),
+				root = {hi:'hi'};
+			variables.setRoot(root);
+			variables.getValue(Variables.ROOT_ID, root);
+		},
+
+		"cannot use id larger than initial size": function() {
+			assert.throws(function() {
+				var variables = new Variables(5);
+				variables.getValue(5, 'hi'); //off by one check
+			});
+			assert.throws(function() {
+				var variables = new Variables(5);
+				variables.getValue(6, 'hi');
+			});
+		},
+
+		"gets the value": function() {
+			var variables = new Variables(5);
+			variables.setValue(1, 'hi');
+			assert.equal(variables.getValue(1), 'hi');
+		},
+
+	},
+
+	"#getDocument": {
+
+		"id must be number": function() {
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getDocument('hi', 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getDocument(null, 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getDocument(new Date(), 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getDocument([], 5);
+			});
+			assert.throws(function() {
+				var variables = new Variables();
+				variables.getDocument({}, 5);
+			});
+			assert.doesNotThrow(function() {
+				var variables = new Variables(5);
+				variables.getDocument(1, 5);
+			});
+		},
+
+		"returns root when given root id": function() {
+			var variables = new Variables(5),
+				root = {hi:'hi'};
+			variables.setRoot(root);
+			variables.getDocument(Variables.ROOT_ID, root);
+		},
+
+		"cannot use id larger than initial size": function() {
+			assert.throws(function() {
+				var variables = new Variables(5);
+				variables.getDocument(5, 'hi'); //off by one check
+			});
+			assert.throws(function() {
+				var variables = new Variables(5);
+				variables.getDocument(6, 'hi');
+			});
+		},
+
+		"gets the value": function() {
+			var variables = new Variables(5),
+				value = {hi:'hi'};
+			variables.setValue(1, value);
+			assert.equal(variables.getDocument(1), value);
+		},
+
+		"only returns documents": function() {
+			var variables = new Variables(5),
+				value = 'hi';
+			variables.setValue(1, value);
+			assert.deepEqual(variables.getDocument(1), {});
+		},
+
+	},
+
+};