Do not cache app settings in session

ChangeLog

@@ -10,6 +10,7 @@ Improvements:
 
 * Add dropdown menu for subtasks, categories, swimlanes, columns, custom filters, task links and groups
 * Add new template hooks
+* Application settings are not cached anymore in the session
 
 Bug fixes:
 

app/Controller/Currency.php

@@ -38,7 +38,7 @@ class Currency extends Base
             'values' => $values,
             'errors' => $errors,
             'rates' => $this->currency->getAll(),
-            'currencies' => $this->config->getCurrencies(),
+            'currencies' => $this->currency->getCurrencies(),
             'title' => t('Settings').' > '.t('Currency rates'),
         )));
     }

app/Core/Session/SessionStorage.php

@@ -8,7 +8,6 @@ namespace Kanboard\Core\Session;
  * @package  session
  * @author   Frederic Guillot
  *
- * @property array  $config
  * @property array  $user
  * @property array  $flash
  * @property array  $csrf

app/Model/Config.php

@@ -14,32 +14,6 @@ use Kanboard\Core\Session\SessionManager;
  */
 class Config extends Setting
 {
-    /**
-     * Get available currencies
-     *
-     * @access public
-     * @return array
-     */
-    public function getCurrencies()
-    {
-        return array(
-            'USD' => t('USD - US Dollar'),
-            'EUR' => t('EUR - Euro'),
-            'GBP' => t('GBP - British Pound'),
-            'CHF' => t('CHF - Swiss Francs'),
-            'CAD' => t('CAD - Canadian Dollar'),
-            'AUD' => t('AUD - Australian Dollar'),
-            'NZD' => t('NZD - New Zealand Dollar'),
-            'INR' => t('INR - Indian Rupee'),
-            'JPY' => t('JPY - Japanese Yen'),
-            'RSD' => t('RSD - Serbian dinar'),
-            'SEK' => t('SEK - Swedish Krona'),
-            'NOK' => t('NOK - Norwegian Krone'),
-            'BAM' => t('BAM - Konvertible Mark'),
-            'RUB' => t('RUB - Russian Ruble'),
-        );
-    }
-
     /**
      * Get available timezones
      *
@@ -59,6 +33,31 @@ class Config extends Setting
         return $listing;
     }
 
+    /**
+     * Get current timezone
+     *
+     * @access public
+     * @return string
+     */
+    public function getCurrentTimezone()
+    {
+        if ($this->userSession->isLogged() && ! empty($this->sessionStorage->user['timezone'])) {
+            return $this->sessionStorage->user['timezone'];
+        }
+
+        return $this->get('application_timezone', 'UTC');
+    }
+
+    /**
+     * Set timezone
+     *
+     * @access public
+     */
+    public function setupTimezone()
+    {
+        date_default_timezone_set($this->getCurrentTimezone());
+    }
+
     /**
      * Get available languages
      *
@@ -156,43 +155,6 @@ class Config extends Setting
         return $this->get('application_language', 'en_US');
     }
 
-    /**
-     * Get a config variable from the session or the database
-     *
-     * @access public
-     * @param  string   $name            Parameter name
-     * @param  string   $default_value   Default value of the parameter
-     * @return string
-     */
-    public function get($name, $default_value = '')
-    {
-        if (! SessionManager::isOpen()) {
-            return $this->getOption($name, $default_value);
-        }
-
-        // Cache config in session
-        if (! isset($this->sessionStorage->config[$name])) {
-            $this->sessionStorage->config = $this->getAll();
-        }
-
-        if (! empty($this->sessionStorage->config[$name])) {
-            return $this->sessionStorage->config[$name];
-        }
-
-        return $default_value;
-    }
-
-    /**
-     * Reload settings in the session and the translations
-     *
-     * @access public
-     */
-    public function reload()
-    {
-        $this->sessionStorage->config = $this->getAll();
-        $this->setupTranslations();
-    }
-
     /**
      * Load translations
      *
@@ -204,28 +166,27 @@ class Config extends Setting
     }
 
     /**
-     * Get current timezone
+     * Get a config variable from the session or the database
      *
      * @access public
+     * @param  string   $name            Parameter name
+     * @param  string   $default_value   Default value of the parameter
      * @return string
      */
-    public function getCurrentTimezone()
+    public function get($name, $default_value = '')
     {
-        if ($this->userSession->isLogged() && ! empty($this->sessionStorage->user['timezone'])) {
+        $options = $this->memoryCache->proxy($this, 'getAll');
-            return $this->sessionStorage->user['timezone'];
+        return isset($options[$name]) && $options[$name] !== '' ? $options[$name] : $default_value;
-        }
-
-        return $this->get('application_timezone', 'UTC');
     }
 
     /**
-     * Set timezone
+     * Reload settings in the session and the translations
      *
      * @access public
      */
-    public function setupTimezone()
+    public function reload()
     {
-        date_default_timezone_set($this->getCurrentTimezone());
+        $this->setupTranslations();
     }
 
     /**
@@ -236,7 +197,7 @@ class Config extends Setting
      */
     public function optimizeDatabase()
     {
-        return $this->db->getconnection()->exec("VACUUM");
+        return $this->db->getconnection()->exec('VACUUM');
     }
 
     /**
@@ -266,10 +227,11 @@ class Config extends Setting
      *
      * @access public
      * @param  string   $option   Parameter name
+     * @return boolean
      */
     public function regenerateToken($option)
     {
-        $this->save(array($option => Token::getToken()));
+        return $this->save(array($option => Token::getToken()));
     }
 
     /**

app/Model/Currency.php

@@ -17,6 +17,32 @@ class Currency extends Base
      */
     const TABLE = 'currencies';
 
+    /**
+     * Get available application currencies
+     *
+     * @access public
+     * @return array
+     */
+    public function getCurrencies()
+    {
+        return array(
+            'USD' => t('USD - US Dollar'),
+            'EUR' => t('EUR - Euro'),
+            'GBP' => t('GBP - British Pound'),
+            'CHF' => t('CHF - Swiss Francs'),
+            'CAD' => t('CAD - Canadian Dollar'),
+            'AUD' => t('AUD - Australian Dollar'),
+            'NZD' => t('NZD - New Zealand Dollar'),
+            'INR' => t('INR - Indian Rupee'),
+            'JPY' => t('JPY - Japanese Yen'),
+            'RSD' => t('RSD - Serbian dinar'),
+            'SEK' => t('SEK - Swedish Krona'),
+            'NOK' => t('NOK - Norwegian Krone'),
+            'BAM' => t('BAM - Konvertible Mark'),
+            'RUB' => t('RUB - Russian Ruble'),
+        );
+    }
+
     /**
      * Get all currency rates
      *

app/Model/Setting.php

@@ -47,10 +47,12 @@ abstract class Setting extends Base
      */
     public function getOption($name, $default = '')
     {
-        return $this->db
+        $value = $this->db
             ->table(self::TABLE)
             ->eq('option', $name)
-            ->findOneColumn('value') ?: $default;
+            ->findOneColumn('value');
+
+        return $value === null || $value === false || $value === '' ? $default : $value;
     }
 
     /**

tests/units/Model/ConfigTest.php

@@ -7,6 +7,74 @@ use Kanboard\Core\Session\SessionManager;
 
 class ConfigTest extends Base
 {
+    public function testGetTimezones()
+    {
+        $configModel = new Config($this->container);
+        $this->assertNotEmpty($configModel->getTimezones());
+        $this->assertArrayHasKey('Europe/Paris', $configModel->getTimezones());
+        $this->assertContains('Europe/Paris', $configModel->getTimezones());
+        $this->assertArrayNotHasKey('', $configModel->getTimezones());
+
+        $this->assertArrayHasKey('', $configModel->getTimezones(true));
+        $this->assertContains('Application default', $configModel->getTimezones(true));
+    }
+
+    public function testGetLanguages()
+    {
+        $configModel = new Config($this->container);
+        $this->assertNotEmpty($configModel->getLanguages());
+        $this->assertArrayHasKey('fr_FR', $configModel->getLanguages());
+        $this->assertContains('Français', $configModel->getLanguages());
+        $this->assertArrayNotHasKey('', $configModel->getLanguages());
+
+        $this->assertArrayHasKey('', $configModel->getLanguages(true));
+        $this->assertContains('Application default', $configModel->getLanguages(true));
+    }
+
+    public function testGetJsLanguage()
+    {
+        $configModel = new Config($this->container);
+        $this->assertEquals('en', $configModel->getJsLanguageCode());
+
+        $this->container['sessionStorage']->user = array('language' => 'fr_FR');
+        $this->assertEquals('fr', $configModel->getJsLanguageCode());
+
+        $this->container['sessionStorage']->user = array('language' => 'xx_XX');
+        $this->assertEquals('en', $configModel->getJsLanguageCode());
+    }
+
+    public function testGetCurrentLanguage()
+    {
+        $configModel = new Config($this->container);
+        $this->assertEquals('en_US', $configModel->getCurrentLanguage());
+
+        $this->container['sessionStorage']->user = array('language' => 'fr_FR');
+        $this->assertEquals('fr_FR', $configModel->getCurrentLanguage());
+
+        $this->container['sessionStorage']->user = array('language' => 'xx_XX');
+        $this->assertEquals('xx_XX', $configModel->getCurrentLanguage());
+    }
+
+    public function testGetCurrentTimezone()
+    {
+        $configModel = new Config($this->container);
+        $this->assertEquals('UTC', $configModel->getCurrentTimezone());
+
+        $this->container['sessionStorage']->user = array('timezone' => 'Europe/Paris');
+        $this->assertEquals('Europe/Paris', $configModel->getCurrentTimezone());
+
+        $this->container['sessionStorage']->user = array('timezone' => 'Something');
+        $this->assertEquals('Something', $configModel->getCurrentTimezone());
+    }
+
+    public function testRegenerateToken()
+    {
+        $configModel = new Config($this->container);
+        $token = $configModel->getOption('api_token');
+        $this->assertTrue($configModel->regenerateToken('api_token'));
+        $this->assertNotEquals($token, $configModel->getOption('api_token'));
+    }
+
     public function testCRUDOperations()
     {
         $c = new Config($this->container);
@@ -37,60 +105,73 @@ class ConfigTest extends Base
         $c = new Config($this->container);
 
         $this->assertTrue($c->save(array('application_url' => 'http://localhost/')));
-        $this->assertEquals('http://localhost/', $c->get('application_url'));
+        $this->assertEquals('http://localhost/', $c->getOption('application_url'));
 
         $this->assertTrue($c->save(array('application_url' => 'http://localhost')));
-        $this->assertEquals('http://localhost/', $c->get('application_url'));
+        $this->assertEquals('http://localhost/', $c->getOption('application_url'));
 
         $this->assertTrue($c->save(array('application_url' => '')));
-        $this->assertEquals('', $c->get('application_url'));
+        $this->assertEquals('', $c->getOption('application_url'));
     }
 
     public function testDefaultValues()
     {
         $c = new Config($this->container);
 
-        $this->assertEquals('en_US', $c->get('application_language'));
+        $this->assertEquals(172800, $c->getOption('board_highlight_period'));
+        $this->assertEquals(60, $c->getOption('board_public_refresh_interval'));
+        $this->assertEquals(10, $c->getOption('board_private_refresh_interval'));
+        $this->assertEmpty($c->getOption('board_columns'));
+
+        $this->assertEquals('yellow', $c->getOption('default_color'));
+        $this->assertEquals('en_US', $c->getOption('application_language'));
+        $this->assertEquals('UTC', $c->getOption('application_timezone'));
+        $this->assertEquals('m/d/Y', $c->getOption('application_date_format'));
+        $this->assertEmpty($c->getOption('application_url'));
+        $this->assertEmpty($c->getOption('application_stylesheet'));
+        $this->assertEquals('USD', $c->getOption('application_currency'));
+
+        $this->assertEquals(0, $c->getOption('calendar_user_subtasks_time_tracking'));
+        $this->assertEquals('date_started', $c->getOption('calendar_user_tasks'));
+        $this->assertEquals('date_started', $c->getOption('calendar_user_tasks'));
+
+        $this->assertEquals(0, $c->getOption('integration_gravatar'));
+        $this->assertEquals(1, $c->getOption('cfd_include_closed_tasks'));
+        $this->assertEquals(1, $c->getOption('password_reset'));
+
+        $this->assertEquals(1, $c->getOption('subtask_time_tracking'));
+        $this->assertEquals(0, $c->getOption('subtask_restriction'));
+        $this->assertEmpty($c->getOption('project_categories'));
+
+        $this->assertEmpty($c->getOption('webhook_url_task_modification'));
+        $this->assertEmpty($c->getOption('webhook_url_task_creation'));
+        $this->assertNotEmpty($c->getOption('webhook_token'));
+        $this->assertEmpty($c->getOption('webhook_url'));
+
+        $this->assertNotEmpty($c->getOption('api_token'));
+    }
+
+    public function testGetOption()
+    {
+        $c = new Config($this->container);
+
+        $this->assertEquals('', $c->getOption('board_columns'));
+        $this->assertEquals('test', $c->getOption('board_columns', 'test'));
+        $this->assertEquals(0, $c->getOption('board_columns', 0));
+    }
+
+    public function testGetWithCaching()
+    {
+        $c = new Config($this->container);
+
         $this->assertEquals('UTC', $c->get('application_timezone'));
+        $this->assertTrue($c->save(array('application_timezone' => 'Europe/Paris')));
 
-        $this->assertEmpty($c->get('webhook_url_task_modification'));
+        $this->assertEquals('UTC', $c->get('application_timezone')); // cached value
-        $this->assertEmpty($c->get('webhook_url_task_creation'));
+        $this->assertEquals('Europe/Paris', $c->getOption('application_timezone'));
-        $this->assertEmpty($c->get('board_columns'));
-        $this->assertEmpty($c->get('application_url'));
-
-        $this->assertNotEmpty($c->get('webhook_token'));
-        $this->assertNotEmpty($c->get('api_token'));
-    }
-
-    public function testGet()
-    {
-        $c = new Config($this->container);
 
         $this->assertEquals('', $c->get('board_columns'));
         $this->assertEquals('test', $c->get('board_columns', 'test'));
-        $this->assertEquals(0, $c->get('board_columns', 0));
-    }
-
-    public function testGetWithSession()
-    {
-        $c = new Config($this->container);
-
-        session_id('test');
-        $this->assertTrue(SessionManager::isOpen());
-
-        $this->assertEquals('', $c->get('board_columns'));
-        $this->assertEquals('test', $c->get('board_columns', 'test'));
-
-        $this->container['sessionStorage']->config = array(
-            'board_columns' => 'foo',
-            'empty_value' => 0
-        );
-
-        $this->assertEquals('foo', $c->get('board_columns'));
-        $this->assertEquals('foo', $c->get('board_columns', 'test'));
         $this->assertEquals('test', $c->get('empty_value', 'test'));
-
-        session_id('');
-        $this->assertFalse(SessionManager::isOpen());
     }
 }

tests/units/Model/CurrencyTest.php

@@ -6,6 +6,13 @@ use Kanboard\Model\Currency;
 
 class CurrencyTest extends Base
 {
+    public function testGetCurrencies()
+    {
+        $currencyModel = new Currency($this->container);
+        $currencies = $currencyModel->getCurrencies();
+        $this->assertArrayHasKey('EUR', $currencies);
+    }
+
     public function testGetAll()
     {
         $currencyModel = new Currency($this->container);