merged branch mazen/fix-memcached-sessions (PR #3399)

Commits
-------

6fbd290 Improved unit tests for MemcacheSessionStorage
b4c5323 Added comma to array initializer, reverted permissions back to 644
3dd851a Use correct parameters
0e01418 Fix default if no serverpool is provided
2a65121 Fix several issues in MemccheSessionStorage which prevented it from being used correctly

Discussion
----------

Fix several issues in MemcacheSessionStorage

Apperently this could never have worked unless someone passed wrong arguments to the options.

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

by mazen at 2012-02-19T07:58:52Z

```
[marcel@development symfony]$ phpunit tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MemcacheSessionStorageTest.php
PHPUnit 3.6.10 by Sebastian Bergmann.

Configuration read from /www/includes/vendor/symfony/phpunit.xml.dist

......

Time: 0 seconds, Memory: 3.75Mb

OK (6 tests, 11 assertions)
```

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

by lsmith77 at 2012-02-19T16:10:13Z

cc @drak

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

by drak at 2012-02-19T17:44:00Z

Looks like we could do with some tests for the constructor that also test the defaults and the internal properties.  And also more extensively tests the mock to test the addServer behaviour.

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

by helmer at 2012-02-19T18:02:03Z

@mazen You've changed file permissions from 644->755 ..

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

by drak at 2012-02-21T12:25:11Z

@fabpot - with the extra tests added in 6fbd290 I believe this code is ready for merge.
This commit is contained in:
Fabien Potencier 2012-02-21 14:47:18 +01:00
commit dc1ff89a94
2 changed files with 51 additions and 8 deletions

View File

@ -54,12 +54,14 @@ class MemcacheSessionStorage extends AbstractSessionStorage implements SessionHa
// defaults
if (!isset($memcacheOptions['serverpool'])) {
$memcacheOptions['serverpool'] = array(
$memcacheOptions['serverpool'] = array(array(
'host' => '127.0.0.1',
'port' => 11211,
'timeout' => 1,
'persistent' => false,
'weight' => 1);
'weight' => 1,
'retry_interval' => 15,
));
}
$memcacheOptions['expiretime'] = isset($memcacheOptions['expiretime']) ? (int)$memcacheOptions['expiretime'] : 86400;
@ -72,14 +74,18 @@ class MemcacheSessionStorage extends AbstractSessionStorage implements SessionHa
protected function addServer(array $server)
{
if (array_key_exists('host', $server)) {
if (!array_key_exists('host', $server)) {
throw new \InvalidArgumentException('host key must be set');
}
$server['port'] = isset($server['port']) ? (int)$server['port'] : 11211;
$server['timeout'] = isset($server['timeout']) ? (int)$server['timeout'] : 1;
$server['presistent'] = isset($server['presistent']) ? (bool)$server['presistent'] : false;
$server['weight'] = isset($server['weight']) ? (bool)$server['weight'] : 1;
$server['persistent'] = isset($server['persistent']) ? (bool)$server['persistent'] : false;
$server['weight'] = isset($server['weight']) ? (int)$server['weight'] : 1;
$server['retry_interval'] = isset($server['retry_interval']) ? (int)$server['retry_interval'] : 15;
$this->memcache->addserver($server['host'], $server['port'], $server['persistent'],$server['weight'],$server['timeout'],$server['retry_interval']);
}
/**
@ -88,7 +94,7 @@ class MemcacheSessionStorage extends AbstractSessionStorage implements SessionHa
public function open($savePath, $sessionName)
{
foreach ($this->memcacheOptions['serverpool'] as $server) {
$this->memcache->addServer($server);
$this->addServer($server);
}
return true;
@ -115,7 +121,7 @@ class MemcacheSessionStorage extends AbstractSessionStorage implements SessionHa
*/
public function write($sessionId, $data)
{
return $this->memcache->set($this->prefix.$sessionId, $data, $this->memcacheOptions['expiretime']);
return $this->memcache->set($this->prefix.$sessionId, $data, 0, $this->memcacheOptions['expiretime']);
}
/**

View File

@ -32,11 +32,48 @@ class MemcacheSessionStorageTest extends \PHPUnit_Framework_TestCase
public function testOpenSession()
{
$this->memcache->expects($this->atLeastOnce())
->method('addServer');
->method('addServer')
->with('127.0.0.1', 11211, false, 1, 1, 15);
$this->assertTrue($this->storage->open('', ''));
}
public function testConstructingWithServerPool()
{
$mock = $this->getMock('Memcache');
$storage = new MemcacheSessionStorage($mock, array(
'serverpool' => array(
array('host' => '127.0.0.2'),
array('host' => '127.0.0.3',
'port' => 11212,
'timeout' => 10,
'persistent' => true,
'weight' => 5,
'retry_interval' => 39,
),
array('host' => '127.0.0.4',
'port' => 11211,
'weight' => 2
),
),
));
$matcher = $mock
->expects($this->at(0))
->method('addServer')
->with('127.0.0.2', 11211, false, 1, 1, 15);
$matcher = $mock
->expects($this->at(1))
->method('addServer')
->with('127.0.0.3', 11212, true, 5, 10, 39);
$matcher = $mock
->expects($this->at(2))
->method('addServer')
->with('127.0.0.4', 11211, false, 2, 1, 15);
$this->assertTrue($storage->open('', ''));
}
public function testCloseSession()
{
$this->memcache->expects($this->once())