8000 [HttpFoundation] enhance PdoSessionHandler by Tobion · Pull Request #10931 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] enhance PdoSessionHandler #10931

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

Merged
merged 8 commits into from
Oct 3, 2014
Merged

[HttpFoundation] enhance PdoSessionHandler #10931

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7dad54c
[HttpFoundation] remove base64 encoding of session data
Tobion May 17, 2014
251238d
[HttpFoundation] implement lazy connect for pdo session handler
Tobion May 18, 2014
af1bb1f
add test for null byte in session data
Tobion May 18, 2014
e79229d
[HttpFoundation] allow different lifetime per session
Tobion May 18, 2014
182a5d3
[HttpFoundation] add create table method to pdo session handler
Tobion May 18, 2014
5978fcf
added upgrade and changelog notes for PdoSessionHandler
Tobion May 25, 2014
6f5748e
adjust sqlite table definition
Tobion Jun 4, 2014
1bc6680
[HttpFoundation] implement different locking strategies for sessions
Tobion Jun 7, 2014
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
Prev Previous commit
Next Next commit
[HttpFoundation] implement lazy connect for pdo session handler
  • Loading branch information
@Tobion
Tobion committed Sep 29, 2014
commit 251238d9a631a974b26855848ec9b5266c7b4626
72 changes: 59 additions & 13 deletions src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,15 @@
class PdoSessionHandler implements \SessionHandlerInterface
{
/**
* @var \PDO PDO instance
* @var \PDO|null PDO instance or null when not connected yet
*/
private $pdo;

/**
* @var string|null|false DNS string or null for session.save_path or false when lazy connection disabled
Copy link

Choose a reason for hiding this comment

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

Maybe you meant DSN instead of DNS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha true

*/
private $dns = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same typo in the var

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same typo everywhere. I'm consistent :)


/**
* @var string Database driver
*/
Expand All @@ -66,6 +71,21 @@ class PdoSessionHandler implements \SessionHandlerInterface
*/
private $timeCol;

/**
* @var string Username when lazy-connect
*/
private $username;

/**
* @var string Password when lazy-connect
*/
private $password;

/**
* @var array Connection options when lazy-connect
*/
private $connectionOptions = array();

/**
* @var bool Whether a transaction is active
*/
Expand All @@ -79,37 +99,54 @@ class PdoSessionHandler implements \SessionHandlerInterface
/**
* Constructor.
*
* You can either pass an existing database connection as PDO instance or
* pass a DNS string that will be used to lazy-connect to the database
* when the session is actually used. Furthermore it's possible to pass null
* which will then use the session.save_path ini setting as PDO DNS parameter.
*
* List of available options:
* * db_table: The name of the table [default: sessions]
* * db_id_col: The column where to store the session id [default: sess_id]
* * db_data_col: The column where to store the session data [default: sess_data]
* * db_time_col: The column where to store the timestamp [default: sess_time]
* * db_username: The username when lazy-connect [default: '']
* * db_password: The password when lazy-connect [default: '']
* * db_connection_options: An array of driver-specific connection options [default: array()]
*
* @param \PDO $pdo A \PDO instance
* @param array $options An associative array of DB options
* @param \PDO|string|null $pdoOrDns A \PDO instance or DNS string or null
* @param array $options An associative array of DB options
*
* @throws \InvalidArgumentException When PDO error mode is not PDO::ERRMODE_EXCEPTION
*/
public function __construct(\PDO $pdo, array $options = array())
public function __construct($pdoOrDns, array $options = array())
{
if (\PDO::ERRMODE_EXCEPTION !== $pdo->getAttribute(\PDO::ATTR_ERRMODE)) {
throw new \InvalidArgumentException(sprintf('"%s" requires PDO error mode attribute be set to throw Exceptions (i.e. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION))', __CLASS__));
}
if ($pdoOrDns instanceof \PDO) {
if (\PDO::ERRMODE_EXCEPTION !== $pdoOrDns->getAttribute(\PDO::ATTR_ERRMODE)) {
throw new \InvalidArgumentException(sprintf('"%s" requires PDO error mode attribute be set to throw Exceptions (i.e. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION))', __CLASS__));
}

$this->pdo = $pdo;
$this->driver = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME);
$this->pdo = $pdoOrDns;
} else {
$this->dns = $pdoOrDns;
}

$options = array_replace(array(
'db_table' => 'sessions',
'db_id_col' => 'sess_id',
'db_data_col' => 'sess_data',
'db_time_col' => 'sess_time',
'db_table' => 'sessions',
'db_id_col' => 'sess_id',
8000 'db_data_col' => 'sess_data',
'db_time_col' => 'sess_time',
'db_username' => '',
'db_password' => '',
'db_connection_options' => array()
), $options);

$this->table = $options['db_table'];
$this->idCol = $options['db_id_col'];
$this->dataCol = $options['db_data_col'];
$this->timeCol = $options['db_time_col'];
$this->username = $options['db_username'];
$this->password = $options['db_password'];
$this->connectionOptions = $options['db_connection_options'];
}

/**
Expand All @@ -118,6 +155,11 @@ public function __construct(\PDO $pdo, array $options = array())
public function open($savePath, $sessionName)
{
$this->gcCalled = false;
if (null === $this->pdo) {
$this->pdo = new \PDO($this->dns ?: $savePath, $this->username, $this->password, $this->connectionOptions);
$this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
}
$this->driver = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME);

return true;
}
Expand Down Expand Up @@ -270,6 +312,10 @@ public function close()
$stmt->execute();
}

if (false !== $this->dns) {
$this->pdo = null;
}

return true;
}

Expand Down
75 changes: 68 additions & 7 deletions src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protected function setUp()

$this->pdo = new \PDO('sqlite::memory:');
$this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
$sql = 'CREATE TABLE sessions (sess_id VARCHAR(128) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)';
$sql = 'CREATE TABLE sessions (sess_id VARCHAR(128) PRIMARY KEY, sess_data BLOB, sess_time INTEGER)';
$this->pdo->exec($sql);
}

Expand All @@ -51,17 +51,74 @@ public function testInexistentTable()
$storage->close();
}

public function testWithLazyDnsConnection()
Copy link

Choose a reason for hiding this comment

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

dsn*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx fixed

{
$dbFile = tempnam(sys_get_temp_dir(), 'sf2_sqlite_sessions');
if (file_exists($dbFile)) {
@unlink($dbFile);
Copy link
Contributor

Choose a reason for hiding this comment

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

should the test be skipped when unlink is not successfull?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather get an error the following call.

}

$pdo = new \PDO('sqlite:' . $dbFile);
$sql = 'CREATE TABLE sessions (sess_id VARCHAR(255) PRIMARY KEY, sess_data BLOB, sess_time INTEGER)';
$pdo->exec($sql);
$pdo = null;

$storage = new PdoSessionHandler('sqlite:' . $dbFile);
$storage->open('', 'sid');
$data = $storage->read('id');
$storage->write('id', 'data');
$storage->close();
$this->assertSame('', $data, 'New session returns empty string data');

$storage->open('', 'sid');
$data = $storage->read('id');
$storage->close();
$this->assertSame('data', $data, 'Written value can be read back correctly');

@unlink($dbFile);
}

public function testWithLazySavePathConnection()
{
$dbFile = tempnam(sys_get_temp_dir(), 'sf2_sqlite_sessions');
if (file_exists($dbFile)) {
@unlink($dbFile);
}

$pdo = new \PDO('sqlite:' . $dbFile);
$sql = 'CREATE TABLE sessions (sess_id VARCHAR(255) PRIMARY KEY, sess_data BLOB, sess_time INTEGER)';
$pdo->exec($sql);
$pdo = null;

// Open is called with what ini_set('session.save_path', 'sqlite:' . $dbFile) would mean
$storage = new PdoSessionHandler(null);
$storage->open('sqlite:' . $dbFile, 'sid');
$data = $storage->read('id');
$storage->write('id', 'data');
$storage->close();
$this->assertSame('', $data, 'New session returns empty string data');

$storage->open('sqlite:' . $dbFile, 'sid');
$data = $storage->read('id');
$storage->close();
$this->assertSame('data', $data, 'Written value can be read back correctly');

@unlink($dbFile);
}

public function testReadWriteRead()
{
$storage = new PdoSessionHandler($this->pdo);
$storage->open('', 'sid');
$this->assertSame('', $storage->read(& DF10 #39;id'), 'New session returns empty string data');
$data = $storage->read('id');
$storage->write('id', 'data');
$storage->close();
$this->assertSame('', $data, 'New session returns empty string data');

$storage->open('', 'sid');
$this->assertSame('data', $storage->read('id'), 'Written value can be read back correctly');
$data = $storage->read('id');
$storage->close();
$this->assertSame('data', $data, 'Written value can be read back correctly');
}

/**
Expand All @@ -77,8 +134,9 @@ public function testWriteDifferentSessionIdThanRead()
$storage->close();

$storage->open('', 'sid');
$this->assertSame('data_of_new_session_id', $storage->read('new_id'), 'Data of regenerated session id is available');
$data = $storage->read('new_id');
$storage->close();
$this->assertSame('data_of_new_session_id', $data, 'Data of regenerated session id is available');
}

/**
Expand Down Expand Up @@ -109,8 +167,9 @@ public function testSessionDestroy()
$this->assertEquals(0, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn());

$storage->open('', 'sid');
$this->assertSame('', $storage->read('id'), 'Destroyed session returns empty string');
$data = $storage->read('id');
$storage->close();
$this->assertSame('', $data, 'Destroyed session returns empty string');
}

public function testSessionGC()
Expand All @@ -125,12 +184,14 @@ public function testSessionGC()
$this->assertEquals(1, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn());

$storage->open('', 'sid');
$this->assertSame('', $storage->read('id'), 'Session already considered garbage, so not returning data even if it is not pruned yet');
$data = $storage->read('id');
$storage->gc(0);
$storage->close();
$this->assertEquals(0, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn());

ini_set('session.gc_maxlifetime', $previousLifeTime);

$this->assertSame('', $data, 'Session already considered garbage, so not returning data even if it is not pruned yet');
$this->assertEquals(0, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn());
}

public function testGetConnection()
Expand Down
0