From 63305fdf9aca81a477f677659062b12136325446 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 09:32:08 +0100 Subject: [PATCH] Add Icinga\Application\Config::saveIni() Simplifies saving INI files. Icinga\File\Ini\IniWriter does already require an instance of Icinga\Application\Config so it's obvious to give "Config" the task to initialize the writer.. We do also have a central place to handle creating missing ancestor directories now. refs #8219 --- .../controllers/DashboardController.php | 41 ++++++++------- application/forms/ConfigForm.php | 12 +---- .../views/scripts/dashboard/error.phtml | 14 ++--- library/Icinga/Application/Config.php | 51 +++++++++++++++++++ .../User/Preferences/Store/IniStore.php | 39 +------------- library/Icinga/Web/Widget/Dashboard.php | 21 ++------ .../library/Monitoring/BackendStep.php | 17 ++----- .../library/Monitoring/InstanceStep.php | 9 ++-- .../library/Monitoring/SecurityStep.php | 9 ++-- .../Setup/Steps/AuthenticationStep.php | 17 +++---- .../library/Setup/Steps/GeneralConfigStep.php | 9 ++-- .../library/Setup/Steps/ResourceStep.php | 10 ++-- 12 files changed, 109 insertions(+), 140 deletions(-) diff --git a/application/controllers/DashboardController.php b/application/controllers/DashboardController.php index ba2125e12..7c70c9c5a 100644 --- a/application/controllers/DashboardController.php +++ b/application/controllers/DashboardController.php @@ -2,14 +2,13 @@ // {{{ICINGA_LICENSE_HEADER}}} // {{{ICINGA_LICENSE_HEADER}}} -use Icinga\Application\Logger; +use Exception; use Icinga\Exception\ProgrammingError; use Icinga\Forms\ConfirmRemovalForm; use Icinga\Forms\Dashboard\DashletForm; use Icinga\Web\Form; use Icinga\Web\Notification; use Icinga\Web\Controller\ActionController; -use Icinga\Web\Request; use Icinga\Web\Url; use Icinga\Web\Widget\Dashboard; use Icinga\Web\Widget\Tabextension\DashboardSettings; @@ -56,11 +55,12 @@ class DashboardController extends ActionController $dashlet = new Dashboard\Dashlet($form->getValue('dashlet'), $form->getValue('url'), $pane); $dashlet->setUserWidget(); $pane->addDashlet($dashlet); + $dashboardConfig = $dashboard->getConfig(); try { - $dashboard->write(); - } catch (\Zend_Config_Exception $e) { + $dashboardConfig->saveIni(); + } catch (Exception $e) { $action->view->error = $e; - $action->view->config = $dashboard->createWriter(); + $action->view->config = $dashboardConfig; $action->render('error'); return false; } @@ -117,11 +117,12 @@ class DashboardController extends ActionController $oldPane = $dashboard->getPane($form->getValue('org_pane')); $oldPane->removeDashlet($dashlet->getTitle()); } + $dashboardConfig = $dashboard->getConfig(); try { - $dashboard->write(); - } catch (\Zend_Config_Exception $e) { + $dashboardConfig->saveIni(); + } catch (Exception $e) { $action->view->error = $e; - $action->view->config = $dashboard->createWriter(); + $action->view->config = $dashboardConfig; $action->render('error'); return false; } @@ -158,15 +159,16 @@ class DashboardController extends ActionController $dashlet = $this->_request->getParam('dashlet'); $action = $this; $form->setOnSuccess(function (Form $form) use ($dashboard, $dashlet, $pane, $action) { + $pane = $dashboard->getPane($pane); + $pane->removeDashlet($dashlet); + $dashboardConfig = $dashboard->getConfig(); try { - $pane = $dashboard->getPane($pane); - $pane->removeDashlet($dashlet); - $dashboard->write(); + $dashboardConfig->saveIni(); Notification::success(t('Dashlet has been removed from') . ' ' . $pane->getTitle()); return true; - } catch (\Zend_Config_Exception $e) { + } catch (Exception $e) { $action->view->error = $e; - $action->view->config = $dashboard->createWriter(); + $action->view->config = $dashboardConfig; $action->render('error'); return false; } catch (ProgrammingError $e) { @@ -196,15 +198,16 @@ class DashboardController extends ActionController $pane = $this->_request->getParam('pane'); $action = $this; $form->setOnSuccess(function (Form $form) use ($dashboard, $pane, $action) { + $pane = $dashboard->getPane($pane); + $dashboard->removePane($pane->getTitle()); + $dashboardConfig = $dashboard->getConfig(); try { - $pane = $dashboard->getPane($pane); - $dashboard->removePane($pane->getTitle()); - $dashboard->write(); + $dashboardConfig->saveIni(); Notification::success(t('Dashboard has been removed') . ': ' . $pane->getTitle()); return true; - } catch (\Zend_Config_Exception $e) { + } catch (Exception $e) { $action->view->error = $e; - $action->view->config = $dashboard->createWriter(); + $action->view->config = $dashboardConfig; $action->render('error'); return false; } catch (ProgrammingError $e) { @@ -241,7 +244,7 @@ class DashboardController extends ActionController $this->view->title = $this->dashboard->getActivePane()->getTitle() . ' :: Dashboard'; if ($this->hasParam('remove')) { $this->dashboard->getActivePane()->removeDashlet($this->getParam('remove')); - $this->dashboard->write(); + $this->dashboard->getConfig()->saveIni(); $this->redirectNow(URL::fromRequest()->remove('remove')); } $this->view->tabs->add( diff --git a/application/forms/ConfigForm.php b/application/forms/ConfigForm.php index 28d3111ea..224633f1f 100644 --- a/application/forms/ConfigForm.php +++ b/application/forms/ConfigForm.php @@ -8,7 +8,6 @@ use Exception; use Zend_Form_Decorator_Abstract; use Icinga\Web\Form; use Icinga\Application\Config; -use Icinga\File\Ini\IniWriter; /** * Form base-class providing standard functionality for configuration forms @@ -44,21 +43,14 @@ class ConfigForm extends Form */ public function save() { - $writer = new IniWriter( - array( - 'config' => $this->config, - 'filename' => $this->config->getConfigFile() - ) - ); - try { - $writer->write(); + $this->config->saveIni(); } catch (Exception $e) { $this->addDecorator('ViewScript', array( 'viewModule' => 'default', 'viewScript' => 'showConfiguration.phtml', 'errorMessage' => $e->getMessage(), - 'configString' => $writer->render(), + 'configString' => $this->config, 'filePath' => $this->config->getConfigFile(), 'placement' => Zend_Form_Decorator_Abstract::PREPEND )); diff --git a/application/views/scripts/dashboard/error.phtml b/application/views/scripts/dashboard/error.phtml index e5a0f3939..56e32faf4 100644 --- a/application/views/scripts/dashboard/error.phtml +++ b/application/views/scripts/dashboard/error.phtml @@ -1,13 +1,13 @@
-

+

translate('Could not persist dashboard'); ?>

- - config->getFilename(); ?>;. + translate('Please copy the following dashboard snippet to '); ?> + config->getConfigFile(); ?>;.
- + translate('Make sure that the webserver can write to this file.'); ?>

- 
config->render(); ?>
+ 
config; ?>
-

+

translate('Error details'); ?>

error->getMessage(); ?>

-
+ \ No newline at end of file diff --git a/library/Icinga/Application/Config.php b/library/Icinga/Application/Config.php index 361f5f11d..8d4914316 100644 --- a/library/Icinga/Application/Config.php +++ b/library/Icinga/Application/Config.php @@ -6,8 +6,10 @@ namespace Icinga\Application; use Iterator; use Countable; +use LogicException; use UnexpectedValueException; use Icinga\Data\ConfigObject; +use Icinga\File\Ini\IniWriter; use Icinga\Exception\NotReadableError; /** @@ -299,6 +301,45 @@ class Config implements Countable, Iterator return $emptyConfig; } + /** + * Save configuration to the given INI file + * + * @param string|null $filePath The path to the INI file or null in case this config's path should be used + * @param int $fileMode The file mode to store the file with + * + * @throws LogicException In case this config has no path and none is passed in either + * @throws NotWritableError In case the INI file cannot be written + * + * @todo create basepath and throw NotWritableError in case its not possible + */ + public function saveIni($filePath = null, $fileMode = 0660) + { + if ($filePath === null && $this->configFile) { + $filePath = $this->configFile; + } elseif ($filePath === null) { + throw new LogicException('You need to pass $filePath or set a path using Config::setConfigFile()'); + } + + $this->getIniWriter($filePath, $fileMode)->write(); + } + + /** + * Return a IniWriter for this config + * + * @param string|null $filePath + * @param int $fileMode + * + * @return IniWriter + */ + protected function getIniWriter($filePath = null, $fileMode = null) + { + return new IniWriter(array( + 'config' => $this, + 'filename' => $filePath, + 'filemode' => $fileMode + )); + } + /** * Prepend configuration base dir to the given relative path * @@ -354,4 +395,14 @@ class Config implements Countable, Iterator return $moduleConfigs[$configname]; } + + /** + * Return this config rendered as a INI structured string + * + * @return string + */ + public function __toString() + { + return $this->getIniWriter()->render(); + } } diff --git a/library/Icinga/User/Preferences/Store/IniStore.php b/library/Icinga/User/Preferences/Store/IniStore.php index 1ba780184..1a096ea7b 100644 --- a/library/Icinga/User/Preferences/Store/IniStore.php +++ b/library/Icinga/User/Preferences/Store/IniStore.php @@ -7,10 +7,8 @@ namespace Icinga\User\Preferences\Store; use Icinga\Application\Config; use Icinga\Exception\NotReadableError; use Icinga\Exception\NotWritableError; -use Icinga\File\Ini\IniWriter; use Icinga\User\Preferences; use Icinga\User\Preferences\PreferencesStore; -use Icinga\Util\File; /** * Load and save user preferences from and to INI files @@ -31,13 +29,6 @@ class IniStore extends PreferencesStore */ protected $preferences = array(); - /** - * Writer which stores the preferences - * - * @var IniWriter - */ - protected $writer; - /** * Initialize the store */ @@ -98,35 +89,7 @@ class IniStore extends PreferencesStore */ public function write() { - if ($this->writer === null) { - if (! file_exists($this->preferencesFile)) { - if (! is_writable($this->getStoreConfig()->location)) { - throw new NotWritableError( - 'Path to the preferences INI files %s is not writable', - $this->getStoreConfig()->location - ); - } - - File::create($this->preferencesFile, 0664); - } - - if (! is_writable($this->preferencesFile)) { - throw new NotWritableError( - 'Preferences INI file %s for user %s is not writable', - $this->preferencesFile, - $this->getUser()->getUsername() - ); - } - - $this->writer = new IniWriter( - array( - 'config' => Config::fromArray($this->preferences), - 'filename' => $this->preferencesFile - ) - ); - } - - $this->writer->write(); + Config::fromArray($this->preferences)->saveIni($this->preferencesFile); } /** diff --git a/library/Icinga/Web/Widget/Dashboard.php b/library/Icinga/Web/Widget/Dashboard.php index 45e1e6830..fe4bba803 100644 --- a/library/Icinga/Web/Widget/Dashboard.php +++ b/library/Icinga/Web/Widget/Dashboard.php @@ -6,12 +6,10 @@ namespace Icinga\Web\Widget; use Icinga\Application\Icinga; use Icinga\Application\Config; -use Icinga\Data\ConfigObject; use Icinga\Exception\ConfigurationError; use Icinga\Exception\NotReadableError; use Icinga\Exception\ProgrammingError; use Icinga\Exception\SystemPermissionException; -use Icinga\File\Ini\IniWriter; use Icinga\User; use Icinga\Web\Widget\Dashboard\Pane; use Icinga\Web\Widget\Dashboard\Dashlet as DashboardDashlet; @@ -86,13 +84,12 @@ class Dashboard extends AbstractWidget } /** - * Create a writer object + * Create and return a Config object for this dashboard * - * @return IniWriter + * @return Config */ - public function createWriter() + public function getConfig() { - $configFile = $this->getConfigFile(); $output = array(); foreach ($this->panes as $pane) { if ($pane->isUserWidget() === true) { @@ -105,17 +102,7 @@ class Dashboard extends AbstractWidget } } - $co = new ConfigObject($output); - $config = new Config($co); - return new IniWriter(array('config' => $config, 'filename' => $configFile)); - } - - /** - * Write user specific dashboards to disk - */ - public function write() - { - $this->createWriter()->write(); + return Config::fromArray($output)->setConfigFile($this->getConfigFile()); } /** diff --git a/modules/monitoring/library/Monitoring/BackendStep.php b/modules/monitoring/library/Monitoring/BackendStep.php index c0dc3d4ea..7f9fc11cf 100644 --- a/modules/monitoring/library/Monitoring/BackendStep.php +++ b/modules/monitoring/library/Monitoring/BackendStep.php @@ -7,7 +7,6 @@ namespace Icinga\Module\Monitoring; use Exception; use Icinga\Module\Setup\Step; use Icinga\Application\Config; -use Icinga\File\Ini\IniWriter; class BackendStep extends Step { @@ -38,11 +37,9 @@ class BackendStep extends Step ); try { - $writer = new IniWriter(array( - 'config' => Config::fromArray($config), - 'filename' => Config::resolvePath('modules/monitoring/backends.ini') - )); - $writer->write(); + Config::fromArray($config) + ->setConfigFile(Config::resolvePath('modules/monitoring/backends.ini')) + ->saveIni(); } catch (Exception $e) { $this->backendIniError = $e; return false; @@ -61,13 +58,7 @@ class BackendStep extends Step try { $config = Config::app('resources', true); $config->setSection($resourceName, $resourceConfig); - - $writer = new IniWriter(array( - 'config' => $config, - 'filename' => Config::resolvePath('resources.ini'), - 'filemode' => 0660 - )); - $writer->write(); + $config->saveIni(); } catch (Exception $e) { $this->resourcesIniError = $e; return false; diff --git a/modules/monitoring/library/Monitoring/InstanceStep.php b/modules/monitoring/library/Monitoring/InstanceStep.php index 2cb8d7d8d..ab54b6548 100644 --- a/modules/monitoring/library/Monitoring/InstanceStep.php +++ b/modules/monitoring/library/Monitoring/InstanceStep.php @@ -7,7 +7,6 @@ namespace Icinga\Module\Monitoring; use Exception; use Icinga\Module\Setup\Step; use Icinga\Application\Config; -use Icinga\File\Ini\IniWriter; class InstanceStep extends Step { @@ -27,11 +26,9 @@ class InstanceStep extends Step unset($instanceConfig['name']); try { - $writer = new IniWriter(array( - 'config' => Config::fromArray(array($instanceName => $instanceConfig)), - 'filename' => Config::resolvePath('modules/monitoring/instances.ini') - )); - $writer->write(); + Config::fromArray(array($instanceName => $instanceConfig)) + ->setConfigFile(Config::resolvePath('modules/monitoring/instances.ini')) + ->saveIni(); } catch (Exception $e) { $this->error = $e; return false; diff --git a/modules/monitoring/library/Monitoring/SecurityStep.php b/modules/monitoring/library/Monitoring/SecurityStep.php index 4a2c0f846..b3eda02c4 100644 --- a/modules/monitoring/library/Monitoring/SecurityStep.php +++ b/modules/monitoring/library/Monitoring/SecurityStep.php @@ -7,7 +7,6 @@ namespace Icinga\Module\Monitoring; use Exception; use Icinga\Module\Setup\Step; use Icinga\Application\Config; -use Icinga\File\Ini\IniWriter; class SecurityStep extends Step { @@ -26,11 +25,9 @@ class SecurityStep extends Step $config['security'] = $this->data['securityConfig']; try { - $writer = new IniWriter(array( - 'config' => Config::fromArray($config), - 'filename' => Config::resolvePath('modules/monitoring/config.ini') - )); - $writer->write(); + Config::fromArray($config) + ->setConfigFile(Config::resolvePath('modules/monitoring/config.ini')) + ->saveIni(); } catch (Exception $e) { $this->error = $e; return false; diff --git a/modules/setup/library/Setup/Steps/AuthenticationStep.php b/modules/setup/library/Setup/Steps/AuthenticationStep.php index ce0b49f58..c06b4e294 100644 --- a/modules/setup/library/Setup/Steps/AuthenticationStep.php +++ b/modules/setup/library/Setup/Steps/AuthenticationStep.php @@ -6,7 +6,6 @@ namespace Icinga\Module\Setup\Steps; use Exception; use Icinga\Application\Config; -use Icinga\File\Ini\IniWriter; use Icinga\Data\ConfigObject; use Icinga\Data\ResourceFactory; use Icinga\Authentication\Backend\DbUserBackend; @@ -50,11 +49,9 @@ class AuthenticationStep extends Step } try { - $writer = new IniWriter(array( - 'config' => Config::fromArray($config), - 'filename' => Config::resolvePath('authentication.ini') - )); - $writer->write(); + Config::fromArray($config) + ->setConfigFile(Config::resolvePath('authentication.ini')) + ->saveIni(); } catch (Exception $e) { $this->authIniError = $e; return false; @@ -73,11 +70,9 @@ class AuthenticationStep extends Step ); try { - $writer = new IniWriter(array( - 'config' => Config::fromArray($config), - 'filename' => Config::resolvePath('permissions.ini') - )); - $writer->write(); + Config::fromArray($config) + ->setConfigFile(Config::resolvePath('permissions.ini')) + ->saveIni(); } catch (Exception $e) { $this->permIniError = $e; return false; diff --git a/modules/setup/library/Setup/Steps/GeneralConfigStep.php b/modules/setup/library/Setup/Steps/GeneralConfigStep.php index 39c160628..bf3d8cb89 100644 --- a/modules/setup/library/Setup/Steps/GeneralConfigStep.php +++ b/modules/setup/library/Setup/Steps/GeneralConfigStep.php @@ -7,7 +7,6 @@ namespace Icinga\Module\Setup\Steps; use Exception; use Icinga\Application\Logger; use Icinga\Application\Config; -use Icinga\File\Ini\IniWriter; use Icinga\Module\Setup\Step; class GeneralConfigStep extends Step @@ -35,11 +34,9 @@ class GeneralConfigStep extends Step } try { - $writer = new IniWriter(array( - 'config' => Config::fromArray($config), - 'filename' => Config::resolvePath('config.ini') - )); - $writer->write(); + Config::fromArray($config) + ->setConfigFile(Config::resolvePath('config.ini')) + ->saveIni(); } catch (Exception $e) { $this->error = $e; return false; diff --git a/modules/setup/library/Setup/Steps/ResourceStep.php b/modules/setup/library/Setup/Steps/ResourceStep.php index c04cdbc79..aae1c3aca 100644 --- a/modules/setup/library/Setup/Steps/ResourceStep.php +++ b/modules/setup/library/Setup/Steps/ResourceStep.php @@ -6,7 +6,6 @@ namespace Icinga\Module\Setup\Steps; use Exception; use Icinga\Application\Config; -use Icinga\File\Ini\IniWriter; use Icinga\Module\Setup\Step; class ResourceStep extends Step @@ -38,12 +37,9 @@ class ResourceStep extends Step } try { - $writer = new IniWriter(array( - 'config' => Config::fromArray($resourceConfig), - 'filename' => Config::resolvePath('resources.ini'), - 'filemode' => 0660 - )); - $writer->write(); + Config::fromArray($resourceConfig) + ->setConfigFile(Config::resolvePath('resources.ini')) + ->saveIni(); } catch (Exception $e) { $this->error = $e; return false;