From 5fc1f85b76dcedc1372c86a0d5bf970253af8ccf Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 29 Oct 2014 13:36:09 +0100 Subject: [PATCH] monitoring: Write transport directive to instances INI configuration Further I replaced if-elseif blocks w/ switch when branching based on the value of a single parameter, which would have to be implied otherwise by looking at all the conditions. --- .../forms/Config/InstanceConfigForm.php | 143 +++++++++--------- 1 file changed, 74 insertions(+), 69 deletions(-) diff --git a/modules/monitoring/application/forms/Config/InstanceConfigForm.php b/modules/monitoring/application/forms/Config/InstanceConfigForm.php index 598a6cea1a..2971cb663b 100644 --- a/modules/monitoring/application/forms/Config/InstanceConfigForm.php +++ b/modules/monitoring/application/forms/Config/InstanceConfigForm.php @@ -5,12 +5,14 @@ namespace Icinga\Module\Monitoring\Form\Config; use InvalidArgumentException; -use Icinga\Web\Request; -use Icinga\Form\ConfigForm; -use Icinga\Web\Notification; use Icinga\Exception\ConfigurationError; +use Icinga\Form\ConfigForm; +use Icinga\Module\Monitoring\Command\Transport\LocalCommandFile; +use Icinga\Module\Monitoring\Command\Transport\RemoteCommandFile; use Icinga\Module\Monitoring\Form\Config\Instance\LocalInstanceForm; use Icinga\Module\Monitoring\Form\Config\Instance\RemoteInstanceForm; +use Icinga\Web\Notification; +use Icinga\Web\Request; /** * Form for modifying/creating monitoring instances @@ -18,7 +20,8 @@ class InstanceConfigForm extends ConfigForm { /** - * Initialize this form + * (non-PHPDoc) + * @see Form::init() For the method documentation. */ public function init() { @@ -27,23 +30,29 @@ public function init() } /** - * Return a form object for the given instance type + * Get 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 Form + * @return LocalInstanceForm|RemoteInstanceForm * * @throws InvalidArgumentException In case the given instance type is invalid */ public function getInstanceForm($type) { - if ($type === 'local') { - return new LocalInstanceForm(); - } elseif ($type === 'remote') { - return new RemoteInstanceForm(); - } else { - throw new InvalidArgumentException(sprintf(mt('monitoring', 'Invalid instance type "%s" provided'), $type)); + switch (strtolower($type)) { + case LocalCommandFile::TRANSPORT: + $form = new LocalInstanceForm(); + break; + case RemoteCommandFile::TRANSPORT; + $form = new RemoteInstanceForm(); + break; + default: + throw new InvalidArgumentException( + sprintf(mt('monitoring', 'Invalid instance type "%s" given'), $type) + ); } + return $form; } /** @@ -62,7 +71,8 @@ public function add(array $values) $name = isset($values['name']) ? $values['name'] : ''; if (! $name) { throw new InvalidArgumentException(mt('monitoring', 'Instance name missing')); - } elseif ($this->config->get($name) !== null) { + } + if (isset($this->config->{$name})) { throw new InvalidArgumentException(mt('monitoring', 'Instance already exists')); } @@ -93,7 +103,7 @@ public function edit($name, array $values) unset($values['name']); unset($this->config->{$name}); - $this->config->{$newName} = array_merge($instanceConfig->toArray(), $values); + $this->config->{$newName} = $values; return $this->config->{$newName}; } @@ -102,7 +112,7 @@ public function edit($name, array $values) * * @param string $name The name of the resource to remove * - * @return array The removed resource confguration + * @return array The removed resource configuration * * @throws InvalidArgumentException In case the resource name is missing or invalid */ @@ -119,12 +129,33 @@ public function remove($name) } /** - * @see Form::onSuccess() + * @see Form::onRequest() For the method documentation. + * @throws ConfigurationError In case the instance name is missing or invalid */ - public function onSuccess(Request $request) + public function onRequest(Request $request) { $instanceName = $request->getQuery('instance'); + if ($instanceName !== null) { + if (! $instanceName) { + throw new ConfigurationError(mt('monitoring', 'Instance name missing')); + } + if (! isset($this->config->{$instanceName})) { + throw new ConfigurationError(mt('monitoring', 'Unknown instance name given')); + } + $instanceConfig = $this->config->{$instanceName}->toArray(); + $instanceConfig['name'] = $instanceName; + $this->populate($instanceConfig); + } + } + + /** + * (non-PHPDoc) + * @see Form::onSuccess() For the method documentation. + */ + public function onSuccess(Request $request) + { + $instanceName = $request->getQuery('instance'); try { if ($instanceName === null) { // create new instance $this->add($this->getValues()); @@ -146,63 +177,37 @@ public function onSuccess(Request $request) } /** - * @see Form::onRequest() - * - * @throws ConfigurationError In case the instance name is missing or invalid - */ - public function onRequest(Request $request) - { - $instanceName = $request->getQuery('instance'); - if ($instanceName !== null) { - if (! $instanceName) { - throw new ConfigurationError(mt('monitoring', 'Instance name missing')); - } elseif (false === isset($this->config->{$instanceName})) { - throw new ConfigurationError(mt('monitoring', 'Unknown instance name provided')); - } - - $instanceConfig = $this->config->{$instanceName}->toArray(); - $instanceConfig['name'] = $instanceName; - if (isset($instanceConfig['host'])) { - // Necessary as we have no config directive for setting the instance's type - $instanceConfig['type'] = 'remote'; - } - $this->populate($instanceConfig); - } - } - - /** - * @see Form::createElements() + * (non-PHPDoc) + * @see Form::createElements() For the method documentation. */ - public function createElements(array $formData) + public function createElements(array $formData = array()) { - $instanceType = isset($formData['type']) ? $formData['type'] : 'local'; + $instanceType = isset($formData['transport']) ? $formData['transport'] : LocalCommandFile::TRANSPORT; - $this->addElement( - 'text', - 'name', + $this->addElements(array( array( - 'required' => true, - 'label' => mt('monitoring', 'Instance Name') - ) - ); - $this->addElement( - 'select', - 'type', + 'text', + 'name', + array( + 'required' => true, + 'label' => mt('monitoring', 'Instance Name') + ) + ), array( - 'required' => true, - 'ignore' => true, - 'autosubmit' => true, - 'label' => mt('monitoring', 'Instance Type'), - 'description' => mt('monitoring', - 'When configuring a remote host, you need to setup passwordless key authentication' - ), - 'multiOptions' => array( - 'local' => mt('monitoring', 'Local Command Pipe'), - 'remote' => mt('monitoring', 'Remote Command Pipe') - ), - 'value' => $instanceType + 'select', + 'transport', + array( + 'required' => true, + 'autosubmit' => true, + 'label' => mt('monitoring', 'Instance Type'), + 'multiOptions' => array( + LocalCommandFile::TRANSPORT => mt('monitoring', 'Local Command File'), + RemoteCommandFile::TRANSPORT => mt('monitoring', 'Remote Command File') + ), + 'value' => $instanceType + ) ) - ); + )); $this->addElements($this->getInstanceForm($instanceType)->createElements($formData)->getElements()); }