merged branch danielholmes/master (PR #2031)

Commits
-------

777f876 [HttpFoundation] Added test that exposes error in session saving

Discussion
----------

[HttpFoundation] Added test that exposes error in session saving

Noticed this commit in the recent release:

34a1b53

And noticed that the save in __destruct won't be fired if a manual save has been called. The testSavedOnDestructAfterManualSave test I've added fails on 2.0.1 but it passes with 2.0.0. An example:

$session->set('foo', 'value');
$session->__destruct(); // eventually called during shutdown, triggers a save
// During next request, $session->get('foo') returns 'value'

$session->set('foo', 'value');
$session->save();
$session->set('foo', 'newvalue');
$session->__destruct(); // eventually called during shutdown, however WON'T trigger save
// During next request, $session->get('foo') returns 'value'

In my eyes the save on destruction should still happen whether a save has been called manually or not - it's more predictable. But i can see arguments for the opposite as well.

If consensus is that this is a bug, I'm happy to provide a fix but wanted to get feedback on the 2 options:

 * Save optimisation can be reverted
 * Can make the save optimisation more intelligent so a write to storage is only done if something has changed. The best way to do this would be to close down the api and mark the session as invalid when an attribute is set for example, however all properties are currently protected and would need to be private, so it might break some people's extensions of session

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

by stof at 2011/09/04 02:13:52 -0700

@fabpot what about it ?

tests/Symfony/Tests/Component/HttpFoundation/SessionTest.php

@@ -200,6 +200,38 @@ public function testStart()
         $this->assertSame(array(), $this->session->getFlashes());
         $this->assertSame(array(), $this->session->all());
     }
+
+    public function testSavedOnDestruct()
+    {
+        $this->session->set('foo', 'bar');
+
+        $this->session->__destruct();
+
+        $expected = array(
+            'attributes'=>array('foo'=>'bar'),
+            'flashes'=>array(),
+            'locale'=>'en'
+        );
+        $saved = $this->storage->read('_symfony2');
+        $this->assertSame($expected, $saved);
+    }
+
+    public function testSavedOnDestructAfterManualSave()
+    {
+        $this->session->set('foo', 'nothing');
+        $this->session->save();
+        $this->session->set('foo', 'bar');
+
+        $this->session->__destruct();
+
+        $expected = array(
+            'attributes'=>array('foo'=>'bar'),
+            'flashes'=>array(),
+            'locale'=>'en'
+        );
+        $saved = $this->storage->read('_symfony2');
+        $this->assertSame($expected, $saved);
+    }
 
     public function testStorageRegenerate()
     {