Rework save preferences

application/controllers/PreferenceController.php

@@ -28,18 +28,17 @@
  */
 // {{{ICINGA_LICENSE_HEADER}}}
 
-use \Icinga\Web\Controller\BasePreferenceController;
+use Icinga\Web\Controller\BasePreferenceController;
-use \Icinga\Web\Widget\Tab;
+use Icinga\Web\Widget\Tab;
-use \Icinga\Application\Config as IcingaConfig;
+use Icinga\Application\Config as IcingaConfig;
-use \Icinga\Web\Url;
+use Icinga\Web\Url;
-use \Icinga\Form\Preference\GeneralForm;
+use Icinga\Form\Preference\GeneralForm;
 
 /**
  * Application wide preference controller for user preferences
  */
 class PreferenceController extends BasePreferenceController
 {
-
     /**
      * Create tabs for this preference controller
      *
@@ -66,34 +65,21 @@ class PreferenceController extends BasePreferenceController
     public function indexAction()
     {
         $form = new GeneralForm();
-        $form->setConfiguration(IcingaConfig::app());
+        $form->setConfiguration(IcingaConfig::app())
-        $form->setRequest($this->getRequest());
+            ->setRequest($this->getRequest());
         if ($form->isSubmittedAndValid()) {
-            $preferences = $form->getPreferences();
-            $userPreferences = $this->getRequest()->getUser()->getPreferences();
-
-            $userPreferences->startTransaction();
-            foreach ($preferences as $key => $value) {
-                if (!$value) {
-                    $userPreferences->remove($key);
-                } else {
-                    $userPreferences->set($key, $value);
-                }
-            }
             try {
-                $userPreferences->commit();
+                $this->savePreferences($form->getPreferences());
-                $this->view->successMessage = "Preferences Updated Successfully";
+                $this->view->successMessage = 'Preferences Updated Successfully';
-
+                // Recreate form to show new values
-                // recreate form to show new values
+                // TODO(el): It must sufficient to call $form->populate(...)
                 $form = new GeneralForm();
                 $form->setConfiguration(IcingaConfig::app());
                 $form->setRequest($this->getRequest());
-
             } catch (Exception $e) {
                 $this->view->exceptionMessage = $e->getMessage();
             }
         }
-
         $this->view->form = $form;
     }
 }

library/Icinga/User/Preferences.php

@@ -29,7 +29,7 @@
 
 namespace Icinga\User;
 
-use \Countable;
+use Countable;
 
 /**
  * User preferences container
@@ -44,9 +44,11 @@ use \Countable;
  *
  * $preferences = new Preferences(array('aPreference' => 'value')); // Start with initial preferences
  *
+ * $prefrences = $user->getPreferences(); // Retrieve preferences from a \Icinga\User instance
+ *
  * $preferences->aNewPreference = 'value'; // Set a preference
  *
- * unset($preferences['aPreference']); // Unset a preference
+ * unset($preferences->aPreference); // Unset a preference
  *
  * // Retrieve a preference and return a default value if the preference does not exist
  * $anotherPreference = $preferences->get('anotherPreference', 'defaultValue');
@@ -161,4 +163,14 @@ class Preferences implements Countable
     {
         $this->remove($name);
     }
+
+    /**
+     * Get preferences as array
+     *
+     * @return array
+     */
+    public function toArray()
+    {
+        return $this->preferences;
+    }
 }

library/Icinga/User/Preferences/PreferencesStore.php

@@ -51,11 +51,11 @@ use Icinga\User\Preferences;
  *         'type'       => 'ini',
  *         'configPath' => '/path/to/preferences'
  *     ),
- *     $user // Instance of Icinga\User
+ *     $user // Instance of \Icinga\User
  * );
  *
  * $preferences = new Preferences($store->load());
- * $prefereces->aPreference = 'value';
+ * $preferences->aPreference = 'value';
  * $store->save($preferences);
  */
 abstract class PreferencesStore
@@ -130,10 +130,13 @@ abstract class PreferencesStore
     public function save(Preferences $preferences)
     {
         $storedPreferences = $this->load();
-        $deletedPreferences = array_keys(array_diff_key($storedPreferences, $preferences));
+        $preferences = $preferences->toArray();
         $newPreferences = array_diff_key($preferences, $storedPreferences);
         $updatedPreferences = array_diff_assoc($preferences, $storedPreferences);
-        $this->cud($newPreferences, $updatedPreferences, $deletedPreferences);
+        $deletedPreferences = array_keys(array_diff_key($storedPreferences, $preferences));
+        if (count($newPreferences) || count($updatedPreferences) || count($deletedPreferences)) {
+            $this->cud($newPreferences, $updatedPreferences, $deletedPreferences);
+        }
     }
 
     /**

library/Icinga/User/Preferences/Store/IniStore.php

@@ -4,7 +4,7 @@
 
 namespace Icinga\User\Preferences\Store;
 
-use Zend_Config_Ini;
+use Zend_Config;
 use Icinga\Application\Config as IcingaConfig;
 use Icinga\Config\PreservingIniWriter;
 use Icinga\Exception\NotReadableError;
@@ -25,13 +25,13 @@ use Icinga\Util\File;
  * use Icinga\User\Preferences\PreferencesStore;
  * use Icinga\User\Preferences\Store\IniStore;
  *
- * // Create the store from the factory (prefered approach)
+ * // Create the store from the factory (preferred approach)
  * $store = new PreferencesStore(
  *     new Zend_Config(
  *         'type'       => 'ini',
  *         'configPath' => '/path/to/preferences'
  *     ),
- *     $user // Instance of Icinga\User
+ *     $user // Instance of \Icinga\User
  * );
  *
  * // Create the store directly
@@ -39,11 +39,11 @@ use Icinga\Util\File;
  *     new Zend_Config(
  *         'configPath' => '/path/to/preferences'
  *     ),
- *     $user // Instance of Icinga\User
+ *     $user // Instance of \Icinga\User
  * );
  *
  * $preferences = new Preferences($store->load());
- * $prefereces->aPreference = 'value';
+ * $preferences->aPreference = 'value';
  * $store->save($preferences);
  * </code>
  */
@@ -59,9 +59,9 @@ class IniStore extends PreferencesStore
     /**
      * Stored preferences
      *
-     * @var Zend_Config_Ini
+     * @var array
      */
-    private $config;
+    private $preferences;
 
     /**
      * Writer which stores the preferences
@@ -87,7 +87,7 @@ class IniStore extends PreferencesStore
                 throw new NotReadableError('Preferences INI file ' . $this->preferencesFile . ' for user '
                     . $this->getUser()->getUsername() . ' is not readable');
             } else {
-                $this->config = new Zend_Config_Ini($this->preferencesFile);
+                $this->preferences = parse_ini_file($this->preferencesFile);
             }
         }
     }
@@ -99,7 +99,7 @@ class IniStore extends PreferencesStore
      */
     public function load()
     {
-        return $this->config !== null ? $this->config->toArray() : array();
+        return $this->preferences !== null ? $this->preferences : array();
     }
 
     /**
@@ -114,33 +114,33 @@ class IniStore extends PreferencesStore
      */
     public function cud($newPreferences, $updatedPreferences, $deletedPreferences)
     {
-        if ($this->config === null) {
+        if ($this->preferences === null) {
             // Preferences INI file does not yet exist
             if (!is_writable($this->getStoreConfig()->configPath)) {
                 throw new NotWritableError('Path to the preferences INI files ' . $this->getStoreConfig()->configPath
                     . ' is not writable');
             }
             File::create($this->preferencesFile);
-            $this->config = new Zend_Config_Ini($this->preferencesFile);
+            $this->preferences = array();
-        }
-        foreach ($newPreferences as $name => $value) {
-            $this->config->{$name} = $value;
-        }
-        foreach ($updatedPreferences as $name => $value) {
-            $this->config->{$name} = $value;
-        }
-        foreach ($deletedPreferences as $name) {
-            unset($this->config->{$name});
-        }
-        if ($this->writer === null) {
-            $this->writer = new PreservingIniWriter(
-                array('config' => $this->config)
-            );
         }
         if (!is_writable($this->preferencesFile)) {
             throw new NotWritableError('Preferences INI file ' . $this->preferencesFile . ' for user '
                 . $this->getUser()->getUsername() . ' is not writable');
         }
+        foreach ($newPreferences as $name => $value) {
+            $this->preferences[$name] = $value;
+        }
+        foreach ($updatedPreferences as $name => $value) {
+            $this->preferences[$name] = $value;
+        }
+        foreach ($deletedPreferences as $name) {
+            unset($this->preferences[$name]);
+        }
+        if ($this->writer === null) {
+            $this->writer = new PreservingIniWriter(
+                array('config' => new Zend_Config($this->preferences), 'filename' => $this->preferencesFile)
+            );
+        }
         $this->writer->write();
     }
 }

library/Icinga/Web/Controller/ActionController.php

@@ -235,7 +235,9 @@ class ActionController extends Zend_Controller_Action
             $this->_helper->layout()->setLayout($target);
         }
         if ($user = $this->getRequest()->getUser()) {
-            if ($user->getPreferences()->get('app.showBenchmark') === true) {
+            // Cast preference app.showBenchmark to bool because preferences loaded from a preferences storage are
+            // always strings
+            if ((bool) $user->getPreferences()->get('app.showBenchmark', false) === true) {
                 Benchmark::measure('Response ready');
                 $this->_helper->layout()->benchmark = $this->renderBenchmark();
             }

library/Icinga/Web/Controller/BasePreferenceController.php

@@ -29,6 +29,12 @@
 
 namespace Icinga\Web\Controller;
 
+use Icinga\Application\Config as IcingaConfig;
+use Icinga\Exception\ConfigurationError;
+use Icinga\Web\Session;
+use Icinga\User\Preferences;
+use Icinga\User\Preferences\PreferencesStore;
+
 /**
  *  Base class for Preference Controllers
  *
@@ -62,4 +68,24 @@ class BasePreferenceController extends ActionController
         parent::init();
         $this->view->tabs = ControllerTabCollector::collectControllerTabs('PreferenceController');
     }
+
+    protected function savePreferences(array $preferences)
+    {
+        $currentPreferences = $this->_request->getUser()->getPreferences();
+        foreach ($preferences as $key => $value) {
+            if ($value === null) {
+                unset($currentPreferences->{$key});
+            } else {
+                $currentPreferences->{$key} = $value;
+            }
+        }
+        Session::getSession()->write();
+        if (($preferencesConfig = IcingaConfig::app()->preferences) === null) {
+            throw new ConfigurationError(
+                'Cannot save preferences changes since you\'ve not configured a preferences backend'
+            );
+        }
+        $store = PreferencesStore::create($preferencesConfig, $this->_request->getUser());
+        $store->save($currentPreferences);
+    }
 }

library/Icinga/Web/Form.php

@@ -242,11 +242,14 @@ class Form extends Zend_Form
     /**
      * Set the configuration to be used for this form when no preferences are set yet
      *
-     * @param IcingaConfig $cfg
+     * @param   IcingaConfig $cfg
+     *
+     * @return  self
      */
     public function setConfiguration($cfg)
     {
         $this->config = $cfg;
+        return $this;
     }
 
     /**