From 8d1038e6221ab36ff9f06a1582e5cc3f9bf20534 Mon Sep 17 00:00:00 2001 From: Marius Hein Date: Tue, 25 Jun 2013 09:43:55 +0200 Subject: [PATCH] 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 | 145 +++++++++++++++--- .../application/views/helpers/QlinkTest.php | 30 ++-- test/php/library/Icinga/Form/BuilderTest.php | 57 ++++++- 3 files changed, 187 insertions(+), 45 deletions(-) diff --git a/library/Icinga/Form/Builder.php b/library/Icinga/Form/Builder.php index 4526f7624..cc2b08591 100644 --- a/library/Icinga/Form/Builder.php +++ b/library/Icinga/Form/Builder.php @@ -5,28 +5,43 @@ namespace Icinga\Form; /** -* Class that helps building and validating forms and offers a rudimentary -* data-binding mechanismn. -* -* 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) -* like: $builder->getElements() -* -**/ + * Class that helps building and validating forms and offers a rudimentary + * data-binding mechanismn. + * + * 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) + * 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 Array an optional array of Options: + * @param \Zend_Form $form The form to use with this Builder + * @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) { @@ -62,7 +91,11 @@ 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(); @@ -103,12 +157,18 @@ 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) { diff --git a/test/php/application/views/helpers/QlinkTest.php b/test/php/application/views/helpers/QlinkTest.php index ad4c5e3be..7e8030ee8 100755 --- a/test/php/application/views/helpers/QlinkTest.php +++ b/test/php/application/views/helpers/QlinkTest.php @@ -1,19 +1,8 @@ view = $this; - $this->basename = $basename; - } - - public function baseUrl($url) { - return $this->basename.$url; - } - }; -} +require_once('Zend/View/Helper/Abstract.php'); +require_once('Zend/View.php'); 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&param2=value2', + '/path?param1=value1&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&param2=value2', + '/path/path1/to/path2?param1=value1&param2=value2', $helper->getFormattedURL($pathTpl,array( 'path1','path2', 'param1'=>'value1', diff --git a/test/php/library/Icinga/Form/BuilderTest.php b/test/php/library/Icinga/Form/BuilderTest.php index 8cae3062b..ede5919fc 100644 --- a/test/php/library/Icinga/Form/BuilderTest.php +++ b/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", - "password" => "Secret$123" + "password" => "Secret$123", + "test" => 'test334' ) ); $this->assertEquals("User input", $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(); + } }