Refactor and test \Icinga\Web\Form

Fix inspection issues, added some other code for testing to
have more coverage, fix test for QLink view helper.

refs #4302
refs #4341

library/Icinga/Form/Builder.php

@@ -5,28 +5,43 @@
 namespace Icinga\Form;
 
 /**
-*   Class that helps building and validating forms and offers a rudimentary
+ * Class that helps building and validating forms and offers a rudimentary
-*   data-binding mechanismn. 
+ * data-binding mechanismn.
-* 
+ *
-*   The underlying form can be accessed by expicitly calling $builder->getForm() or
+ * The underlying form can be accessed by expicitly calling $builder->getForm() or
-*   by directly calling the forms method (which is, in case of populate() the preferred way)
+ * by directly calling the forms method (which is, in case of populate() the preferred way)
-*   like: $builder->getElements()
+ * like: $builder->getElements()
-*
+ *
-**/
+ * @method \Zend_Form_Element getElement(string $name)
+ * @method \Zend_Form addElement(\string $element, string $name = null, array $options = null)
+ * @method \Zend_Form setView(\Zend_View $view)
+ * @package Icinga\Form
+ */
 class Builder
 {
     const CSRF_ID = "icinga_csrf_id";
 
+    /**
+     * @var \Zend_Form
+     */
     private $form;
+
+    /**
+     * @var null
+     */
     private $boundModel = null;
+
+    /**
+     * @var bool
+     */
     private $disableCSRF = false;
 
     /**
     *   Constructrs a new Formbuilder, containing an empty form if no 
     *   $form parameter is given or the Zend form from the $form parameter.
     *   
-    *   @param  \Zend_Form      The form to use with this Builder
+    *   @param  \Zend_Form  $form       The form to use with this Builder
-    *   @param  Array           an optional array of Options:
+    *   @param  Array  $options         an optional array of Options:
     *               - CSRFProtection    true to add a crsf token to the
     *                                   form (default), false to remove it
     *               - model             An referenced array or object to use 
@@ -35,23 +50,37 @@ class Builder
     public function __construct(\Zend_Form $form = null, array $options = array())
     {
         if ($form === null) {
+            $myModel = array(
+                "username" => "",
+                "password" => ""
+            );
+
             $form = new \Zend_Form();
         }
             
-        $this->form = $form;
+        $this->setZendForm($form);
+
         if (isset($options["CSRFProtection"])) {
             $this->disableCSRF  = !$options["CSRFProtection"];
         }
         if (isset($options["model"])) {
-            $this->boundModel = &$options["model"];
+            $this->bindToModel($options["model"]);
         }
     }
 
+    /**
+     * Setter for Zend_Form
+     * @param \Zend_Form $form
+     */
     public function setZendForm(\Zend_Form $form)
     {
         $this->form = $form;
     }
 
+    /**
+     * Getter for Zent_Form
+     * @return \Zend_Form
+     */
     public function getForm()
     {
         if (!$this->disableCSRF) {
@@ -63,6 +92,10 @@ class Builder
         return $this->form;
     }
 
+    /**
+     * Add elements to form
+     * @param array $elements
+     */
     public function addElementsFromConfig(array $elements)
     {
         foreach ($elements as $key => $values) {
@@ -70,6 +103,12 @@ class Builder
         }
     }
 
+    /**
+     * Quick add elements to a new builder instance
+     * @param array $elements
+     * @param array $options
+     * @return Builder
+     */
     public static function fromArray(array $elements, $options = array())
     {
         $builder = new Builder(null, $options);
@@ -77,6 +116,12 @@ class Builder
         return $builder;
     }
 
+    /**
+     * Test that the form is valid
+     *
+     * @param array $data
+     * @return bool
+     */
     public function isValid(array $data = null)
     {
         if ($data === null) {
@@ -85,6 +130,11 @@ class Builder
         return $this->hasValidToken() && $this->form->isValid($data);
     }
 
+    /**
+     * Test if the form was submitted
+     * @param string $btnName
+     * @return bool
+     */
     public function isSubmitted($btnName = 'submit')
     {
         $btn = $this->getElement($btnName);
@@ -94,6 +144,10 @@ class Builder
         return $_POST[$btnName] === $btn->getLabel();
     }
 
+    /**
+     * Render the form
+     * @return string
+     */
     public function render()
     {
         return $this->getForm()->render();
@@ -104,11 +158,17 @@ class Builder
         return $this->getForm()->__toString();
     }
 
+    /**
+     * Enable CSRF token field
+     */
     public function enableCSRF()
     {
         $this->disableCSRF = false;
     }
 
+    /**
+     * Disable CSRF token field
+     */
     public function disableCSRF()
     {
         $this->disableCSRF = true;
@@ -116,6 +176,9 @@ class Builder
         $this->form->removeElement(self::CSRF_ID."_seed");
     }
 
+    /**
+     * Add CSRF field to form
+     */
     private function addCSRFFieldToForm()
     {
         if (!$this->form || $this->disableCSRF || $this->form->getElement(self::CSRF_ID)) {
@@ -135,11 +198,18 @@ class Builder
 
     }
 
+    /**
+     * Bind model to a form
+     * @param $model
+     */
     public function bindToModel(&$model)
     {
         $this->boundModel = &$model;
     }
 
+    /**
+     * Repopulate
+     */
     public function repopulate()
     {
         if (!empty($_POST)) {
@@ -147,6 +217,12 @@ class Builder
         }
     }
 
+    /**
+     * Populate form
+     * @param $data
+     * @param bool $ignoreModel
+     * @throws \InvalidArgumentException
+     */
     public function populate($data, $ignoreModel = false)
     {
         if (is_array($data)) {
@@ -154,7 +230,7 @@ class Builder
         } elseif (is_object($data)) {
             $this->populateFromObject($data);
         } else {
-            throw new InvalidArgumentException("Builder::populate() expects and object or array, $data given");
+            throw new \InvalidArgumentException("Builder::populate() expects and object or array, $data given");
         }
         if ($this->boundModel === null || $ignoreModel) {
             return;
@@ -163,8 +239,14 @@ class Builder
         
     }
 
+    /**
+     * Populate form object
+     * @param $data
+     */
     private function populateFromObject($data)
     {
+        /** @var \Zend_Form_Element $element */
+
         foreach ($this->form->getElements() as $name => $element) {
             if (isset($data->$name)) {
                 $element->setValue($data->$name);
@@ -178,6 +260,9 @@ class Builder
         }
     }
 
+    /**
+     * Update model instance
+     */
     public function updateModel()
     {
         if (is_array($this->boundModel)) {
@@ -187,8 +272,13 @@ class Builder
         }
     }
 
+    /**
+     * Updater for objects
+     */
     private function updateObjectModel()
     {
+        /** @var \Zend_Form_Element $element */
+
         foreach ($this->form->getElements() as $name => $element) {
             if (isset($this->boundModel->$name)) {
                 $this->boundModel->$name = $element->getValue();
@@ -202,8 +292,13 @@ class Builder
         }
     }
 
+    /**
+     * Updater for arrays
+     */
     private function updateArrayModel()
     {
+        /** @var \Zend_Form_Element $element */
+
         foreach ($this->form->getElements() as $name => $element) {
             if (isset($this->boundModel[$name])) {
                 $this->boundModel[$name] = $element->getValue();
@@ -211,11 +306,21 @@ class Builder
         }
     }
 
+    /**
+     * Synchronize model with form
+     */
     public function syncWithModel()
     {
         $this->populate($this->boundModel, true);
     }
 
+    /**
+     * Magic caller, pass through method calls to form
+     * @param $fn
+     * @param array $args
+     * @return mixed
+     * @throws \BadMethodCallException
+     */
     public function __call($fn, array $args)
     {
         if (method_exists($this->form, $fn)) {
@@ -235,7 +340,7 @@ class Builder
      * @param int    $maxAge    Max allowed token age
      * @param string $sessionId A specific session id (useful for tests?)
      *
-     * return bool
+     * @return bool
      */
     public function hasValidToken($maxAge = 600, $sessionId = null)
     {
@@ -265,7 +370,7 @@ class Builder
      * @param int    $maxAge    Max allowed token age
      * @param string $sessionId A specific session id (useful for tests?)
      *
-     * return array
+     * @return array
      */
     public function getSeedTokenPair($maxAge = 600, $sessionId = null)
     {

test/php/application/views/helpers/QlinkTest.php

@@ -1,19 +1,8 @@
 <?php
 
-if(!class_exists('Zend_View_Helper_Abstract')) {
-    abstract class Zend_View_Helper_Abstract {
-        public $basename;
 
-        public function __construct($basename = '') {
+require_once('Zend/View/Helper/Abstract.php');
-            $this->view = $this;
+require_once('Zend/View.php');
-            $this->basename = $basename;
-        }
-
-        public function baseUrl($url) {
-            return $this->basename.$url;
-        }
-    };
-}
 
 require('../../application/views/helpers/Qlink.php');
 
@@ -30,30 +19,37 @@ class Zend_View_Helper_QlinkTest extends \PHPUnit_Framework_TestCase
 
     public function testURLPathParameter()
     {
+        $view = new Zend_View();
+
         $helper = new Zend_View_Helper_Qlink();
-        $pathTpl = "path/%s/to/%s";
+        $helper->setView($view);
+        $pathTpl = "/path/%s/to/%s";
         $this->assertEquals(
-            "path/param1/to/param2",
+            "/path/param1/to/param2",
             $helper->getFormattedURL($pathTpl,array('param1','param2'))
         );
     }
 
     public function testUrlGETParameter()
     {
+        $view = new Zend_View();
         $helper = new Zend_View_Helper_Qlink();
+        $helper->setView($view);
         $pathTpl = 'path';
         $this->assertEquals(
-            'path?param1=value1&amp;param2=value2',
+            '/path?param1=value1&amp;param2=value2',
             $helper->getFormattedURL($pathTpl,array('param1'=>'value1','param2'=>'value2'))
         );
     }
 
     public function testMixedParameters()
     {
+        $view = new Zend_View();
         $helper = new Zend_View_Helper_Qlink();
+        $helper->setView($view);
         $pathTpl = 'path/%s/to/%s';
         $this->assertEquals(
-            'path/path1/to/path2?param1=value1&amp;param2=value2',
+            '/path/path1/to/path2?param1=value1&amp;param2=value2',
             $helper->getFormattedURL($pathTpl,array(
                 'path1','path2',
                 'param1'=>'value1',

test/php/library/Icinga/Form/BuilderTest.php

@@ -8,6 +8,23 @@ require_once("../../library/Icinga/Form/Builder.php");
 
 use Icinga\Form\Builder as Builder;
 
+class BuilderTestModel
+{
+    public $username = '';
+    public $password = '';
+    private $test;
+
+    public function getTest()
+    {
+        return $this->test;
+    }
+
+    public function setTest($test)
+    {
+        $this->test = $test;
+    }
+}
+
 class BuilderTest extends \PHPUnit_Framework_TestCase
 {
     /**
@@ -118,13 +135,23 @@ class BuilderTest extends \PHPUnit_Framework_TestCase
     public function testModelBindingWithArray()
     {
         $view = new \Zend_View();
-        $builder = new Builder(null, array("CSRFProtection" => false));
+
-        $builder->setView($view);
         $myModel = array(
             "username" => "",
             "password" => ""
         );
-        $builder->bindToModel($myModel);
+
+        $builder = new Builder(
+            null,
+            array(
+                "CSRFProtection" => false,
+                "model" => &$myModel
+            )
+        );
+
+        $builder->setView($view);
+
+        // $builder->bindToModel($myModel);
         $builder->addElement("text", "username");
         $builder->addElement("password", "password");
         // test sync from form to model
@@ -155,22 +182,26 @@ class BuilderTest extends \PHPUnit_Framework_TestCase
         $view = new \Zend_View();
         $builder = new Builder(null, array("CSRFProtection" => false));
         $builder->setView($view);
-        $myModel = (object) array(
+
-            "username" => "",
+
-            "password" => ""
+
-        );
+        $myModel = new BuilderTestModel();
+
         $builder->bindToModel($myModel);
         $builder->addElement("text", "username");
         $builder->addElement("password", "password");
+        $builder->addElement("text", "test");
         // test sync from form to model
         $builder->populate(
             (object) array(
                 "username" => "User input<html>",
-                "password" => "Secret$123"
+                "password" => "Secret$123",
+                "test" => 'test334'
             )
         );
         $this->assertEquals("User input<html>", $myModel->username);
         $this->assertEquals("Secret$123", $myModel->password);
+        $this->assertEquals("test334", $myModel->getTest());
 
         // test sync from model to form
         $myModel->username = "Another user";
@@ -180,4 +211,14 @@ class BuilderTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals("Another user", $builder->getElement("username")->getValue());
         $this->assertEquals("Another pass", $builder->getElement("password")->getValue());
     }
+
+    /**
+     * @expectedException \BadMethodCallException
+     * @expectedExceptionMessage Method doesNotExist123 does not exist either in \Icinga\Form\Builder nor in Zend_Form
+     */
+    public function testBadCall1()
+    {
+        $builder = new Builder(null, array("CSRFProtection" => false));
+        $builder->doesNotExist123();
+    }
 }