EAGLESIX-2651: Expression: fix for parseObject isTopLevel, fix misc comments

lib/pipeline/expressions/Expression.js

@@ -12,26 +12,21 @@
  * @namespace mungedb-aggregate.pipeline.expressions
  * @module mungedb-aggregate
  * @constructor
- **/
-
-
+ */
 var Expression = module.exports = function Expression() {
 	if (arguments.length !== 0) throw new Error("zero args expected");
-}, klass = Expression,
-    base = Object,
-    proto = klass.prototype = Object.create(base.prototype, {
-		constructor: {
-			value: klass
-		}
-    });
+}, klass = Expression, base = Object, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+
+
+var Value = require("../Value"),
+	Document = require("../Document");
 
 
-// NESTED CLASSES
 /**
  * Reference to the `mungedb-aggregate.pipeline.expressions.Expression.ObjectCtx` class
  * @static
  * @property ObjectCtx
- **/
+ */
 var ObjectCtx = Expression.ObjectCtx = (function() {
 	// CONSTRUCTOR
 	/**
@@ -47,7 +42,7 @@ var ObjectCtx = Expression.ObjectCtx = (function() {
 	 *      @param [opts.isDocumentOk]      {Boolean}
 	 *      @param [opts.isTopLevel]        {Boolean}
 	 *      @param [opts.isInclusionOk]     {Boolean}
-	 **/
+	 */
 	var klass = function ObjectCtx(opts /*= {isDocumentOk:..., isTopLevel:..., isInclusionOk:...}*/ ) {
 		if (!(opts instanceof Object && opts.constructor == Object)) throw new Error("opts is required and must be an Object containing named args");
 		for (var k in opts) { // assign all given opts to self so long as they were part of klass.prototype as undefined properties
@@ -83,6 +78,7 @@ klass.removeFieldPrefix = function removeFieldPrefix(prefixedField) {
 	return prefixedField.substr(1);
 };
 
+
 /**
  * Parse an Object.  The object could represent a functional expression or a Document expression.
  *
@@ -97,7 +93,7 @@ klass.removeFieldPrefix = function removeFieldPrefix(prefixedField) {
  * @param ctx   a MiniCtx representing the options above
  * @param vps	Variables Parse State
  * @returns the parsed Expression
- **/
+ */
 klass.parseObject = function parseObject(obj, ctx, vps) {
 	if (!(ctx instanceof ObjectCtx)) throw new Error("ctx must be ObjectCtx");
 
@@ -120,24 +116,27 @@ klass.parseObject = function parseObject(obj, ctx, vps) {
 			if (ctx.isTopLevel)
 				throw new Error("$expressions are not allowed at the top-level of $project; uassert code 16404");
 
-			kind = OPERATOR; //we've determined this "object" is an operator expression
+			// we've determined this "object" is an operator expression
+			kind = OPERATOR;
 
 			expression = Expression.parseExpression(fieldName, obj[fieldName], vps); //NOTE: DEVIATION FROM MONGO: c++ code uses 2 arguments. See #parseExpression
 		} else {
 			if (kind === OPERATOR)
 				throw new Error("this object is already an operator expression, and can't be used as a document expression (at '" + fieldName + "'.; uassert code 15990");
 
-			if (!ctx.isTopLevel && fieldName.indexOf(".") != -1)
+			if (!ctx.isTopLevel && fieldName.indexOf(".") !== -1)
 				throw new Error("dotted field names are only allowed at the top level; uassert code 16405");
 
-			if (expression === undefined) { // if it's our first time, create the document expression
-				if (!ctx.isDocumentOk)
-					throw new Error("Assertion failure"); // CW TODO error: document not allowed in this context
+			// if it's our first time, create the document expression
+			if (expression === undefined) {
+				if (!ctx.isDocumentOk) throw new Error("Assertion failure");
+				// CW TODO error: document not allowed in this context
 
-				expressionObject = new ObjectExpression(); //check for top level? //NOTE: DEVIATION FROM MONGO: the c++ calls createRoot() or create() here.
+				expressionObject = ctx.isTopLevel ? ObjectExpression.createRoot() : ObjectExpression.create();
 				expression = expressionObject;
 
-				kind = NOTOPERATOR; //this "object" is not an operator expression
+				// this "object" is not an operator expression
+				kind = NOTOPERATOR;
 			}
 
 			var fieldValue = obj[fieldName];
@@ -153,8 +152,9 @@ klass.parseObject = function parseObject(obj, ctx, vps) {
 
 					break;
 				case "string":
-					// it's a renamed field         // CW TODO could also be a constant
-					expressionObject.addField(fieldName, new FieldPathExpression.parse(fieldValue, vps));
+					// it's a renamed field
+					// CW TODO could also be a constant
+					expressionObject.addField(fieldName, FieldPathExpression.parse(fieldValue, vps));
 					break;
 				case "boolean":
 				case "number":
@@ -170,7 +170,7 @@ klass.parseObject = function parseObject(obj, ctx, vps) {
 					}
 					break;
 				default:
-					throw new Error("disallowed field type " + (fieldValue instanceof Object ? fieldValue.constructor.name + ":" : typeof fieldValue) + typeof(fieldValue) + " in object expression (at '" + fieldName + "') uassert code 15992");
+					throw new Error("disallowed field type " + Value.getType(fieldValue) + " in object expression (at '" + fieldName + "') uassert code 15992");
 			}
 		}
 	}
@@ -181,10 +181,11 @@ klass.parseObject = function parseObject(obj, ctx, vps) {
 
 klass.expressionParserMap = {};
 
-/** Registers an ExpressionParser so it can be called from parseExpression and friends.
- *
- *  As an example, if your expression looks like {"$foo": [1,2,3]} you would add this line:
- *  REGISTER_EXPRESSION("$foo", ExpressionFoo::parse);
+
+/**
+ * Registers an ExpressionParser so it can be called from parseExpression and friends.
+ * As an example, if your expression looks like {"$foo": [1,2,3]} you would add this line:
+ * REGISTER_EXPRESSION("$foo", ExpressionFoo::parse);
  */
 klass.registerExpression = function registerExpression(key, parserFunc) {
 	if (key in klass.expressionParserMap) {
@@ -194,6 +195,7 @@ klass.registerExpression = function registerExpression(key, parserFunc) {
 	return 1;
 };
 
+
 /**
  * Parses a BSONElement which has already been determined to be functional expression.
  * @static
@@ -202,7 +204,7 @@ klass.registerExpression = function registerExpression(key, parserFunc) {
  *    That is the field name should be the $op for the expression.
  * @param vps the variable parse state
  * @returns the parsed Expression
- **/
+ */
 //NOTE: DEVIATION FROM MONGO: the c++ version has 2 arguments, not 3.	//TODO: could easily fix this inconsistency
 klass.parseExpression = function parseExpression(exprElementKey, exprElementValue, vps) {
 	if (!(exprElementKey in Expression.expressionParserMap)) {
@@ -211,6 +213,7 @@ klass.parseExpression = function parseExpression(exprElementKey, exprElementValu
 	return Expression.expressionParserMap[exprElementKey](exprElementValue, vps);
 };
 
+
 /**
  * Parses a BSONElement which is an operand in an Expression.
  *
@@ -224,7 +227,7 @@ klass.parseExpression = function parseExpression(exprElementKey, exprElementValu
  *    That is the field name should be the $op for the expression.
  * @param vps the variable parse state
  * @returns the parsed operand, as an Expression
- **/
+ */
 klass.parseOperand = function parseOperand(exprElement, vps) {
 	var t = typeof(exprElement);
 	if (t === "string" && exprElement[0] == "$") { //if we got here, this is a field path expression
@@ -238,13 +241,13 @@ klass.parseOperand = function parseOperand(exprElement, vps) {
 	}
 };
 
-// PROTOTYPE MEMBERS
+
 /**
  * Evaluate the Expression using the given document as input.
  *
  * @method evaluate
  * @returns the computed value
- **/
+ */
 proto.evaluateInternal = function evaluateInternal(obj) {
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
 };
@@ -256,7 +259,7 @@ proto.evaluateInternal = function evaluateInternal(obj) {
  * While vars is non-const, if properly constructed, subexpressions modifications to it
  * should not effect outer expressions due to unique variable Ids.
  */
-proto.evaluate = function(vars) {
+proto.evaluate = function evaluate(vars) {
 	return this.evaluateInternal(vars);
 };
 
@@ -273,7 +276,7 @@ proto.evaluate = function(vars) {
  *
  * @method optimize
  * @returns the optimized Expression
- **/
+ */
 proto.optimize = function optimize() {
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
 };
@@ -287,7 +290,7 @@ proto.optimize = function optimize() {
  * @method addDependencies
  * @param deps  output parameter
  * @param path  path to self if all ancestors are ExpressionObjects.
- **/
+ */
 proto.addDependencies = function addDependencies(deps, path) {
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
 };
@@ -295,18 +298,17 @@ proto.addDependencies = function addDependencies(deps, path) {
 /**
  * simple expressions are just inclusion exclusion as supported by ExpressionObject
  * @method getIsSimple
- **/
+ */
 proto.getIsSimple = function getIsSimple() {
 	return false;
 };
 
+
 proto.toMatcherBson = function toMatcherBson() {
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!"); //verify(false && "Expression::toMatcherBson()");
 };
 
 
-// DEPENDENCIES
-var Document = require("../Document");
-var ObjectExpression = require("./ObjectExpression");
-var FieldPathExpression = require("./FieldPathExpression");
-var ConstantExpression = require("./ConstantExpression");
+var ObjectExpression = require("./ObjectExpression"),
+	FieldPathExpression = require("./FieldPathExpression"),
+	ConstantExpression = require("./ConstantExpression");