Finished up high level group test case and fixed a bunch of bugs to make it pass. fixes #934

http://source.rd.rcg.local/trac/eagle6/changeset/1362/Eagle6_SVN

lib/pipeline/accumulators/AddToSetAccumulator.js

@@ -34,10 +34,7 @@ var AddToSetAccumulator = module.exports = (function(){
 		if(arguments.length !== 1) throw new Error("One and only one arg expected");
 		var rhs = this.operands[0].evaluate(doc);
 		if ('undefined' != typeof rhs) {
-			Value.verifyArray(rhs);
-			for(var i=0; i<rhs.length; i++) {
-				this.set.set(rhs[i], rhs[i]); //Sorry about variable names here... just following the rules!
-			}
+			this.set.set(rhs, rhs); //Sorry about variable names here... just following the rules!
 		}
 		return undefined;
 	};

lib/pipeline/accumulators/MinMaxAccumulator.js

@@ -45,7 +45,7 @@ var MinMaxAccumulator = module.exports = (function(){
 		var prhs = this.operands[0].evaluate(doc);
 
 		/* if this is the first value, just use it */
-		if (!base.prototype.getValue.call(this))
+		if (!this.hasOwnProperty('value'))
 			this.value = prhs;
 		else {
 			/* compare with the current value; swap if appropriate */

lib/pipeline/accumulators/SingleValueAccumulator.js

@@ -13,9 +13,8 @@ var SingleValueAccumulator = module.exports = (function(){
 	 * @constructor
 	**/
 
-	var klass = module.exports = function AccumulatorSingleValue(value){
+	var klass = module.exports = function AccumulatorSingleValue(){
 		if(arguments.length > 1 ) throw new Error("expects a single value");
-		this.value = value;
 		base.call(this);
 	}, Accumulator = require("./Accumulator"), base = Accumulator, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 

lib/pipeline/documentSources/GroupDocumentSource.js

@@ -35,21 +35,20 @@ var DocumentSource = require("./DocumentSource"),
 		"$avg": Accumulators.Avg,
 		"$first": Accumulators.First,
 		"$last": Accumulators.Last,
-		"$max": Accumulators.MinMax.bind(null, 1),
-		"$min": Accumulators.MinMax.bind(null, -1),
+		"$max": Accumulators.MinMax.createMax,
+		"$min": Accumulators.MinMax.createMin,
 		"$push": Accumulators.Push,
 		"$sum": Accumulators.Sum
 	};
 
-    klass.createFromJson = function createFromJson(groupElement) {
+    klass.createFromJson = function createFromJson(groupObj) {
 
-		if(!(groupElement instanceof Object && groupElement.constructor.name === "Object") || Object.keys(groupElement).length < 1)
+		if(!(groupObj instanceof Object && groupObj.constructor.name === "Object"))
 			throw new Error("a group's fields must be specified in an object");
 
         var idSet = false,
-            group = new GroupDocumentSource(),
-            groupObj = groupElement[group.getSourceName()];
-
+            group = new GroupDocumentSource();
+            
 		for(var groupFieldName in groupObj){
 			if(groupObj.hasOwnProperty(groupFieldName)){
 				var groupField = groupObj[groupFieldName];
@@ -143,9 +142,9 @@ var DocumentSource = require("./DocumentSource"),
 		return typeStr;	
 	};
 
-
+	klass.groupName = "$group";
 	proto.getSourceName = function getSourceName(){
-		return "$group";
+		return klass.groupName;
 	};
 
 	proto.advance = function advance(){
@@ -202,11 +201,12 @@ var DocumentSource = require("./DocumentSource"),
 
             var idHash = JSON.stringify(_id); //! @todo USE A REAL HASH.  I didn't have time to take collision into account.
 
-			if(_id in this.groups){
+			if(idHash in this.groups){
 				group = this.groups[idHash];
 			}else{
 				this.groups[idHash] = group = [];
 				this.groupsKeys[this.currentGroupsKeysIndex] = idHash;
+				++this.currentGroupsKeysIndex;
 				for(var ai =0; ai < this.accumulatorFactories.length; ++ai){
 					var accumulator = new this.accumulatorFactories[ai]();
 					accumulator.addOperand(this.expressions[ai]);
@@ -219,13 +219,13 @@ var DocumentSource = require("./DocumentSource"),
 			for(var gi=0; gi < group.length; ++gi)
 				group[gi].evaluate(currentDocument);
 
-			this.currentGroupsKeysIndex = 0; // Start the group
-			if(this.currentGroupsKeysIndex < this.groupsKeys.length)
-				this.currentDocument = this.makeDocument(this.currentGroupsKeysIndex);
-			this.populated = true;
-
 		}
-
+		
+		this.currentGroupsKeysIndex = 0; // Start the group
+		if(this.groupsKeys.length > 0)
+			this.currentDocument = this.makeDocument(this.currentGroupsKeysIndex);
+		this.populated = true;
+		
 	};
 
 
@@ -238,15 +238,28 @@ var DocumentSource = require("./DocumentSource"),
 
 		for(var i = 0; i < this.fieldNames.length; ++i){
 			var fieldName = this.fieldNames[i],
-				idx = this.groupsKeys[i];
-			if((idx !== "null") && (typeof idx !== "undefined")){
-                var item = group[idx];
-				doc[fieldName] = item.value;
+				item = group[i];
+			if((item !== "null") && (typeof item !== "undefined")){
+				doc[fieldName] = item.getValue();
 			}
 		}
 
 		return doc;
 	};
+	
+    /**
+     * Reset the document source so that it is ready for a new stream of data.
+     * Note that this is a deviation from the mongo implementation.
+     * 
+     * @method	reset
+    **/
+	proto.reset = function reset(){
+		this.populated = false;
+		this.groups = [];
+		this.groupsKeys = [];
+		this.currentDocument = null;
+		this.currentGroupsKeysIndex = 0;
+	};
 
 	return klass;
 

test/lib/munge.js

@@ -97,7 +97,8 @@ module.exports = {
 				p = [{$project:{
 						e:1, 
 						a:{$add:["$e", "$e"]}, 
-						b:{$cond:[{$eq:["$e", 2]}, "two", "not two"]}	
+						b:{$cond:[{$eq:["$e", 2]}, "two", "not two"]}
+						//TODO: high level test of all other expression operators
 					}}],
 				e = [{_id:0, e:1, b:"not two", a:2}, {_id:2, e:2, b:"two", a:4}, {_id:4, e:3, b:"not two", a:6}],
 				munger = munge(p),
@@ -124,6 +125,61 @@ module.exports = {
 			//assert.deepEqual(a, e); //does not work with NaN
 			assert.equal(JSON.stringify(munger(i)), JSON.stringify(e), "Reuse of munger should yield the same results!");
 			assert.equal(JSON.stringify(munge(p, i)), JSON.stringify(e), "Alternate use of munge should yield the same results!");
+		},
+		"should be able to construct an instance with $group operators properly": function(){
+			var i = [
+						{_id:0, a:1},
+						{_id:0, a:2},
+						{_id:0, a:3},
+						{_id:0, a:4},
+						{_id:0, a:1.5},
+						{_id:0, a:null},
+						{_id:1, b:"a"},
+						{_id:1, b:"b"},
+						{_id:1, b:"b"},
+						{_id:1, b:"c"}
+					],
+				p = [{$group:{
+						_id:"$_id",
+						sum_a:{$sum:"$a"},
+						//min_a:{$min:"$a"}, //this is busted in this version of mongo
+						max_a:{$max:"$a"},
+						avg_a:{$avg:"$a"},
+						first_b:{$first:"$b"},
+						last_b:{$last:"$b"},
+						addToSet_b:{$addToSet:"$b"},
+						push_b:{$push:"$b"}
+					}}],
+				e = [
+						{
+							_id:0,
+							sum_a:11.5,
+							//min_a:1,
+							max_a:4,
+							avg_a:2.3,
+							first_b:undefined,
+							last_b:undefined,
+							addToSet_b:[],
+							push_b:[]
+						},
+						{
+							_id:1,
+							sum_a:0,
+							//min_a:null,
+							max_a:undefined,
+							avg_a:0,
+							first_b:"a",
+							last_b:"c",
+							addToSet_b:["a", "b", "c"],
+							push_b:["a", "b", "b", "c"]
+						}
+					],
+				munger = munge(p),
+				a = munger(i);
+			assert.equal(JSON.stringify(a), JSON.stringify(e), "Unexpected value!");
+			//assert.deepEqual(a, e); //does not work with NaN
+			assert.equal(JSON.stringify(munger(i)), JSON.stringify(e), "Reuse of munger should yield the same results!");
+			assert.equal(JSON.stringify(munge(p, i)), JSON.stringify(e), "Alternate use of munge should yield the same results!");
 		}
 
 	}

test/lib/pipeline/accumulators/AddToSetAccumulator.js

@@ -44,13 +44,6 @@ module.exports = {
 					var acc = new createAccumulator();
 					acc.evaluate({}, {});
 				});
-			},
-
-			"should throw an error when given a non-array to evaluate": function testArrayValidity() {
-				assert.throws(function() {
-					var acc = createAccumulator();
-					acc.evaluate({b:5});
-				});
 			}
 
 		},
@@ -66,30 +59,33 @@ module.exports = {
 
 			"should return array with one element that equals 5": function test5InSet() {
 				var acc = createAccumulator();
-				acc.evaluate({b:[5]});
+				acc.evaluate({b:5});
+				acc.evaluate({b:5});
 				var value = acc.getValue();
-				assert.equal((value instanceof Array), true);
-				assert.equal(value.length, 1);
-				assert.equal(value[0], 5);
+				assert.deepEqual(value, [5]);
 			},
 
 			"should produce value that is an array of multiple elements": function testMultipleItems() {
 				var acc = createAccumulator();
-				acc.evaluate({b:[5, {key: "value"}]});
+				acc.evaluate({b:5});
+				acc.evaluate({b:{key: "value"}});
 				var value = acc.getValue();
-				assert.equal((value instanceof Array), true);
-				assert.equal(value.length, 2);
-				assert.equal((value[0] instanceof Object || value[1] instanceof Object) && (typeof value[0] == 'number' || typeof value[1] == 'number'), true);
-				//assert.equal(value[0], 5);
+				assert.deepEqual(value, [5, {key: "value"}]);
 			},
 
 			"should return array with one element that is an object containing a key/value pair": function testKeyValue() {
 				var acc = createAccumulator();
-				acc.evaluate({b:[{key: "value"}]});
+				acc.evaluate({b:{key: "value"}});
+				var value = acc.getValue();
+				assert.deepEqual(value, [{key: "value"}]);
+			},
+
+			"should ignore undefined values": function testKeyValue() {
+				var acc = createAccumulator();
+				acc.evaluate({b:{key: "value"}});
+				acc.evaluate({a:5});
 				var value = acc.getValue();
-				assert.equal((value instanceof Object), true);
-				assert.equal(value.length, 1);
-				assert.equal(value[0].key == "value", true);
+				assert.deepEqual(value, [{key: "value"}]);
 			}
 
 		}

test/lib/pipeline/accumulators/SingleValueAccumulator.js

@@ -30,7 +30,9 @@ module.exports = {
 		"#getValue()": {
 
 			"should return the correct value 'foo'": function testGetValue(){
-				assert.equal(new SingleValueAccumulator("foo").getValue(), "foo");
+				var sva = new SingleValueAccumulator();
+				sva.value = "foo";
+				assert.equal(sva.getValue(), "foo");
 			}
 
 		}

test/lib/pipeline/documentSources/GroupDocumentSource.js

@@ -58,13 +58,13 @@ module.exports = {
 
 			// $group _id is an empty object. g
 			"should not throw when _id is an empty object": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:{}}}, throw:false});
+				assertExpectedResult({spec:{_id:{}}, throw:false});
 			},
 
 			// $group _id is specified as an invalid object expression. g
 			"should throw error when  _id is an invalid object expression": function testConstructor(){
 				assertExpectedResult({
-					spec:{$group:{_id:{$add:1, $and:1}}},
+					spec:{_id:{$add:1, $and:1}},
 				});	
 			},
 
@@ -76,62 +76,62 @@ module.exports = {
 
 			// $group _id is the empty string. g
 			"should not throw when _id is an empty string": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:""}}, throw:false});
+				assertExpectedResult({spec:{_id:""}, throw:false});
 			},
 
 			// $group _id is a string constant. g
 			"should not throw when _id is a string constant": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:"abc"}}, throw:false});
+				assertExpectedResult({spec:{_id:"abc"}, throw:false});
 			},
 
 			// $group with _id set to an invalid field path. g
 			"should throw when _id is an invalid field path": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:"$a.."}}});
+				assertExpectedResult({spec:{_id:"$a.."}});
 			},
 		
 			// $group _id is a numeric constant. g
 			"should not throw when _id is a numeric constant": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:2}}, throw:false});
+				assertExpectedResult({spec:{_id:2}, throw:false});
 			},
 
 			// $group _id is an array constant. g
 			"should not throw when _id is an array constant": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:[1,2]}}, throw:false});
+				assertExpectedResult({spec:{_id:[1,2]}, throw:false});
 			},
 
 			// $group _id is a regular expression (not supported). g
 			"should throw when _id is a regex": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:/a/}}});
+				assertExpectedResult({spec:{_id:/a/}});
 			},
 
 			// The name of an aggregate field is specified with a $ prefix. g
 			"should throw when aggregate field spec is specified with $ prefix": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:1, $foo:{$sum:1}}}});
+				assertExpectedResult({spec:{_id:1, $foo:{$sum:1}}});
 			},
 
 			// An aggregate field spec that is not an object. g
 			"should throw when aggregate field spec is not an object": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:1, a:1}}});
+				assertExpectedResult({spec:{_id:1, a:1}});
 			},
 
 			// An aggregate field spec that is not an object. g
 			"should throw when aggregate field spec is an empty object": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:1, a:{}}}});
+				assertExpectedResult({spec:{_id:1, a:{}}});
 			},
 
 			// An aggregate field spec with an invalid accumulator operator. g
 			"should throw when aggregate field spec is an invalid accumulator": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:1, a:{$bad:1}}}});
+				assertExpectedResult({spec:{_id:1, a:{$bad:1}}});
 			},
 
 			// An aggregate field spec with an array argument. g
 			"should throw when aggregate field spec with an array as an argument": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:1, a:{$sum:[]}}}});
+				assertExpectedResult({spec:{_id:1, a:{$sum:[]}}});
 			},
 
 			// Multiple accumulator operators for a field. g
 			"should throw when aggregate field spec with multiple accumulators": function advanceTest(){
-				assertExpectedResult({spec:{$group:{_id:1, a:{$sum:1, $push:1}}}});
+				assertExpectedResult({spec:{_id:1, a:{$sum:1, $push:1}}});
 			}
 
 			//Not Implementing, not way to support this in Javascript Objects
@@ -144,7 +144,7 @@ module.exports = {
 		"#getSourceName()": {
 
 			"should return the correct source name; $group": function testSourceName(){
-				var gds = new GroupDocumentSource({$group:{_id:{}}});
+				var gds = new GroupDocumentSource({_id:{}});
 				assert.strictEqual(gds.getSourceName(), "$group");
 			}
 		},
@@ -155,7 +155,7 @@ module.exports = {
 			"should compute _id from an object expression": function advanceTest(){
 				assertExpectedResult({
 					docs:[{a:6}],
-					spec:{$group:{_id:{z:"$a"}}},
+					spec:{_id:{z:"$a"}},
 					expected:{_id:{z:6}}
 				});
 			},
@@ -164,7 +164,7 @@ module.exports = {
 			"should compute _id a field path expression": function advanceTest(){
 				assertExpectedResult({
 					docs:[{a:5}],
-					spec:{$group:{_id:"$a"}},
+					spec:{_id:"$a"},
 					expected:{_id:5}
 				});
 			},
@@ -173,7 +173,7 @@ module.exports = {
 			"should aggregate the value of an object expression": function advanceTest(){
 				assertExpectedResult({
 					docs:[{a:6}],
-					spec:{$group:{_id:0, z:{$first:{x:"$a"}}}},
+					spec:{_id:0, z:{$first:{x:"$a"}}},
 					expected:{_id:0, z:{x:6}}
 				});
 			},
@@ -182,7 +182,7 @@ module.exports = {
 			"should aggregate the value of an operator expression": function advanceTest(){
 				assertExpectedResult({
 					docs:[{a:6}],
-					spec:{$group:{_id:0, z:{$first:"$a"}}},
+					spec:{_id:0, z:{$first:"$a"}},
 					expected:{_id:0, z:6}
 				});
 			}

test/lib/pipeline/documentSources/ProjectDocumentSource.js

@@ -183,7 +183,7 @@ module.exports = {
                 });
                 //Empty args
                 assert.throws(function() {
-                    pds = ProjectDocumentSource.createFromJson();
+                    var pds = ProjectDocumentSource.createFromJson();
                 });
                 //Top level operator
                 assert.throws(function() {