From cb873b250b231f74b9542f7497679b07f80e7242 Mon Sep 17 00:00:00 2001 From: Drak Date: Sun, 4 Mar 2012 16:49:40 +0545 Subject: [PATCH] [HttpFoundation] Add tests and some CS/docblocks. --- .../HttpFoundation/Session/Session.php | 6 +- .../Handler/NativeMemcachedSessionHandler.php | 1 - .../Session/Storage/Proxy/AbstractProxy.php | 11 ++- .../Session/Storage/Proxy/NativeProxy.php | 12 +-- .../Storage/Proxy/SessionHandlerProxy.php | 26 +++--- .../Session/Storage/SessionStorage.php | 16 ++-- .../Storage/Proxy/AbstractProxyTest.php | 59 +++++++++---- .../Storage/Proxy/NativeProxyPHP54Test.php | 30 ------- ...ProxyPHP53Test.php => NativeProxyTest.php} | 15 +--- .../Storage/Proxy/SessionHandlerProxyTest.php | 87 ++++++++++++++----- 10 files changed, 147 insertions(+), 116 deletions(-) delete mode 100644 tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP54Test.php rename tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/{NativeProxyPHP53Test.php => NativeProxyTest.php} (57%) diff --git a/src/Symfony/Component/HttpFoundation/Session/Session.php b/src/Symfony/Component/HttpFoundation/Session/Session.php index 63dda8d6bd..6f2c6a4262 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Session.php +++ b/src/Symfony/Component/HttpFoundation/Session/Session.php @@ -38,13 +38,13 @@ class Session implements SessionInterface /** * Constructor. * - * @param SessionStorageInterface $storage A SessionStorageInterface instance. + * @param SessionStorageInterface $storage A SessionStorageInterface instance. * @param AttributeBagInterface $attributes An AttributeBagInterface instance, (defaults null for default AttributeBag) * @param FlashBagInterface $flashes A FlashBagInterface instance (defaults null for default FlashBag) */ - public function __construct(SessionStorageInterface $storage, AttributeBagInterface $attributes = null, FlashBagInterface $flashes = null) + public function __construct(SessionStorageInterface $storage = null, AttributeBagInterface $attributes = null, FlashBagInterface $flashes = null) { - $this->storage = $storage; + $this->storage = $storage ?: new SessionStorage(); $this->registerBag($attributes ?: new AttributeBag()); $this->registerBag($flashes ?: new FlashBag()); } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeMemcachedSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeMemcachedSessionHandler.php index 11f6790aef..d66dc953d6 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeMemcachedSessionHandler.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeMemcachedSessionHandler.php @@ -65,5 +65,4 @@ class NativeMemcachedSessionHandler extends NativeSessionHandler } } } - } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php index 41cd49cb1e..0cd4d73d96 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php @@ -13,6 +13,8 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Proxy; /** * AbstractProxy. + * + * @author Drak */ abstract class AbstractProxy { @@ -43,9 +45,14 @@ abstract class AbstractProxy return $this->saveHandlerName; } + /** + * Is this proxy handler and instance of \SessionHandlerInterface. + * + * @return boolean + */ public function isSessionHandlerInterface() { - return (bool)($this instanceof \SessionHandlerInterface); + return ($this instanceof \SessionHandlerInterface); } /** @@ -75,6 +82,6 @@ abstract class AbstractProxy */ public function setActive($flag) { - $this->active = (bool)$flag; + $this->active = (bool) $flag; } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/NativeProxy.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/NativeProxy.php index 952c7105cd..5bb2c712e3 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/NativeProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/NativeProxy.php @@ -15,21 +15,17 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Proxy; * NativeProxy. * * This proxy is built-in session handlers in PHP 5.3.x + * + * @author Drak */ class NativeProxy extends AbstractProxy { /** * Constructor. - * - * @param $handler */ - public function __construct($handler) + public function __construct() { - if (version_compare(phpversion(), '5.4.0', '>=') && $handler instanceof \SessionHandlerInterface) { - throw new \InvalidArgumentException('This proxy is only for PHP 5.3 and not for instances of \SessionHandler or \SessionHandlerInterface'); - } - - $this->handler = $handler; + // this makes an educated guess as to what the handler is since it should already be set. $this->saveHandlerName = ini_get('session.save_handler'); } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php index 93a1010a73..e925d628df 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php @@ -13,6 +13,8 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Proxy; /** * SessionHandler proxy. + * + * @author Drak */ class SessionHandlerProxy extends AbstractProxy implements \SessionHandlerInterface { @@ -29,7 +31,7 @@ class SessionHandlerProxy extends AbstractProxy implements \SessionHandlerInterf public function __construct(\SessionHandlerInterface $handler) { $this->handler = $handler; - $this->wrapper = (bool)(class_exists('SessionHandler') && $handler instanceof \SessionHandler); + $this->wrapper = ($handler instanceof \SessionHandler); $this->saveHandlerName = $this->wrapper ? ini_get('session.save_handler') : 'user'; } @@ -38,7 +40,7 @@ class SessionHandlerProxy extends AbstractProxy implements \SessionHandlerInterf /** * {@inheritdoc} */ - function open($savePath, $sessionName) + public function open($savePath, $sessionName) { $return = (bool)$this->handler->open($savePath, $sessionName); @@ -52,42 +54,42 @@ class SessionHandlerProxy extends AbstractProxy implements \SessionHandlerInterf /** * {@inheritdoc} */ - function close() + public function close() { $this->active = false; - return (bool)$this->handler->close(); + return (bool) $this->handler->close(); } /** * {@inheritdoc} */ - function read($id) + public function read($id) { - return (string)$this->handler->read($id); + return (string) $this->handler->read($id); } /** * {@inheritdoc} */ - function write($id, $data) + public function write($id, $data) { - return (bool)$this->handler->write($id, $data); + return (bool) $this->handler->write($id, $data); } /** * {@inheritdoc} */ - function destroy($id) + public function destroy($id) { - return (bool)$this->handler->destroy($id); + return (bool) $this->handler->destroy($id); } /** * {@inheritdoc} */ - function gc($maxlifetime) + public function gc($maxlifetime) { - return (bool)$this->handler->gc($maxlifetime); + return (bool) $this->handler->gc($maxlifetime); } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php index f34f5bccf0..f5a1578767 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php @@ -88,13 +88,12 @@ class SessionStorage implements SessionStorageInterface * upload_progress.min-freq, "1" * url_rewriter.tags, "a=href,area=href,frame=src,form=,fieldset=" * - * @param array $options Session configuration options. - * @param $handler SessionHandlerInterface. + * @param array $options Session configuration options. + * @param object $handler SessionHandlerInterface. */ public function __construct(array $options = array(), $handler = null) { $this->setOptions($options); - $this->setSaveHandler($handler); } @@ -118,7 +117,7 @@ class SessionStorage implements SessionStorageInterface } if ($this->options['use_cookies'] && headers_sent()) { - throw new \RuntimeException('Failed to start the session because header have already been sent.'); + throw new \RuntimeException('Failed to start the session because headers have already been sent.'); } // start the session @@ -225,7 +224,7 @@ class SessionStorage implements SessionStorageInterface * * @see http://php.net/session.configuration */ - protected function setOptions(array $options) + public function setOptions(array $options) { $this->options = $options; @@ -234,7 +233,6 @@ class SessionStorage implements SessionStorageInterface 'cache_limiter' => '', // disable by default because it's managed by HeaderBag (if used) 'auto_start' => false, 'use_cookies' => true, - 'cookie_httponly' => true, ); foreach ($defaults as $key => $value) { @@ -272,14 +270,14 @@ class SessionStorage implements SessionStorageInterface * @see http://php.net/sessionhandlerinterface * @see http://php.net/sessionhandler * - * @param object $saveHandler + * @param object $saveHandler Default null means NativeProxy. */ - public function setSaveHandler($saveHandler) + public function setSaveHandler($saveHandler = null) { // Wrap $saveHandler in proxy if (!$saveHandler instanceof AbstractProxy && $saveHandler instanceof \SessionHandlerInterface) { $saveHandler = new SessionHandlerProxy($saveHandler); - } else { + } elseif (!$saveHandler instanceof AbstractProxy) { $saveHandler = new NativeProxy($saveHandler); } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxyTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxyTest.php index a968e2e0a3..efa9f75e64 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxyTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxyTest.php @@ -4,15 +4,44 @@ namespace Symfony\Tests\Component\HttpFoundation\Session\Storage\Proxy; use Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy; +// Note until PHPUnit_Mock_Objects 1.2 is released you cannot mock abstracts due to +// https://github.com/sebastianbergmann/phpunit-mock-objects/issues/73 class ConcreteProxy extends AbstractProxy { } +class ConcreteSessionHandlerInterfaceProxy extends AbstractProxy implements \SessionHandlerInterface +{ + public function open($savePath, $sessionName) + { + } + + public function close() + { + } + + public function read($id) + { + } + + public function write($id, $data) + { + } + + public function destroy($id) + { + } + + public function gc($maxlifetime) + { + } +} + /** * Test class for AbstractProxy. * - * @runTestsInSeparateProcesses + * @author Drak */ class AbstractProxyTest extends \PHPUnit_Framework_TestCase { @@ -23,7 +52,7 @@ class AbstractProxyTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $this->proxy = new ConcreteProxy; + $this->proxy = new ConcreteProxy(); } protected function tearDown() @@ -33,37 +62,31 @@ class AbstractProxyTest extends \PHPUnit_Framework_TestCase public function testGetSaveHandlerName() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->assertNull($this->proxy->getSaveHandlerName()); } public function testIsSessionHandlerInterface() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->assertFalse($this->proxy->isSessionHandlerInterface()); + $sh = new ConcreteSessionHandlerInterfaceProxy(); + $this->assertTrue($sh->isSessionHandlerInterface()); } public function testIsWrapper() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->assertFalse($this->proxy->isWrapper()); } public function testIsActive() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->assertFalse($this->proxy->isActive()); } public function testSetActive() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->proxy->setActive(true); + $this->assertTrue($this->proxy->isActive()); + $this->proxy->setActive(false); + $this->assertFalse($this->proxy->isActive()); } - } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP54Test.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP54Test.php deleted file mode 100644 index f6c3d7aa75..0000000000 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP54Test.php +++ /dev/null @@ -1,30 +0,0 @@ -markTestSkipped('Test skipped, only for PHP 5.4'); - } - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testConstructor() - { - $proxy = new NativeProxy(new NativeFileSessionHandler()); - $this->assertTrue($proxy->isWrapper()); - } -} diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP53Test.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyTest.php similarity index 57% rename from tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP53Test.php rename to tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyTest.php index b62ce8192b..0dc0ce92a3 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP53Test.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyTest.php @@ -8,27 +8,20 @@ use Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHa /** * Test class for NativeProxy. * - * @runTestsInSeparateProcesses + * @author Drak */ -class NativeProxyPHP53Test extends \PHPUnit_Framework_TestCase +class NativeProxyTest extends \PHPUnit_Framework_TestCase { - protected function setUp() - { - if (version_compare(phpversion(), '5.4.0', '>=')) { - $this->markTestSkipped('Test skipped, only for PHP 5.3'); - } - } - public function testIsWrapper() { - $proxy = new NativeProxy(new NativeFileSessionHandler()); + $proxy = new NativeProxy(); $this->assertFalse($proxy->isWrapper()); } public function testGetSaveHandlerName() { $name = ini_get('session.save_handler'); - $proxy = new NativeProxy(new NativeFileSessionHandler()); + $proxy = new NativeProxy(); $this->assertEquals($name, $proxy->getSaveHandlerName()); } } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxyTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxyTest.php index fb7d36adda..61393f3102 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxyTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxyTest.php @@ -3,68 +3,111 @@ namespace Symfony\Tests\Component\HttpFoundation\Session\Storage\Proxy; use Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy; -use Symfony\Component\HttpFoundation\Session\Storage\Handler\NullSessionHandler; /** + * Tests for SessionHandlerProxy class. + * + * @author Drak + * * @runTestsInSeparateProcesses */ class SessionHandlerProxyTest extends \PHPUnit_Framework_TestCase { + /** + * @var PHPUnit_Framework_MockObject_Matcher + */ + private $mock; + /** * @var SessionHandlerProxy */ - protected $proxy; + private $proxy; protected function setUp() { - $this->proxy = new SessionHandlerProxy(new NullSessionHandler()); + $this->mock = $this->getMock('SessionHandlerInterface'); + $this->proxy = new SessionHandlerProxy($this->mock); } protected function tearDown() { + $this->mock = null; $this->proxy = null; } public function testOpen() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->mock->expects($this->once()) + ->method('open') + ->will($this->returnValue(true)); + + $this->assertFalse($this->proxy->isActive()); + $this->proxy->open('name', 'id'); + $this->assertTrue($this->proxy->isActive()); + } + + public function testOpenFalse() + { + $this->mock->expects($this->once()) + ->method('open') + ->will($this->returnValue(false)); + + $this->assertFalse($this->proxy->isActive()); + $this->proxy->open('name', 'id'); + $this->assertFalse($this->proxy->isActive()); } public function testClose() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->mock->expects($this->once()) + ->method('close') + ->will($this->returnValue(true)); + + $this->assertFalse($this->proxy->isActive()); + $this->proxy->close(); + $this->assertFalse($this->proxy->isActive()); + } + + public function testCloseFalse() + { + $this->mock->expects($this->once()) + ->method('close') + ->will($this->returnValue(false)); + + $this->assertFalse($this->proxy->isActive()); + $this->proxy->close(); + $this->assertFalse($this->proxy->isActive()); } public function testRead() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->mock->expects($this->once()) + ->method('read'); + + $this->proxy->read('id'); } public function testWrite() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->mock->expects($this->once()) + ->method('write'); + + $this->proxy->write('id', 'data'); } public function testDestroy() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); + $this->mock->expects($this->once()) + ->method('destroy'); + + $this->proxy->destroy('id'); } public function testGc() { - $this->markTestIncomplete( - 'This test has not been implemented yet.' - ); - } + $this->mock->expects($this->once()) + ->method('gc'); + $this->proxy->gc(86400); + } }