From 5fc1f85b76dcedc1372c86a0d5bf970253af8ccf Mon Sep 17 00:00:00 2001
From: Eric Lippmann <eric.lippmann@netways.de>
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 598a6cea1..2971cb663 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 @@ use Icinga\Module\Monitoring\Form\Config\Instance\RemoteInstanceForm;
class InstanceConfigForm extends ConfigForm
{
/**
- * Initialize this form
+ * (non-PHPDoc)
+ * @see Form::init() For the method documentation.
*/
public function init()
{
@@ -27,23 +30,29 @@ class InstanceConfigForm extends ConfigForm
}
/**
- * 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 @@ class InstanceConfigForm extends ConfigForm
$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 @@ class InstanceConfigForm extends ConfigForm
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 @@ class InstanceConfigForm extends ConfigForm
*
* @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 @@ class InstanceConfigForm extends ConfigForm
}
/**
- * @see Form::onSuccess()
+ * @see Form::onRequest() For the method documentation.
+ * @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'));
+ }
+ 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 @@ class InstanceConfigForm extends ConfigForm
}
/**
- * @see Form::onRequest()
- *
- * @throws ConfigurationError In case the instance name is missing or invalid
+ * (non-PHPDoc)
+ * @see Form::createElements() For the method documentation.
*/
- public function onRequest(Request $request)
+ public function createElements(array $formData = array())
{
- $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'));
- }
+ $instanceType = isset($formData['transport']) ? $formData['transport'] : LocalCommandFile::TRANSPORT;
- $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()
- */
- public function createElements(array $formData)
- {
- $instanceType = isset($formData['type']) ? $formData['type'] : 'local';
-
- $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());
}