From 766ff8ed839affcf0bb8a7dbb0f604ce28b9095d Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 29 Jun 2015 12:01:43 +0200 Subject: [PATCH] InstanceConfigForm: Adjust how to process requests fixes #7486 fixes #7488 fixes #7489 fixes #7490 refs #9516 --- .../controllers/ConfigController.php | 119 +++++--- .../forms/Config/InstanceConfigForm.php | 255 ++++++++++-------- .../application/forms/Setup/InstancePage.php | 4 - .../views/scripts/config/createinstance.phtml | 6 - .../views/scripts/config/editinstance.phtml | 6 - .../views/scripts/config/removeinstance.phtml | 6 - 6 files changed, 215 insertions(+), 181 deletions(-) delete mode 100644 modules/monitoring/application/views/scripts/config/createinstance.phtml delete mode 100644 modules/monitoring/application/views/scripts/config/editinstance.phtml delete mode 100644 modules/monitoring/application/views/scripts/config/removeinstance.phtml diff --git a/modules/monitoring/application/controllers/ConfigController.php b/modules/monitoring/application/controllers/ConfigController.php index 33bf142f3e..5187a56919 100644 --- a/modules/monitoring/application/controllers/ConfigController.php +++ b/modules/monitoring/application/controllers/ConfigController.php @@ -91,82 +91,113 @@ public function removebackendAction() } /** - * Display a confirmation form to remove the instance identified by the 'instance' parameter + * Remove a monitoring instance */ public function removeinstanceAction() { - $config = $this->Config('instances'); - $form = new ConfirmRemovalForm(array( - 'onSuccess' => function ($form) use ($config) { - $instanceName = $form->getRequest()->getQuery('instance'); - $configForm = new InstanceConfigForm(); - $configForm->setIniConfig($config); - - try { - $configForm->remove($instanceName); - } catch (InvalidArgumentException $e) { - Notification::error($e->getMessage()); - return; - } + $instanceName = $this->params->getRequired('instance'); - if ($configForm->save()) { - Notification::success(sprintf( - $this->translate('Instance "%s" successfully removed.'), - $instanceName - )); - } else { - return false; - } - } - )); - $form->setTitle($this->translate('Remove Existing Instance')); + $instanceForm = new InstanceConfigForm(); + $instanceForm->setIniConfig($this->Config('instances')); + $form = new ConfirmRemovalForm(); + $form->setRedirectUrl('monitoring/config'); + $form->setTitle(sprintf($this->translate('Remove Monitoring Instance %s'), $instanceName)); $form->addDescription($this->translate( 'If you have still any environments or views referring to this instance, ' . 'you won\'t be able to send commands anymore after deletion.' )); - $form->addElement( - 'note', - 'question', - array( - 'value' => $this->translate('Are you sure you want to remove this instance?'), - 'decorators' => array( - 'ViewHelper', - array('HtmlTag', array('tag' => 'p')) - ) - ) - ); - $form->setRedirectUrl('monitoring/config'); + $form->setOnSuccess(function (ConfirmRemovalForm $form) use ($instanceName, $instanceForm) { + try { + $instanceForm->delete($instanceName); + } catch (Exception $e) { + $form->error($e->getMessage()); + return false; + } + + if ($instanceForm->save()) { + Notification::success(sprintf(t('Monitoring instance "%s" successfully removed'), $instanceName)); + return true; + } + + return false; + }); $form->handleRequest(); $this->view->form = $form; + $this->render('form'); } /** - * Display a form to edit the instance identified by the 'instance' parameter of the request + * Edit a monitoring instance */ public function editinstanceAction() { + $instanceName = $this->params->getRequired('instance'); + $form = new InstanceConfigForm(); - $form->setTitle($this->translate('Edit Existing Instance')); - $form->setIniConfig($this->Config('instances')); $form->setRedirectUrl('monitoring/config'); - $form->handleRequest(); + $form->setTitle(sprintf($this->translate('Edit Monitoring Instance %s'), $instanceName)); + $form->setIniConfig($this->Config('instances')); + $form->setOnSuccess(function (InstanceConfigForm $form) use ($instanceName) { + try { + $form->edit($instanceName, array_map( + function ($v) { + return $v !== '' ? $v : null; + }, + $form->getValues() + )); + } catch (Exception $e) { + $form->error($e->getMessage()); + return false; + } + + if ($form->save()) { + Notification::success(sprintf(t('Monitoring instance "%s" successfully updated'), $instanceName)); + return true; + } + + return false; + }); + + try { + $form->load($instanceName); + $form->handleRequest(); + } catch (NotFoundError $_) { + $this->httpNotFound(sprintf($this->translate('Monitoring instance "%s" not found'), $instanceName)); + } $this->view->form = $form; + $this->render('form'); } /** - * Display a form to create a new instance + * Create a new monitoring instance */ public function createinstanceAction() { $form = new InstanceConfigForm(); - $form->setTitle($this->translate('Add New Instance')); - $form->setIniConfig($this->Config('instances')); $form->setRedirectUrl('monitoring/config'); + $form->setTitle($this->translate('Create New Monitoring Instance')); + $form->setIniConfig($this->Config('instances')); + $form->setOnSuccess(function (InstanceConfigForm $form) { + try { + $form->add(array_filter($form->getValues())); + } catch (Exception $e) { + $form->error($e->getMessage()); + return false; + } + + if ($form->save()) { + Notification::success(t('Monitoring instance successfully created')); + return true; + } + + return false; + }); $form->handleRequest(); $this->view->form = $form; + $this->render('form'); } /** diff --git a/modules/monitoring/application/forms/Config/InstanceConfigForm.php b/modules/monitoring/application/forms/Config/InstanceConfigForm.php index c44e480ac7..6522797227 100644 --- a/modules/monitoring/application/forms/Config/InstanceConfigForm.php +++ b/modules/monitoring/application/forms/Config/InstanceConfigForm.php @@ -4,22 +4,28 @@ namespace Icinga\Module\Monitoring\Forms\Config; use InvalidArgumentException; -use Icinga\Exception\ConfigurationError; +use Icinga\Exception\IcingaException; +use Icinga\Exception\NotFoundError; use Icinga\Forms\ConfigForm; use Icinga\Module\Monitoring\Command\Transport\LocalCommandFile; use Icinga\Module\Monitoring\Command\Transport\RemoteCommandFile; use Icinga\Module\Monitoring\Forms\Config\Instance\LocalInstanceForm; use Icinga\Module\Monitoring\Forms\Config\Instance\RemoteInstanceForm; -use Icinga\Web\Notification; /** - * Form for modifying/creating monitoring instances + * Form for managing monitoring instances */ class InstanceConfigForm extends ConfigForm { /** - * (non-PHPDoc) - * @see Form::init() For the method documentation. + * The instance to load when displaying the form for the first time + * + * @var string + */ + protected $instanceToLoad; + + /** + * Initialize this form */ public function init() { @@ -28,11 +34,11 @@ public function init() } /** - * Get a form object for the given instance type + * Return a form object for the given instance type * - * @param string $type The instance type for which to return a form + * @param string $type The instance type for which to return a form * - * @return LocalInstanceForm|RemoteInstanceForm + * @return Form * * @throws InvalidArgumentException In case the given instance type is invalid */ @@ -40,162 +46,157 @@ public function getInstanceForm($type) { switch (strtolower($type)) { case LocalCommandFile::TRANSPORT: - $form = new LocalInstanceForm(); - break; + return new LocalInstanceForm(); case RemoteCommandFile::TRANSPORT; - $form = new RemoteInstanceForm(); - break; + return new RemoteInstanceForm(); default: throw new InvalidArgumentException( - sprintf($this->translate('Invalid instance type "%s" given'), $type) + sprintf($this->translate('Invalid monitoring instance type "%s" given'), $type) ); } - return $form; } /** - * Add a new instance - * - * The resource to add is identified by the array-key `name'. + * Populate the form with the given instance's config * - * @param array $values The values to extend the configuration with + * @param string $name * * @return $this * - * @throws InvalidArgumentException In case the resource already exists + * @throws NotFoundError In case no instance with the given name is found */ - public function add(array $values) + public function load($name) { - $name = isset($values['name']) ? $values['name'] : ''; - if (! $name) { - throw new InvalidArgumentException($this->translate('Instance name missing')); - } - if ($this->config->hasSection($name)) { - throw new InvalidArgumentException($this->translate('Instance already exists')); + if (! $this->config->hasSection($name)) { + throw new NotFoundError('No monitoring instance called "%s" found', $name); } - unset($values['name']); - $this->config->setSection($name, $values); + $this->instanceToLoad = $name; return $this; } /** - * Edit an existing instance + * Add a new instance + * + * The instance to add is identified by the array-key `name'. * - * @param string $name The name of the resource to edit - * @param array $values The values to edit the configuration with + * @param array $data * - * @return array The edited resource configuration + * @return $this * - * @throws InvalidArgumentException In case the resource name is missing or invalid + * @throws InvalidArgumentException In case $data does not contain a instance name + * @throws IcingaException In case a instance with the same name already exists */ - public function edit($name, array $values) + public function add(array $data) { - if (! $name) { - throw new InvalidArgumentException($this->translate('Old instance name missing')); - } elseif (! ($newName = isset($values['name']) ? $values['name'] : '')) { - throw new InvalidArgumentException($this->translate('New instance name missing')); - } elseif (! $this->config->hasSection($name)) { - throw new InvalidArgumentException($this->translate('Unknown instance name provided')); + if (! isset($data['name'])) { + throw new InvalidArgumentException('Key \'name\' missing'); + } + + $instanceName = $data['name']; + if ($this->config->hasSection($instanceName)) { + throw new IcingaException('A monitoring instance with the name "%s" does already exist', $instanceName); } - unset($values['name']); - $this->config->setSection($name, $values); - return $this->config->getSection($name); + unset($data['name']); + $this->config->setSection($instanceName, $data); + return $this; } /** - * Remove a instance + * Edit an existing instance * - * @param string $name The name of the resource to remove + * @param string $name + * @param array $data * - * @return array The removed resource configuration + * @return $this * - * @throws InvalidArgumentException In case the resource name is missing or invalid + * @throws NotFoundError In case no instance with the given name is found */ - public function remove($name) + public function edit($name, array $data) { - if (! $name) { - throw new InvalidArgumentException($this->translate('Instance name missing')); - } elseif (! $this->config->hasSection($name)) { - throw new InvalidArgumentException($this->translate('Unknown instance name provided')); + if (! $this->config->hasSection($name)) { + throw new NotFoundError('No monitoring instance called "%s" found', $name); } $instanceConfig = $this->config->getSection($name); - $this->config->removeSection($name); - return $instanceConfig; - } - - /** - * @see Form::onRequest() For the method documentation. - * @throws ConfigurationError In case the instance name is missing or invalid - */ - public function onRequest() - { - $instanceName = $this->request->getQuery('instance'); - if ($instanceName !== null) { - if (! $instanceName) { - throw new ConfigurationError($this->translate('Instance name missing')); - } - if (! $this->config->hasSection($instanceName)) { - throw new ConfigurationError($this->translate('Unknown instance name given')); + if (isset($data['name'])) { + if ($data['name'] !== $name) { + $this->config->removeSection($name); + $name = $data['name']; } - $instanceConfig = $this->config->getSection($instanceName)->toArray(); - $instanceConfig['name'] = $instanceName; + unset($data['name']); + } - if (isset($instanceConfig['resource'])) { - $instanceConfig['use_resource'] = true; + $instanceConfig->merge($data); + foreach ($instanceConfig->toArray() as $k => $v) { + if ($v === null) { + unset($instanceConfig->$k); } - - $this->populate($instanceConfig); } + + $this->config->setSection($name, $instanceConfig); + return $this; } /** - * (non-PHPDoc) - * @see Form::onSuccess() For the method documentation. + * Remove a instance + * + * @param string $name + * + * @return $this */ - public function onSuccess() + public function delete($name) { - $instanceName = $this->request->getQuery('instance'); - try { - if ($instanceName === null) { // create new instance - $this->add($this->getValues()); - $message = $this->translate('Instance "%s" created successfully.'); - } else { // edit existing instance - $this->edit($instanceName, $this->getValues()); - $message = $this->translate('Instance "%s" edited successfully.'); - } - } catch (InvalidArgumentException $e) { - Notification::error($e->getMessage()); - return; - } - - if ($this->save()) { - Notification::success(sprintf($message, $this->getElement('name')->getValue())); - } else { - return false; - } + $this->config->removeSection($name); + return $this; } /** - * (non-PHPDoc) - * @see Form::createElements() For the method documentation. + * Create and add elements to this form + * + * @param array $formData */ - public function createElements(array $formData = array()) + public function createElements(array $formData) { - $instanceType = isset($formData['transport']) ? $formData['transport'] : LocalCommandFile::TRANSPORT; - - $this->addElements(array( + $this->addElement( + 'text', + 'name', array( - 'text', - 'name', - array( - 'required' => true, - 'label' => $this->translate('Instance Name') + 'required' => true, + 'label' => $this->translate('Instance Name'), + 'description' => $this->translate( + 'The name of this monitoring instance that is used to differentiate it from others' + ), + 'validators' => array( + array( + 'Regex', + false, + array( + 'pattern' => '/^[^\\[\\]:]+$/', + 'messages' => array( + 'regexNotMatch' => $this->translate( + 'The name cannot contain \'[\', \']\' or \':\'.' + ) + ) + ) + ) ) - ), + ) + ); + + $instanceTypes = array( + LocalCommandFile::TRANSPORT => $this->translate('Local Command File'), + RemoteCommandFile::TRANSPORT => $this->translate('Remote Command File') + ); + + $instanceType = isset($formData['transport']) ? $formData['transport'] : null; + if ($instanceType === null) { + $instanceType = key($instanceTypes); + } + + $this->addElements(array( array( 'select', 'transport', @@ -203,15 +204,39 @@ public function createElements(array $formData = array()) 'required' => true, 'autosubmit' => true, 'label' => $this->translate('Instance Type'), - 'multiOptions' => array( - LocalCommandFile::TRANSPORT => $this->translate('Local Command File'), - RemoteCommandFile::TRANSPORT => $this->translate('Remote Command File') - ), - 'value' => $instanceType + 'description' => $this->translate('The type of transport to use for this monitoring instance'), + 'multiOptions' => $instanceTypes ) ) )); - $this->addElements($this->getInstanceForm($instanceType)->createElements($formData)->getElements()); + $this->addSubForm($this->getInstanceForm($instanceType)->create($formData), 'instance_form'); + } + + /** + * Populate the configuration of the instance to load + */ + public function onRequest() + { + if ($this->instanceToLoad) { + $data = $this->config->getSection($this->instanceToLoad)->toArray(); + $data['name'] = $this->instanceToLoad; + $this->populate($data); + } + } + + /** + * Retrieve all form element values + * + * @param bool $suppressArrayNotation Ignored + * + * @return array + */ + public function getValues($suppressArrayNotation = false) + { + $values = parent::getValues(); + $values = array_merge($values, $values['instance_form']); + unset($values['instance_form']); + return $values; } } diff --git a/modules/monitoring/application/forms/Setup/InstancePage.php b/modules/monitoring/application/forms/Setup/InstancePage.php index 25525ffaf7..4dd6a5a093 100644 --- a/modules/monitoring/application/forms/Setup/InstancePage.php +++ b/modules/monitoring/application/forms/Setup/InstancePage.php @@ -19,10 +19,6 @@ public function init() public function createElements(array $formData) { - if (isset($formData['host'])) { - $formData['type'] = 'remote'; // This is necessary as the type element gets ignored by Form::getValues() - } - $instanceConfigForm = new InstanceConfigForm(); $instanceConfigForm->createElements($formData); $this->addElements($instanceConfigForm->getElements()); diff --git a/modules/monitoring/application/views/scripts/config/createinstance.phtml b/modules/monitoring/application/views/scripts/config/createinstance.phtml deleted file mode 100644 index d2c7b6bdd9..0000000000 --- a/modules/monitoring/application/views/scripts/config/createinstance.phtml +++ /dev/null @@ -1,6 +0,0 @@ -
- showOnlyCloseButton() ?> -
-
- -
\ No newline at end of file diff --git a/modules/monitoring/application/views/scripts/config/editinstance.phtml b/modules/monitoring/application/views/scripts/config/editinstance.phtml deleted file mode 100644 index d2c7b6bdd9..0000000000 --- a/modules/monitoring/application/views/scripts/config/editinstance.phtml +++ /dev/null @@ -1,6 +0,0 @@ -
- showOnlyCloseButton() ?> -
-
- -
\ No newline at end of file diff --git a/modules/monitoring/application/views/scripts/config/removeinstance.phtml b/modules/monitoring/application/views/scripts/config/removeinstance.phtml deleted file mode 100644 index d2c7b6bdd9..0000000000 --- a/modules/monitoring/application/views/scripts/config/removeinstance.phtml +++ /dev/null @@ -1,6 +0,0 @@ -
- showOnlyCloseButton() ?> -
-
- -
\ No newline at end of file