From 5f7652133e4479405294ca1d9ba1ab209d39aff7 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 25 Jul 2014 08:39:27 +0200 Subject: [PATCH 1/6] Fix code-style and documentation --- .../controllers/ConfigController.php | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php index c06ded57f..a6b3ab6d4 100644 --- a/modules/monitoring/application/controllers/ConfigController.php +++ b/modules/monitoring/application/controllers/ConfigController.php @@ -2,19 +2,15 @@ // {{{ICINGA_LICENSE_HEADER}}} // {{{ICINGA_LICENSE_HEADER}}} -use \Exception; - +use Exception; use Icinga\Config\PreservingIniWriter; use Icinga\Web\Controller\ModuleActionController; use Icinga\Web\Notification; -use Icinga\Web\Url; - use Icinga\Module\Monitoring\Form\Config\ConfirmRemovalForm; use Icinga\Module\Monitoring\Form\Config\Backend\EditBackendForm; use Icinga\Module\Monitoring\Form\Config\Backend\CreateBackendForm; use Icinga\Module\Monitoring\Form\Config\Instance\EditInstanceForm; use Icinga\Module\Monitoring\Form\Config\Instance\CreateInstanceForm; - use Icinga\Exception\NotReadableError; /** @@ -22,7 +18,6 @@ use Icinga\Exception\NotReadableError; */ class Monitoring_ConfigController extends ModuleActionController { - /** * Display a list of available backends and instances */ @@ -74,7 +69,7 @@ class Monitoring_ConfigController extends ModuleActionController } /** - * Display a form to create a new backends + * Display a form to create a new backend */ public function createbackendAction() { @@ -128,7 +123,7 @@ class Monitoring_ConfigController extends ModuleActionController } /** - * Display a form to remove the instance identified by the 'instance' parameter + * Display a confirmation form to remove the instance identified by the 'instance' parameter */ public function removeinstanceAction() { @@ -214,9 +209,14 @@ class Monitoring_ConfigController extends ModuleActionController } /** - * Display a form to remove the instance identified by the 'instance' parameter + * Write configuration to an ini file + * + * @param Zend_Config $config The configuration to write + * @param string $file The config file to write to + * + * @return bool Whether the configuration was written or not */ - private function writeConfiguration($config, $file) + protected function writeConfiguration($config, $file) { $target = $this->Config($file)->getConfigFile(); $writer = new PreservingIniWriter(array('filename' => $target, 'config' => $config)); @@ -234,26 +234,26 @@ class Monitoring_ConfigController extends ModuleActionController } /** - * Return true if the backend exists in the current configuration + * Return whether the given backend exists in the current configuration * - * @param string $backend The name of the backend to check for existence + * @param string $backend The name of the backend to check * - * @return bool True if the backend name exists, otherwise false + * @return bool Whether the backend exists or not */ - private function isExistingBackend($backend) + protected function isExistingBackend($backend) { $backendCfg = $this->Config('backends'); return $backend && $backendCfg->get($backend); } /** - * Return true if the instance exists in the current configuration + * Return whether the given instance exists in the current configuration * - * @param string $instance The name of the instance to check for existence + * @param string $instance The name of the instance to check * - * @return bool True if the instance name exists, otherwise false + * @return bool Whether the instance exists or not */ - private function isExistingInstance($instance) + protected function isExistingInstance($instance) { $instanceCfg = $this->Config('instances'); return $instanceCfg && $instanceCfg->get($instance); From a6bd802e3e565b2b9f873c12ebeb5eacbe08f8a4 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 25 Jul 2014 08:58:11 +0200 Subject: [PATCH 2/6] Remove unnecessary properties and its getters/setters refs #5525 --- .../forms/Config/Backend/EditBackendForm.php | 66 ------------------- 1 file changed, 66 deletions(-) diff --git a/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php b/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php index 52e420a76..93d306d72 100644 --- a/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php +++ b/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php @@ -14,72 +14,6 @@ use Icinga\Data\ResourceFactory; */ class EditBackendForm extends Form { - /** - * Database resources to use instead of the one's from ResourceFactory (used for testing) - * - * @var array - */ - protected $resources; - - /** - * The Backend configuration to use for populating the form - * - * @var Zend_Config - */ - protected $backend; - - /** - * Set the configuration to be used for initial population of the form - * - * @param Zend_Form $config - */ - public function setBackendConfiguration($config) - { - $this->backend = $config; - } - - /** - * Set a custom array of resources to be used in this form instead of the ones from ResourceFactory - * (used for testing) - */ - public function setResources($resources) - { - $this->resources = $resources; - } - - /** - * Return content of the resources.ini or previously set resources for displaying in the database selection field - * - * @return array - */ - public function getResources() - { - if ($this->resources === null) { - return ResourceFactory::getResourceConfigs()->toArray(); - } else { - return $this->resources; - } - } - - /** - * Return a list of all resources of the given type ready to be used as content for a select input - * - * @param string $type The type of resources to return - * - * @return array - */ - protected function getResourcesByType($type) - { - $backends = array(); - foreach ($this->getResources() as $name => $resource) { - if ($resource['type'] === $type) { - $backends[$name] = $name; - } - } - - return $backends; - } - /** * Create this form * From 26d42da6352f750c0753780ef13b3473333179b1 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 25 Jul 2014 08:58:50 +0200 Subject: [PATCH 3/6] Re-add getResourcesByType as simplified version refs #5525 --- .../forms/Config/Backend/EditBackendForm.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php b/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php index 93d306d72..b66383f73 100644 --- a/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php +++ b/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php @@ -79,4 +79,21 @@ class EditBackendForm extends Form ) ); } + + /** + * Return a list of all resources of the given type ready to be used as content for a select input + * + * @param string $type The type of resources to return + * + * @return array + */ + protected function getResourcesByType($type) + { + $backends = array(); + foreach (array_keys(ResourceFactory::getResourceConfigs($type)->toArray()) as $name) { + $backends[$name] = $name; + } + + return $backends; + } } From af5a3262be89a58dfd3fba0a4f2a24bc7977bd5f Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 25 Jul 2014 09:12:12 +0200 Subject: [PATCH 4/6] Revert backslash removal in front of namespace-less use statement --- modules/monitoring/application/controllers/ConfigController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php index a6b3ab6d4..9e74943f3 100644 --- a/modules/monitoring/application/controllers/ConfigController.php +++ b/modules/monitoring/application/controllers/ConfigController.php @@ -2,7 +2,7 @@ // {{{ICINGA_LICENSE_HEADER}}} // {{{ICINGA_LICENSE_HEADER}}} -use Exception; +use \Exception; use Icinga\Config\PreservingIniWriter; use Icinga\Web\Controller\ModuleActionController; use Icinga\Web\Notification; From 488ea4633d232dfc54acbadeb2fd8469a50257fa Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 25 Jul 2014 14:50:53 +0200 Subject: [PATCH 5/6] Merge EditBackendForm with CreateBackendForm and rename it It is not necessary to distinct between whether a backend is created or edited as a user perhaps likes to change the name of a backend directly instead of the need to remove and recreate it. refs #5525 --- .../Config/Backend/CreateBackendForm.php | 45 ------ .../forms/Config/Backend/EditBackendForm.php | 99 ------------- .../application/forms/Config/BackendForm.php | 134 ++++++++++++++++++ 3 files changed, 134 insertions(+), 144 deletions(-) delete mode 100644 modules/monitoring/application/forms/Config/Backend/CreateBackendForm.php delete mode 100644 modules/monitoring/application/forms/Config/Backend/EditBackendForm.php create mode 100644 modules/monitoring/application/forms/Config/BackendForm.php diff --git a/modules/monitoring/application/forms/Config/Backend/CreateBackendForm.php b/modules/monitoring/application/forms/Config/Backend/CreateBackendForm.php deleted file mode 100644 index 513dea238..000000000 --- a/modules/monitoring/application/forms/Config/Backend/CreateBackendForm.php +++ /dev/null @@ -1,45 +0,0 @@ -setBackendConfiguration(new Zend_Config(array('type' => 'ido'))); - $this->addElement( - 'text', - 'backend_name', - array( - 'label' => 'Backend Name', - 'required' => true, - 'helptext' => 'This will be the identifier of this backend' - ) - ); - parent::create(); - } - - /** - * Return the name of the backend that is to be created - * - * @return string The name of the backend as entered in the form - */ - public function getBackendName() - { - return $this->getValue('backend_name'); - } -} diff --git a/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php b/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php deleted file mode 100644 index b66383f73..000000000 --- a/modules/monitoring/application/forms/Config/Backend/EditBackendForm.php +++ /dev/null @@ -1,99 +0,0 @@ -getRequest()->getParam('backend_type', $this->backend->type); - - $this->addElement( - 'select', - 'backend_type', - array( - 'label' => 'Backend Type', - 'value' => $this->backend->type, - 'required' => true, - 'helptext' => 'The data source used for retrieving monitoring information', - 'multiOptions' => array( - 'ido' => 'IDO Backend', - 'statusdat' => 'Status.dat', - 'livestatus' => 'Livestatus' - ) - ) - ); - $this->addElement( - 'select', - 'backend_resource', - array( - 'label' => 'Resource', - 'value' => $this->backend->resource, - 'required' => true, - 'multiOptions' => $this->getResourcesByType($backendType === 'ido' ? 'db' : $backendType), - 'helptext' => 'The resource to use' - ) - ); - $this->addElement( - 'checkbox', - 'backend_disable', - array( - 'label' => 'Disable This Backend', - 'required' => true, - 'value' => $this->backend->disabled - ) - ); - - $this->enableAutoSubmit(array('backend_type')); - $this->setSubmitLabel('{{SAVE_ICON}} Save Changes'); - } - - /** - * Return a configuration containing the backend settings entered in this form - * - * @return Zend_Config The updated configuration for this backend - */ - public function getConfig() - { - $values = $this->getValues(); - return new Zend_Config( - array( - 'type' => $values['backend_type'], - 'disabled' => $values['backend_disable'], - 'resource' => $values['backend_resource'] - ) - ); - } - - /** - * Return a list of all resources of the given type ready to be used as content for a select input - * - * @param string $type The type of resources to return - * - * @return array - */ - protected function getResourcesByType($type) - { - $backends = array(); - foreach (array_keys(ResourceFactory::getResourceConfigs($type)->toArray()) as $name) { - $backends[$name] = $name; - } - - return $backends; - } -} diff --git a/modules/monitoring/application/forms/Config/BackendForm.php b/modules/monitoring/application/forms/Config/BackendForm.php new file mode 100644 index 000000000..19f870778 --- /dev/null +++ b/modules/monitoring/application/forms/Config/BackendForm.php @@ -0,0 +1,134 @@ +createElement( + 'text', + 'name', + array( + 'required' => true, + 'label' => t('Backend Name'), + 'helptext' => t('The identifier of this backend') + ) + ), + $this->createElement( + 'select', + 'type', + array( + 'required' => true, + 'class' => 'autosubmit', + 'label' => t('Backend Type'), + 'helptext' => t('The data source used for retrieving monitoring information'), + 'multiOptions' => array( + 'ido' => 'IDO Backend', + 'statusdat' => 'Status.dat', + 'livestatus' => 'Livestatus' + ) + ) + ), + $this->createElement( + 'select', + 'resource', + array( + 'required' => true, + 'label' => t('Resource'), + 'helptext' => t('The resource to use'), + 'multiOptions' => $this->getResourcesByType( + false === isset($formData['type']) || $formData['type'] === 'ido' ? 'db' : $formData['type'] + ) + ) + ), + $this->createElement( + 'checkbox', + 'disabled', + array( + 'required' => true, + 'label' => t('Disable This Backend') + ) + ) + ); + } + + /** + * @see Form::addSubmitButton() + */ + public function addSubmitButton() + { + $this->addElement( + 'submit', + 'btn_submit', + array( + 'label' => t('Save Changes') + ) + ); + + return $this; + } + + /** + * Return the backend configuration values and its name + * + * The first value is the name and the second one the values as array. + * + * @return array + */ + public function getBackendConfig() + { + $values = $this->getValues(); + $name = $values['name']; + + if ($values['disabled'] == '0') { + unset($values['disabled']); + } + + unset($values['name']); + unset($values['btn_submit']); + unset($values[$this->getTokenElementName()]); + return array($name, $values); + } + + /** + * Populate the form with the given configuration values + * + * @param string $name The name of the backend + * @param array $config The configuration values + */ + public function setBackendConfig($name, array $config) + { + $config['name'] = $name; + $this->populate($config); + } + + /** + * Return a list of all resources of the given type ready to be used as content for a select input + * + * @param string $type The type of resources to return + * + * @return array + */ + protected function getResourcesByType($type) + { + $backends = array(); + foreach (array_keys(ResourceFactory::getResourceConfigs($type)->toArray()) as $name) { + $backends[$name] = $name; + } + + return $backends; + } +} From b743cf814319d2db301a028b41cf9557bc93e26b Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 25 Jul 2014 14:55:36 +0200 Subject: [PATCH 6/6] Adjust editbackend-action to suit the new backend-form interface refs #5525 --- .../controllers/ConfigController.php | 54 ++++++++++++------- .../views/scripts/config/editbackend.phtml | 15 +----- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php index 9e74943f3..2e01709e6 100644 --- a/modules/monitoring/application/controllers/ConfigController.php +++ b/modules/monitoring/application/controllers/ConfigController.php @@ -3,12 +3,12 @@ // {{{ICINGA_LICENSE_HEADER}}} use \Exception; +use \Zend_Config; use Icinga\Config\PreservingIniWriter; use Icinga\Web\Controller\ModuleActionController; use Icinga\Web\Notification; +use Icinga\Module\Monitoring\Form\Config\BackendForm; use Icinga\Module\Monitoring\Form\Config\ConfirmRemovalForm; -use Icinga\Module\Monitoring\Form\Config\Backend\EditBackendForm; -use Icinga\Module\Monitoring\Form\Config\Backend\CreateBackendForm; use Icinga\Module\Monitoring\Form\Config\Instance\EditInstanceForm; use Icinga\Module\Monitoring\Form\Config\Instance\CreateInstanceForm; use Icinga\Exception\NotReadableError; @@ -43,29 +43,40 @@ class Monitoring_ConfigController extends ModuleActionController */ public function editbackendAction() { + // Fetch the backend to be edited $backend = $this->getParam('backend'); - if (!$this->isExistingBackend($backend)) { - $this->view->error = 'Unknown backend ' . $backend; - return; + $backendsConfig = $this->Config('backends')->toArray(); + if (false === array_key_exists($backend, $backendsConfig)) { + // TODO: Should behave as in the app's config controller (Specific redirect to an error action) + Notification::error(sprintf($this->translate('Cannot edit "%s". Backend not found.'), $backend)); + $this->redirectNow('monitoring/config'); } - $backendForm = new EditBackendForm(); - $backendForm->setRequest($this->getRequest()); - $backendForm->setBackendConfiguration($this->Config('backends')->get($backend)); - if ($backendForm->isSubmittedAndValid()) { - $newConfig = $backendForm->getConfig(); - $config = $this->Config('backends'); - $config->$backend = $newConfig; - if ($this->writeConfiguration($config, 'backends')) { - Notification::success('Backend ' . $backend . ' Modified.'); - $this->redirectNow('monitoring/config'); - } else { - $this->render('show-configuration'); - return; + $form = new BackendForm(); + $request = $this->getRequest(); + if ($request->isPost()) { + if ($form->isValid($request->getPost())) { + list($newName, $config) = $form->getBackendConfig(); + + if ($newName !== $backend) { + // Backend name has changed + unset($backendsConfig[$backend]); // We can safely use unset as all values are part of the form + } + + $backendsConfig[$newName] = $config; + if ($this->writeConfiguration($backendsConfig, 'backends')) { + Notification::success(sprintf($this->translate('Backend "%s" successfully modified.'), $backend)); + $this->redirectNow('monitoring/config'); + } else { + $this->render('show-configuration'); + return; + } } + } else { + $form->setBackendConfig($backend, $backendsConfig[$backend]); } - $this->view->name = $backend; - $this->view->form = $backendForm; + + $this->view->form = $form; } /** @@ -218,6 +229,9 @@ class Monitoring_ConfigController extends ModuleActionController */ protected function writeConfiguration($config, $file) { + if (is_array($config)) { + $config = new Zend_Config($config); + } $target = $this->Config($file)->getConfigFile(); $writer = new PreservingIniWriter(array('filename' => $target, 'config' => $config)); diff --git a/modules/monitoring/application/views/scripts/config/editbackend.phtml b/modules/monitoring/application/views/scripts/config/editbackend.phtml index 6f2f19cbb..58b6c18f1 100644 --- a/modules/monitoring/application/views/scripts/config/editbackend.phtml +++ b/modules/monitoring/application/views/scripts/config/editbackend.phtml @@ -1,13 +1,2 @@ -name): ?> -

Edit Backend "escape($this->name) ?>"

- -

Create New Backend

- - -error): ?> -
- escape($this->error); ?> -
- - -form; ?> \ No newline at end of file +

translate('Edit Existing Backend'); ?>

+ \ No newline at end of file