[HttpFoundation] Add tests and some CS/docblocks.

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());
     }

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeMemcachedSessionHandler.php

@@ -65,5 +65,4 @@ class NativeMemcachedSessionHandler extends NativeSessionHandler
             }
         }
     }
-
 }

src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php

@@ -13,6 +13,8 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Proxy;
 
 /**
  * AbstractProxy.
+ *
+ * @author Drak <drak@zikula.org>
  */
 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;
     }
 }

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 <drak@zikula.org>
  */
 class NativeProxy extends AbstractProxy
 {
     /**
      * Constructor.
-     *
-     * @param $handler
      */
-    public function __construct($handler)
+    public function __construct()
     {
-        if (version_compare(phpversion(), '5.4.0', '>=') && $handler instanceof \SessionHandlerInterface) {
+        // this makes an educated guess as to what the handler is since it should already be set.
-            throw new \InvalidArgumentException('This proxy is only for PHP 5.3 and not for instances of \SessionHandler or \SessionHandlerInterface');
-        }
-        
-        $this->handler = $handler;
         $this->saveHandlerName = ini_get('session.save_handler');
     }
 

src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php

@@ -13,6 +13,8 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Proxy;
 
 /**
  * SessionHandler proxy.
+ *
+ * @author Drak <drak@zikula.org>
  */
 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);
     }
 }

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 array  $options Session configuration options.
-     * @param       $handler SessionHandlerInterface.
+     * @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);
         }
 

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 <drak@zikula.org>
  */
 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->assertNull($this->proxy->getSaveHandlerName());
-            'This test has not been implemented yet.'
-        );
     }
 
     public function testIsSessionHandlerInterface()
     {
-        $this->markTestIncomplete(
+        $this->assertFalse($this->proxy->isSessionHandlerInterface());
-            'This test has not been implemented yet.'
+        $sh = new ConcreteSessionHandlerInterfaceProxy();
-        );
+        $this->assertTrue($sh->isSessionHandlerInterface());
     }
 
     public function testIsWrapper()
     {
-        $this->markTestIncomplete(
+        $this->assertFalse($this->proxy->isWrapper());
-            'This test has not been implemented yet.'
-        );
     }
 
     public function testIsActive()
     {
-        $this->markTestIncomplete(
+        $this->assertFalse($this->proxy->isActive());
-            'This test has not been implemented yet.'
-        );
     }
 
     public function testSetActive()
     {
-        $this->markTestIncomplete(
+        $this->proxy->setActive(true);
-            'This test has not been implemented yet.'
+        $this->assertTrue($this->proxy->isActive());
-        );
+        $this->proxy->setActive(false);
+        $this->assertFalse($this->proxy->isActive());
     }
-
 }

tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Proxy/NativeProxyPHP54Test.php

@@ -1,30 +0,0 @@
-<?php
-
-namespace Symfony\Tests\Component\HttpFoundation\Session\Storage\Proxy;
-
-use Symfony\Component\HttpFoundation\Session\Storage\Proxy\NativeProxy;
-use Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHandler;
-
-/**
- * Test class for NativeProxy.
- *
- * @runTestsInSeparateProcesses
- */
-class NativeProxyPHP54Test extends \PHPUnit_Framework_TestCase
-{
-    protected function setUp()
-    {
-        if (version_compare(phpversion(), '5.4.0', '<')) {
-            $this->markTestSkipped('Test skipped, only for PHP 5.4');
-        }
-    }
-
-    /**
-     * @expectedException \InvalidArgumentException
-     */
-    public function testConstructor()
-    {
-        $proxy = new NativeProxy(new NativeFileSessionHandler());
-        $this->assertTrue($proxy->isWrapper());
-    }
-}

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 <drak@zikula.org>
  */
-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());
     }
 }

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 <drak@zikula.org>
+ *
  * @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->mock->expects($this->once())
-            'This test has not been implemented yet.'
+            ->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->mock->expects($this->once())
-            'This test has not been implemented yet.'
+            ->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->mock->expects($this->once())
-            'This test has not been implemented yet.'
+            ->method('read');
-        );
+
+        $this->proxy->read('id');
     }
 
     public function testWrite()
     {
-        $this->markTestIncomplete(
+        $this->mock->expects($this->once())
-            'This test has not been implemented yet.'
+            ->method('write');
-        );
+
+        $this->proxy->write('id', 'data');
     }
 
     public function testDestroy()
     {
-        $this->markTestIncomplete(
+        $this->mock->expects($this->once())
-            'This test has not been implemented yet.'
+            ->method('destroy');
-        );
+
+        $this->proxy->destroy('id');
     }
 
     public function testGc()
     {
-        $this->markTestIncomplete(
+        $this->mock->expects($this->once())
-            'This test has not been implemented yet.'
+            ->method('gc');
-        );
-    }
 
+        $this->proxy->gc(86400);
+    }
 }