From e185c8d63b5b84bad89136c33db1d5bf413aa184 Mon Sep 17 00:00:00 2001 From: Drak Date: Thu, 8 Dec 2011 11:35:01 +0545 Subject: [PATCH] [HttpFoundation] Refactored component for session workflow. --- .../Component/HttpFoundation/Request.php | 10 +- .../FilesystemSessionStorage.php | 192 ------------------ .../SessionStorage/NativeSessionStorage.php | 180 ---------------- .../SessionStorage/PdoSessionStorage.php | 102 +++------- .../Component/HttpFoundation/RequestTest.php | 8 +- .../FilesystemSessionStorageTest.php | 104 ---------- 6 files changed, 41 insertions(+), 555 deletions(-) delete mode 100644 src/Symfony/Component/HttpFoundation/SessionStorage/FilesystemSessionStorage.php delete mode 100644 src/Symfony/Component/HttpFoundation/SessionStorage/NativeSessionStorage.php delete mode 100644 tests/Symfony/Tests/Component/HttpFoundation/SessionStorage/FilesystemSessionStorageTest.php diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index fc33610906..bd6bf43629 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -11,6 +11,8 @@ namespace Symfony\Component\HttpFoundation; +use Symfony\Component\HttpFoundation\SessionInterface; + /** * Request represents an HTTP request. * @@ -122,7 +124,7 @@ class Request protected $format; /** - * @var \Symfony\Component\HttpFoundation\Session + * @var \Symfony\Component\HttpFoundation\SessionInterface */ protected $session; @@ -466,7 +468,7 @@ class Request /** * Gets the Session. * - * @return Session|null The session + * @return SessionInterface|null The session * * @api */ @@ -504,11 +506,11 @@ class Request /** * Sets the Session. * - * @param Session $session The Session + * @param SessionInterface $session The Session * * @api */ - public function setSession(Session $session) + public function setSession(SessionInterface $session) { $this->session = $session; } diff --git a/src/Symfony/Component/HttpFoundation/SessionStorage/FilesystemSessionStorage.php b/src/Symfony/Component/HttpFoundation/SessionStorage/FilesystemSessionStorage.php deleted file mode 100644 index ceb5913fe9..0000000000 --- a/src/Symfony/Component/HttpFoundation/SessionStorage/FilesystemSessionStorage.php +++ /dev/null @@ -1,192 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\HttpFoundation\SessionStorage; - -/** - * FilesystemSessionStorage simulates sessions for functional tests. - * - * This storage does not start the session (session_start()) - * as it is not "available" when running tests on the command line. - * - * @author Fabien Potencier - * - * @api - */ -class FilesystemSessionStorage extends NativeSessionStorage -{ - /** - * File path. - * - * @var string - */ - private $path; - - /** - * Data. - * - * @var array - */ - private $data; - - /** - * Session started flag. - * - * @var boolean - */ - private $started; - - /** - * Constructor. - */ - public function __construct($path, array $options = array()) - { - $this->path = $path; - $this->started = false; - - parent::__construct($options); - } - - /** - * Starts the session. - * - * @api - */ - public function start() - { - if ($this->started) { - return; - } - - session_set_cookie_params( - $this->options['lifetime'], - $this->options['path'], - $this->options['domain'], - $this->options['secure'], - $this->options['httponly'] - ); - - if (!ini_get('session.use_cookies') && isset($this->options['id']) && $this->options['id'] && $this->options['id'] != session_id()) { - session_id($this->options['id']); - } - - if (!session_id()) { - session_id(hash('md5', uniqid(mt_rand(), true))); - } - - $file = $this->path.'/'.session_id().'.session'; - - $this->data = is_file($file) ? unserialize(file_get_contents($file)) : array(); - $this->started = true; - } - - /** - * Returns the session ID - * - * @return mixed The session ID - * - * @throws \RuntimeException If the session was not started yet - * - * @api - */ - public function getId() - { - if (!$this->started) { - throw new \RuntimeException('The session must be started before reading its ID'); - } - - return session_id(); - } - - /** - * Reads data from this storage. - * - * The preferred format for a key is directory style so naming conflicts can be avoided. - * - * @param string $key A unique key identifying your data - * @param string $default The default value - * - * @return mixed Data associated with the key - * - * @throws \RuntimeException If an error occurs while reading data from this storage - * - * @api - */ - public function read($key, $default = null) - { - return array_key_exists($key, $this->data) ? $this->data[$key] : $default; - } - - /** - * Removes data from this storage. - * - * The preferred format for a key is directory style so naming conflicts can be avoided. - * - * @param string $key A unique key identifying your data - * - * @return mixed Data associated with the key - * - * @throws \RuntimeException If an error occurs while removing data from this storage - * - * @api - */ - public function remove($key) - { - $retval = $this->data[$key]; - - unset($this->data[$key]); - - return $retval; - } - - /** - * Writes data to this storage. - * - * The preferred format for a key is directory style so naming conflicts can be avoided. - * - * @param string $key A unique key identifying your data - * @param mixed $data Data associated with your key - * - * @throws \RuntimeException If an error occurs while writing to this storage - * - * @api - */ - public function write($key, $data) - { - $this->data[$key] = $data; - - if (!is_dir($this->path)) { - mkdir($this->path, 0777, true); - } - - file_put_contents($this->path.'/'.session_id().'.session', serialize($this->data)); - } - - /** - * Regenerates id that represents this storage. - * - * @param Boolean $destroy Destroy session when regenerating? - * - * @return Boolean True if session regenerated, false if error - * - * @throws \RuntimeException If an error occurs while regenerating this storage - * - * @api - */ - public function regenerate($destroy = false) - { - if ($destroy) { - $this->data = array(); - } - - return true; - } -} diff --git a/src/Symfony/Component/HttpFoundation/SessionStorage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/SessionStorage/NativeSessionStorage.php deleted file mode 100644 index b759f7411a..0000000000 --- a/src/Symfony/Component/HttpFoundation/SessionStorage/NativeSessionStorage.php +++ /dev/null @@ -1,180 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\HttpFoundation\SessionStorage; - -/** - * NativeSessionStorage. - * - * @author Fabien Potencier - * - * @api - */ -class NativeSessionStorage implements SessionStorageInterface -{ - static protected $sessionIdRegenerated = false; - static protected $sessionStarted = false; - - protected $options; - - /** - * Available options: - * - * * name: The cookie name (null [omitted] by default) - * * id: The session id (null [omitted] by default) - * * lifetime: Cookie lifetime - * * path: Cookie path - * * domain: Cookie domain - * * secure: Cookie secure - * * httponly: Cookie http only - * - * The default values for most options are those returned by the session_get_cookie_params() function - * - * @param array $options An associative array of session options - */ - public function __construct(array $options = array()) - { - $cookieDefaults = session_get_cookie_params(); - - $this->options = array_merge(array( - 'lifetime' => $cookieDefaults['lifetime'], - 'path' => $cookieDefaults['path'], - 'domain' => $cookieDefaults['domain'], - 'secure' => $cookieDefaults['secure'], - 'httponly' => isset($cookieDefaults['httponly']) ? $cookieDefaults['httponly'] : false, - ), $options); - - // Skip setting new session name if user don't want it - if (isset($this->options['name'])) { - session_name($this->options['name']); - } - } - - /** - * Starts the session. - * - * @api - */ - public function start() - { - if (self::$sessionStarted) { - return; - } - - session_set_cookie_params( - $this->options['lifetime'], - $this->options['path'], - $this->options['domain'], - $this->options['secure'], - $this->options['httponly'] - ); - - // disable native cache limiter as this is managed by HeaderBag directly - session_cache_limiter(false); - - if (!ini_get('session.use_cookies') && isset($this->options['id']) && $this->options['id'] && $this->options['id'] != session_id()) { - session_id($this->options['id']); - } - - session_start(); - - self::$sessionStarted = true; - } - - /** - * {@inheritDoc} - * - * @api - */ - public function getId() - { - if (!self::$sessionStarted) { - throw new \RuntimeException('The session must be started before reading its ID'); - } - - return session_id(); - } - - /** - * Reads data from this storage. - * - * The preferred format for a key is directory style so naming conflicts can be avoided. - * - * @param string $key A unique key identifying your data - * @param string $default Default value - * - * @return mixed Data associated with the key - * - * @api - */ - public function read($key, $default = null) - { - return array_key_exists($key, $_SESSION) ? $_SESSION[$key] : $default; - } - - /** - * Removes data from this storage. - * - * The preferred format for a key is directory style so naming conflicts can be avoided. - * - * @param string $key A unique key identifying your data - * - * @return mixed Data associated with the key - * - * @api - */ - public function remove($key) - { - $retval = null; - - if (isset($_SESSION[$key])) { - $retval = $_SESSION[$key]; - unset($_SESSION[$key]); - } - - return $retval; - } - - /** - * Writes data to this storage. - * - * The preferred format for a key is directory style so naming conflicts can be avoided. - * - * @param string $key A unique key identifying your data - * @param mixed $data Data associated with your key - * - * @api - */ - public function write($key, $data) - { - $_SESSION[$key] = $data; - } - - /** - * Regenerates id that represents this storage. - * - * @param Boolean $destroy Destroy session when regenerating? - * - * @return Boolean True if session regenerated, false if error - * - * @api - */ - public function regenerate($destroy = false) - { - if (self::$sessionIdRegenerated) { - return; - } - - session_regenerate_id($destroy); - - self::$sessionIdRegenerated = true; - } -} diff --git a/src/Symfony/Component/HttpFoundation/SessionStorage/PdoSessionStorage.php b/src/Symfony/Component/HttpFoundation/SessionStorage/PdoSessionStorage.php index 24898b7c20..1a264f1753 100644 --- a/src/Symfony/Component/HttpFoundation/SessionStorage/PdoSessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/SessionStorage/PdoSessionStorage.php @@ -11,24 +11,28 @@ namespace Symfony\Component\HttpFoundation\SessionStorage; +use Symfony\Component\HttpFoundation\AttributeBagInterface; +use Symfony\Component\HttpFoundation\FlashBagInterface; + /** * PdoSessionStorage. * * @author Fabien Potencier * @author Michael Williams */ -class PdoSessionStorage extends NativeSessionStorage +class PdoSessionStorage extends AbstractSessionStorage implements SessionSaveHandlerInterface { /** * PDO instance. * * @var \PDO */ - private $db; + private $pdo; /** * Database options. * + * * @var array */ private $dbOptions; @@ -36,59 +40,35 @@ class PdoSessionStorage extends NativeSessionStorage /** * Constructor. * - * @param \PDO $db A PDO instance - * @param array $options An associative array of session options - * @param array $dbOptions An associative array of DB options + * + * @param \PDO $pdo A \PDO instance + * @param array $dbOptions An associative array of DB options + * @param array $options Session configuration options + * @param AttributeBagInterface $attributes An AttributeBagInterface instance, (defaults null for default AttributeBag) + * @param FlashBagInterface $flashes A FlashBagInterface instance (defaults null for defaul FlashBag) * * @throws \InvalidArgumentException When "db_table" option is not provided * - * @see NativeSessionStorage::__construct() + * @see AbstractSessionStorage::__construct() */ - public function __construct(\PDO $db, array $options = array(), array $dbOptions = array()) + public function __construct(\PDO $pdo, array $dbOptions = array(), array $options = array(), AttributeBagInterface $attributes = null, FlashBagInterface $flashes = null) { if (!array_key_exists('db_table', $dbOptions)) { throw new \InvalidArgumentException('You must provide the "db_table" option for a PdoSessionStorage.'); } - $this->db = $db; + $this->pdo = $pdo; $this->dbOptions = array_merge(array( 'db_id_col' => 'sess_id', 'db_data_col' => 'sess_data', 'db_time_col' => 'sess_time', ), $dbOptions); - parent::__construct($options); + parent::__construct($attributes, $flashes, $options); } /** - * Starts the session. - */ - public function start() - { - if (self::$sessionStarted) { - return; - } - - // use this object as the session handler - session_set_save_handler( - array($this, 'sessionOpen'), - array($this, 'sessionClose'), - array($this, 'sessionRead'), - array($this, 'sessionWrite'), - array($this, 'sessionDestroy'), - array($this, 'sessionGC') - ); - - parent::start(); - } - - /** - * Opens a session. - * - * @param string $path (ignored) - * @param string $name (ignored) - * - * @return Boolean true, if the session was opened, otherwise an exception is thrown + * {@inheritdoc} */ public function sessionOpen($path = null, $name = null) { @@ -96,22 +76,15 @@ class PdoSessionStorage extends NativeSessionStorage } /** - * Closes a session. - * - * @return Boolean true, if the session was closed, otherwise false + * {@inheritdoc} */ public function sessionClose() { - // do nothing return true; } /** - * Destroys a session. - * - * @param string $id A session ID - * - * @return Boolean true, if the session was destroyed, otherwise an exception is thrown + * {@inheritdoc} * * @throws \RuntimeException If the session cannot be destroyed */ @@ -125,7 +98,7 @@ class PdoSessionStorage extends NativeSessionStorage $sql = "DELETE FROM $dbTable WHERE $dbIdCol = :id"; try { - $stmt = $this->db->prepare($sql); + $stmt = $this->pdo->prepare($sql); $stmt->bindParam(':id', $id, \PDO::PARAM_STR); $stmt->execute(); } catch (\PDOException $e) { @@ -136,15 +109,11 @@ class PdoSessionStorage extends NativeSessionStorage } /** - * Cleans up old sessions. - * - * @param int $lifetime The lifetime of a session - * - * @return Boolean true, if old sessions have been cleaned, otherwise an exception is thrown + * {@inheritdoc} * * @throws \RuntimeException If any old sessions cannot be cleaned */ - public function sessionGC($lifetime) + public function sessionGc($lifetime) { // get table/column $dbTable = $this->dbOptions['db_table']; @@ -154,7 +123,7 @@ class PdoSessionStorage extends NativeSessionStorage $sql = "DELETE FROM $dbTable WHERE $dbTimeCol < (:time - $lifetime)"; try { - $stmt = $this->db->prepare($sql); + $stmt = $this->pdo->prepare($sql); $stmt->bindValue(':time', time(), \PDO::PARAM_INT); $stmt->execute(); } catch (\PDOException $e) { @@ -165,11 +134,7 @@ class PdoSessionStorage extends NativeSessionStorage } /** - * Reads a session. - * - * @param string $id A session ID - * - * @return string The session data if the session was read or created, otherwise an exception is thrown + * {@inheritdoc} * * @throws \RuntimeException If the session cannot be read */ @@ -183,7 +148,7 @@ class PdoSessionStorage extends NativeSessionStorage try { $sql = "SELECT $dbDataCol FROM $dbTable WHERE $dbIdCol = :id"; - $stmt = $this->db->prepare($sql); + $stmt = $this->pdo->prepare($sql); $stmt->bindParam(':id', $id, \PDO::PARAM_STR, 255); $stmt->execute(); @@ -200,17 +165,12 @@ class PdoSessionStorage extends NativeSessionStorage return ''; } catch (\PDOException $e) { - throw new \RuntimeException(sprintf('PDOException was thrown when trying to manipulate session data: %s', $e->getMessage()), 0, $e); + throw new \RuntimeException(sprintf('PDOException was thrown when trying to read the session data: %s', $e->getMessage()), 0, $e); } } /** - * Writes session data. - * - * @param string $id A session ID - * @param string $data A serialized chunk of session data - * - * @return Boolean true, if the session was written, otherwise an exception is thrown + * {@inheritdoc} * * @throws \RuntimeException If the session data cannot be written */ @@ -222,7 +182,7 @@ class PdoSessionStorage extends NativeSessionStorage $dbIdCol = $this->dbOptions['db_id_col']; $dbTimeCol = $this->dbOptions['db_time_col']; - $sql = ('mysql' === $this->db->getAttribute(\PDO::ATTR_DRIVER_NAME)) + $sql = ('mysql' === $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME)) ? "INSERT INTO $dbTable ($dbIdCol, $dbDataCol, $dbTimeCol) VALUES (:id, :data, :time) " ."ON DUPLICATE KEY UPDATE $dbDataCol = VALUES($dbDataCol), $dbTimeCol = CASE WHEN $dbTimeCol = :time THEN (VALUES($dbTimeCol) + 1) ELSE VALUES($dbTimeCol) END" : "UPDATE $dbTable SET $dbDataCol = :data, $dbTimeCol = :time WHERE $dbIdCol = :id"; @@ -230,7 +190,7 @@ class PdoSessionStorage extends NativeSessionStorage try { //session data can contain non binary safe characters so we need to encode it $encoded = base64_encode($data); - $stmt = $this->db->prepare($sql); + $stmt = $this->pdo->prepare($sql); $stmt->bindParam(':id', $id, \PDO::PARAM_STR); $stmt->bindParam(':data', $encoded, \PDO::PARAM_STR); $stmt->bindValue(':time', time(), \PDO::PARAM_INT); @@ -242,7 +202,7 @@ class PdoSessionStorage extends NativeSessionStorage $this->createNewSession($id, $data); } } catch (\PDOException $e) { - throw new \RuntimeException(sprintf('PDOException was thrown when trying to manipulate session data: %s', $e->getMessage()), 0, $e); + throw new \RuntimeException(sprintf('PDOException was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e); } return true; @@ -268,7 +228,7 @@ class PdoSessionStorage extends NativeSessionStorage //session data can contain non binary safe characters so we need to encode it $encoded = base64_encode($data); - $stmt = $this->db->prepare($sql); + $stmt = $this->pdo->prepare($sql); $stmt->bindParam(':id', $id, \PDO::PARAM_STR); $stmt->bindParam(':data', $encoded, \PDO::PARAM_STR); $stmt->bindValue(':time', time(), \PDO::PARAM_INT); diff --git a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php index 4b208bb3ec..9f144724f8 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php @@ -13,10 +13,10 @@ namespace Symfony\Tests\Component\HttpFoundation; use Symfony\Component\HttpFoundation\SessionStorage\ArraySessionStorage; - use Symfony\Component\HttpFoundation\Session; - use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\FlashBag; +use Symfony\Component\HttpFoundation\AttributeBag; class RequestTest extends \PHPUnit_Framework_TestCase { @@ -848,7 +848,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase $request = new Request; $this->assertFalse($request->hasSession()); - $request->setSession(new Session(new ArraySessionStorage())); + $request->setSession(new Session(new ArraySessionStorage(new AttributeBag(), new FlashBag()))); $this->assertTrue($request->hasSession()); } @@ -859,7 +859,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase $this->assertFalse($request->hasPreviousSession()); $request->cookies->set(session_name(), 'foo'); $this->assertFalse($request->hasPreviousSession()); - $request->setSession(new Session(new ArraySessionStorage())); + $request->setSession(new Session(new ArraySessionStorage(new AttributeBag(), new FlashBag()))); $this->assertTrue($request->hasPreviousSession()); } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/SessionStorage/FilesystemSessionStorageTest.php b/tests/Symfony/Tests/Component/HttpFoundation/SessionStorage/FilesystemSessionStorageTest.php deleted file mode 100644 index 060cb0e913..0000000000 --- a/tests/Symfony/Tests/Component/HttpFoundation/SessionStorage/FilesystemSessionStorageTest.php +++ /dev/null @@ -1,104 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Tests\Component\HttpFoundation\SessionStorage; - -use Symfony\Component\HttpFoundation\SessionStorage\FilesystemSessionStorage; - -class FilesystemSessionStorageTest extends \PHPUnit_Framework_TestCase -{ - private $path; - - protected function setUp() - { - $this->path = sys_get_temp_dir().'/sf2/session_test'; - if (!file_exists($this->path)) { - mkdir($this->path, 0777, true); - } - } - - protected function tearDown() - { - array_map('unlink', glob($this->path.'/*.session')); - rmdir($this->path); - - $this->path = null; - } - - public function testMultipleInstances() - { - $storage1 = new FilesystemSessionStorage($this->path); - $storage1->start(); - $storage1->write('foo', 'bar'); - - $storage2 = new FilesystemSessionStorage($this->path); - $storage2->start(); - $this->assertEquals('bar', $storage2->read('foo'), 'values persist between instances'); - } - - public function testGetIdThrowsErrorBeforeStart() - { - $this->setExpectedException('RuntimeException'); - - $storage = new FilesystemSessionStorage($this->path); - $storage->getId(); - } - - public function testGetIdWorksAfterStart() - { - $storage = new FilesystemSessionStorage($this->path); - $storage->start(); - $storage->getId(); - } - - public function testGetIdSetByOptions() - { - $previous = ini_get('session.use_cookies'); - - ini_set('session.use_cookies', false); - - $storage = new FilesystemSessionStorage($this->path, array('id' => 'symfony2-sessionId')); - $storage->start(); - - $this->assertEquals('symfony2-sessionId', $storage->getId()); - - ini_set('session.use_cookies', $previous); - } - - public function testRemoveVariable() - { - $storage = new FilesystemSessionStorage($this->path); - $storage->start(); - - $storage->write('foo', 'bar'); - - $this->assertEquals('bar', $storage->read('foo')); - - $storage->remove('foo', 'bar'); - - $this->assertNull($storage->read('foo')); - } - - public function testRegenerate() - { - $storage = new FilesystemSessionStorage($this->path); - $storage->start(); - $storage->write('foo', 'bar'); - - $storage->regenerate(); - - $this->assertEquals('bar', $storage->read('foo')); - - $storage->regenerate(true); - - $this->assertNull($storage->read('foo')); - } -}