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

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;
-	}
-}
+	},
+
+};

lib/pipeline/expressions/SetIsSubsetExpression.js

@@ -10,107 +10,88 @@
  **/
 
 var SetIsSubsetExpression = module.exports = function SetIsSubsetExpression() {
-
-	if (arguments.length !== 0) throw new Error("SetIsSubsetExpression constructor must be called with no args");
-
+	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}});
 
-// DEPENDENCIES
 var Value = require("../Value"),
 	Expression = require("./Expression"),
+	NaryExpression = require("./NaryExpression"),
+	ConstantExpression = require("./ConstantExpression"),
 	Helpers = require("./Helpers");
 
-// PROTOTYPE MEMBERS
-proto.getOpName = function getOpName() {
-	return "$setIsSubset";
-};
-
-
-// lhs should be array, rhs should be set (object). See arrayToSet implementation.
-var setIsSubsetHelper = function setIsSubsetHelper(lhs, rhs){
-
-	var rkeys = Object.keys(rhs);
-
-	// do not short-circuit when lhs.size() > rhs.size()
+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
-	for (var i = 0; i < lhs.length; i++){
-		if (rkeys.indexOf(JSON.stringify(lhs[i])) < 0) { //TODO Why do we stringify the lhs and not the rhs?
-			return false
+	for (var i = 0; i < lhs.length; i++) {
+		if (!(JSON.stringify(lhs[i]) in rhs)) {
+			return false;
 		}
 	}
-
 	return true;
 };
 
-/**
- * Takes 2 arrays. Returns true if the first is a subset of the second. Returns false otherwise.
- * @method evaluateInternal
- **/
 proto.evaluateInternal = function evaluateInternal(vars) {
-	if (this.operands.length !== 2) throw new Error("two args expected");
 	var lhs = this.operands[0].evaluateInternal(vars),
 		rhs = this.operands[1].evaluateInternal(vars);
 
-	if (!(lhs instanceof Array)) throw new Error("Both operands of " + this.getOpName() + ": must be arrays. First argument is of type " + typeof lhs+"; code 17046");
-	if (!(rhs instanceof Array)) throw new Error("Both operands of " + this.getOpName() + ": must be arrays. First argument is of type " + typeof rhs+"; code 17042");
+	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");
 
 	return setIsSubsetHelper(lhs, Helpers.arrayToSet(rhs));
-
 };
 
 
-
-var Optimized = function Optimized(cachedRhsSet, operands) {
-	this.operands = operands;
+/**
+ * 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 = 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("uassert 17310: both operands of " + this.getOpName() + "  must be arrays. First argument is of type " + typeof lhs);
+	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");
 
 	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 (this.operands[1] instanceof ConstantExpression){
-		var ce = this.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;
-
 };
 
-/** Register Expression */
 Expression.registerExpression("$setIsSubset", base.parse);
+
+proto.getOpName = function getOpName() {
+	return "$setIsSubset";
+};

test/lib/pipeline/expressions/SetIsSubsetExpression.js

@@ -1,137 +1,378 @@
 "use strict";
 var assert = require("assert"),
-		VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
-		VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
-		SetIsSubsetExpression = require("../../../../lib/pipeline/expressions/SetIsSubsetExpression"),
-		Expression = require("../../../../lib/pipeline/expressions/Expression");
+	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));
 
-function errMsg(expr, args, tree, expected, result) {
-	return 	"for expression " + expr +
-			" with argument " + args +
-			" full tree: " + JSON.stringify(tree) +
-			" expected: " + expected +
-			" result: " + result;
-}
+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;
+})();
 
-module.exports = {
+exports.SetIsSubsetExpression = {
 
-	"SetIsSubsetExpression": {
+	"constructor()": {
 
-		"constructor()": {
+		"should not throw Error when constructing without args": function() {
+			assert.doesNotThrow(function() {
+				new SetIsSubsetExpression();
+			});
+		},
 
-			"should not throw Error when constructing without args": function testConstructor() {
-				assert.doesNotThrow(function() {
-					new SetIsSubsetExpression();
-				});
-			},
+		"should throw Error when constructing with args": function() {
+			assert.throws(function() {
+				new SetIsSubsetExpression("someArg");
+			});
+		}
 
-			"should throw Error when constructing with args": function testConstructor() {
-				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();
 		},
 
-		"#getOpName()": {
+		"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 return the correct op name; $setIsSubset": function testOpName() {
-				assert.equal(new SetIsSubsetExpression().getOpName(), "$setIsSubset");
-			}
+		"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();
 		},
 
-		"#evaluateInternal()": {
+		"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();
+		},
 
-			beforeEach: function() {
-				this.compare = function(array1, array2, expected) {
-					var input = [array1, array2],
-						idGenerator = new VariablesIdGenerator(),
-						vps = new VariablesParseState(idGenerator),
-						expr = Expression.parseExpression("$setIsSubset", input, vps),
-						result = expr.evaluate({}),
-						msg = errMsg("$setIsSubset", input, expr.serialize(false), expected, result);
-					assert.equal(result, expected, msg);
-				}
-			},
+		"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 fail if array1 is not an array": function testArg1() {
-				var array1 = "not an array",
-					array2 = [6, 7, 8, 9],
-					input = [array1,array2],
-					idGenerator = new VariablesIdGenerator(),
-					vps = new VariablesParseState(idGenerator),
-					expr = Expression.parseExpression("$setIsSubset", input, vps);
-				assert.throws(function () {
-					expr.evaluate({});
-				});
-			},
-
-			"Should fail if array2 is not an array": function testArg2() {
-				var array1 = [1, 2, 3, 4],
-					array2 = "not an array",
-					input = [array1,array2],
-					idGenerator = new VariablesIdGenerator(),
-					vps = new VariablesParseState(idGenerator),
-					expr = Expression.parseExpression("$setIsSubset", input, vps);
-				assert.throws(function () {
-						expr.evaluate({});
-				});
-			},
-
-			"Should fail if both are not an array": function testArg1andArg2() {
-				var array1 = "not an array",
-					array2 = "not an array",
-					input = [array1,array2],
-					idGenerator = new VariablesIdGenerator(),
-					vps = new VariablesParseState(idGenerator),
-					expr = Expression.parseExpression("$setIsSubset", input, vps);
-				assert.throws(function () {
-					expr.evaluate({});
-				});
-			},
+		"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 pass and return a true": function(){
-				this.compare([2, 3], [1, 2, 3, 4, 5], true);
-			},
+		"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 pass and return false": function() {
-				this.compare([1, 2, 3, 4, 5], [7, 8, 9], false);
-			},
+		"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 return false when the 1st array is not empty and the 2nd array is": function() {
-				this.compare([1, 2, 3, 4, 5], [], false);
-			},
+		"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 return true if an 1st array is empty and the 2nd is not": function () {
-				this.compare([],[1, 2, 3, 4, 5], true);
-			},
+		"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 return true if both are empty": function () {
-				this.compare([],[],true);
-			},
+		"should handle when the input has no args": function NoArg(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [],
+					expected: {
+						// $setIntersection: [],
+						// $setUnion: [],
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+						// "$setDifference"
+					],
+				},
+			}).run();
+		},
 
-			"should not consider a non-nested source array the same as a nested object array": function() {
-				this.compare([1], [[1]], false);
-			},
+		"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();
+		},
 
-			"should not consider a nested source the same as an unnested object array": function(){
-				this.compare([[1]], [1], false);
-			},
+		"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 ignore dups in the source": function(){
-				this.compare([1,2,1], [1,2], true);
-			},
+		"should handle when the input has empty left arg": function LeftArgEmpty(){
+			new ExpectedResultBase({
+				getSpec: {
+					input: [[]],
+					expected: {
+						// $setIntersection: [],
+						// $setUnion: [],
+					},
+					error: [
+						// "$setEquals"
+						"$setIsSubset"
+						// "$setDifference"
+					],
+				},
+			}).run();
+		},
 
-			"should know the difference between a number and a string": function(){
-				this.compare([1], ["1"], true);
-			}
+		"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();
+		},
 
-	}
+	},
 
 };
 
-if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);
+if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);