diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch stof/session_cleanup (PR #3333)

Browse Source 
Commits
-------

2c767d1 [HttpFoundation] Fixed closeSession for the Memcached storage
ec44e68 [HttpFoundation] Fixed the use of the prefix for the Memcached storage
5808773 [HttpFoundation] Fixed a typo and updated the phpdoc for the session
0550bef Removed methods duplicated from parent class

Discussion
----------

Session cleanup

This cleans the phpdoc of the refactored session by using inheritdoc in all appropriate places. For the SessionInterface, I added the ``@api`` tag in all methods as the Session class had them.
It also fixes a few typos in the variable names.

I figured a few things:

- the Session class implements some methods that are not part of the SessionInterface. Is it intended or simply a left-over ?
- the MemcachedSessionStorage uses a ``prefix`` property which does not exist. This is clearly a copy-paste from the MemcacheSessionStorage which has it. It also call the ``Memcached#close`` method which does not exist according to PhpStorm. @drak could you check this class ? I don't know Memcached at all.
- as I said on the refactoring PR, the Serializable implementation of the Session class seems totally wrong as the SessionStorage is not serializable.

/cc @drak

---------------------------------------------------------------------------

by drak at 2012-02-12T02:09:38Z

@stof there is a ticket about this, the problem exists from Symfony 2.0 also - refs #3000 PDOSessionStorage

---------------------------------------------------------------------------

by drak at 2012-02-12T02:10:12Z

@stof I will look at the memcache issue and make a quick PR

---------------------------------------------------------------------------

by stof at 2012-02-12T02:11:07Z

@drak the issue could exist with any SessionStorage as your SessionStorageInterface does not enforce making it serializable

---------------------------------------------------------------------------

by stof at 2012-02-12T02:14:15Z

@drak note that this PR already fixes 2 typo in the memcached storage. It seems like some tests are missing for it as they should have been found (they would have raised a notice in the constructor)

---------------------------------------------------------------------------

by drak at 2012-02-12T02:14:50Z

Afaik, only PDO is not serializable@ but the `Serializable` is on the SessionInterface.  The problem is with #3000, the serialization is necessary but impossible.

---------------------------------------------------------------------------

by stof at 2012-02-12T02:21:42Z

@drak what about a Mongo storage or things like that ?

---------------------------------------------------------------------------

by drak at 2012-02-12T02:23:58Z

@stof Since you've started this PR would you mind removing `$prefix.` from the `*session()` methods in `MemcachedSessionStorage` - they are not required because Memcached handles prefixes itself.  I'll write some tests in another PR for them.  I guess I omitted it because at the time I didn't have them on my test env.

---------------------------------------------------------------------------

by drak at 2012-02-12T02:24:35Z

@stof - Let's take the serialization issue to #3000 because it's being discussed there.

---------------------------------------------------------------------------

by stof at 2012-02-12T02:27:27Z

@drak what about the ``close()`` method which does not exist in the Memcached class ?

---------------------------------------------------------------------------

by drak at 2012-02-12T03:58:11Z

The method just needs to `return true;`.
This commit is contained in:
Fabien Potencier 2012-02-12 13:06:54 +01:00
commit f6ee66f75a
14 changed files with 88 additions and 144 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Symfony/Component/HttpFoundation/Session/Attribute/AttributeBag.php
View File

@ -31,7 +31,7 @@ class AttributeBag implements AttributeBagInterface
/**
* Constructor.
*
* @param type $storageKey The key used to store flashes in the session.
* @param string $storageKey The key used to store flashes in the session.
*/
public function __construct($storageKey = '_sf2_attributes')
{

2
src/Symfony/Component/HttpFoundation/Session/Attribute/AttributeBagInterface.php
View File

@ -65,6 +65,8 @@ interface AttributeBagInterface extends SessionBagInterface
* Removes an attribute.
*
* @param string $name
*
* @return mixed The removed value
*/
function remove($name);
}

34
src/Symfony/Component/HttpFoundation/Session/Attribute/NamespacedAttributeBag.php
View File

@ -29,8 +29,8 @@ class NamespacedAttributeBag extends AttributeBag
/**
* Constructor.
*
* @param type $storageKey Session storage key.
* @param type $namespaceCharacter Namespace character to use in keys.
* @param string $storageKey Session storage key.
* @param string $namespaceCharacter Namespace character to use in keys.
*/
public function __construct($storageKey = '_sf2_attributes', $namespaceCharacter = '/')
{
@ -70,25 +70,6 @@ class NamespacedAttributeBag extends AttributeBag
$attributes[$name] = $value;
}
/**
* {@inheritdoc}
*/
public function all()
{
return $this->attributes;
}
/**
* {@inheritdoc}
*/
public function replace(array $attributes)
{
$this->attributes = array();
foreach ($attributes as $key => $value) {
$this->set($key, $value);
}
}
/**
* {@inheritdoc}
*/
@ -105,17 +86,6 @@ class NamespacedAttributeBag extends AttributeBag
return $retval;
}
/**
* {@inheritdoc}
*/
public function clear()
{
$return = $this->attributes;
$this->attributes = array();
return $return;
}
/**
* Resolves a path in attributes property and returns it as a reference.
*

2
src/Symfony/Component/HttpFoundation/Session/Flash/AutoExpireFlashBag.php
View File

@ -37,7 +37,7 @@ class AutoExpireFlashBag implements FlashBagInterface
/**
* Constructor.
*
* @param type $storageKey The key used to store flashes in the session.
* @param string $storageKey The key used to store flashes in the session.
*/
public function __construct($storageKey = '_sf2_flashes')
{

2
src/Symfony/Component/HttpFoundation/Session/Flash/FlashBag.php
View File

@ -37,7 +37,7 @@ class FlashBag implements FlashBagInterface
/**
* Constructor.
*
* @param type $storageKey The key used to store flashes in the session.
* @param string $storageKey The key used to store flashes in the session.
*/
public function __construct($storageKey = '_sf2_flashes')
{

94
src/Symfony/Component/HttpFoundation/Session/Session.php
View File

@ -50,11 +50,7 @@ class Session implements SessionInterface
}
/**
* Starts the session storage.
*
* @return boolean True if session started.
*
* @api
* {@inheritdoc}
*/
public function start()
{
@ -62,13 +58,7 @@ class Session implements SessionInterface
}
/**
* Checks if an attribute is defined.
*
* @param string $name The attribute name
*
* @return Boolean true if the attribute is defined, false otherwise
*
* @api
* {@inheritdoc}
*/
public function has($name)
{
@ -76,14 +66,7 @@ class Session implements SessionInterface
}
/**
* Returns an attribute.
*
* @param string $name The attribute name
* @param mixed $default The default value
*
* @return mixed
*
* @api
* {@inheritdoc}
*/
public function get($name, $default = null)
{
@ -91,12 +74,7 @@ class Session implements SessionInterface
}
/**
* Sets an attribute.
*
* @param string $name
* @param mixed $value
*
* @api
* {@inheritdoc}
*/
public function set($name, $value)
{
@ -104,11 +82,7 @@ class Session implements SessionInterface
}
/**
* Returns attributes.
*
* @return array Attributes
*
* @api
* {@inheritdoc}
*/
public function all()
{
@ -116,11 +90,7 @@ class Session implements SessionInterface
}
/**
* Sets attributes.
*
* @param array $attributes Attributes
*
* @api
* {@inheritdoc}
*/
public function replace(array $attributes)
{
@ -128,11 +98,7 @@ class Session implements SessionInterface
}
/**
* Removes an attribute.
*
* @param string $name
*
* @api
* {@inheritdoc}
*/
public function remove($name)
{
@ -140,9 +106,7 @@ class Session implements SessionInterface
}
/**
* Clears all attributes.
*
* @api
* {@inheritdoc}
*/
public function clear()
{
@ -150,14 +114,7 @@ class Session implements SessionInterface
}
/**
* Invalidates the current session.
*
* Clears all session attributes and flashes and regenerates the
* session and deletes the old session from persistence.
*
* @return boolean True if session invalidated, false if error.
*
* @api
* {@inheritdoc}
*/
public function invalidate()
{
@ -167,14 +124,7 @@ class Session implements SessionInterface
}
/**
* Migrates the current session to a new session id while maintaining all
* session attributes.
*
* @param boolean $destroy Whether to delete the old session or leave it to garbage collection.
*
* @return boolean True if session migrated, false if error
*
* @api
* {@inheritdoc}
*/
public function migrate($destroy = false)
{
@ -204,7 +154,7 @@ class Session implements SessionInterface
/**
* Implements the \Serialize interface.
*
* @return SessionStorageInterface
* @return string
*/
public function serialize()
{
@ -214,6 +164,8 @@ class Session implements SessionInterface
/**
* Implements the \Serialize interface.
*
* @param string $serialized
*
* @throws \InvalidArgumentException If the passed string does not unserialize to an instance of SessionStorageInterface
*/
public function unserialize($serialized)
@ -261,6 +213,8 @@ class Session implements SessionInterface
// the following methods are kept for compatibility with Symfony 2.0 (they will be removed for Symfony 2.3)
/**
* @return array
*
* @deprecated since 2.1, will be removed from 2.3
*/
public function getFlashes()
@ -269,6 +223,8 @@ class Session implements SessionInterface
}
/**
* @param array $values
*
* @deprecated since 2.1, will be removed from 2.3
*/
public function setFlashes($values)
@ -277,6 +233,11 @@ class Session implements SessionInterface
}
/**
* @param string $name
* @param string $default
*
* @return string
*
* @deprecated since 2.1, will be removed from 2.3
*/
public function getFlash($name, $default = null)
@ -285,6 +246,9 @@ class Session implements SessionInterface
}
/**
* @param string $name
* @param string $value
*
* @deprecated since 2.1, will be removed from 2.3
*/
public function setFlash($name, $value)
@ -293,6 +257,10 @@ class Session implements SessionInterface
}
/**
* @param string $name
*
* @return Boolean
*
* @deprecated since 2.1, will be removed from 2.3
*/
public function hasFlash($name)
@ -301,6 +269,8 @@ class Session implements SessionInterface
}
/**
* @param string $name
*
* @deprecated since 2.1, will be removed from 2.3
*/
public function removeFlash($name)
@ -309,6 +279,8 @@ class Session implements SessionInterface
}
/**
* @return array
*
* @deprecated since 2.1, will be removed from 2.3
*/
public function clearFlashes()

29
src/Symfony/Component/HttpFoundation/Session/SessionInterface.php
View File

@ -21,14 +21,23 @@ interface SessionInterface extends \Serializable
/**
* Starts the session storage.
*
* @return Boolean True if session started.
*
* @throws \RuntimeException If session fails to start.
*
* @api
*/
function start();
/**
* Invalidates the current session.
*
* @return boolean True if session invalidated, false if error.
* Clears all session attributes and flashes and regenerates the
* session and deletes the old session from persistence.
*
* @return Boolean True if session invalidated, false if error.
*
* @api
*/
function invalidate();
@ -36,9 +45,9 @@ interface SessionInterface extends \Serializable
* Migrates the current session to a new session id while maintaining all
* session attributes.
*
* @param boolean $destroy Whether to delete the old session or leave it to garbage collection.
* @param Boolean $destroy Whether to delete the old session or leave it to garbage collection.
*
* @return boolean True if session migrated, false if error.
* @return Boolean True if session migrated, false if error.
*
* @api
*/
@ -59,6 +68,8 @@ interface SessionInterface extends \Serializable
* @param string $name The attribute name
*
* @return Boolean true if the attribute is defined, false otherwise
*
* @api
*/
function has($name);
@ -69,6 +80,8 @@ interface SessionInterface extends \Serializable
* @param mixed $default The default value if not found.
*
* @return mixed
*
* @api
*/
function get($name, $default = null);
@ -77,6 +90,8 @@ interface SessionInterface extends \Serializable
*
* @param string $name
* @param mixed $value
*
* @api
*/
function set($name, $value);
@ -84,6 +99,8 @@ interface SessionInterface extends \Serializable
* Returns attributes.
*
* @return array Attributes
*
* @api
*/
function all();
@ -98,11 +115,17 @@ interface SessionInterface extends \Serializable
* Removes an attribute.
*
* @param string $name
*
* @return mixed The removed value
*
* @api
*/
function remove($name);
/**
* Clears all attributes.
*
* @api
*/
function clear();
}

25
src/Symfony/Component/HttpFoundation/Session/Storage/AbstractSessionStorage.php
View File

@ -128,16 +128,7 @@ abstract class AbstractSessionStorage implements SessionStorageInterface
}
/**
* Regenerates the session.
*
* This method will regenerate the session ID and optionally
* destroy the old ID. Session regeneration should be done
* periodically and for example, should be done when converting
* an anonymous session to a logged in user session.
*
* @param boolean $destroy
*
* @return boolean Returns true on success or false on failure.
* {@inheritdoc}
*/
public function regenerate($destroy = false)
{
@ -171,9 +162,7 @@ abstract class AbstractSessionStorage implements SessionStorageInterface
}
/**
* Register a SessionBagInterface for use.
*
* @param SessionBagInterface $bag
* {@inheritdoc}
*/
public function registerBag(SessionBagInterface $bag)
{
@ -181,13 +170,7 @@ abstract class AbstractSessionStorage implements SessionStorageInterface
}
/**
* Gets a bag by name.
*
* @param string $name
*
* @return SessionBagInterface
*
* @throws \InvalidArgumentException
* {@inheritdoc}
*/
public function getBag($name)
{
@ -323,6 +306,8 @@ abstract class AbstractSessionStorage implements SessionStorageInterface
* are set to (either PHP's internal, custom set with session_set_save_handler()).
* PHP takes the return value from the sessionRead() handler, unserializes it
* and populates $_SESSION with the result automatically.
*
* @param array|null $session
*/
protected function loadSession(array &$session = null)
{

4
src/Symfony/Component/HttpFoundation/Session/Storage/MemcacheSessionStorage.php
View File

@ -21,7 +21,7 @@ class MemcacheSessionStorage extends AbstractSessionStorage implements SessionSa
/**
* Memcache driver.
*
* @var Memcache
* @var \Memcache
*/
private $memcache;
@ -63,7 +63,7 @@ class MemcacheSessionStorage extends AbstractSessionStorage implements SessionSa
}
$memcacheOptions['expiretime'] = isset($memcacheOptions['expiretime']) ? (int)$memcacheOptions['expiretime'] : 86400;
$this->prefix = isset($memcachedOptions['prefix']) ? $memcachedOptions['prefix'] : 'sf2s';
$this->prefix = isset($memcacheOptions['prefix']) ? $memcacheOptions['prefix'] : 'sf2s';
$this->memcacheOptions = $memcacheOptions;

18
src/Symfony/Component/HttpFoundation/Session/Storage/MemcachedSessionStorage.php
View File

@ -21,7 +21,7 @@ class MemcachedSessionStorage extends AbstractSessionStorage implements SessionS
/**
* Memcached driver.
*
* @var Memcached
* @var \Memcached
*/
private $memcached;
@ -41,7 +41,7 @@ class MemcachedSessionStorage extends AbstractSessionStorage implements SessionS
*
* @see AbstractSessionStorage::__construct()
*/
public function __construct(\Memcached $memcache, array $memcachedOptions = array(), array $options = array())
public function __construct(\Memcached $memcached, array $memcachedOptions = array(), array $options = array())
{
$this->memcached = $memcached;
@ -57,7 +57,7 @@ class MemcachedSessionStorage extends AbstractSessionStorage implements SessionS
$memcachedOptions['expiretime'] = isset($memcachedOptions['expiretime']) ? (int)$memcachedOptions['expiretime'] : 86400;
$this->memcached->setOption(\Memcached::OPT_PREFIX_KEY, isset($memcachedOptions['prefix']) ? $memcachedOption['prefix'] : 'sf2s');
$this->memcached->setOption(\Memcached::OPT_PREFIX_KEY, isset($memcachedOptions['prefix']) ? $memcachedOptions['prefix'] : 'sf2s');
$this->memcacheOptions = $memcachedOptions;
@ -77,13 +77,11 @@ class MemcachedSessionStorage extends AbstractSessionStorage implements SessionS
}
/**
* Close session.
*
* @return boolean
* {@inheritdoc}
*/
public function closeSession()
{
return $this->memcached->close();
return true;
}
/**
@ -91,7 +89,7 @@ class MemcachedSessionStorage extends AbstractSessionStorage implements SessionS
*/
public function readSession($sessionId)
{
return $this->memcached->get($this->prefix.$sessionId) ?: '';
return $this->memcached->get($sessionId) ?: '';
}
/**
@ -99,7 +97,7 @@ class MemcachedSessionStorage extends AbstractSessionStorage implements SessionS
*/
public function writeSession($sessionId, $data)
{
return $this->memcached->set($this->prefix.$sessionId, $data, false, $this->memcachedOptions['expiretime']);
return $this->memcached->set($sessionId, $data, false, $this->memcachedOptions['expiretime']);
}
/**
@ -107,7 +105,7 @@ class MemcachedSessionStorage extends AbstractSessionStorage implements SessionS
*/
public function destroySession($sessionId)
{
return $this->memcached->delete($this->prefix.$sessionId);
return $this->memcached->delete($sessionId);
}
/**

2
src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php
View File

@ -33,7 +33,7 @@ class MockArraySessionStorage extends AbstractSessionStorage
/**
* @var array
*/
private $sessionData = array();
protected $sessionData = array();
public function setSessionData(array $array)
{

4
src/Symfony/Component/HttpFoundation/Session/Storage/NullSessionStorage.php
View File

@ -31,9 +31,7 @@ class NullSessionStorage extends AbstractSessionStorage implements SessionSaveHa
}
/**
* Close session.
*
* @return boolean
* {@inheritdoc}
*/
public function closeSession()
{

10
src/Symfony/Component/HttpFoundation/Session/Storage/PdoSessionStorage.php
View File

@ -65,7 +65,7 @@ class PdoSessionStorage extends AbstractSessionStorage implements SessionSaveHan
/**
* {@inheritdoc}
*/
public function openSession($path = null, $name = null)
public function openSession($path, $name)
{
return true;
}
@ -80,8 +80,6 @@ class PdoSessionStorage extends AbstractSessionStorage implements SessionSaveHan
/**
* {@inheritdoc}
*
* @throws \RuntimeException If the session cannot be destroyed
*/
public function destroySession($id)
{
@ -105,8 +103,6 @@ class PdoSessionStorage extends AbstractSessionStorage implements SessionSaveHan
/**
* {@inheritdoc}
*
* @throws \RuntimeException If any old sessions cannot be cleaned
*/
public function gcSession($lifetime)
{
@ -130,8 +126,6 @@ class PdoSessionStorage extends AbstractSessionStorage implements SessionSaveHan
/**
* {@inheritdoc}
*
* @throws \RuntimeException If the session cannot be read
*/
public function readSession($id)
{
@ -166,8 +160,6 @@ class PdoSessionStorage extends AbstractSessionStorage implements SessionSaveHan
/**
* {@inheritdoc}
*
* @throws \RuntimeException If the session data cannot be written
*/
public function writeSession($id, $data)
{

4
src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php
View File

@ -79,7 +79,11 @@ interface SessionStorageInterface
/**
* Gets a SessionBagInterface by name.
*
* @param string $name
*
* @return SessionBagInterface
*
* @throws \InvalidArgumentException If the bag does not exist
*/
function getBag($name);