merged branch baldurrensch/testlistener_fix (PR #6362)

This PR was merged into the 2.1 branch.

Commits
-------

098b593 [Session] Added exception to save method
6b9ee87 [Session] Fixed a bug with the TestListener

Discussion
----------

[Session] Fixed bug with TestListener

Fixed a bug where an unstarted mock session would be emptied with a save. Here are the steps to reproduce:

Use the test client from Symfony\Bundle\FrameworkBundle\Test\WebTestCase::createClient(), and add something to its session. (I actually had it authenticate against a firewall).
Take the cookies of this first test client and add them to a second test client
Have the second test client request a URL that results in a 404
Since the 404 does not need to start the session, hence when save is called (automatically), the mock session is overwritten with an empty array. This does not happen with the other session handlers.
The added unit test in this PR shows this problem. If this PR gets accepted, will it also get merged into the 2.1.x-dev branch?

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes (The broken test seems to be unrelated to this change)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

This is a follow up PR since my original one (https://github.com/symfony/symfony/pull/6342) was against the wrong upstream branch.

src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php

@@ -68,7 +68,9 @@ class TestSessionListener implements EventSubscriberInterface
         }
 
         if ($session = $event->getRequest()->getSession()) {
-            $session->save();
+            if ($session->isStarted()) {
+                $session->save();
+            }
 
             $params = session_get_cookie_params();
 

src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php

@@ -43,6 +43,7 @@ class TestSessionListenerTest extends \PHPUnit_Framework_TestCase
 
     public function testShouldSaveMasterRequestSession()
     {
+        $this->sessionHasBeenStarted();
         $this->sessionMustBeSaved();
 
         $this->filterResponse(new Request());
@@ -66,6 +67,14 @@ class TestSessionListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(0, reset($cookies)->getExpiresTime());
     }
 
+    public function testUnstartedSessionIsNotSave()
+    {
+        $this->sessionHasNotBeenStarted();
+        $this->sessionMustNotBeSaved();
+
+        $this->filterResponse(new Request());
+    }
+
     private function filterResponse(Request $request, $type = HttpKernelInterface::MASTER_REQUEST)
     {
         $request->setSession($this->session);
@@ -92,6 +101,20 @@ class TestSessionListenerTest extends \PHPUnit_Framework_TestCase
             ->method('save');
     }
 
+    private function sessionHasBeenStarted()
+    {
+        $this->session->expects($this->once())
+            ->method('isStarted')
+            ->will($this->returnValue(true));
+    }
+
+    private function sessionHasNotBeenStarted()
+    {
+        $this->session->expects($this->once())
+            ->method('isStarted')
+            ->will($this->returnValue(false));
+    }
+
     private function getSession()
     {
         $mock = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\Session')

src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php

@@ -159,6 +159,9 @@ class MockArraySessionStorage implements SessionStorageInterface
      */
     public function save()
     {
+        if (!$this->started || $this->closed) {
+            throw new \RuntimeException("Trying to save a session that was not started yet or was already closed");
+        }
         // nothing to do since we don't persist the session data
         $this->closed = false;
     }

src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php

@@ -97,6 +97,10 @@ class MockFileSessionStorage extends MockArraySessionStorage
      */
     public function save()
     {
+        if (!$this->started) {
+            throw new \RuntimeException("Trying to save a session that was not started yet or was already closed");
+        }
+
         file_put_contents($this->getFilePath(), serialize($this->data));
 
         // this is needed for Silex, where the session object is re-used across requests

src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php

@@ -110,6 +110,9 @@ interface SessionStorageInterface
      * used for a storage object design for unit or functional testing where
      * a real PHP session would interfere with testing, in which case it
      * it should actually persist the session data if required.
+     *
+     * @throws \RuntimeException If the session is saved without being started, or if the session
+     *                           is already closed.
      */
     public function save();
 

src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php

@@ -151,6 +151,7 @@ class SessionTest extends \PHPUnit_Framework_TestCase
 
     public function testSave()
     {
+        $this->session->start();
         $this->session->save();
     }
 

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php

@@ -95,4 +95,12 @@ class MockArraySessionStorageTest extends \PHPUnit_Framework_TestCase
         $this->storage->start();
         $this->assertNotEquals('', $this->storage->getId());
     }
+
+    /**
+     * @expectedException RuntimeException
+     */
+    public function testUnstartedSave()
+    {
+        $this->storage->save();
+    }
 }

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockFileSessionStorageTest.php

@@ -106,6 +106,15 @@ class MockFileSessionStorageTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('bar', $storage2->getBag('attributes')->get('foo'), 'values persist between instances');
     }
 
+    /**
+     * @expectedException RuntimeException
+     */
+    public function testSaveWithoutStart()
+    {
+        $storage1 = $this->getStorage();
+        $storage1->save();
+    }
+
     private function getStorage()
     {
         $storage = new MockFileSessionStorage($this->sessionDir);