EAGLESIX-2651: Substr: better sync w/ 2.6.5 code and tests

lib/pipeline/expressions/SubstrExpression.js

@@ -7,45 +7,36 @@
  * @namespace mungedb-aggregate.pipeline.expressions
  * @module mungedb-aggregate
  * @constructor
- **/
+ */
 var SubstrExpression = module.exports = function SubstrExpression() {
+	if (arguments.length !== 0) throw new Error(klass.name + ": no args expected");
 	base.call(this);
-}, klass = SubstrExpression,
-	FixedArityExpression = require("./FixedArityExpressionT")(klass, 3),
-	base = FixedArityExpression,
-	proto = klass.prototype = Object.create(base.prototype, {
-		constructor: {
-			value: klass
-		}
-	});
+}, klass = SubstrExpression, base = require("./FixedArityExpressionT")(klass, 3), proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
-// DEPENDENCIES
 var Value = require("../Value"),
 	Expression = require("./Expression");
 
-// PROTOTYPE MEMBERS
-proto.getOpName = function getOpName() {
-	return "$substr";
-};
-
-/**
- * Takes a string and two numbers. The first number represents the number of bytes in the string to skip, and the second number specifies the number of bytes to return from the string.
- * @method evaluateInternal
- **/
 proto.evaluateInternal = function evaluateInternal(vars) {
 	var string = this.operands[0].evaluateInternal(vars),
-		lower = this.operands[1].evaluateInternal(vars),
-		length = this.operands[2].evaluateInternal(vars),
-		str = Value.coerceToString(string);
+		pLower = this.operands[1].evaluateInternal(vars),
+		pLength = this.operands[2].evaluateInternal(vars);
 
-	if (typeof(lower) !== "number") throw new Error(this.getOpName() + ": starting index must be a numeric type; code 16034");
-	if (typeof(length) !== "number") throw new Error(this.getOpName() + ": length must be a numeric type; code 16035");
+	var str = Value.coerceToString(string);
+	if (typeof pLower !== "number") throw new Error(this.getOpName() + ":  starting index must be a numeric type (is type " + Value.getType(pLower) + "); uassert code 16034");
+	if (typeof pLength !== "number") throw new Error(this.getOpName() + ":  length must be a numeric type (is type " + Value.getType(pLength) + "); uassert code 16035");
 
-	// If lower > str.length() then string::substr() will throw out_of_range, so return an
-    // empty string if lower is not a valid string index.
-	if (lower >= str.length) return "";
+	var lower = Value.coerceToLong(pLower),
+		length = Value.coerceToLong(pLength);
+	if (lower >= str.length) {
+		// If lower > str.length() then string::substr() will throw out_of_range, so return an
+		// empty string if lower is not a valid string index.
+		return "";
+	}
 	return str.substr(lower, length);
 };
 
-/** Register Expression */
 Expression.registerExpression("$substr", base.parse);
+
+proto.getOpName = function getOpName() {
+	return "$substr";
+};

test/lib/pipeline/expressions/SubstrExpression_test.js

@@ -8,91 +8,102 @@ var assert = require("assert"),
 	constify = utils.constify,
 	expressionToJson = utils.expressionToJson;
 
-module.exports = {
+// Mocha one-liner to make these tests self-hosted
+if(!module.parent)return(require.cache[__filename]=null,(new(require("mocha"))({ui:"exports",reporter:"spec",grep:process.env.TEST_GREP})).addFile(__filename).run(process.exit));
+
+var TestBase = function TestBase(overrides) {
+		//NOTE: DEVIATION FROM MONGO: using this base class to make things easier to initialize
+		for (var key in overrides)
+			this[key] = overrides[key];
+	},
+	ExpectedResultBase = (function() {
+		var klass = function ExpectedResultBase() {
+			base.apply(this, arguments);
+		}, base = TestBase, proto = klass.prototype = Object.create(base.prototype);
+		proto.run = function(){
+			var specElement = this.spec(),
+				idGenerator = new VariablesIdGenerator(),
+				vps = new VariablesParseState(idGenerator),
+				expr = Expression.parseOperand(specElement, vps);
+			assert.deepEqual(constify(specElement), expressionToJson(expr));
+			assert.deepEqual(this.expectedResult, expr.evaluate({}));
+		};
+		proto.spec = function() { return {$substr:[this.str, this.offset, this.length]}; };
+		return klass;
+	})();
+
+exports.SubstrExpression = {
+
+	"constructor()": {
+
+		"should construct instance": function() {
+			assert(new SubstrExpression() instanceof SubstrExpression);
+			assert(new SubstrExpression() instanceof Expression);
+		},
 
-	"SubstrExpression": {
+		"should error if given args": function() {
+			assert.throws(function() {
+				new SubstrExpression("bad stuff");
+			});
+		},
 
-		"constructor()": {
+	},
 
-			"should not throw Error when constructing without args": function testConstructor() {
-				assert.doesNotThrow(function() {
-					new SubstrExpression();
-				});
-			}
+	"#getOpName()": {
 
+		"should return the correct op name; $substr": function() {
+			assert.equal(new SubstrExpression().getOpName(), "$substr");
 		},
 
-		"#getOpName()": {
+	},
 
-			"should return the correct op name; $substr": function testOpName() {
-				assert.equal(new SubstrExpression().getOpName(), "$substr");
-			}
+	"evaluate": {
 
+		"should return full string (if contains null)": function FullNull() {
+			new ExpectedResultBase({
+				str: "a\0b",
+				offset: 0,
+				length: 3,
+				get expectedResult(){ return this.str; },
+			}).run();
 		},
 
-		"evaluate": {
-
-			"before": function before() {
-				this.run = function run() {
-					var idGenerator = new VariablesIdGenerator(),
-						vps = new VariablesParseState(idGenerator),
-						spec = this.spec(),
-						expectedResult = this.expectedResult,
-						expression = Expression.parseOperand(spec, vps);
-					assert.deepEqual(constify(spec), expressionToJson(expression));
-					assert.equal(expectedResult, expression.evaluate({}));
-				};
-				this.str = undefined;
-				this.offset = undefined;
-				this.length = undefined;
-				this.expectedResult = undefined;
-				this.spec = function spec() {return {$substr:[this.str, this.offset, this.length]}; };
-			},
-
-			"FullNull": function FullNull() {
-				this.str = "a\0b";
-				this.offset = 0;
-				this.length = 3;
-				this.expectedResult = this.str;
-				this.run();
-			},
-
-			"BeginAtNull": function BeginAtNull() {
-				this.str = "a\0b";
-				this.offset = 1;
-				this.length = 2;
-				this.expectedResult = "\0b";
-				this.run();
-			},
-
-			"EndAtNull": function EndAtNull() {
-				this.str = "a\0b";
-				this.offset = 0;
-				this.length = 2;
-				this.expectedResult = "a\0";
-				this.run();
-			},
-
-			"DropBeginningNull": function DropBeginningNull() {
-				this.str = "\0b";
-				this.offset = 1;
-				this.length = 1;
-				this.expectedResult = "b";
-				this.run();
-			},
-
-			"DropEndingNull": function DropEndingNull() {
-				this.str = "a\0";
-				this.offset = 0;
-				this.length = 1;
-				this.expectedResult = "a";
-				this.run();
-			},
-
-
-		}
-	}
+		"should return tail of string (if begin at null)": function BeginAtNull() {
+			new ExpectedResultBase({
+				str: "a\0b",
+				offset: 1,
+				length: 2,
+				expectedResult: "\0b",
+			}).run();
+		},
 
-};
+		"should return head of string (if end at null)": function EndAtNull() {
+			new ExpectedResultBase({
+				str: "a\0b",
+				offset: 0,
+				length: 2,
+				expectedResult: "a\0",
+			}).run();
+		},
 
-if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);
+		"should return tail of string (if head has null) ": function DropBeginningNull() {
+			new ExpectedResultBase({
+				str: "\0b",
+				offset: 1,
+				length: 1,
+				expectedResult: "b",
+			}).run();
+		},
+
+		"should return head of string (if tail has null)": function DropEndingNull() {
+			new ExpectedResultBase({
+				str: "a\0",
+				offset: 0,
+				length: 1,
+				expectedResult: "a",
+			}).run();
+		},
+
+	},
+
+};