[Session] Added exception to save method

Baldur Rensch · Baldur Rensch · commit 098b593591d1 · 2012-12-14T13:03:29.000-08:00
A RuntimeException is thrown if there is an attempt to save the session
without it being started, or if it has already been closed.
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php
@@ -159,6 +159,9 @@ public function setName($name)
      */
     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;
     }
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php
@@ -97,6 +97,10 @@ public function regenerate($destroy = false, $lifetime = null)
      */
     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
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php
@@ -110,6 +110,9 @@ public function regenerate($destroy = false, $lifetime = null);
      * 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();
 
diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php
@@ -151,6 +151,7 @@ public function testMigrateDestroy()
 
     public function testSave()
     {
+        $this->session->start();
         $this->session->save();
     }
 
diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php
@@ -95,4 +95,12 @@ public function testGetId()
         $this->storage->start();
         $this->assertNotEquals('', $this->storage->getId());
     }
+
+    /**
+     * @expectedException RuntimeException
+     */
+    public function testUnstartedSave()
+    {
+        $this->storage->save();
+    }
 }
diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockFileSessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockFileSessionStorageTest.php
@@ -106,6 +106,15 @@ public function testMultipleInstances()
         $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);

Original file line number	Diff line number	Diff line change
`@@ -159,6 +159,9 @@ public function setName($name)`
`159`	`159`	`*/`
`160`	`160`	`public function save()`
`161`	`161`	`{`
	`162`	`+ if (!$this->started \|\| $this->closed) {`
	`163`	`+ throw new \RuntimeException("Trying to save a session that was not started yet or was already closed");`
	`164`	`+ }`
`162`	`165`	`// nothing to do since we don't persist the session data`
`163`	`166`	`$this->closed = false;`
`164`	`167`	`}`
Original file line number	Diff line number	Diff line change
`@@ -151,6 +151,7 @@ public function testMigrateDestroy()`
`151`	`151`
`152`	`152`	`public function testSave()`
`153`	`153`	`{`
	`154`	`+ $this->session->start();`
`154`	`155`	`$this->session->save();`
`155`	`156`	`}`
`156`	`157`
Original file line number	Diff line number	Diff line change
`@@ -95,4 +95,12 @@ public function testGetId()`
`95`	`95`	`$this->storage->start();`
`96`	`96`	`$this->assertNotEquals('', $this->storage->getId());`
`97`	`97`	`}`
	`98`	`+`
	`99`	`+ /**`
	`100`	`+ * @expectedException RuntimeException`
	`101`	`+ */`
	`102`	`+ public function testUnstartedSave()`
	`103`	`+ {`
	`104`	`+ $this->storage->save();`
	`105`	`+ }`
`98`	`106`	`}`