Added tweaks and documentation to previous pull request

src/Jenssegers/Mongodb/Builder.php

@@ -64,72 +64,70 @@ class Builder extends \Illuminate\Database\Query\Builder {
         // all of the columns on the table using the "wildcard" column character.
         if (is_null($this->columns)) $this->columns = $columns;
 
-        // Drop all columns if * is present
-        if (in_array('*', $this->columns))
-        {
-            $this->columns = array();
-        }
+        // Drop all columns if * is present, MongoDB does not work this way
+        if (in_array('*', $this->columns)) $this->columns = array();
 
         // Compile wheres
         $wheres = $this->compileWheres();
 
-        // Use aggregation framework if needed
+        // Aggregation query
         if ($this->groups || $this->aggregate)
         {
-            $pipeline = array();
             $group = array();
 
-            // Grouping
+            // Apply grouping
             if ($this->groups)
             {
                 foreach ($this->groups as $column)
                 {
+                    // Mark column as grouped
                     $group['_id'][$column] = '$' . $column;
+
+                    // Aggregate by $last when grouping, this mimics MySQL's behaviour a bit
                     $group[$column] = array('$last' => '$' . $column);
                 }
             }
             else
             {
-                $group['_id'] = 0;
+                // If we don't use grouping, set the _id to null to prepare the pipeline for 
+                // other aggregation functions
+                $group['_id'] = null;
             }
 
-            // Columns
+            // When additional columns are requested, aggregate them by $last as well
             foreach ($this->columns as $column)
             {
-                $group[$column] = array('$last' => '$' . $column);
+                // Replace possible dots in subdocument queries with underscores
+                $key = str_replace('.', '_', $column);
+                $group[$key] = array('$last' => '$' . $column);
             }
 
-            // Apply aggregation functions
+            // Apply aggregation functions, these may override previous aggregations
             if ($this->aggregate)
             {
                 $function = $this->aggregate['function'];
 
                 foreach ($this->aggregate['columns'] as $column)
                 {
+                    // Replace possible dots in subdocument queries with underscores
+                    $key = str_replace('.', '_', $column);
+
                     // Translate count into sum
                     if ($function == 'count')
                     {
-                        $group[$column] = array('$sum' => 1);
+                        $group[$key] = array('$sum' => 1);
                     }
                     // Pass other functions directly
                     else
                     {
-                        // Normally this aggregate function would overwrite the
-                        // $last group set above, but since we are modifying
-                        // the string, we need to unset it directly.
-                        if (isset($group[$column]))
-                        {
-                            unset($group[$column]);
-                        }
-
-                        $key = str_replace('.', '_', $column);
                         $group[$key] = array('$' . $function => '$' . $column);
                     }
                 }
             }
 
+            // Build pipeline
+            $pipeline = array();
             if ($wheres)  $pipeline[] = array('$match' => $wheres);
-
             $pipeline[] = array('$group' => $group);
 
             // Apply order and limit
@@ -137,28 +135,31 @@ class Builder extends \Illuminate\Database\Query\Builder {
             if ($this->offset) $pipeline[] = array('$skip' => $this->offset);
             if ($this->limit)  $pipeline[] = array('$limit' => $this->limit);
 
+            // Execute aggregation
             $results = $this->collection->aggregate($pipeline);
 
             // Return results
             return $results['result'];
         }
-        else
-        {
-            // Execute distinct
-            if ($this->distinct)
+
+        // Distinct query
+        else if ($this->distinct)
         {
+            // Return distinct results directly
             $column = isset($this->columns[0]) ? $this->columns[0] : '_id';
             return $this->collection->distinct($column, $wheres);
         }
         
-            // Columns
+        // Normal query
+        else
+        {
             $columns = array();
             foreach ($this->columns as $column)
             {
                 $columns[$column] = true;
             }
 
-            // Get the MongoCursor
+            // Execute query and get MongoCursor
             $cursor = $this->collection->find($wheres, $columns);
 
             // Apply order, offset and limit
@@ -166,7 +167,7 @@ class Builder extends \Illuminate\Database\Query\Builder {
             if ($this->offset) $cursor->skip($this->offset);
             if ($this->limit)  $cursor->limit($this->limit);
 
-            // Return results
+            // Return results as an array with numeric keys
             return iterator_to_array($cursor, false);
         }
     }
@@ -212,6 +213,7 @@ class Builder extends \Illuminate\Database\Query\Builder {
 
         if (isset($results[0]))
         {
+            // Replace possible dots in subdocument queries with underscores
             $key = str_replace('.', '_', $columns[0]);
             return $results[0][$key];
         }

tests/QueryBuilderTest.php

@@ -337,4 +337,39 @@ class QueryBuilderTest extends PHPUnit_Framework_TestCase {
 		$this->assertEquals(array('knife' => 'sharp', 'fork' => 'sharp', 'spoon' => 'round'), $list);
 	}
 
+	public function testAggregate()
+	{
+		DB::collection('items')->insert(array(
+			array('name' => 'knife', 'type' => 'sharp', 'amount' => 34),
+			array('name' => 'fork',  'type' => 'sharp', 'amount' => 20),
+			array('name' => 'spoon', 'type' => 'round', 'amount' => 3),
+			array('name' => 'spoon', 'type' => 'round', 'amount' => 14)
+		));
+
+		$this->assertEquals(71, DB::collection('items')->sum('amount'));
+		$this->assertEquals(4, DB::collection('items')->count('amount'));
+		$this->assertEquals(3, DB::collection('items')->min('amount'));
+		$this->assertEquals(34, DB::collection('items')->max('amount'));
+		$this->assertEquals(17.75, DB::collection('items')->avg('amount'));
+
+		$this->assertEquals(2, DB::collection('items')->where('name', 'spoon')->count('amount'));
+		$this->assertEquals(14, DB::collection('items')->where('name', 'spoon')->max('amount'));
+	}
+
+	public function testSubdocumentAggregate()
+	{
+		DB::collection('items')->insert(array(
+			array('name' => 'knife', 'amount' => array('hidden' => 10, 'found' => 3)),
+			array('name' => 'fork',  'amount' => array('hidden' => 35, 'found' => 12)),
+			array('name' => 'spoon', 'amount' => array('hidden' => 14, 'found' => 21)),
+			array('name' => 'spoon', 'amount' => array('hidden' => 6, 'found' => 4))
+		));
+
+		$this->assertEquals(65, DB::collection('items')->sum('amount.hidden'));
+		$this->assertEquals(4, DB::collection('items')->count('amount.hidden'));
+		$this->assertEquals(6, DB::collection('items')->min('amount.hidden'));
+		$this->assertEquals(35, DB::collection('items')->max('amount.hidden'));
+		$this->assertEquals(16.25, DB::collection('items')->avg('amount.hidden'));
+	}
+
 }
\ No newline at end of file

tests/QueryTest.php

@@ -3,17 +3,23 @@ require_once('tests/app.php');
 
 class QueryTest extends PHPUnit_Framework_TestCase {
 
-	public static function setUpBeforeClass()
+	protected static $started = false;
+
+	public function setUp()
 	{
-		User::create(array('name' => 'John Doe', 'age' => 35, 'title' => 'admin', 'subdocument' => array('age' => 35)));
-		User::create(array('name' => 'Jane Doe', 'age' => 33, 'title' => 'admin', 'subdocument' => array('age' => 33)));
-		User::create(array('name' => 'Harry Hoe', 'age' => 13, 'title' => 'user', 'subdocument' => array('age' => 13)));
-		User::create(array('name' => 'Robert Roe', 'age' => 37, 'title' => 'user', 'subdocument' => array('age' => 37)));
-		User::create(array('name' => 'Mark Moe', 'age' => 23, 'title' => 'user', 'subdocument' => array('age' => 23)));
-		User::create(array('name' => 'Brett Boe', 'age' => 35, 'title' => 'user', 'subdocument' => array('age' => 35)));
-		User::create(array('name' => 'Tommy Toe', 'age' => 33, 'title' => 'user', 'subdocument' => array('age' => 33)));
-		User::create(array('name' => 'Yvonne Yoe', 'age' => 35, 'title' => 'admin', 'subdocument' => array('age' => 35)));
+		if (self::$started) return;
+
+		User::create(array('name' => 'John Doe', 'age' => 35, 'title' => 'admin'));
+		User::create(array('name' => 'Jane Doe', 'age' => 33, 'title' => 'admin'));
+		User::create(array('name' => 'Harry Hoe', 'age' => 13, 'title' => 'user'));
+		User::create(array('name' => 'Robert Roe', 'age' => 37, 'title' => 'user'));
+		User::create(array('name' => 'Mark Moe', 'age' => 23, 'title' => 'user'));
+		User::create(array('name' => 'Brett Boe', 'age' => 35, 'title' => 'user'));
+		User::create(array('name' => 'Tommy Toe', 'age' => 33, 'title' => 'user'));
+		User::create(array('name' => 'Yvonne Yoe', 'age' => 35, 'title' => 'admin'));
 		User::create(array('name' => 'Error', 'age' => null, 'title' => null));
+
+		self::$started = true;
 	}
 
 	public static function tearDownAfterClass()
@@ -168,26 +174,6 @@ class QueryTest extends PHPUnit_Framework_TestCase {
 		$this->assertEquals(8, $num);
 	}
 
-	public function testAggregates()
-	{
-		$this->assertEquals(9, User::count());
-		$this->assertEquals(37, User::max('age'));
-		$this->assertEquals(13, User::min('age'));
-		$this->assertEquals(30.5, User::avg('age'));
-		$this->assertEquals(244, User::sum('age'));
-
-		$this->assertEquals(37, User::max('subdocument.age'));
-		$this->assertEquals(13, User::min('subdocument.age'));
-		$this->assertEquals(30.5, User::avg('subdocument.age'));
-		$this->assertEquals(244, User::sum('subdocument.age'));
-
-		$this->assertEquals(35, User::where('title', 'admin')->max('age'));
-		$this->assertEquals(37, User::where('title', 'user')->max('age'));
-
-		$this->assertEquals(33, User::where('title', 'admin')->min('age'));
-		$this->assertEquals(13, User::where('title', 'user')->min('age'));
-	}
-
 	public function testGroupBy()
 	{
 		$users = User::groupBy('title')->get();