diff --git a/core/Db.php b/core/Db.php index 556bd2ada5d27009ae0e9fec7c46256662dd37ca..fa26f34f02c95ddc4935fb994360dc97926aa0e6 100644 --- a/core/Db.php +++ b/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. * diff --git a/core/Plugin/Settings.php b/core/Plugin/Settings.php index 57f8c813fb6f80522f0029c9f07ae285a5dfceae..0b0bb0953aa7eb015823535001411c2858e3b6d5 100644 --- a/core/Plugin/Settings.php +++ b/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) { diff --git a/tests/PHPUnit/Framework/TestCase/SystemTestCase.php b/tests/PHPUnit/Framework/TestCase/SystemTestCase.php index 879fc915e2c91a446dcbeb5ff082dd397f356b64..d0a7c340ca4c3ea59268a71b241c8de523002944 100755 --- a/tests/PHPUnit/Framework/TestCase/SystemTestCase.php +++ b/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); + } + } diff --git a/tests/PHPUnit/Integration/Plugin/SettingsTest.php b/tests/PHPUnit/Integration/Plugin/SettingsTest.php index 763ce4f09a22913f8584277cc7b4f2b0c385bc5b..3c146f5d31a6eb8476ff6aed17296e1ce07fe6f7 100644 --- a/tests/PHPUnit/Integration/Plugin/SettingsTest.php +++ b/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"