From 8c6f2a07ae10ed082865adb6eeb3e3f2fdb68e11 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 10 Jul 2013 11:40:48 +0200 Subject: [PATCH] Remove magic retrieval of configuration files This change leads to expected exceptions when bootstrapping the application since the \Icinga\Application\Config class was refactored but its usage not. refs #4354 refs #4353 --- doc/CONFIG.md | 24 +++ library/Icinga/Application/Config.php | 159 +++++++++++------- .../Application/Config/files/config.ini | 7 + .../Icinga/Application/Config/files/extra.ini | 2 + .../Config/files/modules/amodule/config.ini | 2 + .../Config/files/modules/amodule/extra.ini | 2 + .../library/Icinga/Application/ConfigTest.php | 100 +++++++---- test/php/library/Icinga/ConfigTest.php | 50 ------ 8 files changed, 200 insertions(+), 146 deletions(-) create mode 100644 doc/CONFIG.md create mode 100644 test/php/library/Icinga/Application/Config/files/config.ini create mode 100644 test/php/library/Icinga/Application/Config/files/extra.ini create mode 100644 test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini create mode 100644 test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini delete mode 100755 test/php/library/Icinga/ConfigTest.php diff --git a/doc/CONFIG.md b/doc/CONFIG.md new file mode 100644 index 000000000..5ebaa607b --- /dev/null +++ b/doc/CONFIG.md @@ -0,0 +1,24 @@ +# Application and Module Configuration + +The \Icinga\Application\Config class is a general purpose service to help you find, load and save +configuration data. It is used both by the Icinga 2 Web modules and the framework itself. With +INI files as source it enables you to store configuration in a familiar format. Icinga 2 Web +defines some configuration files for its own purposes. Please note that both modules and framework +keep their main configuration in the INI file called config.ini. Here's some example code: + + global->get('defaultTimezone', 'Europe/Berlin'); + + // If you don't pass a configuration name to IcingaConfig::app it tries to load values from the + // application's config.ini. For using other files you have to pass this parameter though. + // The following example loads a section from the application's authentication.ini: + IcingaConfig::app('authentication')->get('ldap-authentication'); + + // If you don't pass a configuration name to IcingaConfig::module it tries to load values from + // the module's config.ini. For using other files you have to pass this parameter though. + // The following example loads values from the example module's extra.ini: + IcingaConfig::module('example', 'extra')->logging->get('enabled', true); + diff --git a/library/Icinga/Application/Config.php b/library/Icinga/Application/Config.php index e19d15345..b77861310 100755 --- a/library/Icinga/Application/Config.php +++ b/library/Icinga/Application/Config.php @@ -1,79 +1,110 @@ $what === null) { + if (!@is_readable($filename)) { + throw new \Exception('Cannot read config file: ' . $filename); + }; + $this->configFile = $filename; + parent::__construct($filename); + } + + /** + * Retrieve a application config instance. + * + * @param string $configname + * @return mixed + */ + public static function app($configname = 'config') + { + if (!isset(self::$app[$configname])) { + $filename = self::$configDir . '/' . $configname . '.ini'; + self::$app[$configname] = new Config($filename); + } + return self::$app[$configname]; + } + + /** + * Retrieve a module config instance. + * + * @param string $modulename + * @param string $configname + * @return Config + */ + public static function module($modulename, $configname = 'config') + { + if (!isset(self::$modules[$modulename])) { + self::$modules[$modulename] = array(); + } + $moduleConfigs = self::$modules[$modulename]; + if (!isset($moduleConfigs[$configname])) { + $filename = self::$configDir . '/modules/' . $modulename . '/' . $configname . '.ini'; + $moduleConfigs[$configname] = new Config($filename); + } + return $moduleConfigs[$configname]; + } + + /** + * Retrieve names of accessible sections or properties. + * + * @param $name + * @return array + */ + public function keys($name = null) + { + if ($name === null) { + return array_keys($this->toArray()); + } elseif ($this->$name === null) { return array(); } else { - return array_keys($this->$what->toArray()); + return array_keys($this->$name->toArray()); } } - - public function getConfigDir() - { - return $this->configDir; - } - - public function __construct($filename, $section = null, $options = false) - { - $options['allowModifications'] = true; - $this->configDir = dirname($filename); - return parent::__construct($filename, $section, $options); - } - - public static function module($name, $file = null) - { - if ($file === null) { - $file = $name . '.ini'; // TODO: default should be module/config.ini - } - $filename = Module::get($name)->getConfigDir() . '/' . $file; - if (file_exists($filename)) { - $config = new Config($filename); - // Compat: $config->$module->$whatever - self::getInstance()->$name = $config; - return $config; - } - return null; - } - - public function __get($key) - { - $res = parent::__get($key); - if ($res === null) { - $app = Icinga::app(); - if ($app->hasModule($key)) { - $filename = $app->getModule($key)->getConfigDir() . "/$key.ini"; - } else { - $filename = $this->configDir . '/' . $key . '.ini'; - } - if (file_exists($filename)) { - $res = $this->$key = new Config($filename); - } - } - return $res; - } - - public static function getInstance($configFile = null) - { - if (self::$instance === null) { - if ($configFile === null) { - $configFile = dirname(dirname(dirname(dirname(__FILE__)))) - . '/config/icinga.ini'; - } - self::$instance = new Config($configFile); - } - return self::$instance; - } } diff --git a/test/php/library/Icinga/Application/Config/files/config.ini b/test/php/library/Icinga/Application/Config/files/config.ini new file mode 100644 index 000000000..b74ea65c5 --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/config.ini @@ -0,0 +1,7 @@ +[logging] +enable = 1 + +[backend] +db.user = 'user' +db.password = 'password' +disable = 1 diff --git a/test/php/library/Icinga/Application/Config/files/extra.ini b/test/php/library/Icinga/Application/Config/files/extra.ini new file mode 100644 index 000000000..fc203052e --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/extra.ini @@ -0,0 +1,2 @@ +[meta] +version = 1 diff --git a/test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini b/test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini new file mode 100644 index 000000000..012a5188a --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/modules/amodule/config.ini @@ -0,0 +1,2 @@ +[menu] +breadcrumb = 1 diff --git a/test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini b/test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini new file mode 100644 index 000000000..38fd04b27 --- /dev/null +++ b/test/php/library/Icinga/Application/Config/files/modules/amodule/extra.ini @@ -0,0 +1,2 @@ +[ldap] +user.ldap_object_class = inetOrgPerson diff --git a/test/php/library/Icinga/Application/ConfigTest.php b/test/php/library/Icinga/Application/ConfigTest.php index 72cd2cfad..52967c220 100644 --- a/test/php/library/Icinga/Application/ConfigTest.php +++ b/test/php/library/Icinga/Application/ConfigTest.php @@ -1,50 +1,86 @@ markTestIncomplete('testListAll is not implemented yet'); + IcingaConfig::$configDir = dirname(__FILE__) . '/Config/files'; } - /** - * Test for Config::GetConfigDir() - * - **/ - public function testGetConfigDir() + public function testAppConfig() { - $this->markTestIncomplete('testGetConfigDir is not implemented yet'); + $config = IcingaConfig::app(); + $this->assertEquals(1, $config->logging->enable); + // Test non-existent property where null is the default value + $this->assertEquals(null, $config->logging->get('disable')); + // Test non-existent property using zero as the default value + $this->assertEquals(0, $config->logging->get('disable', 0)); + // Test retrieve full section + $this->assertEquals( + array( + 'disable' => 1, + 'db' => array( + 'user' => 'user', + 'password' => 'password' + ) + ), + $config->backend->toArray() + ); + // Test non-existent section using 'default' as default value + $this->assertEquals('default', $config->get('magic', 'default')); + // Test sub-properties + $this->assertEquals('user', $config->backend->db->user); + // Test non-existent sub-property using 'UTF-8' as the default value + $this->assertEquals('UTF-8', $config->backend->db->get('encoding', 'UTF-8')); + // Test invalid property names using false as default value + $this->assertEquals(false, $config->backend->get('.', false)); + $this->assertEquals(false, $config->backend->get('db.', false)); + $this->assertEquals(false, $config->backend->get('.user', false)); + // Test retrieve array of sub-properties + $this->assertEquals( + array( + 'user' => 'user', + 'password' => 'password' + ), + $config->backend->db->toArray() + ); + // Test singleton + $this->assertEquals($config, IcingaConfig::app()); + $this->assertEquals(array('logging', 'backend'), $config->keys()); + $this->assertEquals(array('enable'), $config->keys('logging')); } - /** - * Test for Config::__get() - * - **/ - public function test__get() + public function testAppExtraConfig() { - $this->markTestIncomplete('test__get is not implemented yet'); + $extraConfig = IcingaConfig::app('extra'); + $this->assertEquals(1, $extraConfig->meta->version); + $this->assertEquals($extraConfig, IcingaConfig::app('extra')); } - /** - * Test for Config::GetInstance() - * Note: This method is static! - * - **/ - public function testGetInstance() + public function testModuleConfig() { - $this->markTestIncomplete('testGetInstance is not implemented yet'); + $moduleConfig = IcingaConfig::module('amodule'); + $this->assertEquals(1, $moduleConfig->menu->get('breadcrumb')); + $this->assertEquals($moduleConfig, IcingaConfig::module('amodule')); } + public function testModuleExtraConfig() + { + $moduleExtraConfig = IcingaConfig::module('amodule', 'extra'); + $this->assertEquals( + 'inetOrgPerson', + $moduleExtraConfig->ldap->user->get('ldap_object_class') + ); + $this->assertEquals($moduleExtraConfig, IcingaConfig::module('amodule', 'extra')); + } } diff --git a/test/php/library/Icinga/ConfigTest.php b/test/php/library/Icinga/ConfigTest.php deleted file mode 100755 index 4d700a579..000000000 --- a/test/php/library/Icinga/ConfigTest.php +++ /dev/null @@ -1,50 +0,0 @@ -markTestIncomplete('testGet is not implemented yet'); - } - - /** - * Test for Config::__get() - * - **/ - public function test__get() - { - $this->markTestIncomplete('test__get is not implemented yet'); - } - - /** - * Test for Config::__set() - * - **/ - public function test__set() - { - $this->markTestIncomplete('test__set is not implemented yet'); - } - - /** - * Test for Config::Create() - * Note: This method is static! - * - **/ - public function testCreate() - { - $this->markTestIncomplete('testCreate is not implemented yet'); - } - -}