diff --git a/application/forms/Config/Resource/DbResourceForm.php b/application/forms/Config/Resource/DbResourceForm.php index 75d935361..3339b59bf 100644 --- a/application/forms/Config/Resource/DbResourceForm.php +++ b/application/forms/Config/Resource/DbResourceForm.php @@ -3,10 +3,7 @@ namespace Icinga\Forms\Config\Resource; -use Exception; use Icinga\Web\Form; -use Icinga\Data\ConfigObject; -use Icinga\Data\ResourceFactory; use Icinga\Application\Platform; /** @@ -23,7 +20,9 @@ class DbResourceForm extends Form } /** - * @see Form::createElements() + * Create and add elements to this form + * + * @param array $formData The data sent by the user */ public function createElements(array $formData) { @@ -107,35 +106,4 @@ class DbResourceForm extends Form return $this; } - - /** - * Validate that the current configuration points to a valid resource - * - * @see Form::onSuccess() - */ - public function onSuccess() - { - if (false === static::isValidResource($this)) { - return false; - } - } - - /** - * Validate the resource configuration by trying to connect with it - * - * @param Form $form The form to fetch the configuration values from - * - * @return bool Whether validation succeeded or not - */ - public static function isValidResource(Form $form) - { - $result = ResourceFactory::createResource(new ConfigObject($form->getValues()))->inspect(); - if ($result->hasError()) { - $form->addError(sprintf($form->translate('Connectivity validation failed: %s'), $result->getError())); - } - - // TODO: display diagnostics in $result->toArray() to the user - - return ! $result->hasError(); - } } diff --git a/application/forms/Config/Resource/LdapResourceForm.php b/application/forms/Config/Resource/LdapResourceForm.php index b385610a8..c6e452d2c 100644 --- a/application/forms/Config/Resource/LdapResourceForm.php +++ b/application/forms/Config/Resource/LdapResourceForm.php @@ -3,10 +3,7 @@ namespace Icinga\Forms\Config\Resource; -use Exception; use Icinga\Web\Form; -use Icinga\Data\ConfigObject; -use Icinga\Data\ResourceFactory; use Icinga\Protocol\Ldap\LdapConnection; /** @@ -23,7 +20,9 @@ class LdapResourceForm extends Form } /** - * @see Form::createElements() + * Create and add elements to this form + * + * @param array $formData The data sent by the user */ public function createElements(array $formData) { @@ -132,39 +131,4 @@ class LdapResourceForm extends Form return $this; } - - /** - * Validate that the current configuration points to a valid resource - * - * @see Form::onSuccess() - */ - public function onSuccess() - { - if (false === static::isValidResource($this)) { - return false; - } - } - - /** - * Validate the resource configuration by trying to connect with it - * - * @param Form $form The form to fetch the configuration values from - * - * @return bool Whether validation succeeded or not - */ - public static function isValidResource(Form $form) - { - $result = ResourceFactory::createResource(new ConfigObject($form->getValues()))->inspect(); - if ($result->hasError()) { - $form->addError(sprintf( - '%s (%s)', - $form->translate('Connectivity validation failed, connection to the given resource not possible.'), - $result->getError() - )); - } - - // TODO: display diagnostics in $result->toArray() to the user - - return ! $result->hasError(); - } } diff --git a/application/forms/Config/ResourceConfigForm.php b/application/forms/Config/ResourceConfigForm.php index 5d6626d02..51e1b1bff 100644 --- a/application/forms/Config/ResourceConfigForm.php +++ b/application/forms/Config/ResourceConfigForm.php @@ -4,15 +4,20 @@ namespace Icinga\Forms\Config; use InvalidArgumentException; -use Icinga\Web\Notification; +use Icinga\Application\Platform; +use Icinga\Exception\ConfigurationError; +use Icinga\Data\ConfigObject; +use Icinga\Data\Inspectable; +use Icinga\Data\Inspection; +use Icinga\Data\ResourceFactory; use Icinga\Forms\ConfigForm; use Icinga\Forms\Config\Resource\DbResourceForm; use Icinga\Forms\Config\Resource\FileResourceForm; use Icinga\Forms\Config\Resource\LdapResourceForm; use Icinga\Forms\Config\Resource\LivestatusResourceForm; use Icinga\Forms\Config\Resource\SshResourceForm; -use Icinga\Application\Platform; -use Icinga\Exception\ConfigurationError; +use Icinga\Web\Form; +use Icinga\Web\Notification; class ResourceConfigForm extends ConfigForm { @@ -23,6 +28,7 @@ class ResourceConfigForm extends ConfigForm { $this->setName('form_config_resource'); $this->setSubmitLabel($this->translate('Save Changes')); + $this->setValidatePartial(true); } /** @@ -141,7 +147,9 @@ class ResourceConfigForm extends ConfigForm $resourceForm = $this->getResourceForm($this->getElement('type')->getValue()); if (($el = $this->getElement('force_creation')) === null || false === $el->isChecked()) { - if (method_exists($resourceForm, 'isValidResource') && false === $resourceForm::isValidResource($this)) { + $inspection = static::inspectResource($this); + if ($inspection !== null && $inspection->hasError()) { + $this->error($inspection->getError()); $this->addElement($this->getForceCreationCheckbox()); return false; } @@ -255,4 +263,103 @@ class ResourceConfigForm extends ConfigForm $this->addElements($this->getResourceForm($resourceType)->createElements($formData)->getElements()); } + + /** + * Create a resource by using the given form's values and return its inspection results + * + * @param Form $form + * + * @return Inspection + */ + public static function inspectResource(Form $form) + { + if ($form->getValue('type') !== 'ssh') { + $resource = ResourceFactory::createResource(new ConfigObject($form->getValues())); + if ($resource instanceof Inspectable) { + return $resource->inspect(); + } + } + } + + /** + * Run the configured resource's inspection checks and show the result, if necessary + * + * This will only run any validation if the user pushed the 'resource_validation' button. + * + * @param array $formData + * + * @return bool + */ + public function isValidPartial(array $formData) + { + if ($this->getElement('resource_validation')->isChecked() && parent::isValid($formData)) { + $inspection = static::inspectResource($this); + if ($inspection !== null) { + $join = function ($e) use (& $join) { + return is_string($e) ? $e : join("\n", array_map($join, $e)); + }; + $this->addElement( + 'note', + 'inspection_output', + array( + 'order' => 0, + 'value' => '' . $this->translate('Validation Log') . "\n\n" + . join("\n", array_map($join, $inspection->toArray())), + 'decorators' => array( + 'ViewHelper', + array('HtmlTag', array('tag' => 'pre', 'class' => 'log-output')), + ) + ) + ); + + if ($inspection->hasError()) { + $this->warning(sprintf( + $this->translate('Failed to successfully validate the configuration: %s'), + $inspection->getError() + )); + return false; + } + } + + $this->info($this->translate('The configuration has been successfully validated.')); + } + + return true; + } + + /** + * Add a submit button to this form and one to manually validate the configuration + * + * Calls parent::addSubmitButton() to add the submit button. + * + * @return $this + */ + public function addSubmitButton() + { + parent::addSubmitButton() + ->getElement('btn_submit') + ->setDecorators(array('ViewHelper')); + + $this->addElement( + 'submit', + 'resource_validation', + array( + 'ignore' => true, + 'label' => $this->translate('Validate Configuration'), + 'decorators' => array('ViewHelper') + ) + ); + $this->addDisplayGroup( + array('btn_submit', 'resource_validation'), + 'submit_validation', + array( + 'decorators' => array( + 'FormElements', + array('HtmlTag', array('tag' => 'div', 'class' => 'control-group')) + ) + ) + ); + + return $this; + } } diff --git a/application/forms/Config/UserBackendConfigForm.php b/application/forms/Config/UserBackendConfigForm.php index 0afc54a44..5ee4674bb 100644 --- a/application/forms/Config/UserBackendConfigForm.php +++ b/application/forms/Config/UserBackendConfigForm.php @@ -11,6 +11,7 @@ use Icinga\Exception\IcingaException; use Icinga\Exception\NotFoundError; use Icinga\Data\ConfigObject; use Icinga\Data\Inspectable; +use Icinga\Data\Inspection; use Icinga\Forms\ConfigForm; use Icinga\Forms\Config\UserBackend\ExternalBackendForm; use Icinga\Forms\Config\UserBackend\DbBackendForm; @@ -43,6 +44,7 @@ class UserBackendConfigForm extends ConfigForm { $this->setName('form_config_authbackend'); $this->setSubmitLabel($this->translate('Save Changes')); + $this->setValidatePartial(true); } /** @@ -348,8 +350,9 @@ class UserBackendConfigForm extends ConfigForm } if (($el = $this->getElement('skip_validation')) === null || false === $el->isChecked()) { - $backendForm = $this->getBackendForm($this->getValue('type')); - if (! static::isValidUserBackend($this)) { + $inspection = static::inspectUserBackend($this); + if ($inspection && $inspection->hasError()) { + $this->error($inspection->getError()); if ($el === null) { $this->addSkipValidationCheckbox(); } @@ -362,24 +365,20 @@ class UserBackendConfigForm extends ConfigForm } /** - * Validate the configuration by creating a backend and running its inspection checks + * Create a user backend by using the given form's values and return its inspection results * - * @param Form $form The form to fetch the configuration values from + * Returns null for non-inspectable backends. * - * @return bool Whether inspection succeeded or not + * @param Form $form + * + * @return Inspection|null */ - public static function isValidUserBackend(Form $form) + public static function inspectUserBackend(Form $form) { $backend = UserBackend::create(null, new ConfigObject($form->getValues())); if ($backend instanceof Inspectable) { - $inspection = $backend->inspect(); - if ($inspection->hasError()) { - $form->error($inspection->getError()); - return false; - } + return $backend->inspect(); } - - return true; } /** @@ -401,4 +400,86 @@ class UserBackendConfigForm extends ConfigForm ) ); } + + /** + * Run the configured backend's inspection checks and show the result, if necessary + * + * This will only run any validation if the user pushed the 'backend_validation' button. + * + * @param array $formData + * + * @return bool + */ + public function isValidPartial(array $formData) + { + if ($this->getElement('backend_validation')->isChecked() && parent::isValid($formData)) { + $inspection = static::inspectUserBackend($this); + if ($inspection !== null) { + $join = function ($e) use (& $join) { + return is_string($e) ? $e : join("\n", array_map($join, $e)); + }; + $this->addElement( + 'note', + 'inspection_output', + array( + 'order' => 0, + 'value' => '' . $this->translate('Validation Log') . "\n\n" + . join("\n", array_map($join, $inspection->toArray())), + 'decorators' => array( + 'ViewHelper', + array('HtmlTag', array('tag' => 'pre', 'class' => 'log-output')), + ) + ) + ); + + if ($inspection->hasError()) { + $this->warning(sprintf( + $this->translate('Failed to successfully validate the configuration: %s'), + $inspection->getError() + )); + return false; + } + } + + $this->info($this->translate('The configuration has been successfully validated.')); + } + + return true; + } + + /** + * Add a submit button to this form and one to manually validate the configuration + * + * Calls parent::addSubmitButton() to add the submit button. + * + * @return $this + */ + public function addSubmitButton() + { + parent::addSubmitButton() + ->getElement('btn_submit') + ->setDecorators(array('ViewHelper')); + + $this->addElement( + 'submit', + 'backend_validation', + array( + 'ignore' => true, + 'label' => $this->translate('Validate Configuration'), + 'decorators' => array('ViewHelper') + ) + ); + $this->addDisplayGroup( + array('btn_submit', 'backend_validation'), + 'submit_validation', + array( + 'decorators' => array( + 'FormElements', + array('HtmlTag', array('tag' => 'div', 'class' => 'control-group')) + ) + ) + ); + + return $this; + } } diff --git a/modules/monitoring/application/forms/Setup/IdoResourcePage.php b/modules/monitoring/application/forms/Setup/IdoResourcePage.php index 7470e730a..ea244d057 100644 --- a/modules/monitoring/application/forms/Setup/IdoResourcePage.php +++ b/modules/monitoring/application/forms/Setup/IdoResourcePage.php @@ -4,6 +4,7 @@ namespace Icinga\Module\Monitoring\Forms\Setup; use Icinga\Data\ConfigObject; +use Icinga\Forms\Config\ResourceConfigForm; use Icinga\Forms\Config\Resource\DbResourceForm; use Icinga\Web\Form; use Icinga\Module\Monitoring\Forms\Config\BackendConfigForm; @@ -71,13 +72,17 @@ class IdoResourcePage extends Form } if (! isset($formData['skip_validation']) || !$formData['skip_validation']) { - $configObject = new ConfigObject($this->getValues()); - if (! DbResourceForm::isValidResource($this, $configObject)) { + $inspection = ResourceConfigForm::inspectResource($this); + if ($inspection !== null && $inspection->hasError()) { + $this->error($inspection->getError()); $this->addSkipValidationCheckbox($this->translate( 'Check this to not to validate connectivity with the given database server.' )); return false; - } elseif ( + } + + $configObject = new ConfigObject($this->getValues()); + if ( ! BackendConfigForm::isValidIdoSchema($this, $configObject) || !BackendConfigForm::isValidIdoInstance($this, $configObject) ) { diff --git a/modules/setup/application/forms/AuthBackendPage.php b/modules/setup/application/forms/AuthBackendPage.php index 06a2fec2c..ceeb08ed7 100644 --- a/modules/setup/application/forms/AuthBackendPage.php +++ b/modules/setup/application/forms/AuthBackendPage.php @@ -146,7 +146,9 @@ class AuthBackendPage extends Form 'value' => $this->getResourceConfig() ) ); - if (! UserBackendConfigForm::isValidUserBackend($self)) { + $inspection = UserBackendConfigForm::inspectUserBackend($self); + if ($inspection && $inspection->hasError()) { + $this->error($inspection->getError()); $this->addSkipValidationCheckbox(); return false; } diff --git a/modules/setup/application/forms/LdapResourcePage.php b/modules/setup/application/forms/LdapResourcePage.php index e7342185e..316b66f31 100644 --- a/modules/setup/application/forms/LdapResourcePage.php +++ b/modules/setup/application/forms/LdapResourcePage.php @@ -4,6 +4,7 @@ namespace Icinga\Module\Setup\Forms; use Icinga\Web\Form; +use Icinga\Forms\Config\ResourceConfigForm; use Icinga\Forms\Config\Resource\LdapResourceForm; /** @@ -65,12 +66,14 @@ class LdapResourcePage extends Form */ public function isValid($data) { - if (false === parent::isValid($data)) { + if (! parent::isValid($data)) { return false; } - if (false === isset($data['skip_validation']) || $data['skip_validation'] == 0) { - if (false === LdapResourceForm::isValidResource($this)) { + if (! isset($data['skip_validation']) || $data['skip_validation'] == 0) { + $inspection = ResourceConfigForm::inspectResource($this); + if ($inspection !== null && $inspection->hasError()) { + $this->error($inspection->getError()); $this->addSkipValidationCheckbox(); return false; } diff --git a/public/css/icinga/main-content.less b/public/css/icinga/main-content.less index c51ef8722..d0f182913 100644 --- a/public/css/icinga/main-content.less +++ b/public/css/icinga/main-content.less @@ -12,6 +12,15 @@ p code { padding: 0.3em; } +pre.log-output { + padding: 0.5em; + margin-top: 0; + margin-bottom: 2em; + max-height: 12em; + overflow: auto; + border: 1px solid #ddd; +} + a { color: #39a; } diff --git a/test/php/application/forms/Config/Resource/DbResourceFormTest.php b/test/php/application/forms/Config/Resource/DbResourceFormTest.php deleted file mode 100644 index 6aa51dbff..000000000 --- a/test/php/application/forms/Config/Resource/DbResourceFormTest.php +++ /dev/null @@ -1,78 +0,0 @@ -setUpResourceFactoryMock( - Mockery::mock()->shouldReceive('inspect')->andReturn(self::createInspector(false))->getMock() - ); - - $this->assertTrue( - DbResourceForm::isValidResource(new DbResourceForm()), - 'ResourceForm claims that a valid db resource is not valid' - ); - } - - /** - * @runInSeparateProcess - * @preserveGlobalState disabled - */ - public function testInvalidDbResourceIsNotValid() - { - $this->setUpResourceFactoryMock( - Mockery::mock()->shouldReceive('inspect')->andReturn(self::createInspector(true))->getMock() - ); - - $this->assertFalse( - DbResourceForm::isValidResource(new DbResourceForm()), - 'ResourceForm claims that an invalid db resource is valid' - ); - } - - protected function setUpResourceFactoryMock($resourceMock) - { - Mockery::mock('alias:Icinga\Data\ResourceFactory') - ->shouldReceive('createResource') - ->with(Mockery::type('Icinga\Data\ConfigObject')) - ->andReturn($resourceMock); - } - - public static function createInspector($error = false, $log = array('log')) - { - if (! $error) { - $calls = array( - 'hasError' => false, - 'toArray' => $log - ); - } else { - $calls = array( - 'hasError' => true, - 'getError' => 'Error', - 'toArray' => $log - ); - } - return Mockery::mock('Icinga\Data\Inspection', $calls); - } -} diff --git a/test/php/application/forms/Config/Resource/LdapResourceFormTest.php b/test/php/application/forms/Config/Resource/LdapResourceFormTest.php deleted file mode 100644 index bef49edca..000000000 --- a/test/php/application/forms/Config/Resource/LdapResourceFormTest.php +++ /dev/null @@ -1,96 +0,0 @@ -setUpResourceFactoryMock( - Mockery::mock()->shouldReceive('inspect')->andReturn(self::createInspector(false))->getMock() - ); - - // Passing array(null) is required to make Mockery call the constructor... - $form = Mockery::mock('Icinga\Forms\Config\Resource\LdapResourceForm[getView]', array(null)); - $form->shouldReceive('getView->escape') - ->with(Mockery::type('string')) - ->andReturnUsing(function ($s) { - return $s; - }); - $form->setTokenDisabled(); - - $this->assertTrue( - LdapResourceForm::isValidResource($form->create()), - 'ResourceForm claims that a valid ldap resource is not valid' - ); - } - - /** - * @runInSeparateProcess - * @preserveGlobalState disabled - */ - public function testInvalidLdapResourceIsNotValid() - { - $this->setUpResourceFactoryMock( - Mockery::mock()->shouldReceive('inspect')->andReturn(self::createInspector(true))->getMock() - ); - - // Passing array(null) is required to make Mockery call the constructor... - $form = Mockery::mock('Icinga\Forms\Config\Resource\LdapResourceForm[getView]', array(null)); - $form->shouldReceive('getView->escape') - ->with(Mockery::type('string')) - ->andReturnUsing(function ($s) { - return $s; - }); - $form->setTokenDisabled(); - - $this->assertFalse( - LdapResourceForm::isValidResource($form->create()), - 'ResourceForm claims that an invalid ldap resource is valid' - ); - } - - protected function setUpResourceFactoryMock($resourceMock) - { - Mockery::mock('alias:Icinga\Data\ResourceFactory') - ->shouldReceive('createResource') - ->with(Mockery::type('Icinga\Data\ConfigObject')) - ->andReturn($resourceMock); - } - - public static function createInspector($error = false, $log = array('log')) - { - if (! $error) { - $calls = array( - 'hasError' => false, - 'toArray' => $log - ); - } else { - $calls = array( - 'hasError' => true, - 'getError' => 'Error', - 'toArray' => $log - ); - } - return Mockery::mock('Icinga\Data\Inspection', $calls); - } -} diff --git a/test/php/application/forms/Config/Resource/LivestatusResourceFormTest.php b/test/php/application/forms/Config/Resource/LivestatusResourceFormTest.php deleted file mode 100644 index a634752de..000000000 --- a/test/php/application/forms/Config/Resource/LivestatusResourceFormTest.php +++ /dev/null @@ -1,62 +0,0 @@ -setUpResourceFactoryMock( - Mockery::mock()->shouldReceive('connect')->andReturn(Mockery::self()) - ->shouldReceive('disconnect')->getMock() - ); - - $this->assertTrue( - LivestatusResourceForm::isValidResource(new LivestatusResourceForm()), - 'ResourceForm claims that a valid livestatus resource is not valid' - ); - } - - /** - * @runInSeparateProcess - * @preserveGlobalState disabled - */ - public function testInvalidLivestatusResourceIsNotValid() - { - $this->setUpResourceFactoryMock( - Mockery::mock()->shouldReceive('connect')->once()->andThrow('\Exception')->getMock() - ); - - $this->assertFalse( - LivestatusResourceForm::isValidResource(new LivestatusResourceForm()), - 'ResourceForm claims that an invalid livestatus resource is valid' - ); - } - - protected function setUpResourceFactoryMock($resourceMock) - { - Mockery::mock('alias:Icinga\Data\ResourceFactory') - ->shouldReceive('createResource') - ->with(Mockery::type('Icinga\Data\ConfigObject')) - ->andReturn($resourceMock); - } -}