8000 Fix for #19183 to add support for new PHP MongoDB extension in sessions. by omanizer · Pull Request #19186 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Fix for #19183 to add support for new PHP MongoDB extension in sessions. #19186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Fix for #19183 to add support for new PHP MongoDB extension in sessions. #19186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c3e4c6
Fix for #19183 to add support for new PHP MongoDB extension in sessions.
omanizer Jun 25, 2016
2cf1928
Improved fields array declaration to use less code. Modified unit te…
omanizer Jun 26, 2016
ea9ddd2
style improvements
omanizer Jun 26, 2016
76865e8
Implemented correct way to test whether either Mongo or MongoDB exten…
omanizer Jun 27, 2016
a6bd3c7
Refactored write method to consolidate what ends up inside conditiona…
omanizer Jun 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 55 additions & 13 deletions src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
* If you use such an index, you can drop `gc_probability` to 0 since
* no garbage-collection is required.
*
* @param \Mongo|\MongoClient $mongo A MongoClient or Mongo instance
* @param array $options An associative array of field options
* @param \Mongo|\MongoClient|\MongoDB\Client $mongo A MongoDB\Client, MongoClient or Mongo instance
* @param array $options An associative array of field options
*
* @throws \InvalidArgumentException When MongoClient or Mongo instance not provided
* @throws \InvalidArgumentException When "database" or "collection" not provided
*/
public function __construct($mongo, array $options)
{
if (!($mongo instanceof \MongoClient || $mongo instanceof \Mongo)) {
if (!($mongo instanceof \MongoDB\Client || $mongo instanceof \MongoClient || $mongo instanceof \Mongo)) {
throw new \InvalidArgumentException('MongoClient or Mongo instance required');
}

Expand Down Expand Up @@ -108,7 +108,9 @@ public function close()
*/
public function destroy($sessionId)
{
$this->getCollection()->remove(array(
$methodName = ($this->mongo instanceof \MongoDB\Client) ? 'deleteOne' : 'remove';

$this->getCollection()->$methodName(array(
$this->options['id_field'] => $sessionId,
));

Expand All @@ -120,8 +122,10 @@ public function destroy($sessionId)
*/
public function gc($maxlifetime)
{
$this->getCollection()->remove(array(
$this->options['expiry_field'] => array('$lt' => new \MongoDate()),
$methodName = ($this->mongo instanceof \MongoDB\Client) ? 'deleteOne' : 'remove';

$this->getCollection()->$methodName(array(
$this->options['expiry_field'] => array('$lt' => $this->createDateTime()),
));

return true;
Expand All @@ -132,18 +136,28 @@ public function gc($maxlifetime)
*/
public function write($sessionId, $data)
{
$expiry = new \MongoDate(time() + (int) ini_get('session.gc_maxlifetime'));
$expiry = $this->createDateTime(time() + (int) ini_get('session.gc_maxlifetime'));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use a

  $fields = array(
        $this->options['time_field'] => $this->createDateTime(),
       $this->options['expiry_field'] => $expiry,
      );

Then add the MongoDbBin part ?

Copy link
Contributor Author
@omanizer omanizer Jun 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I'd suggest keeping 2 different blocks due to the update method name changing too (update vs updateOne as well as needing the 'multiple' property passed in the options parameter for the first one. I'll modify this to condense the $fields declaration though.

$fields = array(
$this->options['data_field'] => new \MongoBinData($data, \MongoBinData::BYTE_ARRAY),
$this->options['time_field'] => new \MongoDate(),
$this->options['time_field'] => $this->createDateTime(),
$this->options['expiry_field'] => $expiry,
);

$this->getCollection()->update(
$options = array('upsert' => true);

if ($this->mongo instanceof \MongoDB\Client) {
$fields[$this->options['data_field']] = new \MongoDB\BSON\Binary($data, \MongoDB\BSON\Binary::TYPE_OLD_BINARY);
} else {
$fields[$this->options['data_field']] = new \MongoBinData($data, \MongoBinData::BYTE_ARRAY);
$options['multiple'] = false;
}

$methodName = ($this->mongo instanceof \MongoDB\Client) ? 'updateOne' : 'update';

$this->getCollection()->$methodName(
array($this->options['id_field'] => $sessionId),
array('$set' => $fields),
array('upsert' => true, 'multiple' => false)
$options
);

return true;
Expand All @@ -156,10 +170,18 @@ public function read($sessionId)
{
$dbData = $this->getCollection()->findOne(array(
$this->options['id_field'] => $sessionId,
$this->options['expiry_field'] => array('$gte' => new \MongoDate()),
$this->options['expiry_field'] => array('$gte' => $this->createDateTime()),
));

return null === $dbData ? '' : $dbData[$this->options['data_field']]->bin;
if (null === $dbData) {
return '';
}

if ($dbData[$this->options['data_field']] instanceof \MongoDB\BSON\Binary) {
return $dbData[$this->options['data_field']]->getData();
}

return $dbData[$this->options['data_field']]->bin;
}

/**
Expand All @@ -185,4 +207,24 @@ protected function getMongo()
{
return $this->mongo;
}

/**
* Create a date object using the class appropriate for the current mongo connection.
*
* Return an instance of a MongoDate or \MongoDB\BSON\UTCDateTime
*
* @param int $seconds An integer representing UTC seconds since Jan 1 1970. Defaults to now.
*/
private function createDateTime($seconds = null)
{
if (is_null($seconds)) {
$seconds = time();
}

if ($this->mongo instanceof \MongoDB\Client) {
return new \MongoDB\BSON\UTCDateTime($seconds * 1000);
}

return new \MongoDate($seconds);
}
}
124 changes: 98 additions & 26 deletions ...fony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

/**
* @author Markus Bachmann <markus.bachmann@bachi.biz>
* @requires extension mongo
* @group time-sensitive
*/
class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
Expand All @@ -31,7 +30,15 @@ protected function setUp()
{
parent::setUp();

$mongoClass = version_compare(phpversion('mongo'), '1.3.0', '<') ? 'Mongo' : 'MongoClient';
if (!extension_loaded('mongo') && !extension_loaded('mongodb')) {
$this->markTestSkipped('The Mongo or MongoDB extension is required.');
}

if (phpversion('mongodb')) {
$mongoClass = 'MongoDB\Client';
} else {
$mongoClass = version_compare(phpversion('mongo'), '1.3.0', '<') ? 'Mongo' : 'MongoClient';
}

$this->mongo = $this->getMockBuilder($mongoClass)
->disableOriginalConstructor()
Expand Down Expand Up @@ -96,14 +103,28 @@ public function testRead()

$this->assertArrayHasKey($this->options['expiry_field'], $criteria);
$this->assertArrayHasKey('$gte', $criteria[$this->options['expiry_field']]);
$this->assertInstanceOf('MongoDate', $criteria[$this->options['expiry_field']]['$gte']);
$this->assertGreaterThanOrEqual($criteria[$this->options['expiry_field']]['$gte']->sec, $testTimeout);

return array(
if (phpversion('mongodb')) {
$this->assertInstanceOf('MongoDB\BSON\UTCDateTime', $criteria[$this->options['expiry_field']]['$gte']);
$this->assertGreaterThanOrEqual(round(intval((string) $criteria[$this->options['expiry_field']]['$gte']) / 1000), $testTimeout);
} else {
$this->assertInstanceOf('MongoDate', $criteria[$this->options['expiry_field']]['$gte']);
$this->assertGreaterThanOrEqual($criteria[$this->options['expiry_field']]['$gte']->sec, $testTimeout);
}

$fields = array(
$this->options['id_field'] => 'foo',
$this->options['data_field'] => new \MongoBinData('bar', \MongoBinData::BYTE_ARRAY),
$this->options['id_field'] => new \MongoDate(),
);

if (phpversion('mongodb')) {
$fields[$this->options['data_field']] = new \MongoDB\BSON\Binary('bar', \MongoDB\BSON\Binary::TYPE_OLD_BINARY);
$fields[$this->options['id_field']] = new \MongoDB\BSON\UTCDateTime(time() * 1000);
} else {
$fields[$this->options['data_field']] = new \MongoBinData('bar', \MongoBinData::BYTE_ARRAY);
$fields[$this->options['id_field']] = new \MongoDate();
}

return $fields;
}));

$this->assertEquals('bar', $this->storage->read('foo'));
Expand All @@ -120,22 +141,36 @@ public function testWrite()

$data = array();

$methodName = phpversion('mongodb') ? 'updateOne' : 'update';

$collection->expects($this->once())
->method('update')
->method($methodName)
->will($this->returnCallback(function ($criteria, $updateData, $options) use (&$data) {
$this->assertEquals(array($this->options['id_field'] => 'foo'), $criteria);
$this->assertEquals(array('upsert' => true, 'multiple' => false), $options);

if (phpversion('mongodb')) {
$this->assertEquals(array('upsert' => true), $options);
} else {
$this->assertEquals(array('upsert' => true, 'multiple' => false), $options);
}

$data = $updateData['$set'];
}));

$expectedExpiry = time() + (int) ini_get('session.gc_maxlifetime');
$this->assertTrue($this->storage->write('foo', 'bar'));

$this->assertEquals('bar', $data[$this->options['data_field']]->bin);
$this->assertInstanceOf('MongoDate', $data[$this->options['time_field']]);
$this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]);
$this->assertGreaterThanOrEqual($expectedExpiry, $data[$this->options['expiry_field']]->sec);
if (phpversion('mongodb')) {
$this->assertEquals('bar', $data[$this->options['data_field']]->getData());
$this->assertInstanceOf('MongoDB\BSON\UTCDateTime', $data[$this->options['time_field']]);
$this->assertInstanceOf('MongoDB\BSON\UTCDateTime', $data[$this->options['expiry_field']]);
$this->assertGreaterThanOrEqual($expectedExpiry, round(intval((string) $data[$this->options['expiry_field']]) / 1000));
} else {
$this->assertEquals('bar', $data[$this->options['data_field']]->bin);
$this->assertInstanceOf('MongoDate', $data[$this->options['time_field']]);
$this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]);
$this->assertGreaterThanOrEqual($expectedExpiry, $data[$this->options['expiry_field']]->sec);
}
}

public function testWriteWhenUsingExpiresField()
Expand All @@ -160,20 +195,33 @@ public function testWriteWhenUsingExpiresField()

$data = array();

$methodName = phpversion('mongodb') ? 'updateOne' : 'update';

$collection->expects($this->once())
->method('update')
->method($methodName)
->will($this->returnCallback(function A93C ($criteria, $updateData, $options) use (&$data) {
$this->assertEquals(array($this->options['id_field'] => 'foo'), $criteria);
$this->assertEquals(array('upsert' => true, 'multiple' => false), $options);

if (phpversion('mongodb')) {
$this->assertEquals(array('upsert' => true), $options);
} else {
$this->assertEquals(array('upsert' => true, 'multiple' => false), $options);
}

$data = $updateData['$set'];
}));

$this->assertTrue($this->storage->write('foo', 'bar'));

$this->assertEquals('bar', $data[$this->options['data_field']]->bin);
$this->assertInstanceOf('MongoDate', $data[$this->options['time_field']]);
$this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]);
if (phpversion('mongodb')) {
$this->assertEquals('bar', $data[$this->options['data_field']]->getData());
$this->assertInstanceOf('MongoDB\BSON\UTCDateTime', $data[$this->options['time_field']]);
$this->assertInstanceOf('MongoDB\BSON\UTCDateTime', $data[$this->options['expiry_field']]);
} else {
$this->assertEquals('bar', $data[$this->options['data_field']]->bin);
$this->assertInstanceOf('MongoDate', $data[$this->options['time_field']]);
$this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]);
}
}

public function testReplaceSessionData()
Expand All @@ -187,16 +235,22 @@ public function testReplaceSessionData()

$data = array();

$methodName = phpversion('mongodb') ? 'updateOne' : 'update';

$collection->expects($this->exactly(2))
->method('update')
->method($methodName)
->will($this->returnCallback(function ($criteria, $updateData, $options) use (&$data) {
$data = $updateData;
}));

$this->storage->write('foo', 'bar');
$this->storage->write('foo', 'foobar');

$this->assertEquals('foobar', $data['$set'][$this->options['data_field']]->bin);
if (phpversion('mongodb')) {
$this->assertEquals('foobar', $data['$set'][$this->options['data_field']]->getData());
} else {
$this->assertEquals('foobar', $data['$set'][$this->options['data_field']]->bin);
}
}

public function testDestroy()
Expand All @@ -208,8 +262,10 @@ public function testDestroy()
->with($this->options['database'], $this->options['collection'])
->will($this->returnValue($collection));

$methodName = phpversion('mongodb') ? 'deleteOne' : 'remove';

$collection->expects($this->once())
->method('remove')
->method($methodName)
->with(array($this->options['id_field'] => 'foo'));

$this->assertTrue($this->storage->destroy('foo'));
Expand All @@ -224,11 +280,18 @@ public function testGc()
->with($this->options['database'], $this->options['collection'])
->will($this->returnValue($collection));

$methodName = phpversion('mongodb') ? 'deleteOne' : 'remove';

$collection->expects($this->once())
->method('remove')
->method($methodName)
->will($this->returnCallback(function ($criteria) {
$this->assertInstanceOf('MongoDate', $criteria[$this->options['expiry_field']]['$lt']);
$this->assertGreaterThanOrEqual(time() - 1, $criteria[$this->options['expiry_field']]['$lt']->sec);
if (phpversion('mongodb')) {
$this->assertInstanceOf('MongoDB\BSON\UTCDateTime', $criteria[$this->options['expiry_field']]['$lt']);
$this->assertGreaterThanOrEqual(time() - 1, round(intval((string) $criteria[$this->options['expiry_field']]['$lt']) / 1000));
} else {
$this->assertInstanceOf('MongoDate', $criteria[$this->options['expiry_field']]['$lt']);
$this->assertGreaterThanOrEqual(time() - 1, $criteria[$this->options['expiry_field']]['$lt']->sec);
}
}));

$this->assertTrue($this->storage->gc(1));
Expand All @@ -239,14 +302,23 @@ public function testGetConnection()
$method = new \ReflectionMethod($this->storage, 'getMongo');
$method->setAccessible(true);

$mongoClass = (version_compare(phpversion('mongo'), '1.3.0', '<')) ? '\Mongo' : '\MongoClient';
if (phpversion('mongodb')) {
$mongoClass = 'MongoDB\Client';
} else {
$mongoClass = version_compare(phpversion('mongo'), '1.3.0', '<') ? 'Mongo' : 'MongoClient';
}

$this->assertInstanceOf($mongoClass, $method->invoke($this->storage));
}

private function createMongoCollectionMock()
{
$collection = $this->getMockBuilder('MongoCollection')
$collectionClass = 'MongoCollection';
if (phpversion('mongodb')) {
$collectionClass = 'MongoDB\Collection';
}

$collection = $this->getMockBuilder($collectionClass)
->disableOriginalConstructor()
->getMock();

Expand Down
0