Fixes #1001: DocumentSourceGroup is working now.

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

lib/pipeline/documentSources/GroupDocumentSource.js

@@ -1,7 +1,9 @@
 var DocumentSource = require("./DocumentSource"),
+	Accumulators = require("../accumulators/"),
 	Document = require("../Document"),
 	Expression = require("../expressions/Expression"),
-	Accumulators = require("../accumulators/"),
+	ConstantExpression = require("../expressions/ConstantExpression"),
+	FieldPathExpression = require("../expressions/FieldPathExpression"),
 	GroupDocumentSource = module.exports = (function(){
 	// CONSTRUCTOR
 	/**
@@ -13,10 +15,8 @@ var DocumentSource = require("./DocumentSource"),
 	 * @constructor
 	 * @param	{ExpressionContext}	
 	**/
-	var klass = module.exports = GroupDocumentSource = function GroupDocumentSource(groupElement){
-		if(!(groupElement instanceof Object && groupElement.constructor.name === "Object") || Object.keys(groupElement).length < 1)
-			throw new Error("a group's fields must be specified in an object");			
-	
+	var klass = module.exports = GroupDocumentSource = function GroupDocumentSource(){
+
 		this.populated = false;
 		this.idExpression = null;
 		this.groups = {}; // GroupsType Value -> Accumulators[]
@@ -26,31 +26,65 @@ var DocumentSource = require("./DocumentSource"),
 		this.accumulatorFactories = [];
 		this.expressions = [];
 		this.currentDocument = null;
-		this.groupCurrentIndex = 0;
+		this.currentGroupsKeysIndex = 0;
+
+	}, base = DocumentSource, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+
+	klass.GroupOps = {
+		"$addToSet": Accumulators.AddToSet,
+		"$avg": Accumulators.Avg,
+		"$first": Accumulators.First,
+		"$last": Accumulators.Last,
+		"$max": Accumulators.MinMax.bind(null, 1),
+		"$min": Accumulators.MinMax.bind(null, -1),
+		"$push": Accumulators.Push,
+		"$sum": Accumulators.Sum
+	};
+
+    klass.createFromJson = function createFromJson(groupElement) {
+
+		if(!(groupElement instanceof Object && groupElement.constructor.name === "Object") || Object.keys(groupElement).length < 1)
+			throw new Error("a group's fields must be specified in an object");
+
+        var idSet = false,
+            group = new GroupDocumentSource(),
+            groupObj = groupElement[group.getSourceName()];
 
-		var groupObj = groupElement[this.getSourceName()];
 		for(var groupFieldName in groupObj){
 			if(groupObj.hasOwnProperty(groupFieldName)){
 				var groupField = groupObj[groupFieldName];
-				
+
 				if(groupFieldName === "_id"){
+
+                    if(idSet) {
+                        throw new Error("15948 a group's _id may only be specified once");
+                    }
+
 					if(groupField instanceof Object && groupField.constructor.name === "Object"){
 						var objCtx = new Expression.ObjectCtx({isDocumentOk:true});
-						this.idExpression = Expression.parseObject(groupField, objCtx);
+						group.idExpression = Expression.parseObject(groupField, objCtx);
+                        idSet = true;
 
 					}else if( typeof groupField === "string"){
-						if(groupField[0] !== "$")
-							this.idExpression = new ConstantExpression(groupField);		
-						var pathString = Expression.removeFieldPrefix(groupField);
-						this.idExpression = new FieldPathExpression(pathString);
+						if(groupField[0] !== "$") {
+							group.idExpression = new ConstantExpression(groupField);
+                        }
+                        else {
+                            var pathString = Expression.removeFieldPrefix(groupField);
+                            group.idExpression = new FieldPathExpression(pathString);
+                        }
+
+                        idSet = true;
 					}else{
-						var typeStr = this._getTypeStr(groupField);
+						var typeStr = group._getTypeStr(groupField);
 						switch(typeStr){
 							case "number":
 							case "string":
 							case "boolean":
-							case "object":
-								this.idExpression = new ConstantExpression(groupField);
+							case "Object":
+                            case "Array":
+								group.idExpression = new ConstantExpression(groupField);
+                                idSet = true;
 								break;
 							default:
 								throw new Error("a group's _id may not include fields of type " + typeStr  + ""); 
@@ -63,56 +97,50 @@ var DocumentSource = require("./DocumentSource"),
 						throw new Error("16414 the group aggregate field name '" + groupFieldName + "' cannot contain '.'");
 					if(groupFieldName[0] === "$")
 						throw new Error("15950 the group aggregate field name '" + groupFieldName + "' cannot be an operator name");
-					if(this._getTypeStr(groupFieldName) === "object")
+					if(group._getTypeStr(groupFieldName) === "Object")
 						throw new Error("15951 the group aggregate field '" + groupFieldName + "' must be defined as an expression inside an object");
 
 					var subFieldCount = 0;
 					for(var subFieldName in groupField){
 						if(groupField.hasOwnProperty(subFieldName)){
-							var subField = groupField[subField],
-								op = DocumentSource.GroupOps[subFieldName];
+							var subField = groupField[subFieldName],
+								op = klass.GroupOps[subFieldName];
 							if(!op)
-								throw new Exception("15952 unknown group operator '" + subFieldName + "'");
+								throw new Error("15952 unknown group operator '" + subFieldName + "'");
 
 							var groupExpression,
-								subFieldTypeStr = this._getTypeStr(subField);
-							if(subFieldTypeStr === "object"){
+								subFieldTypeStr = group._getTypeStr(subField);
+							if(subFieldTypeStr === "Object"){
 								var subFieldObjCtx = new Expression.ObjectCtx({isDocumentOk:true});
-								groupExpression = Expression.parseObject(groupField, subFieldObjCtx);
+								groupExpression = Expression.parseObject(subField, subFieldObjCtx);
 							}else if(subFieldTypeStr === "Array"){
-								throw new Exception("15953 aggregating group operators are unary (" + subFieldName + ")");
+								throw new Error("15953 aggregating group operators are unary (" + subFieldName + ")");
 							}else{
 								groupExpression = Expression.parseOperand(subField);
 							}
-							this.addAccumulator(groupFieldName,op, groupExpression); 
+							group.addAccumulator(groupFieldName,op, groupExpression); 
 							
 							++subFieldCount;
 						}
+					}
 					if(subFieldCount != 1)
 						throw new Error("15954 the computed aggregate '" + groupFieldName + "' must specify exactly one operator");
-					}
 				}
 			}
-		}	
-
+		}
 
-	}, base = DocumentSource, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+        if(!idSet) {
+            throw new Error("15955 a group specification must include an _id");
+        }
 
-
-	klass.GroupOps = {
-		"$addToSet": Accumulators.AddToSet,
-		"$avg": Accumulators.Avg,
-		"$first": Accumulators.First,
-		"$last": Accumulators.Last,
-		"$max": Accumulators.MinMax.bind(null, 1),
-		"$min": Accumulators.MinMax.bind(null, -1),
-		"$push": Accumulators.Push,
-		"$sum": Accumulators.Sum
-	};
+        return group;
+    };
 
 	proto._getTypeStr = function _getTypeStr(obj){
-		var typeofStr=typeof groupField, typeStr = (typeofStr == "object" ? groupField.constructor.name : typeStr);
-		return typeofStr;	
+		var typeofStr=typeof obj,
+            typeStr=(typeofStr == "object" ? obj.constructor.name : typeofStr);
+
+		return typeStr;	
 	};
 
 
@@ -122,17 +150,18 @@ var DocumentSource = require("./DocumentSource"),
 
 	proto.advance = function advance(){
 		base.prototype.advance.call(this); // Check for interupts ????
-
 		if(!this.populated)
 			this.populate();
 
+        //verify(this.currentGroupsKeysIndex < this.groupsKeys.length);
+
 		++this.currentGroupsKeysIndex;
-		if(this.currentGroupsKeysIndex === this.groupKeys.length){
+		if(this.currentGroupsKeysIndex === this.groupsKeys.length){
 			this.currentDocument = null;
 			return false;	
 		}
 
-		
+        this.currentDocument = this.makeDocument(this.currentGroupsKeysIndex);
 		return true;
 	};
 
@@ -163,18 +192,24 @@ var DocumentSource = require("./DocumentSource"),
 
 	proto.populate = function populate(){
 		for(var hasNext = !this.pSource.eof(); hasNext; hasNext = this.pSource.advance()){
-			var currentDocument = this.pSource.getCurrent(),
-				_id = this.idExpression.evaluate(currentDocument) || null,
-				group;
+			var group,
+                currentDocument = this.pSource.getCurrent(),
+				_id = this.idExpression.evaluate(currentDocument);
+
+            if(undefined === _id) {
+                _id = null;
+            }
+
+            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){
-				group = this.groups[_id];
+				group = this.groups[idHash];
 			}else{
-				this.groups[_id] = group = [];
-				this.groupsKeys[this.currentGroupsKeysIndex] = _id;
-				for(var ai =0; ai < this.accumulators.length; ++ai){
+				this.groups[idHash] = group = [];
+				this.groupsKeys[this.currentGroupsKeysIndex] = idHash;
+				for(var ai =0; ai < this.accumulatorFactories.length; ++ai){
 					var accumulator = new this.accumulatorFactories[ai]();
-					accumulators.addOperand(this.expressions[ai]);
+					accumulator.addOperand(this.expressions[ai]);
 					group.push(accumulator);
 				}
 			}
@@ -184,10 +219,9 @@ 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.groups.length-1)
-				this.currentDocument = makeDocument(this.currentGroupsKeysIndex);
+			if(this.currentGroupsKeysIndex < this.groupsKeys.length)
+				this.currentDocument = this.makeDocument(this.currentGroupsKeysIndex);
 			this.populated = true;
 
 		}
@@ -196,17 +230,18 @@ var DocumentSource = require("./DocumentSource"),
 
 
 	proto.makeDocument = function makeDocument(groupKeyIndex){
-		var groupKey = this.groupKeys[groupKeyIndex],
+		var groupKey = this.groupsKeys[groupKeyIndex],
 			group = this.groups[groupKey],
 			doc = {};
-		doc[Document.ID_PROPERTY_NAME] = groupKey;
-			
-	
+
+		doc[Document.ID_PROPERTY_NAME] = JSON.parse(groupKey);
+
 		for(var i = 0; i < this.fieldNames.length; ++i){
 			var fieldName = this.fieldNames[i],
-				value = this.group[i].getValue();
-			if(typeof value !== "undefined"){
-				doc[fieldName] = value;
+				idx = this.groupsKeys[i];
+			if((idx !== "null") && (typeof idx !== "undefined")){
+                var item = group[idx];
+				doc[fieldName] = item.value;
 			}
 		}
 

test/lib/pipeline/documentSources/GroupDocumentSource.js

@@ -15,22 +15,21 @@ function assertExpectedResult(args) {
 
 	// run implementation
 	if(args.expected && args.docs){
-		var gds = new GroupDocumentSource(args.spec),
+		var gds = GroupDocumentSource.createFromJson(args.spec),
 			cwc = new CursorDocumentSource.CursorWithContext();
 		cwc._cursor = new Cursor( args.docs );
 		var cds = new CursorDocumentSource(cwc);
 		gds.setSource(cds);
-		gds.advance();
 		var result = gds.getCurrent();
 		assert.deepEqual(result, args.expected);
 	}else{
 		if(args.throw)
 			assert.throws(function(){
-				new GroupDocumentSource(args.spec);
+				GroupDocumentSource.createFromJson(args.spec);
 			});
 		else
 			assert.doesNotThrow(function(){
-				new GroupDocumentSource(args.spec);
+				GroupDocumentSource.createFromJson(args.spec);
 			});
 	}
 
@@ -42,27 +41,27 @@ module.exports = {
 
 		"constructor()": {
 
-			/** $group spec is not an object. */
+			// $group spec is not an object. g
 			"should throw Error when constructing without args": function testConstructor(){
 				assertExpectedResult({throw:true});
 			},
 
-			/** $group spec is not an object. */
+			// $group spec is not an object. g
 			"should throw Error when $group spec is not an object": function testConstructor(){
 				assertExpectedResult({spec:"Foo"});
 			},
 
-			/** $group spec is an empty object. */
+			// $group spec is an empty object. g
 			"should throw Error when $group spec is an empty object": function testConstructor(){
 				assertExpectedResult({spec:{}});
 			},
 
-//			/** $group _id is an empty object. */
-//			"should not throw when _id is an empty object": function advanceTest(){
-//				assertExpectedResult({spec:{$group:{_id:{}}}, throw:false});
-//			},
+			// $group _id is an empty object. g
+			"should not throw when _id is an empty object": function advanceTest(){
+				assertExpectedResult({spec:{$group:{_id:{}}}, throw:false});
+			},
 
-			/** $group _id is specified as an invalid object expression. */
+			// $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}}},
@@ -70,73 +69,73 @@ module.exports = {
 			},
 
 
-			/** $group with two _id specs. */
+			// $group with two _id specs. g
 			//NOT Implemented can't do this in Javascript
 
 
 
-//			/** $group _id is the empty string. */
-//			"should not throw when _id is an empty string": function advanceTest(){
-//				assertExpectedResult({spec:{$group:{_id:""}}, throw:false});
-//			},
+			// $group _id is the empty string. g
+			"should not throw when _id is an empty string": function advanceTest(){
+				assertExpectedResult({spec:{$group:{_id:""}}, throw:false});
+			},
 
-//			/** $group _id is a string constant. */
-//			"should not throw when _id is a string constant": function advanceTest(){
-//				assertExpectedResult({spec:{$group:{_id:"abc"}}, 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});
+			},
 
-			/** $group with _id set to an invalid field path. */
+			// $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.."}}});
 			},
 		
-//			/** $group _id is a numeric constant. */
-//			"should not throw when _id is a numeric constant": function advanceTest(){
-//				assertExpectedResult({spec:{$group:{_id:2}}, throw:false});
-//			},
+			// $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});
+			},
 
-//			/** $group _id is an array constant. */
-//			"should not throw when _id is an array constant": function advanceTest(){
-//				assertExpectedResult({spec:{$group:{_id:[1,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});
+			},
 
-			/** $group _id is a regular expression (not supported). */
+			// $group _id is a regular expression (not supported). g
 			"should throw when _id is a regex": function advanceTest(){
 				assertExpectedResult({spec:{$group:{_id:/a/}}});
 			},
 
-			/** The name of an aggregate field is specified with a $ prefix. */
+			// 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}}}});
 			},
 
-			/** An aggregate field spec that is not an object. */
+			// 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}}});
 			},
 
-			/** An aggregate field spec that is not an object. */
+			// 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:{}}}});
 			},
 
-			/** An aggregate field spec with an invalid accumulator operator. */
+			// 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}}}});
 			},
 
-			/** An aggregate field spec with an array argument. */
+			// 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:[]}}}});
 			},
 
-			/** Multiple accumulator operators for a field. */
+			// 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}}}});
 			}
 
 			//Not Implementing, not way to support this in Javascript Objects
-			/** Aggregation using duplicate field names is allowed currently. */
+			// Aggregation using duplicate field names is allowed currently. g
 
 
 
@@ -144,49 +143,49 @@ module.exports = {
 
 		"#getSourceName()": {
 
-//			"should return the correct source name; $group": function testSourceName(){
-//				var gds = new GroupDocumentSource({$group:{_id:{}}});
-//				assert.strictEqual(gds.getSourceName(), "$group");
-//			}
+			"should return the correct source name; $group": function testSourceName(){
+				var gds = new GroupDocumentSource({$group:{_id:{}}});
+				assert.strictEqual(gds.getSourceName(), "$group");
+			}
 		},
 
 		"#advance": {
 
-//			/** $group _id is computed from an object expression. */
-//			"should compute _id from an object expression": function advanceTest(){
-//				assertExpectedResult({
-//					docs:[{a:6}],
-//					spec:{$group:{_id:{z:"$a"}}},
-//					expected:{_id:{z:6}}
-//				});	
-//			},
-
-//			/** $group _id is a field path expression. */
-//			"should compute _id a field path expression": function advanceTest(){
-//				assertExpectedResult({
-//					docs:[{a:5}],
-//					spec:{$group:{_id:"$a"}},
-//					expected:{_id:{z:5}}
-//				});	
-//			},
-
-//			/** Aggregate the value of an object expression. */
-//			"should aggregate the value of an object expression": function advanceTest(){
-//				assertExpectedResult({
-//					docs:[{a:6}],
-//					spec:{$group:{_id:0, z:{$first:{x:"$a"}}}},
-//					expected:{_id:0, z:6}
-//				});	
-//			},
-
-//			/** Aggregate the value of an operator expression. */
-//			"should aggregate the value of an operator expression": function advanceTest(){
-//				assertExpectedResult({
-//					docs:[{a:6}],
-//					spec:{$group:{_id:0, z:{$first:"$a"}}},
-//					expected:{_id:0, z:6}
-//				});	
-//			}
+			// $group _id is computed from an object expression. g
+			"should compute _id from an object expression": function advanceTest(){
+				assertExpectedResult({
+					docs:[{a:6}],
+					spec:{$group:{_id:{z:"$a"}}},
+					expected:{_id:{z:6}}
+				});
+			},
+
+			// $group _id is a field path expression. g
+			"should compute _id a field path expression": function advanceTest(){
+				assertExpectedResult({
+					docs:[{a:5}],
+					spec:{$group:{_id:"$a"}},
+					expected:{_id:5}
+				});
+			},
+
+			// Aggregate the value of an object expression. g
+			"should aggregate the value of an object expression": function advanceTest(){
+				assertExpectedResult({
+					docs:[{a:6}],
+					spec:{$group:{_id:0, z:{$first:{x:"$a"}}}},
+					expected:{_id:0, z:{x:6}}
+				});
+			},
+
+//			// Aggregate the value of an operator expression. g
+			"should aggregate the value of an operator expression": function advanceTest(){
+				assertExpectedResult({
+					docs:[{a:6}],
+					spec:{$group:{_id:0, z:{$first:"$a"}}},
+					expected:{_id:0, z:6}
+				});
+			}
 		}
 	}