fixes #6727 lazy load plugin settings

core/Db.php

@@ -46,7 +46,7 @@ class Db
             return Tracker::getDatabase();
         }
 
-        if (self::$connection === null) {
+        if (!self::hasDatabaseObject()) {
             self::createDatabaseObject();
         }
 
@@ -103,6 +103,16 @@ class Db
         self::$connection = $db;
     }
 
+    /**
+     * Detect whether a database object is initialized / created or not.
+     *
+     * @internal
+     */
+    public static function hasDatabaseObject()
+    {
+        return isset(self::$connection);
+    }
+
     /**
      * Disconnects and destroys the database connection.
      *

core/Plugin/Settings.php

@@ -58,6 +58,9 @@ abstract class Settings implements StorageInterface
     private $introduction;
     private $pluginName;
 
+    // for lazy loading of setting values
+    private $settingValuesLoaded = false;
+
     /**
      * Constructor.
      */
@@ -76,7 +79,6 @@ abstract class Settings implements StorageInterface
         }
 
         $this->init();
-        $this->loadSettings();
     }
 
     /**
@@ -126,7 +128,7 @@ abstract class Settings implements StorageInterface
         });
 
         $settings2 = $settings;
-        
+
         uasort($settings, function ($setting1, $setting2) use ($settings2) {
 
             /** @var Setting $setting1 */ /** @var Setting $setting2 */
@@ -166,6 +168,8 @@ abstract class Settings implements StorageInterface
      */
     public function save()
     {
+        $this->loadSettingsIfNotDoneYet();
+
         Option::set($this->getOptionKey(), serialize($this->settingsValues));
 
         $pluginName = $this->getPluginName();
@@ -195,6 +199,7 @@ abstract class Settings implements StorageInterface
 
         Option::delete($this->getOptionKey());
         $this->settingsValues = array();
+        $this->settingValuesLoaded = false;
     }
 
     /**
@@ -210,6 +215,7 @@ abstract class Settings implements StorageInterface
     {
         $this->checkIsValidSetting($setting->getName());
         $this->checkHasEnoughReadPermission($setting);
+        $this->loadSettingsIfNotDoneYet();
 
         if (array_key_exists($setting->getKey(), $this->settingsValues)) {
 
@@ -247,6 +253,7 @@ abstract class Settings implements StorageInterface
             settype($value, $setting->type);
         }
 
+        $this->loadSettingsIfNotDoneYet();
         $this->settingsValues[$setting->getKey()] = $value;
     }
 
@@ -259,6 +266,7 @@ abstract class Settings implements StorageInterface
     public function removeSettingValue(Setting $setting)
     {
         $this->checkHasEnoughWritePermission($setting);
+        $this->loadSettingsIfNotDoneYet();
 
         $key = $setting->getKey();
 
@@ -298,6 +306,16 @@ abstract class Settings implements StorageInterface
         return 'Plugin_' . $this->pluginName . '_Settings';
     }
 
+    private function loadSettingsIfNotDoneYet()
+    {
+        if ($this->settingValuesLoaded) {
+            return;
+        }
+
+        $this->settingValuesLoaded = true;
+        $this->loadSettings();
+    }
+
     private function loadSettings()
     {
         $values = Option::get($this->getOptionKey());
@@ -412,7 +430,7 @@ abstract class Settings implements StorageInterface
         $setting->validate = function ($value) use ($setting, $pluginName) {
 
             $errorMsg = Piwik::translate('CoreAdminHome_PluginSettingsValueNotAllowed',
-                                         array($setting->title, $pluginName));
+                array($setting->title, $pluginName));
 
             if (is_array($value) && $setting->type == Settings::TYPE_ARRAY) {
                 foreach ($value as $val) {

tests/PHPUnit/Framework/TestCase/SystemTestCase.php

@@ -589,4 +589,15 @@ abstract class SystemTestCase extends PHPUnit_Framework_TestCase
     {
         self::assertThat($url, new ResponseCode($expectedResponseCode), $message);
     }
+
+    public function assertNotDbConnectionCreated($message = 'A database connection was created but should not.')
+    {
+        self::assertFalse(Db::hasDatabaseObject(), $message);
+    }
+
+    public function assertDbConnectionCreated($message = 'A database connection was not created but should.')
+    {
+        self::assertTrue(Db::hasDatabaseObject(), $message);
+    }
+
 }

tests/PHPUnit/Integration/Plugin/SettingsTest.php

@@ -9,6 +9,7 @@
 namespace Piwik\Tests\Integration\Plugin;
 
 use Piwik\Access;
+use Piwik\Db;
 use Piwik\Plugin\Settings as PluginSettings;
 use Piwik\Settings\Setting;
 use Piwik\Tests\Framework\Mock\FakeAccess;
@@ -42,6 +43,7 @@ class SettingsTest extends IntegrationTestCase
     {
         parent::setUp();
         Access::setSingletonInstance(null);
+        Db::destroyDatabaseObject();
 
         $this->settings = $this->createSettingsInstance();
     }
@@ -56,6 +58,49 @@ class SettingsTest extends IntegrationTestCase
         parent::tearDown();
     }
 
+    public function test_constructor_shouldNotEstablishADatabaseConnection()
+    {
+        Db::destroyDatabaseObject();
+
+        $this->assertNotDbConnectionCreated();
+
+        $this->createSettingsInstance();
+
+        $this->assertNotDbConnectionCreated();
+    }
+
+    public function test_constructor_shouldEstablishADatabaseConnection_AsSoonAsWeGetAValue()
+    {
+        $this->setSuperUser();
+        Db::destroyDatabaseObject();
+
+        $setting  = $this->buildUserSetting('testSetting', 'Test Setting');
+        $settings = $this->createSettingsInstance();
+        $settings->addSetting($setting);
+
+        $this->assertNotDbConnectionCreated();
+
+        $settings->getSettingValue($setting);
+
+        $this->assertDbConnectionCreated();
+    }
+
+    public function test_constructor_shouldEstablishADatabaseConnection_AsSoonAsWeSetAValue()
+    {
+        $this->setSuperUser();
+        Db::destroyDatabaseObject();
+
+        $setting  = $this->buildUserSetting('testSetting', 'Test Setting');
+        $settings = $this->createSettingsInstance();
+        $settings->addSetting($setting);
+
+        $this->assertNotDbConnectionCreated();
+
+        $settings->setSettingValue($setting, '5');
+
+        $this->assertDbConnectionCreated();
+    }
+
     /**
      * @expectedException \Exception
      * @expectedExceptionMessage A setting with name "myname" does already exist for plugin "ExampleSettingsPlugin"