merged branch jmikola/2.1-mongo-session (PR #5916)
This PR was submitted for the 2.1 branch but it was merged into the master branch instead (closes #5916). Commits ------- 917cc14 [HttpFoundation] Revise Mongo session storage Discussion ---------- [HttpFoundation] Revise MongoDB session storage ``` Bug fix: yes Feature addition: no Backwards compatibility break: yes Symfony2 tests pass: yes Fixes the following tickets: - Todo: - License of the code: MIT Documentation PR: - ``` I decided to take a look at the MongoDB session driver after reading @pgodel's [blog post](http://blog.servergrove.com/2012/11/05/storing-sessions-in-mongodb-with-symfony2/) today. This PR contains some fixes to make this session handler integrate better with MongoDB, as well as make it more in line with the work I did in zendframework/zf2/#2031: * Default to _id for storing session ID (BC break) * 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) In my opinion, the BC breaks are reasonable. `_id` is the logical field to store the session ID, as I'd expect many users may not even think to index the `sess_id` field to avoid inefficient queries otherwise. Also, MongoTimestamp should never have been used in the existing manner. Per the [documentation](http://php.net/manual/en/class.mongotimestamp.php): > This class is not for measuring time, creating a timestamp on a document or automatically adding or updating a timestamp on a document. Unless you are writing something that interacts with the sharding internals, stop, go directly to MongoDate, do not pass go, do not collect 200 dollars. This is not the class you are looking for. On a side note, I'm not sure why `sess_` prefixes exist for the PDO driver. It seems redundant in either case (the table/collection would logically have "session" in the name). The fix to the update statement actually addresses a bug were `_id` to appear in the `$set` query. I'm not sure how to document the BC breaks or changes, as the 2.1 branch's readme files look a lot sparser than those for 2.0. Let me know if there's something else to be done, though. --------------------------------------------------------------------------- by jmikola at 2012-11-06T02:49:06Z FYI: the Travis CI build failure looks unrelated to these changes (something to do with Form tests): https://travis-ci.org/#!/jmikola/symfony/jobs/3076587 --------------------------------------------------------------------------- by pgodel at 2012-11-06T04:27:24Z I think the BC breaks are very mino, so there is no reason to not merge this. --------------------------------------------------------------------------- by jmikola at 2012-11-06T07:32:44Z Thanks, @stof. That looked like something @pborreli would have caught sooner or later :) --------------------------------------------------------------------------- by fabpot at 2012-11-06T08:00:48Z As there is a BC break, I'm going to merge it on master. --------------------------------------------------------------------------- by pborreli at 2012-11-06T08:05:08Z @jmikola merging with master you will have a little conflict as i already fixed the citeria => criteria typo :)
This commit is contained in:
Fabien Potencier
commit
1b53bf68e8
@ -50,9 +50,9 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
|
||||
$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 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
|
||||
*/
|
||||
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,11 +89,21 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
|
||||
*/
|
||||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -102,16 +111,13 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
|
||||
*/
|
||||
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;
|
||||
|
||||
@ -28,7 +28,7 @@ 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')
|
||||
@ -36,9 +36,9 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
|
||||
->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 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
$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 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
$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 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
$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 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
$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));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user