ResourceConfigForm: Replace isValid- with inspectResource()

refs #7588

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();
-    }
 }

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();
-    }
 }

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\Web\Form;
-use Icinga\Exception\ConfigurationError;
+use Icinga\Web\Notification;
 
 class ResourceConfigForm extends ConfigForm
 {
@@ -141,7 +146,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 +262,21 @@ 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();
+            }
+        }
+    }
 }

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());
+            $inspection = ResourceConfigForm::inspectResource($this);
-            if (! DbResourceForm::isValidResource($this, $configObject)) {
+            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)
             ) {

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 (! isset($data['skip_validation']) || $data['skip_validation'] == 0) {
-            if (false === LdapResourceForm::isValidResource($this)) {
+            $inspection = ResourceConfigForm::inspectResource($this);
+            if ($inspection !== null && $inspection->hasError()) {
+                $this->error($inspection->getError());
                 $this->addSkipValidationCheckbox();
                 return false;
             }

test/php/application/forms/Config/Resource/DbResourceFormTest.php

@@ -1,78 +0,0 @@
-<?php
-/* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
-
-namespace Tests\Icinga\Forms\Config\Resource;
-
-// Necessary as some of these tests disable phpunit's preservation
-// of the global state (e.g. autoloaders are in the global state)
-require_once realpath(dirname(__FILE__) . '/../../../../bootstrap.php');
-
-use Mockery;
-use Icinga\Test\BaseTestCase;
-use Icinga\Forms\Config\Resource\DbResourceForm;
-
-class DbResourceFormTest extends BaseTestCase
-{
-    public function tearDown()
-    {
-        parent::tearDown();
-        Mockery::close(); // Necessary because some tests run in a separate process
-    }
-
-    /**
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
-     */
-    public function testValidDbResourceIsValid()
-    {
-        $this->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);
-    }
-}

test/php/application/forms/Config/Resource/LdapResourceFormTest.php

@@ -1,96 +0,0 @@
-<?php
-/* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
-
-namespace Tests\Icinga\Forms\Config\Resource;
-
-// Necessary as some of these tests disable phpunit's preservation
-// of the global state (e.g. autoloaders are in the global state)
-require_once realpath(dirname(__FILE__) . '/../../../../bootstrap.php');
-
-use Mockery;
-use Icinga\Test\BaseTestCase;
-use Icinga\Forms\Config\Resource\LdapResourceForm;
-
-class LdapResourceFormTest extends BaseTestCase
-{
-    public function tearDown()
-    {
-        parent::tearDown();
-        Mockery::close(); // Necessary because some tests run in a separate process
-    }
-
-    /**
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
-     */
-    public function testValidLdapResourceIsValid()
-    {
-        $this->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);
-    }
-}

test/php/application/forms/Config/Resource/LivestatusResourceFormTest.php

@@ -1,62 +0,0 @@
-<?php
-/* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
-
-namespace Tests\Icinga\Forms\Config\Resource;
-
-// Necessary as some of these tests disable phpunit's preservation
-// of the global state (e.g. autoloaders are in the global state)
-require_once realpath(dirname(__FILE__) . '/../../../../bootstrap.php');
-
-use Mockery;
-use Icinga\Test\BaseTestCase;
-use Icinga\Forms\Config\Resource\LivestatusResourceForm;
-
-class LivestatusResourceFormTest extends BaseTestCase
-{
-    public function tearDown()
-    {
-        parent::tearDown();
-        Mockery::close(); // Necessary because some tests run in a separate process
-    }
-
-    /**
-     * @runInSeparateProcess
-     * @preserveGlobalState disabled
-     */
-    public function testValidLivestatusResourceIsValid()
-    {
-        $this->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);
-    }
-}