Write session on response

There should not be any necessity to write the session once changes are
being made to it. We now track whether changes were made and write
the session when responding to the user's request if so.

application/forms/PreferenceForm.php

@@ -99,10 +99,7 @@ class PreferenceForm extends Form
         }
         $this->preferences->icingaweb = $webPreferences;
 
-        // TODO: Is this even necessary in case the session is written on response?
-        $session = Session::getSession();
-        $session->user->setPreferences($this->preferences);
-        $session->write();
+        Session::getSession()->user->setPreferences($this->preferences);
 
         try {
             $this->save();

library/Icinga/Authentication/Manager.php

@@ -106,10 +106,7 @@ class Manager
      */
     public function persistCurrentUser()
     {
-        $session = Session::getSession();
-        $session->set('user', $this->user);
-        $session->write();
-        $session->refreshId();
+        Session::getSession()->set('user', $this->user)->refreshId();
     }
 
     /**

library/Icinga/Web/Controller/ActionController.php

@@ -18,8 +18,6 @@ use Icinga\File\Pdf;
 use Icinga\Exception\ProgrammingError;
 use Icinga\Web\Session;
 use Icinga\Web\UrlParams;
-use Icinga\Session\SessionNamespace;
-use Icinga\Exception\NotReadableError;
 use Zend_Controller_Action;
 use Zend_Controller_Action_HelperBroker as ActionHelperBroker;
 use Zend_Controller_Request_Abstract as Request;
@@ -325,14 +323,24 @@ class ActionController extends Zend_Controller_Action
             $this->getResponse()->setHeader('X-Icinga-Reload-Css', 'now');
         }
 
+        $this->shutdownSession();
+
         $this->getResponse()
             ->setHeader('X-Icinga-Redirect', rawurlencode($url->getAbsoluteUrl()))
             ->sendHeaders();
 
-        // TODO: Session shutdown?
         exit;
     }
 
+    protected function redirectHttp($url)
+    {
+        if (! $url instanceof Url) {
+            $url = Url::fromPath($url);
+        }
+        $this->shutdownSession();
+        $this->_helper->Redirector->gotoUrlAndExit($url->getRelativeUrl());
+    }
+
     /**
     *  Redirect to a specific url, updating the browsers URL field
     *
@@ -343,10 +351,7 @@ class ActionController extends Zend_Controller_Action
         if ($this->isXhr()) {
             $this->redirectXhr($url);
         } else {
-            if (! $url instanceof Url) {
-                $url = Url::fromPath($url);
-            }
-            $this->_helper->Redirector->gotoUrlAndExit($url->getRelativeUrl());
+            $this->redirectHttp($url);
         }
     }
 
@@ -372,6 +377,8 @@ class ActionController extends Zend_Controller_Action
             }
         }
 
+        $this->shutdownSession();
+
         if ($req->getParam('format') === 'pdf') {
             $layout->setLayout('pdf');
             $this->sendAsPdf();
@@ -430,6 +437,14 @@ class ActionController extends Zend_Controller_Action
         $pdf->renderControllerAction($this);
     }
 
+    protected function shutdownSession()
+    {
+        $session = Session::getSession();
+        if ($session->hasChanged()) {
+            $session->write();
+        }
+    }
+
     /**
      * Render the benchmark
      *

library/Icinga/Web/Notification.php

@@ -80,30 +80,24 @@ class Notification
             'message' => $message,
         );
 
-        // Get, change, set - just to be on the safe side:
         $session = Session::getSession();
-        $msgs = $session->messages;
-        $msgs[] = $mo;
-        $session->messages = $msgs;
-        $session->write();
+        $session->messages[] = $mo;
     }
 
     public function hasMessages()
     {
         $session = Session::getSession();
-        return !empty($session->messages);
+        return false === empty($session->messages);
     }
 
     public function getMessages()
     {
         $session = Session::getSession();
-        $msgs = $session->messages;
-        if (false === empty($msgs)) {
+        if (false === empty($session->messages)) {
             $session->messages = array();
-            $session->write();
         }
 
-        return $msgs;
+        return $session->messages;
     }
 
     final private function __construct()

library/Icinga/Web/Response.php

@@ -21,6 +21,12 @@ class Response extends Zend_Controller_Response_Http
         } else {
             $this->setRedirect($url->getAbsoluteUrl());
         }
+
+        $session = Session::getSession();
+        if ($session->hasChanged()) {
+            $session->write();
+        }
+
         $this->sendHeaders();
         exit;
     }

library/Icinga/Web/Session/Session.php

@@ -104,13 +104,22 @@ abstract class Session extends SessionNamespace
         $this->removedNamespaces[] = $identifier;
     }
 
+    /**
+     * Return whether the session has changed
+     *
+     * @return  bool
+     */
+    public function hasChanged()
+    {
+        return parent::hasChanged() || false === empty($this->namespaces) || false === empty($this->removedNamespaces);
+    }
+
     /**
      * Clear all values and namespaces from the session cache
      */
     public function clear()
     {
-        $this->values = array();
-        $this->removed = array();
+        parent::clear();
         $this->namespaces = array();
         $this->removedNamespaces = array();
     }

library/Icinga/Web/Session/SessionNamespace.php

@@ -208,4 +208,23 @@ class SessionNamespace implements IteratorAggregate
 
         $this->session->write();
     }
+
+    /**
+     * Return whether the session namespace has been changed
+     *
+     * @return  bool
+     */
+    public function hasChanged()
+    {
+        return false === empty($this->values) || false === empty($this->removed);
+    }
+
+    /**
+     * Clear all values from the session namespace
+     */
+    public function clear()
+    {
+        $this->values = array();
+        $this->removed = array();
+    }
 }

modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php

@@ -655,7 +655,6 @@ abstract class IdoQuery extends DbQuery
             );
             if ($session !== null) {
                 $session->version = self::$idoVersion;
-                $session->write(); // <- WHY? I don't want to care about this!
             }
         }
         return self::$idoVersion;

modules/monitoring/library/Monitoring/Timeline/TimeLine.php

@@ -288,7 +288,6 @@ class TimeLine implements IteratorAggregate
 
                     if ($this->session !== null) {
                         $this->session->calculationBase = $new;
-                        $this->session->write();
                     }
                 } else {
                     $this->calculationBase = $calculationBase;