EAGLESIX-2651: Expression: fixes during FieldPath testing

lib/pipeline/expressions/Expression.js

@@ -64,31 +64,19 @@ var ObjectCtx = Expression.ObjectCtx = (function() {
 })();
 
 
-/**
- * Produce a field path string with the field prefix removed.
- * Throws an error if the field prefix is not present.
- *
- * @static
- * @param prefixedField the prefixed field
- * @returns the field path with the prefix removed
- **/
-klass.removeFieldPrefix = function removeFieldPrefix(prefixedField) {
-	if (prefixedField.indexOf("\0") != -1) throw new Error("field path must not contain embedded null characters; uassert code 16419");
-	if (prefixedField[0] !== "$") throw new Error("field path references must be prefixed with a '$' ('" + prefixedField + "'); uassert code 15982");
-	return prefixedField.substr(1);
-};
-
+//
+// Diagram of relationship between parse functions when parsing a $op:
+//
+// { someFieldOrArrayIndex: { $op: [ARGS] } }
+//                             ^ parseExpression on inner $op BSONElement
+//                          ^ parseObject on BSONObject
+//             ^ parseOperand on outer BSONElement wrapping the $op Object
+//
 
 /**
- * Parse an Object.  The object could represent a functional expression or a Document expression.
- *
- * An object expression can take any of the following forms:
- *
- *      f0: {f1: ..., f2: ..., f3: ...}
- *      f0: {$operator:[operand1, operand2, ...]}
- *
- * @static
+ * Parses a JSON Object that could represent a functional expression or a Document expression.
  * @method parseObject
+ * @static
  * @param obj   the element representing the object
  * @param ctx   a MiniCtx representing the options above
  * @param vps	Variables Parse State
@@ -96,6 +84,12 @@ klass.removeFieldPrefix = function removeFieldPrefix(prefixedField) {
  */
 klass.parseObject = function parseObject(obj, ctx, vps) {
 	if (!(ctx instanceof ObjectCtx)) throw new Error("ctx must be ObjectCtx");
+    /*
+      An object expression can take any of the following forms:
+
+      f0: {f1: ..., f2: ..., f3: ...}
+      f0: {$operator:[operand1, operand2, ...]}
+    */
 
 	var expression, // the result
 		expressionObject, // the alt result
@@ -196,6 +190,7 @@ klass.registerExpression = function registerExpression(key, parserFunc) {
 };
 
 
+//NOTE: DEVIATION FROM MONGO: the c++ version has 2 arguments, not 3.	//TODO: could easily fix this inconsistency
 /**
  * Parses a BSONElement which has already been determined to be functional expression.
  * @static
@@ -205,12 +200,13 @@ klass.registerExpression = function registerExpression(key, parserFunc) {
  * @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)) {
-		throw new Error("Invalid operator : " + exprElementKey + "; code 15999");
-	}
-	return Expression.expressionParserMap[exprElementKey](exprElementValue, vps);
+	var opName = exprElementKey,
+		op = Expression.expressionParserMap;
+	if (!op) throw new Error("invalid operator : " + exprElementKey + "; uassert code 15999");
+
+    // make the expression node
+	return op(exprElementValue, vps);
 };
 
 
@@ -230,84 +226,109 @@ klass.parseExpression = function parseExpression(exprElementKey, exprElementValu
  */
 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
+	if (t === "string" && exprElement[0] === "$") {
+		//if we got here, this is a field path expression
 	    return new FieldPathExpression.parse(exprElement, vps);
 	} else if (t === "object" && exprElement && exprElement.constructor === Object) {
-		return Expression.parseObject(exprElement, new ObjectCtx({
+		var oCtx = new ObjectCtx({
 			isDocumentOk: true
-		}), vps);
+		});
+		return Expression.parseObject(exprElement, oCtx, vps);
 	} else {
 		return ConstantExpression.parse(exprElement, vps);
 	}
 };
 
 
-/**
- * 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!");
-};
-
-
-/**
- * Evaluate expression with specified inputs and return result.
- *
- * 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 evaluate(vars) {
-	return this.evaluateInternal(vars);
-};
-
-
 /**
  * Optimize the Expression.
  *
  * This provides an opportunity to do constant folding, or to collapse nested
- *  operators that have the same precedence, such as $add, $and, or $or.
+ * operators that have the same precedence, such as $add, $and, or $or.
  *
  * The Expression should be replaced with the return value, which may or may
- *  not be the same object.  In the case of constant folding, a computed
- *  expression may be replaced by a constant.
+ * not be the same object.  In the case of constant folding, a computed
+ * expression may be replaced by a constant.
  *
  * @method optimize
  * @returns the optimized Expression
  */
 proto.optimize = function optimize() {
-	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
+	return this;
 };
 
+
 /**
- * Add this expression's field dependencies to the set Expressions are trees, so this is often recursive.
- *
- * Top-level ExpressionObject gets pointer to empty vector.
- * If any other Expression is an ancestor, or in other cases where {a:1} inclusion objects aren't allowed, they get NULL.
+ * Add this expression's field dependencies to the set.
+ * Expressions are trees, so this is often recursive.
  *
  * @method addDependencies
- * @param deps  output parameter
- * @param path  path to self if all ancestors are ExpressionObjects.
+ * @param deps Fully qualified paths to depended-on fields are added to this set.
+ *             Empty string means need full document.
+ * @param path path to self if all ancestors are ExpressionObjects.
+ *             Top-level ExpressionObject gets pointer to empty vector.
+ *             If any other Expression is an ancestor, or in other cases
+ *             where {a:1} inclusion objects aren't allowed, they get
+ *             NULL.
  */
 proto.addDependencies = function addDependencies(deps, path) {
 	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
 };
 
+
 /**
  * simple expressions are just inclusion exclusion as supported by ExpressionObject
- * @method getIsSimple
+ * @method isSimple
  */
-proto.getIsSimple = function getIsSimple() {
+proto.isSimple = function isSimple() {
 	return false;
 };
 
+/**
+ * Serialize the Expression tree recursively.
+ * If explain is false, returns a Value parsable by parseOperand().
+ * @method serialize
+ */
+proto.serialize = function serialize(explain) {
+	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
+};
 
-proto.toMatcherBson = function toMatcherBson() {
-	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!"); //verify(false && "Expression::toMatcherBson()");
+/**
+ * Evaluate expression with specified inputs and return result.
+ *
+ * While vars is non-const, if properly constructed, subexpressions modifications to it
+ * should not effect outer expressions due to unique variable Ids.
+ *
+ * @method evaluate
+ * @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)
+	return this.evaluateInternal(vars);
+};
+
+/**
+ * Produce a field path string with the field prefix removed.
+ * Throws an error if the field prefix is not present.
+ * @method removeFieldPrefix
+ * @static
+ * @param prefixedField the prefixed field
+ * @returns the field path with the prefix removed
+ */
+klass.removeFieldPrefix = function removeFieldPrefix(prefixedField) {
+	if (prefixedField.indexOf("\0") != -1) throw new Error("field path must not contain embedded null characters; uassert code 16419");
+	if (prefixedField[0] !== "$") throw new Error("field path references must be prefixed with a '$' ('" + prefixedField + "'); uassert code 15982");
+	return prefixedField.substr(1);
 };
 
+/**
+ * Evaluate the subclass Expression using the given Variables as context and return result.
+ * @method evaluate
+ * @returns the computed value
+ */
+proto.evaluateInternal = function evaluateInternal(vars) {
+	throw new Error("WAS NOT IMPLEMENTED BY INHERITOR!");
+};
 
 var ObjectExpression = require("./ObjectExpression"),
 	FieldPathExpression = require("./FieldPathExpression"),