From 63305fdf9aca81a477f677659062b12136325446 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 09:32:08 +0100 Subject: [PATCH 1/8] 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; From 0108d44d635e628004c6bc5b711ef9ba2f0e6f36 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 09:38:59 +0100 Subject: [PATCH 2/8] Fix "The use of non compound name 'Exception'..." --- application/controllers/DashboardController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/controllers/DashboardController.php b/application/controllers/DashboardController.php index 7c70c9c5a..49a58426f 100644 --- a/application/controllers/DashboardController.php +++ b/application/controllers/DashboardController.php @@ -2,7 +2,7 @@ // {{{ICINGA_LICENSE_HEADER}}} // {{{ICINGA_LICENSE_HEADER}}} -use Exception; +use \Exception; use Icinga\Exception\ProgrammingError; use Icinga\Forms\ConfirmRemovalForm; use Icinga\Forms\Dashboard\DashletForm; From fdcec046e057f0eb4a765459caa71ebd2e3affce Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 15:42:22 +0100 Subject: [PATCH 3/8] Make File::create(.., $recursive = true) create missing nested directories refs #8219 --- library/Icinga/Util/File.php | 72 +++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 6 deletions(-) diff --git a/library/Icinga/Util/File.php b/library/Icinga/Util/File.php index 9ce1ec893..99421b33d 100644 --- a/library/Icinga/Util/File.php +++ b/library/Icinga/Util/File.php @@ -23,6 +23,13 @@ class File extends SplFileObject */ protected $openMode; + /** + * The access mode to use when creating directories + * + * @var int + */ + public static $dirMode = 1528; // 2770 + /** * @see SplFileObject::__construct() */ @@ -37,21 +44,74 @@ class File extends SplFileObject } /** - * Create a file with an access mode + * Create a file using the given access mode and return a instance of File open for writing * * @param string $path The path to the file * @param int $accessMode The access mode to set + * @param bool $recursive Whether missing nested directories of the given path should be created + * + * @return File * * @throws RuntimeException In case the file cannot be created or the access mode cannot be set + * @throws NotWritableError In case the path's (existing) parent is not writable */ - public static function create($path, $accessMode) + public static function create($path, $accessMode, $recursive = true) { - if (!@touch($path)) { - throw new RuntimeException('Cannot create file "' . $path . '" with acces mode "' . $accessMode . '"'); + $dirPath = dirname($path); + if ($recursive && !is_dir($dirPath)) { + static::createDirectories($dirPath); + } elseif (! is_writable($dirPath)) { + throw new NotWritableError(sprintf('Path "%s" is not writable', $dirPath)); } - if (!@chmod($path, $accessMode)) { - throw new RuntimeException('Cannot set access mode "' . $accessMode . '" on file "' . $path . '"'); + $file = new static($path, 'x'); + + if (! @chmod($path, $accessMode)) { + $error = error_get_last(); + throw new RuntimeException(sprintf( + 'Cannot set access mode "%s" on file "%s" (%s)', + decoct($accessMode), + $path, + $error['message'] + )); + } + + return $file; + } + + /** + * Create missing directories + * + * @param string $path + * + * @throws RuntimeException In case a directory cannot be created or the access mode cannot be set + */ + protected static function createDirectories($path) + { + $part = strpos($path, DIRECTORY_SEPARATOR) === 0 ? DIRECTORY_SEPARATOR : ''; + foreach (explode(DIRECTORY_SEPARATOR, ltrim($path, DIRECTORY_SEPARATOR)) as $dir) { + $part .= $dir . DIRECTORY_SEPARATOR; + + if (! is_dir($part)) { + if (! @mkdir($part, static::$dirMode)) { + $error = error_get_last(); + throw new RuntimeException(sprintf( + 'Failed to create missing directory "%s" (%s)', + $part, + $error['message'] + )); + } + + if (! @chmod($part, static::$dirMode)) { + $error = error_get_last(); + throw new RuntimeException(sprintf( + 'Failed to set access mode "%s" for directory "%s" (%s)', + decoct(static::$dirMode), + $part, + $error['message'] + )); + } + } } } From 9426a5bd234ef75eb468cdb98d2468029dd0a57b Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 15:43:02 +0100 Subject: [PATCH 4/8] Use File::create() in Config::saveIni() to create missing nested directories refs #8219 --- library/Icinga/Application/Config.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/Icinga/Application/Config.php b/library/Icinga/Application/Config.php index 8d4914316..484cff6df 100644 --- a/library/Icinga/Application/Config.php +++ b/library/Icinga/Application/Config.php @@ -8,6 +8,7 @@ use Iterator; use Countable; use LogicException; use UnexpectedValueException; +use Icinga\Util\File; use Icinga\Data\ConfigObject; use Icinga\File\Ini\IniWriter; use Icinga\Exception\NotReadableError; @@ -320,6 +321,10 @@ class Config implements Countable, Iterator throw new LogicException('You need to pass $filePath or set a path using Config::setConfigFile()'); } + if (! file_exists($filePath)) { + File::create($filePath, $fileMode); + } + $this->getIniWriter($filePath, $fileMode)->write(); } From 6416fc421c8c4d89edf92d4db5734b9a6131e4ab Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 15:43:39 +0100 Subject: [PATCH 5/8] Do not create directories which are created automatically if necessary refs #8219 --- .../library/Monitoring/MonitoringWizard.php | 4 ---- modules/setup/library/Setup/WebWizard.php | 12 +----------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/modules/monitoring/library/Monitoring/MonitoringWizard.php b/modules/monitoring/library/Monitoring/MonitoringWizard.php index d7a3671a4..f02faa4f3 100644 --- a/modules/monitoring/library/Monitoring/MonitoringWizard.php +++ b/modules/monitoring/library/Monitoring/MonitoringWizard.php @@ -4,14 +4,12 @@ namespace Icinga\Module\Monitoring; -use Icinga\Application\Icinga; use Icinga\Web\Form; use Icinga\Web\Wizard; use Icinga\Web\Request; use Icinga\Module\Setup\Setup; use Icinga\Module\Setup\SetupWizard; use Icinga\Module\Setup\Requirements; -use Icinga\Module\Setup\Utils\MakeDirStep; use Icinga\Module\Setup\Forms\SummaryPage; use Icinga\Module\Monitoring\Forms\Setup\WelcomePage; use Icinga\Module\Monitoring\Forms\Setup\BackendPage; @@ -108,8 +106,6 @@ class MonitoringWizard extends Wizard implements SetupWizard $pageData = $this->getPageData(); $setup = new Setup(); - $setup->addStep(new MakeDirStep(array(Icinga::app()->getConfigDir() . '/modules/monitoring'), 2770)); - $setup->addStep( new BackendStep(array( 'backendConfig' => $pageData['setup_monitoring_backend'], diff --git a/modules/setup/library/Setup/WebWizard.php b/modules/setup/library/Setup/WebWizard.php index 8ae7344ea..c073723a3 100644 --- a/modules/setup/library/Setup/WebWizard.php +++ b/modules/setup/library/Setup/WebWizard.php @@ -334,17 +334,7 @@ class WebWizard extends Wizard implements SetupWizard ); } - $configDir = Icinga::app()->getConfigDir(); - $setup->addStep( - new MakeDirStep( - array( - $configDir . DIRECTORY_SEPARATOR . 'modules', - $configDir . DIRECTORY_SEPARATOR . 'preferences', - $configDir . DIRECTORY_SEPARATOR . 'enabledModules' - ), - 2770 - ) - ); + $setup->addStep(new MakeDirStep(array(Config::resolvePath('enabledModules')), 2770)); foreach ($this->getWizards() as $wizard) { if ($wizard->isComplete()) { From d2dd66c9fd25b63287ae94363415f73007a3c64b Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 15:47:11 +0100 Subject: [PATCH 6/8] Revert "setup: Fix octdec for directory modes" This reverts commit c0444a81b24aea332b07eb1409af168c1c53f0c9. --- modules/setup/library/Setup/Utils/MakeDirStep.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/setup/library/Setup/Utils/MakeDirStep.php b/modules/setup/library/Setup/Utils/MakeDirStep.php index d7813541b..27919820b 100644 --- a/modules/setup/library/Setup/Utils/MakeDirStep.php +++ b/modules/setup/library/Setup/Utils/MakeDirStep.php @@ -21,7 +21,7 @@ class MakeDirStep extends Step public function __construct($paths, $dirmode) { $this->paths = $paths; - $this->dirmode = octdec((string) $dirmode); + $this->dirmode = octdec($dirmode); $this->errors = array(); } From a95fd561cd09c91f094bfd65f58560945863c92b Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 15:47:21 +0100 Subject: [PATCH 7/8] Revert "setup: Convert octal directory mode to decimal notation" This reverts commit e93e8f633045d7fd0f844e797848d4d6fe32ca9e. --- modules/setup/library/Setup/Utils/MakeDirStep.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/setup/library/Setup/Utils/MakeDirStep.php b/modules/setup/library/Setup/Utils/MakeDirStep.php index 27919820b..b758ccd79 100644 --- a/modules/setup/library/Setup/Utils/MakeDirStep.php +++ b/modules/setup/library/Setup/Utils/MakeDirStep.php @@ -16,12 +16,12 @@ class MakeDirStep extends Step /** * @param array $paths - * @param int $dirmode Directory mode in octal notation + * @param int $dirmode */ public function __construct($paths, $dirmode) { $this->paths = $paths; - $this->dirmode = octdec($dirmode); + $this->dirmode = $dirmode; $this->errors = array(); } From f5a651664c2a2085c761aa8fbbff927c11fb310f Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 30 Jan 2015 16:16:12 +0100 Subject: [PATCH 8/8] Create the enabledModules directory when necessary only as well refs #8219 --- .../Icinga/Application/Modules/Manager.php | 46 +++++++++---------- modules/setup/library/Setup/WebWizard.php | 2 - 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/library/Icinga/Application/Modules/Manager.php b/library/Icinga/Application/Modules/Manager.php index 7ee2461f9..0a1b23f78 100644 --- a/library/Icinga/Application/Modules/Manager.php +++ b/library/Icinga/Application/Modules/Manager.php @@ -103,25 +103,22 @@ class Manager */ private function detectEnabledModules() { - $canonical = $this->enableDir; - if ($canonical === false || ! file_exists($canonical)) { - // TODO: I guess the check for false has something to do with a - // call to realpath no longer present + if (! file_exists($this->enableDir)) { return; } - if (!is_dir($this->enableDir)) { + if (! is_dir($this->enableDir)) { throw new NotReadableError( 'Cannot read enabled modules. Module directory "%s" is not a directory', $this->enableDir ); } - if (!is_readable($this->enableDir)) { + if (! is_readable($this->enableDir)) { throw new NotReadableError( 'Cannot read enabled modules. Module directory "%s" is not readable', $this->enableDir ); } - if (($dh = opendir($canonical)) !== false) { + if (($dh = opendir($this->enableDir)) !== false) { $this->enabledDirs = array(); while (($file = readdir($dh)) !== false) { @@ -129,7 +126,7 @@ class Manager continue; } - $link = $this->enableDir . '/' . $file; + $link = $this->enableDir . DIRECTORY_SEPARATOR . $file; if (! is_link($link)) { Logger::warning( 'Found invalid module in enabledModule directory "%s": "%s" is not a symlink', @@ -140,7 +137,7 @@ class Manager } $dir = realpath($link); - if (!file_exists($dir) || !is_dir($dir)) { + if (! file_exists($dir) || !is_dir($dir)) { Logger::warning( 'Found invalid module in enabledModule directory "%s": "%s" points to non existing path "%s"', $this->enableDir, @@ -208,7 +205,7 @@ class Manager */ public function enableModule($name) { - if (!$this->hasInstalled($name)) { + if (! $this->hasInstalled($name)) { throw new ConfigurationError( 'Cannot enable module "%s". Module is not installed.', $name @@ -217,11 +214,16 @@ class Manager clearstatcache(true); $target = $this->installedBaseDirs[$name]; - $link = $this->enableDir . '/' . $name; + $link = $this->enableDir . DIRECTORY_SEPARATOR . $name; - if (! is_dir($this->enableDir)) { - throw new NotFoundError('Cannot enable module "%s". Path "%s" not found.', $name, $this->enableDir); - } elseif (!is_writable($this->enableDir)) { + if (! is_dir($this->enableDir) && !@mkdir($this->enableDir, 02770, true)) { + $error = error_get_last(); + throw new SystemPermissionException( + 'Failed to create enabledModule directory "%s" (%s)', + $this->enableDir, + $error['message'] + ); + } elseif (! is_writable($this->enableDir)) { throw new SystemPermissionException( 'Cannot enable module "%s". Insufficient system permissions for enabling modules.', $name @@ -232,7 +234,7 @@ class Manager return $this; } - if (!@symlink($target, $link)) { + if (! @symlink($target, $link)) { $error = error_get_last(); if (strstr($error["message"], "File exists") === false) { throw new SystemPermissionException( @@ -246,9 +248,7 @@ class Manager } $this->enabledDirs[$name] = $link; - $this->loadModule($name); - return $this; } @@ -264,22 +264,22 @@ class Manager */ public function disableModule($name) { - if (!$this->hasEnabled($name)) { + if (! $this->hasEnabled($name)) { return $this; } - if (!is_writable($this->enableDir)) { + if (! is_writable($this->enableDir)) { throw new SystemPermissionException( 'Could not disable module. Module path is not writable.' ); } - $link = $this->enableDir . '/' . $name; - if (!file_exists($link)) { + $link = $this->enableDir . DIRECTORY_SEPARATOR . $name; + if (! file_exists($link)) { throw new ConfigurationError( 'Could not disable module. The module %s was not found.', $name ); } - if (!is_link($link)) { + if (! is_link($link)) { throw new ConfigurationError( 'Could not disable module. The module "%s" is not a symlink. ' . 'It looks like you have installed this module manually and moved it to your module folder. ' @@ -290,7 +290,7 @@ class Manager } if (file_exists($link) && is_link($link)) { - if (!@unlink($link)) { + if (! @unlink($link)) { $error = error_get_last(); throw new SystemPermissionException( 'Could not disable module "%s" due to file system errors. ' diff --git a/modules/setup/library/Setup/WebWizard.php b/modules/setup/library/Setup/WebWizard.php index c073723a3..b580101ed 100644 --- a/modules/setup/library/Setup/WebWizard.php +++ b/modules/setup/library/Setup/WebWizard.php @@ -334,8 +334,6 @@ class WebWizard extends Wizard implements SetupWizard ); } - $setup->addStep(new MakeDirStep(array(Config::resolvePath('enabledModules')), 2770)); - foreach ($this->getWizards() as $wizard) { if ($wizard->isComplete()) { $setup->addSteps($wizard->getSetup()->getSteps());