Revise Mongo session storage

 * Default to _id for storing session ID
 * Use MongoDate instead of MongoTimestamp (BC break)
 * Rename default field names ("sess_" is redundant)
 * "justOne" is redundant for session removal
 * Assert true return values in method tests
 * Add note about TTL collections for gc()
 * Don't set identifier in upsert (invalid behavior)

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php

@@ -50,9 +50,9 @@ public function __construct(\Mongo $mongo, array $options)
         $this->mongo = $mongo;
 
         $this->options = array_merge(array(
-            'id_field'   => 'sess_id',
-            'data_field' => 'sess_data',
-            'time_field' => 'sess_time',
+            'id_field'   => '_id',
+            'data_field' => 'data',
+            'time_field' => 'time',
         ), $options);
     }
 
@@ -77,10 +77,9 @@ public function close()
      */
     public function destroy($sessionId)
     {
-        $this->getCollection()->remove(
-            array($this->options['id_field'] => $sessionId),
-            array('justOne' => true)
-        );
+        $this->getCollection()->remove(array(
+            $this->options['id_field'] => $sessionId
+        ));
 
         return true;
     }
@@ -90,28 +89,35 @@ public function destroy($sessionId)
      */
     public function gc($lifetime)
     {
-        $time = new \MongoTimestamp(time() - $lifetime);
+        /* Note: MongoDB 2.2+ supports TTL collections, which may be used in
+         * place of this method by indexing the "time_field" field with an
+         * "expireAfterSeconds" option. Regardless of whether TTL collections
+         * are used, consider indexing this field to make the remove query more
+         * efficient.
+         *
+         * See: http://docs.mongodb.org/manual/tutorial/expire-data/
+         */
+        $time = new \MongoDate(time() - $lifetime);
 
         $this->getCollection()->remove(array(
             $this->options['time_field'] => array('$lt' => $time),
         ));
+
+        return true;
     }
 
     /**
      * {@inheritDoc]
      */
     public function write($sessionId, $data)
     {
-        $data = array(
-            $this->options['id_field']   => $sessionId,
-            $this->options['data_field'] => new \MongoBinData($data, \MongoBinData::BYTE_ARRAY),
-            $this->options['time_field'] => new \MongoTimestamp()
-        );
-
         $this->getCollection()->update(
             array($this->options['id_field'] => $sessionId),
-            array('$set' => $data),
-            array('upsert' => true)
+            array('$set' => array(
+                $this->options['data_field'] => new \MongoBinData($data, \MongoBinData::BYTE_ARRAY),
+                $this->options['time_field'] => new \MongoDate(),
+            )),
+            array('upsert' => true, 'multiple' => false)
         );
 
         return true;

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php

@@ -28,17 +28,17 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
     protected function setUp()
     {
         if (!class_exists('\Mongo')) {
-            $this->markTestSkipped('MongoDbSessionHandler requires the php "mongo" extension');
+            $this->markTestSkipped('MongoDbSessionHandler requires the mongo extension.');
         }
 
         $this->mongo = $this->getMockBuilder('Mongo')
             ->disableOriginalConstructor()
             ->getMock();
 
         $this->options = array(
-            'id_field'   => 'sess_id',
-            'data_field' => 'sess_data',
-            'time_field' => 'sess_time',
+            'id_field'   => '_id',
+            'data_field' => 'data',
+            'time_field' => 'time',
             'database' => 'sf2-test',
             'collection' => 'session-test'
         );
@@ -81,17 +81,17 @@ public function testWrite()
 
         $collection->expects($this->once())
             ->method('update')
-            ->will($this->returnCallback(function($citeria, $updateData, $options) use ($that, &$data) {
-                $that->assertEquals(array($that->options['id_field'] => 'foo'), $citeria);
-                $that->assertEquals(array('upsert' => true), $options);
+            ->will($this->returnCallback(function($criteria, $updateData, $options) use ($that, &$data) {
+                $that->assertEquals(array($that->options['id_field'] => 'foo'), $criteria);
+                $that->assertEquals(array('upsert' => true, 'multiple' => false), $options);
 
                 $data = $updateData['$set'];
             }));
 
         $this->assertTrue($this->storage->write('foo', 'bar'));
 
-        $this->assertEquals('foo', $data[$this->options['id_field']]);
         $this->assertEquals('bar', $data[$this->options['data_field']]->bin);
+        $that->assertInstanceOf('MongoDate', $data[$this->options['time_field']]);
     }
 
     public function testReplaceSessionData()
@@ -118,7 +118,7 @@ public function testReplaceSessionData()
 
         $collection->expects($this->exactly(2))
             ->method('update')
-            ->will($this->returnCallback(function($citeria, $updateData, $options) use (&$data) {
+            ->will($this->returnCallback(function($criteria, $updateData, $options) use (&$data) {
                 $data = $updateData;
             }));
 
@@ -150,13 +150,9 @@ public function testDestroy()
 
         $collection->expects($this->once())
             ->method('remove')
-            ->with(
-                array($this->options['id_field'] => 'foo'),
-                array('justOne' => true)
-            );
+            ->with(array($this->options['id_field'] => 'foo'));
 
-
-        $this->storage->destroy('foo');
+        $this->assertTrue($this->storage->destroy('foo'));
     }
 
     public function testGc()
@@ -183,11 +179,11 @@ public function testGc()
 
         $collection->expects($this->once())
             ->method('remove')
-            ->will($this->returnCallback(function($citeria) use($that) {
-                $that->assertInstanceOf('MongoTimestamp', $citeria[$that->options['time_field']]['$lt']);
-                $that->assertGreaterThanOrEqual(time() - -1, $citeria[$that->options['time_field']]['$lt']->sec);
+            ->will($this->returnCallback(function($criteria) use($that) {
+                $that->assertInstanceOf('MongoDate', $criteria[$that->options['time_field']]['$lt']);
+                $that->assertGreaterThanOrEqual(time() - -1, $criteria[$that->options['time_field']]['$lt']->sec);
             }));
 
-        $this->storage->gc(-1);
+        $this->assertTrue($this->storage->gc(-1));
     }
 }