Merge branch 'bugfix/fix-monitoring-config-forms-9516'

fixes #9516

application/forms/Config/UserGroup/UserGroupBackendForm.php

@@ -35,6 +35,8 @@ class UserGroupBackendForm extends ConfigForm
      * @param   string  $type               The backend type for which to return a form
      *
      * @return  Form
+     *
+     * @throws  InvalidArgumentException    In case the given backend type is invalid
      */
     public function getBackendForm($type)
     {
@@ -74,6 +76,8 @@ class UserGroupBackendForm extends ConfigForm
     /**
      * Add a new user group backend
      *
+     * The backend to add is identified by the array-key `name'.
+     *
      * @param   array   $data
      *
      * @return  $this
@@ -171,7 +175,7 @@ class UserGroupBackendForm extends ConfigForm
                             'pattern'  => '/^[^\\[\\]:]+$/',
                             'messages' => array(
                                 'regexNotMatch' => $this->translate(
-                                    'The backend name cannot contain \'[\', \']\' or \':\'.'
+                                    'The name cannot contain \'[\', \']\' or \':\'.'
                                 )
                             )
                         )

modules/monitoring/application/controllers/ConfigController.php

@@ -1,10 +1,11 @@
 <?php
 /* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
 
-use Icinga\Web\Notification;
 use Icinga\Data\ResourceFactory;
+use Icinga\Exception\ConfigurationError;
 use Icinga\Forms\ConfirmRemovalForm;
 use Icinga\Web\Controller;
+use Icinga\Web\Notification;
 use Icinga\Module\Monitoring\Forms\Config\BackendConfigForm;
 use Icinga\Module\Monitoring\Forms\Config\InstanceConfigForm;
 use Icinga\Module\Monitoring\Forms\Config\SecurityConfigForm;
@@ -25,33 +26,89 @@ class Monitoring_ConfigController extends Controller
     }
 
     /**
-     * Display a form to modify the backend identified by the 'backend' parameter of the request
+     * Edit a monitoring backend
      */
     public function editbackendAction()
     {
+        $backendName = $this->params->getRequired('backend');
+
         $form = new BackendConfigForm();
-        $form->setTitle($this->translate('Edit Existing Backend'));
+        $form->setRedirectUrl('monitoring/config');
+        $form->setTitle(sprintf($this->translate('Edit Monitoring Backend %s'), $backendName));
         $form->setIniConfig($this->Config('backends'));
         $form->setResourceConfig(ResourceFactory::getResourceConfigs());
-        $form->setRedirectUrl('monitoring/config');
+        $form->setOnSuccess(function (BackendConfigForm $form) use ($backendName) {
+            try {
+                $form->edit($backendName, 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 backend "%s" successfully updated'), $backendName));
+                return true;
+            }
+
+            return false;
+        });
+
+        try {
+            $form->load($backendName);
             $form->handleRequest();
+        } catch (NotFoundError $_) {
+            $this->httpNotFound(sprintf($this->translate('Monitoring backend "%s" not found'), $backendName));
+        }
 
         $this->view->form = $form;
+        $this->render('form');
     }
 
     /**
-     * Display a form to create a new backend
+     * Create a new monitoring backend
      */
     public function createbackendAction()
     {
         $form = new BackendConfigForm();
-        $form->setTitle($this->translate('Add New Backend'));
-        $form->setIniConfig($this->Config('backends'));
-        $form->setResourceConfig(ResourceFactory::getResourceConfigs());
         $form->setRedirectUrl('monitoring/config');
+        $form->setTitle($this->translate('Create New Monitoring Backend'));
+        $form->setIniConfig($this->Config('backends'));
+
+        try {
+            $form->setResourceConfig(ResourceFactory::getResourceConfigs());
+        } catch (ConfigurationError $e) {
+            if ($this->hasPermission('config/application/resources')) {
+                Notification::error($e->getMessage());
+                $this->redirectNow('config/createresource');
+            }
+
+            throw $e; // No permission for resource configuration, show the error
+        }
+
+        $form->setOnSuccess(function (BackendConfigForm $form) {
+            try {
+                $form->add(array_filter($form->getValues()));
+            } catch (Exception $e) {
+                $form->error($e->getMessage());
+                return false;
+            }
+
+            if ($form->save()) {
+                Notification::success(t('Monitoring backend successfully created'));
+                return true;
+            }
+
+            return false;
+        });
         $form->handleRequest();
 
         $this->view->form = $form;
+        $this->render('form');
     }
 
     /**
@@ -59,114 +116,142 @@ class Monitoring_ConfigController extends Controller
      */
     public function removebackendAction()
     {
-        $config = $this->Config('backends');
+        $backendName = $this->params->getRequired('backend');
-        $form = new ConfirmRemovalForm(array(
-            'onSuccess' => function ($form) use ($config) {
-                $backendName = $form->getRequest()->getQuery('backend');
-                $configForm = new BackendConfigForm();
-                $configForm->setIniConfig($config);
 
+        $backendForm = new BackendConfigForm();
+        $backendForm->setIniConfig($this->Config('backends'));
+        $form = new ConfirmRemovalForm();
+        $form->setRedirectUrl('monitoring/config');
+        $form->setTitle(sprintf($this->translate('Remove Monitoring Backend %s'), $backendName));
+        $form->setOnSuccess(function (ConfirmRemovalForm $form) use ($backendName, $backendForm) {
             try {
-                    $configForm->remove($backendName);
+                $backendForm->delete($backendName);
-                } catch (InvalidArgumentException $e) {
+            } catch (Exception $e) {
-                    Notification::error($e->getMessage());
+                $form->error($e->getMessage());
-                    return;
-                }
-
-                if ($configForm->save()) {
-                    Notification::success(sprintf(
-                        $this->translate('Backend "%s" successfully removed.'),
-                        $backendName
-                    ));
-                } else {
                 return false;
             }
+
+            if ($backendForm->save()) {
+                Notification::success(sprintf(t('Monitoring backend "%s" successfully removed'), $backendName));
+                return true;
             }
-        ));
+
-        $form->setTitle($this->translate('Remove Existing Backend'));
+            return false;
-        $form->setRedirectUrl('monitoring/config');
+        });
         $form->handleRequest();
 
         $this->view->form = $form;
+        $this->render('form');
     }
 
     /**
-     * Display a confirmation form to remove the instance identified by the 'instance' parameter
+     * Remove a monitoring instance
      */
     public function removeinstanceAction()
     {
-        $config = $this->Config('instances');
+        $instanceName = $this->params->getRequired('instance');
-        $form = new ConfirmRemovalForm(array(
-            'onSuccess' => function ($form) use ($config) {
-                $instanceName = $form->getRequest()->getQuery('instance');
-                $configForm = new InstanceConfigForm();
-                $configForm->setIniConfig($config);
 
-                try {
+        $instanceForm = new InstanceConfigForm();
-                    $configForm->remove($instanceName);
+        $instanceForm->setIniConfig($this->Config('instances'));
-                } catch (InvalidArgumentException $e) {
+        $form = new ConfirmRemovalForm();
-                    Notification::error($e->getMessage());
+        $form->setRedirectUrl('monitoring/config');
-                    return;
+        $form->setTitle(sprintf($this->translate('Remove Monitoring Instance %s'), $instanceName));
-                }
-
-                if ($configForm->save()) {
-                    Notification::success(sprintf(
-                        $this->translate('Instance "%s" successfully removed.'),
-                        $instanceName
-                    ));
-                } else {
-                    return false;
-                }
-            }
-        ));
-        $form->setTitle($this->translate('Remove Existing Instance'));
         $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(
+        $form->setOnSuccess(function (ConfirmRemovalForm $form) use ($instanceName, $instanceForm) {
-            'note',
+            try {
-            'question',
+                $instanceForm->delete($instanceName);
-            array(
+            } catch (Exception $e) {
-                'value'         => $this->translate('Are you sure you want to remove this instance?'),
+                $form->error($e->getMessage());
-                'decorators'    => array(
+                return false;
-                    'ViewHelper',
+            }
-                    array('HtmlTag', array('tag' => 'p'))
+
-                )
+            if ($instanceForm->save()) {
-            )
+                Notification::success(sprintf(t('Monitoring instance "%s" successfully removed'), $instanceName));
-        );
+                return true;
-        $form->setRedirectUrl('monitoring/config');
+            }
+
+            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->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');
     }
 
     /**
@@ -180,5 +265,6 @@ class Monitoring_ConfigController extends Controller
 
         $this->view->form = $form;
         $this->view->tabs = $this->Module()->getConfigTabs()->activate('security');
+        $this->render('form');
     }
 }

modules/monitoring/application/forms/Config/BackendConfigForm.php

@@ -4,17 +4,18 @@
 namespace Icinga\Module\Monitoring\Forms\Config;
 
 use Exception;
-use Icinga\Data\ConfigObject;
-use Icinga\Data\ResourceFactory;
-use Icinga\Web\Form;
 use InvalidArgumentException;
 use Icinga\Application\Config;
+use Icinga\Data\ConfigObject;
+use Icinga\Data\ResourceFactory;
 use Icinga\Exception\ConfigurationError;
+use Icinga\Exception\IcingaException;
+use Icinga\Exception\NotFoundError;
 use Icinga\Forms\ConfigForm;
-use Icinga\Web\Notification;
+use Icinga\Web\Form;
 
 /**
- * Form class for creating/modifying monitoring backends
+ * Form for managing monitoring backends
  */
 class BackendConfigForm extends ConfigForm
 {
@@ -25,6 +26,13 @@ class BackendConfigForm extends ConfigForm
      */
     protected $resources;
 
+    /**
+     * The backend to load when displaying the form for the first time
+     *
+     * @var string
+     */
+    protected $backendToLoad;
+
     /**
      * Initialize this form
      */
@@ -47,16 +55,15 @@ class BackendConfigForm extends ConfigForm
     {
         $resources = array();
         foreach ($resourceConfig as $name => $resource) {
-//            if ($resource->type === 'db' || $resource->type === 'livestatus') {
-//                $resources[$resource->type === 'db' ? 'ido' : 'livestatus'][$name] = $name;
-//            }
             if ($resource->type === 'db') {
                 $resources['ido'][$name] = $name;
             }
         }
 
         if (empty($resources)) {
-            throw new ConfigurationError($this->translate('Could not find any valid monitoring backend resources'));
+            throw new ConfigurationError($this->translate(
+                'Could not find any valid monitoring backend resources. Please configure a database resource first.'
+            ));
         }
 
         $this->resources = $resources;
@@ -64,146 +71,109 @@ class BackendConfigForm extends ConfigForm
     }
 
     /**
-     * Add a particular monitoring backend
+     * Populate the form with the given backend's config
      *
-     * The backend to add is identified by the array-key `name'.
+     * @param   string  $name
-     *
-     * @param   array   $values             The values to extend the configuration with
      *
      * @return  $this
      *
-     * @throws  InvalidArgumentException    In case the backend does already exist
+     * @throws  NotFoundError   In case no backend with the given name is found
      */
-    public function add(array $values)
+    public function load($name)
     {
-        $name = isset($values['name']) ? $values['name'] : '';
+        if (! $this->config->hasSection($name)) {
-        if (! $name) {
+            throw new NotFoundError('No monitoring backend called "%s" found', $name);
-            throw new InvalidArgumentException($this->translate('Monitoring backend name missing'));
-        } elseif ($this->config->hasSection($name)) {
-            throw new InvalidArgumentException($this->translate('Monitoring backend already exists'));
         }
 
-        unset($values['name']);
+        $this->backendToLoad = $name;
-        $this->config->setSection($name, $values);
         return $this;
     }
 
     /**
-     * Edit a particular monitoring backend
+     * Add a new monitoring backend
      *
-     * @param   string  $name               The name of the backend to edit
+     * The backend to add is identified by the array-key `name'.
-     * @param   array   $values             The values to edit the configuration with
      *
-     * @return  array                       The edited backend configuration
+     * @param   array   $data
      *
-     * @throws  InvalidArgumentException    In case the backend does not exist
+     * @return  $this
+     *
+     * @throws  InvalidArgumentException    In case $data does not contain a backend name
+     * @throws  IcingaException             In case a backend with the same name already exists
      */
-    public function edit($name, array $values)
+    public function add(array $data)
     {
-        if (! $name) {
+        if (! isset($data['name'])) {
-            throw new InvalidArgumentException($this->translate('Old monitoring backend name missing'));
+            throw new InvalidArgumentException('Key \'name\' missing');
-        } elseif (! ($newName = isset($values['name']) ? $values['name'] : '')) {
-            throw new InvalidArgumentException($this->translate('New monitoring backend name missing'));
-        } elseif (! $this->config->hasSection($name)) {
-            throw new InvalidArgumentException($this->translate('Unknown monitoring backend provided'));
         }
 
-        unset($values['name']);
+        $backendName = $data['name'];
-        $this->config->setSection($name, $values);
+        if ($this->config->hasSection($backendName)) {
-        return $this->config->getSection($name);
+            throw new IcingaException('A monitoring backend with the name "%s" does already exist', $backendName);
+        }
+
+        unset($data['name']);
+        $this->config->setSection($backendName, $data);
+        return $this;
     }
 
     /**
-     * Remove the given monitoring backend
+     * Edit a monitoring backend
      *
-     * @param   string      $name           The name of the backend to remove
+     * @param   string  $name
+     * @param   array   $data
      *
-     * @return  array                       The removed backend configuration
+     * @return  $this
      *
-     * @throws  InvalidArgumentException    In case the backend does not exist
+     * @throws  NotFoundError   In case no backend with the given name is found
      */
-    public function remove($name)
+    public function edit($name, array $data)
     {
-        if (! $name) {
+        if (! $this->config->hasSection($name)) {
-            throw new InvalidArgumentException($this->translate('Monitoring backend name missing'));
+            throw new NotFoundError('No monitoring backend called "%s" found', $name);
-        } elseif (! $this->config->hasSection($name)) {
-            throw new InvalidArgumentException($this->translate('Unknown monitoring backend provided'));
         }
 
         $backendConfig = $this->config->getSection($name);
+        if (isset($data['name'])) {
+            if ($data['name'] !== $name) {
                 $this->config->removeSection($name);
-        return $backendConfig;
+                $name = $data['name'];
+            }
+
+            unset($data['name']);
+        }
+
+        $backendConfig->merge($data);
+        foreach ($backendConfig->toArray() as $k => $v) {
+            if ($v === null) {
+                unset($backendConfig->$k);
+            }
+        }
+
+        $this->config->setSection($name, $backendConfig);
+        return $this;
     }
 
     /**
-     * Add or edit a monitoring backend and save the configuration
+     * Remove a monitoring backend
-     */
-    public function onSuccess()
-    {
-        $monitoringBackend = $this->request->getQuery('backend');
-        try {
-            if ($monitoringBackend === null) { // Create new backend
-                $this->add($this->getValues());
-                $message = $this->translate('Monitoring backend "%s" has been successfully created');
-            } else { // Edit existing backend
-                $this->edit($monitoringBackend, $this->getValues());
-                $message = $this->translate('Monitoring backend "%s" has been successfully changed');
-            }
-        } catch (InvalidArgumentException $e) {
-            Notification::error($e->getMessage());
-            return null;
-        }
-
-        if ($this->save()) {
-            Notification::success(sprintf($message, $this->getElement('name')->getValue()));
-        } else {
-            return false;
-        }
-    }
-
-    /**
-     * Populate the form in case a monitoring backend is being edited
      *
-     * @throws  ConfigurationError  In case the backend name is missing in the request or is invalid
+     * @param   string  $name
+     *
+     * @return  $this
      */
-    public function onRequest()
+    public function delete($name)
     {
-        $monitoringBackend = $this->request->getQuery('backend');
+        $this->config->removeSection($name);
-        if ($monitoringBackend !== null) {
+        return $this;
-            if ($monitoringBackend === '') {
-                throw new ConfigurationError($this->translate('Monitoring backend name missing'));
-            } elseif (! $this->config->hasSection($monitoringBackend)) {
-                throw new ConfigurationError($this->translate('Unknown monitoring backend provided'));
-            }
-
-            $backendConfig = $this->config->getSection($monitoringBackend)->toArray();
-            $backendConfig['name'] = $monitoringBackend;
-            $this->populate($backendConfig);
-        }
     }
 
     /**
-     * (non-PHPDoc)
+     * Create and add elements to this form
-     * @see Form::createElements() For the method documentation.
+     *
+     * @param   array   $formData
      */
     public function createElements(array $formData)
     {
-        $resourceType = isset($formData['type']) ? $formData['type'] : key($this->resources);
-
-        if ($resourceType === 'livestatus') {
-            throw new ConfigurationError(
-                'We\'ve disabled livestatus support for now because it\'s not feature complete yet'
-            );
-        }
-
-        $resourceTypes = array();
-        if ($resourceType === 'ido' || array_key_exists('ido', $this->resources)) {
-            $resourceTypes['ido'] = 'IDO Backend';
-        }
-//        if ($resourceType === 'livestatus' || array_key_exists('livestatus', $this->resources)) {
-//            $resourceTypes['livestatus'] = 'Livestatus';
-//        }
-
         $this->addElement(
             'checkbox',
             'disabled',
@@ -218,9 +188,41 @@ class BackendConfigForm extends ConfigForm
             array(
                 'required'      => true,
                 'label'         => $this->translate('Backend Name'),
-                'description'   => $this->translate('The identifier of this backend')
+                'description'   => $this->translate(
+                    'The name of this monitoring backend 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 \':\'.'
+                                )
+                            )
+                        )
+                    )
+                )
             )
         );
+
+        $resourceType = isset($formData['type']) ? $formData['type'] : null;
+
+        $resourceTypes = array();
+        if ($resourceType === 'ido' || array_key_exists('ido', $this->resources)) {
+            $resourceTypes['ido'] = 'IDO Backend';
+        }
+
+        if ($resourceType === null) {
+            $resourceType = key($resourceTypes);
+        } elseif ($resourceType === 'livestatus') {
+            throw new ConfigurationError(
+                'We\'ve disabled livestatus support for now because it\'s not feature complete yet'
+            );
+        }
+
         $this->addElement(
             'select',
             'type',
@@ -228,9 +230,10 @@ class BackendConfigForm extends ConfigForm
                 'required'      => true,
                 'autosubmit'    => true,
                 'label'         => $this->translate('Backend Type'),
-                'description'   => $this->translate('The data source used for retrieving monitoring information'),
+                'description'   => $this->translate(
-                'multiOptions'  => $resourceTypes,
+                    'The type of data source used for retrieving monitoring information'
-                'value'         => $resourceType
+                ),
+                'multiOptions'  => $resourceTypes
             )
         );
 
@@ -276,6 +279,18 @@ class BackendConfigForm extends ConfigForm
         );
     }
 
+    /**
+     * Populate the configuration of the backend to load
+     */
+    public function onRequest()
+    {
+        if ($this->backendToLoad) {
+            $data = $this->config->getSection($this->backendToLoad)->toArray();
+            $data['name'] = $this->backendToLoad;
+            $this->populate($data);
+        }
+    }
+
     /**
      * Validate the ido instance schema resource
      *

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)
+     * The instance to load when displaying the form for the first time
-     * @see Form::init() For the method documentation.
+     *
+     * @var string
+     */
+    protected $instanceToLoad;
+
+    /**
+     * Initialize this form
      */
     public function init()
     {
@@ -28,11 +34,11 @@ class InstanceConfigForm extends ConfigForm
     }
 
     /**
-     * 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
      *
-     * @return  LocalInstanceForm|RemoteInstanceForm
+     * @return  Form
      *
      * @throws  InvalidArgumentException    In case the given instance type is invalid
      */
@@ -40,162 +46,157 @@ class InstanceConfigForm extends ConfigForm
     {
         switch (strtolower($type)) {
             case LocalCommandFile::TRANSPORT:
-                $form = new LocalInstanceForm();
+                return new LocalInstanceForm();
-                break;
             case RemoteCommandFile::TRANSPORT;
-                $form = new RemoteInstanceForm();
+                return new RemoteInstanceForm();
-                break;
             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;
+    }
+
+    /**
+     * Populate the form with the given instance's config
+     *
+     * @param   string  $name
+     *
+     * @return  $this
+     *
+     * @throws  NotFoundError   In case no instance with the given name is found
+     */
+    public function load($name)
+    {
+        if (! $this->config->hasSection($name)) {
+            throw new NotFoundError('No monitoring instance called "%s" found', $name);
+        }
+
+        $this->instanceToLoad = $name;
+        return $this;
     }
 
     /**
      * Add a new instance
      *
-     * The resource to add is identified by the array-key `name'.
+     * The instance to add is identified by the array-key `name'.
      *
-     * @param   array   $values             The values to extend the configuration with
+     * @param   array   $data
      *
      * @return  $this
      *
-     * @throws  InvalidArgumentException    In case the resource already exists
+     * @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 add(array $values)
+    public function add(array $data)
     {
-        $name = isset($values['name']) ? $values['name'] : '';
+        if (! isset($data['name'])) {
-        if (! $name) {
+            throw new InvalidArgumentException('Key \'name\' missing');
-            throw new InvalidArgumentException($this->translate('Instance name missing'));
-        }
-        if ($this->config->hasSection($name)) {
-            throw new InvalidArgumentException($this->translate('Instance already exists'));
         }
 
-        unset($values['name']);
+        $instanceName = $data['name'];
-        $this->config->setSection($name, $values);
+        if ($this->config->hasSection($instanceName)) {
+            throw new IcingaException('A monitoring instance with the name "%s" does already exist', $instanceName);
+        }
+
+        unset($data['name']);
+        $this->config->setSection($instanceName, $data);
         return $this;
     }
 
     /**
      * Edit an existing instance
      *
-     * @param   string      $name           The name of the resource to edit
+     * @param   string  $name
-     * @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  NotFoundError   In case no instance with the given name is found
      */
-    public function edit($name, array $values)
+    public function edit($name, array $data)
     {
-        if (! $name) {
+        if (! $this->config->hasSection($name)) {
-            throw new InvalidArgumentException($this->translate('Old instance name missing'));
+            throw new NotFoundError('No monitoring instance called "%s" found', $name);
-        } 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'));
         }
 
-        unset($values['name']);
+        $instanceConfig = $this->config->getSection($name);
-        $this->config->setSection($name, $values);
+        if (isset($data['name'])) {
-        return $this->config->getSection($name);
+            if ($data['name'] !== $name) {
+                $this->config->removeSection($name);
+                $name = $data['name'];
+            }
+
+            unset($data['name']);
+        }
+
+        $instanceConfig->merge($data);
+        foreach ($instanceConfig->toArray() as $k => $v) {
+            if ($v === null) {
+                unset($instanceConfig->$k);
+            }
+        }
+
+        $this->config->setSection($name, $instanceConfig);
+        return $this;
     }
 
     /**
      * Remove a instance
      *
-     * @param   string      $name           The name of the resource to remove
+     * @param   string  $name
      *
-     * @return  array                       The removed resource configuration
+     * @return  $this
-     *
-     * @throws  InvalidArgumentException    In case the resource name is missing or invalid
      */
-    public function remove($name)
+    public function delete($name)
     {
-        if (! $name) {
-            throw new InvalidArgumentException($this->translate('Instance name missing'));
-        } elseif (! $this->config->hasSection($name)) {
-            throw new InvalidArgumentException($this->translate('Unknown instance name provided'));
-        }
-
-        $instanceConfig = $this->config->getSection($name);
         $this->config->removeSection($name);
-        return $instanceConfig;
+        return $this;
     }
 
     /**
-     * @see     Form::onRequest()   For the method documentation.
+     * Create and add elements to this form
-     * @throws  ConfigurationError  In case the instance name is missing or invalid
+     *
+     * @param   array   $formData
      */
-    public function onRequest()
+    public function createElements(array $formData)
     {
-        $instanceName = $this->request->getQuery('instance');
+        $this->addElement(
-        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'));
-            }
-
-            $instanceConfig = $this->config->getSection($instanceName)->toArray();
-            $instanceConfig['name'] = $instanceName;
-
-            if (isset($instanceConfig['resource'])) {
-                $instanceConfig['use_resource'] = true;
-            }
-
-            $this->populate($instanceConfig);
-        }
-    }
-
-    /**
-     * (non-PHPDoc)
-     * @see Form::onSuccess() For the method documentation.
-     */
-    public function onSuccess()
-    {
-        $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;
-        }
-    }
-
-    /**
-     * (non-PHPDoc)
-     * @see Form::createElements() For the method documentation.
-     */
-    public function createElements(array $formData = array())
-    {
-        $instanceType = isset($formData['transport']) ? $formData['transport'] : LocalCommandFile::TRANSPORT;
-
-        $this->addElements(array(
-            array(
             'text',
             'name',
             array(
                 'required'      => true,
-                    'label'     => $this->translate('Instance Name')
+                '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 @@ class InstanceConfigForm extends ConfigForm
                     'required'      => true,
                     'autosubmit'    => true,
                     'label'         => $this->translate('Instance Type'),
-                    'multiOptions'  => array(
+                    'description'   => $this->translate('The type of transport to use for this monitoring instance'),
-                        LocalCommandFile::TRANSPORT     => $this->translate('Local Command File'),
+                    'multiOptions'  => $instanceTypes
-                        RemoteCommandFile::TRANSPORT    => $this->translate('Remote Command File')
-                    ),
-                    'value' => $instanceType
                 )
             )
         ));
 
-        $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;
     }
 }

modules/monitoring/application/forms/Setup/InstancePage.php

@@ -19,10 +19,6 @@ class InstancePage extends Form
 
     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());

modules/monitoring/application/views/scripts/config/createinstance.phtml

@@ -1,6 +0,0 @@
-<div class="controls">
-    <?= $tabs->showOnlyCloseButton() ?>
-</div>
-<div class="content">
-    <?= $form; ?>
-</div>

modules/monitoring/application/views/scripts/config/editbackend.phtml

@@ -1,6 +0,0 @@
-<div class="controls">
-    <?= $tabs->showOnlyCloseButton() ?>
-</div>
-<div class="content">
-    <?= $form; ?>
-</div>

modules/monitoring/application/views/scripts/config/editinstance.phtml

@@ -1,6 +0,0 @@
-<div class="controls">
-    <?= $tabs->showOnlyCloseButton() ?>
-</div>
-<div class="content">
-    <?= $form; ?>
-</div>

modules/monitoring/application/views/scripts/config/form.phtml

@@ -1,5 +1,5 @@
 <div class="controls">
-    <?= $tabs->showOnlyCloseButton() ?>
+  <?= $tabs->showOnlyCloseButton(); ?>
 </div>
 <div class="content">
   <?= $form; ?>

modules/monitoring/application/views/scripts/config/removebackend.phtml

@@ -1,6 +0,0 @@
-<div class="controls">
-    <?= $tabs->showOnlyCloseButton() ?>
-</div>
-<div class="content">
-    <?= $form; ?>
-</div>

modules/monitoring/application/views/scripts/config/removeinstance.phtml

@@ -1,6 +0,0 @@
-<div class="controls">
-    <?= $tabs->showOnlyCloseButton() ?>
-</div>
-<div class="content">
-    <?= $form; ?>
-</div>

modules/monitoring/application/views/scripts/config/security.phtml

@@ -1,6 +0,0 @@
-<div class="controls">
-    <?= $this->tabs ?>
-</div>
-<div class="content">
-    <?= $this->form ?>
-</div>