From 5cddbd1162d5f057a9e1ee8111c89a2d1a8705eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Mon, 24 Jun 2013 18:40:37 +0200 Subject: [PATCH 01/10] Add Form\Builder The builder class allows to create forms and bind them to models. The tests are currently failing because of the zend pluginloader, this must be evaluated refs #4302 --- library/Icinga/Form/Builder.php | 280 +++++++++++++++++++ test/php/library/Icinga/Form/BuilderTest.php | 183 ++++++++++++ 2 files changed, 463 insertions(+) create mode 100644 library/Icinga/Form/Builder.php create mode 100644 test/php/library/Icinga/Form/BuilderTest.php diff --git a/library/Icinga/Form/Builder.php b/library/Icinga/Form/Builder.php new file mode 100644 index 000000000..4526f7624 --- /dev/null +++ b/library/Icinga/Form/Builder.php @@ -0,0 +1,280 @@ +getForm() or +* by directly calling the forms method (which is, in case of populate() the preferred way) +* like: $builder->getElements() +* +**/ +class Builder +{ + const CSRF_ID = "icinga_csrf_id"; + + private $form; + private $boundModel = null; + 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: + * - CSRFProtection true to add a crsf token to the + * form (default), false to remove it + * - model An referenced array or object to use + * for value binding + **/ + public function __construct(\Zend_Form $form = null, array $options = array()) + { + if ($form === null) { + $form = new \Zend_Form(); + } + + $this->form = $form; + if (isset($options["CSRFProtection"])) { + $this->disableCSRF = !$options["CSRFProtection"]; + } + if (isset($options["model"])) { + $this->boundModel = &$options["model"]; + } + } + + public function setZendForm(\Zend_Form $form) + { + $this->form = $form; + } + + public function getForm() + { + if (!$this->disableCSRF) { + $this->addCSRFFieldToForm(); + } + if (!$this->form) { + return new \Zend_Form(); + } + return $this->form; + } + + public function addElementsFromConfig(array $elements) + { + foreach ($elements as $key => $values) { + $this->addElement($values[0], $key, $values[1]); + } + } + + public static function fromArray(array $elements, $options = array()) + { + $builder = new Builder(null, $options); + $builder->addElementsFromConfig($elements); + return $builder; + } + + public function isValid(array $data = null) + { + if ($data === null) { + $data = $_POST; + } + return $this->hasValidToken() && $this->form->isValid($data); + } + + public function isSubmitted($btnName = 'submit') + { + $btn = $this->getElement($btnName); + if (!$btn || !isset($_POST[$btnName])) { + return false; + } + return $_POST[$btnName] === $btn->getLabel(); + } + + public function render() + { + return $this->getForm()->render(); + } + + public function __toString() + { + return $this->getForm()->__toString(); + } + + public function enableCSRF() + { + $this->disableCSRF = false; + } + + public function disableCSRF() + { + $this->disableCSRF = true; + $this->form->removeElement(self::CSRF_ID); + $this->form->removeElement(self::CSRF_ID."_seed"); + } + + private function addCSRFFieldToForm() + { + if (!$this->form || $this->disableCSRF || $this->form->getElement(self::CSRF_ID)) { + return; + } + list($seed, $token) = $this->getSeedTokenPair(); + + $this->form->addElement("hidden", self::CSRF_ID); + $this->form->getElement(self::CSRF_ID) + ->setValue($token) + ->setDecorators(array('ViewHelper')); + + $this->form->addElement("hidden", self::CSRF_ID."_seed"); + $this->form->getElement(self::CSRF_ID."_seed") + ->setValue($seed) + ->setDecorators(array('ViewHelper')); + + } + + public function bindToModel(&$model) + { + $this->boundModel = &$model; + } + + public function repopulate() + { + if (!empty($_POST)) { + $this->populate($_POST); + } + } + + public function populate($data, $ignoreModel = false) + { + if (is_array($data)) { + $this->form->populate($data); + } elseif (is_object($data)) { + $this->populateFromObject($data); + } else { + throw new InvalidArgumentException("Builder::populate() expects and object or array, $data given"); + } + if ($this->boundModel === null || $ignoreModel) { + return; + } + $this->updateModel(); + + } + + private function populateFromObject($data) + { + foreach ($this->form->getElements() as $name => $element) { + if (isset($data->$name)) { + $element->setValue($data->$name); + + } else { + $getter = "get".ucfirst($name); + if (method_exists($data, $getter)) { + $element->setValue($data->$getter()); + } + } + } + } + + public function updateModel() + { + if (is_array($this->boundModel)) { + $this->updateArrayModel(); + } elseif (is_object($this->boundModel)) { + $this->updateObjectModel(); + } + } + + private function updateObjectModel() + { + foreach ($this->form->getElements() as $name => $element) { + if (isset($this->boundModel->$name)) { + $this->boundModel->$name = $element->getValue(); + } else { + $setter = "set".ucfirst($name); + if (method_exists($this->boundModel, $setter)) { + $this->boundModel->$setter($element->getValue()); + } + + } + } + } + + private function updateArrayModel() + { + foreach ($this->form->getElements() as $name => $element) { + if (isset($this->boundModel[$name])) { + $this->boundModel[$name] = $element->getValue(); + } + } + } + + public function syncWithModel() + { + $this->populate($this->boundModel, true); + } + + public function __call($fn, array $args) + { + if (method_exists($this->form, $fn)) { + return call_user_func_array(array($this->form, $fn), $args); + } else { + throw new \BadMethodCallException( + "Method $fn does not exist either ". + "in \Icinga\Form\Builder nor in Zend_Form" + ); + } + } + + + /** + * Whether the token parameter is valid + * + * @param int $maxAge Max allowed token age + * @param string $sessionId A specific session id (useful for tests?) + * + * return bool + */ + public function hasValidToken($maxAge = 600, $sessionId = null) + { + if ($this->disableCSRF) { + return true; + } + + if ($this->form->getElement(self::CSRF_ID) == null) { + return false; + } + + $sessionId = $sessionId ? $sessionId : session_id(); + $seed = $this->form->getElement(self::CSRF_ID.'_seed')->getValue(); + if (! is_numeric($seed)) { + return false; + } + + // Remove quantitized timestamp portion so maxAge applies + $seed -= (intval(time() / $maxAge) * $maxAge); + $token = $this->getElement(self::CSRF_ID)->getValue(); + return $token === hash('sha256', $sessionId . $seed); + } + + /** + * Get a new seed/token pair + * + * @param int $maxAge Max allowed token age + * @param string $sessionId A specific session id (useful for tests?) + * + * return array + */ + public function getSeedTokenPair($maxAge = 600, $sessionId = null) + { + $sessionId = $sessionId ? $sessionId : session_id(); + $seed = mt_rand(); + $hash = hash('sha256', $sessionId . $seed); + + // Add quantitized timestamp portion to apply maxAge + $seed += (intval(time() / $maxAge) * $maxAge); + return array($seed, $hash); + } +} diff --git a/test/php/library/Icinga/Form/BuilderTest.php b/test/php/library/Icinga/Form/BuilderTest.php new file mode 100644 index 000000000..8cae3062b --- /dev/null +++ b/test/php/library/Icinga/Form/BuilderTest.php @@ -0,0 +1,183 @@ + false)); + $this->assertInstanceOf("Zend_Form", $builder->getForm()); + } + + /** + * + **/ + public function testCSRFProtectionTokenCreation() + { + $view = new \Zend_View(); + $builder = new Builder(); // when no token is given, a CRSF field should be added + $builder->setView($view); + + $DOM = new \DOMDocument; + $DOM->loadHTML($builder); + $this->assertNotNull($DOM->getElementById(Builder::CSRF_ID)); + + $builder->disableCSRF(); + $DOM->loadHTML($builder); + $this->assertNull($DOM->getElementById(Builder::CSRF_ID)); + + } + /** + * Test whether form methods are passed to the Zend_Form object + * When called in the Builder instance + * + **/ + public function testMethodPassing() + { + $DOM = new \DOMDocument; + $view = new \Zend_View(); + $builder = new Builder(null, array("CSRFProtection" => false)); + $builder->setView($view); + + $DOM->loadHTML($builder); + $this->assertEquals(0, $DOM->getElementsByTagName("input")->length); + + $builder->addElement("text", "username"); + $DOM->loadHTML($builder); + $inputEls = $DOM->getElementsByTagName("input"); + $this->assertEquals(1, $inputEls->length); + $this->assertEquals("username", $inputEls->item(0)->attributes->getNamedItem("name")->value); + } + /** + * + * + **/ + public function testCreateByArray() + { + $DOM = new \DOMDocument; + $view = new \Zend_View(); + $builder = Builder::fromArray( + array( + 'username' => array( + 'text', + array( + 'label' => 'Username', + 'required' => true, + ) + ), + 'password' => array( + 'password', + array( + 'label' => 'Password', + 'required' => true, + ) + ), + 'submit' => array( + 'submit', + array( + 'label' => 'Login' + ) + ) + ), + array( + "CSRFProtection" => false + ) + ); + $builder->setView($view); + + $DOM->loadHTML($builder); + $inputEls = $DOM->getElementsByTagName("input"); + $this->assertEquals(3, $inputEls->length); + + $username = $inputEls->item(0); + $this->assertEquals("username", $username->attributes->getNamedItem("name")->value); + + $password= $inputEls->item(1); + $this->assertEquals("password", $password->attributes->getNamedItem("name")->value); + $this->assertEquals("password", $password->attributes->getNamedItem("type")->value); + + $submitBtn= $inputEls->item(2); + $this->assertEquals("submit", $submitBtn->attributes->getNamedItem("name")->value); + $this->assertEquals("submit", $submitBtn->attributes->getNamedItem("type")->value); + } + + /** + * + * + */ + 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->addElement("text", "username"); + $builder->addElement("password", "password"); + // test sync from form to model + $builder->populate( + array( + "username" => "User input", + "password" => "Secret$123" + ) + ); + $this->assertEquals("User input", $myModel["username"]); + $this->assertEquals("Secret$123", $myModel["password"]); + + // test sync from model to form + $myModel["username"] = "Another user"; + $myModel["password"] = "Another pass"; + + $builder->syncWithModel(); + $this->assertEquals("Another user", $builder->getElement("username")->getValue()); + $this->assertEquals("Another pass", $builder->getElement("password")->getValue()); + } + + /** + * + * + */ + public function testModelBindingWithObject() + { + $view = new \Zend_View(); + $builder = new Builder(null, array("CSRFProtection" => false)); + $builder->setView($view); + $myModel = (object) array( + "username" => "", + "password" => "" + ); + $builder->bindToModel($myModel); + $builder->addElement("text", "username"); + $builder->addElement("password", "password"); + // test sync from form to model + $builder->populate( + (object) array( + "username" => "User input", + "password" => "Secret$123" + ) + ); + $this->assertEquals("User input", $myModel->username); + $this->assertEquals("Secret$123", $myModel->password); + + // test sync from model to form + $myModel->username = "Another user"; + $myModel->password = "Another pass"; + + $builder->syncWithModel(); + $this->assertEquals("Another user", $builder->getElement("username")->getValue()); + $this->assertEquals("Another pass", $builder->getElement("password")->getValue()); + } +} From 8d1038e6221ab36ff9f06a1582e5cc3f9bf20534 Mon Sep 17 00:00:00 2001 From: Marius Hein Date: Tue, 25 Jun 2013 09:43:55 +0200 Subject: [PATCH 02/10] 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(); + } } From 0f8648dcb1a3cc27f7fcc6a060b4cc285dc53e68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Thu, 20 Jun 2013 13:47:51 +0200 Subject: [PATCH 03/10] Add SystemPermissionException This exception should be fired when the OS restricts the web user from executing certain actions. Also the template for errors has been modified so it displays the action and the target that was prevented. refs #4092 --- application/controllers/ErrorController.php | 3 +-- application/views/scripts/error/error.phtml | 11 ++++++++-- .../Exception/SystemPermissionException.php | 22 +++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 library/Icinga/Exception/SystemPermissionException.php diff --git a/application/controllers/ErrorController.php b/application/controllers/ErrorController.php index efcc20e9b..c523bb77f 100755 --- a/application/controllers/ErrorController.php +++ b/application/controllers/ErrorController.php @@ -34,7 +34,6 @@ class ErrorController extends ActionController $this->view->message = 'Application error'; break; } - // conditionally display exceptions if ($this->getInvokeArg('displayExceptions') == true) { $this->view->exception = $errors->exception; @@ -44,4 +43,4 @@ class ErrorController extends ActionController } } -// @codingStandardsIgnoreEnd \ No newline at end of file +// @codingStandardsIgnoreEnd diff --git a/application/views/scripts/error/error.phtml b/application/views/scripts/error/error.phtml index 9ae6ecd1f..b5cfcddcb 100755 --- a/application/views/scripts/error/error.phtml +++ b/application/views/scripts/error/error.phtml @@ -9,8 +9,15 @@

Exception information:

-

- Message: exception->getMessage() ?> +

+ Message: exception->getMessage() ?> + exception->action)): ?> +
Action: exception->action; ?> + + exception->action)): ?> +
Target: exception->target; ?> + +

Stack trace:

diff --git a/library/Icinga/Exception/SystemPermissionException.php b/library/Icinga/Exception/SystemPermissionException.php new file mode 100644 index 000000000..02849ec34 --- /dev/null +++ b/library/Icinga/Exception/SystemPermissionException.php @@ -0,0 +1,22 @@ +action = $action; + $this->target = $target; + } +} From 81e27689bb79c0901975bb24bdfab9eb6605c6cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Thu, 20 Jun 2013 14:02:58 +0200 Subject: [PATCH 04/10] Add rule for 'body' container and QLink property 'target' QLinks can now define the target for their link. Additionally, the targte 'body' is now always the tag. This allows the modulemanager to reload the whole page (with the navigation bar). refs #4092 --- .../layouts/scripts/just-the-body.phtml | 8 --- application/layouts/scripts/layout.phtml | 4 +- .../layouts/scripts/parts/navigation.phtml | 3 +- application/views/helpers/Qlink.php | 5 +- public/js/icinga/modules/mainDetail.js | 71 ++++++++++--------- public/js/icinga/util/async.js | 8 +-- 6 files changed, 49 insertions(+), 50 deletions(-) diff --git a/application/layouts/scripts/just-the-body.phtml b/application/layouts/scripts/just-the-body.phtml index e243e63ed..ee29c44e6 100755 --- a/application/layouts/scripts/just-the-body.phtml +++ b/application/layouts/scripts/just-the-body.phtml @@ -12,14 +12,6 @@
-
- - - - - - -
diff --git a/application/layouts/scripts/layout.phtml b/application/layouts/scripts/layout.phtml index cfe89626c..d70627d1b 100755 --- a/application/layouts/scripts/layout.phtml +++ b/application/layouts/scripts/layout.phtml @@ -21,12 +21,12 @@ ICINGA_DEBUG = true; - + - + render('parts/topbar.phtml') ?>
diff --git a/application/layouts/scripts/parts/navigation.phtml b/application/layouts/scripts/parts/navigation.phtml index 1184d1830..e7de8f369 100755 --- a/application/layouts/scripts/parts/navigation.phtml +++ b/application/layouts/scripts/parts/navigation.phtml @@ -5,7 +5,8 @@ $item = $this->navigation->listAll("menu"); ?> -auth()->isAuthenticated()): ?> +auth()->isAuthenticated()): ?> +
    view->baseUrl($url) : $baseUrl . $url; } } -// @codingStandardsIgnoreEnd \ No newline at end of file +// @codingStandardsIgnoreEnd diff --git a/public/js/icinga/modules/mainDetail.js b/public/js/icinga/modules/mainDetail.js index 9a207c933..368c5c5f7 100755 --- a/public/js/icinga/modules/mainDetail.js +++ b/public/js/icinga/modules/mainDetail.js @@ -16,45 +16,52 @@ define(['jquery','logging','icinga/util/async'],function($,log,async) { href = a.attr("href"); ev.stopImmediatePropagation(); collapseDetailView(); + async.loadToTarget("icinga-main",href); + return false; +}; + +var onLinkTagClick = function(ev) { + + var a = $(ev.currentTarget), + target = a.attr("target"), + href = a.attr("href"); + + // check for protocol:// + if(/^[A-Z]{2,10}\:\/\//i.test(href)) { + window.open(href); + ev.stopImmediatePropagation(); + return false; + } + + // check for link in table header + if(a.parents('th').length > 0) { + ev.stopImmediatePropagation(); + return false; + } + + if(typeof target === "undefined") { + if(a.parents("#icinga-detail").length) { + log.debug("Parent is detail, loading into detail"); + async.loadToTarget("icinga-detail",href); + } else { + log.debug("Parent is not detail, loading into main"); async.loadToTarget("icinga-main",href); - return false; - }; - - var onLinkTagClick = function(ev) { - - var a = $(ev.currentTarget), - target = a.attr("target"), - href = a.attr("href"); - - // check for protocol:// - if(/^[A-Z]{2,10}\:\/\//i.test(href)) { - window.open(href); - ev.stopImmediatePropagation(); - return false; - } - - // check for link in table header - if(a.parents('th').length > 0) { - ev.stopImmediatePropagation(); - return false; - } - - if(typeof target === "undefined") { - if(a.parents("#icinga-detail").length) { - async.loadToTarget("icinga-detail",href); - } else { - async.loadToTarget("icinga-main",href); - } - } else { - switch(target) { + } + } else { + switch(target) { + case "body": + async.loadToTarget("body", href); + break; case "main": async.loadToTarget("icinga-main",href); collapseDetailView(); break; case "detail": + log.debug("Target: detail"); async.loadToTarget("icinga-detail",href); break; case "popup": + log.debug("No target"); async.loadToTarget(null,href); break; default: @@ -67,7 +74,7 @@ define(['jquery','logging','icinga/util/async'],function($,log,async) { var expandDetailView = function() { $("#icinga-detail").parents(".collapsed").removeClass('collapsed'); - }; + }; var collapseDetailView = function(elementInDetailView) { $("#icinga-detail").parents(".layout-main-detail").addClass('collapsed'); @@ -88,7 +95,5 @@ define(['jquery','logging','icinga/util/async'],function($,log,async) { } }; }; - return new MainDetailBehaviour(); }); - diff --git a/public/js/icinga/util/async.js b/public/js/icinga/util/async.js index efab52ed7..8e015b45a 100644 --- a/public/js/icinga/util/async.js +++ b/public/js/icinga/util/async.js @@ -5,11 +5,9 @@ define(['icinga/container','logging','icinga/behaviour','jquery'],function(containerMgr,log,behaviour,$) { - var pending = { }; - var getDOMForDestination = function(destination) { var target = destination; if(typeof destination === "string") { @@ -84,13 +82,14 @@ this.loadToTarget = function(destination,url,data) { if(destination) { + log.debug("Laoding to container", destination, url); this.clearPendingRequestsFor(destination); } var req = this.createRequest(url,data); if(destination) { pending.push({ request: req, - DOM: getDOMForDestination(destination) + DOM: getDOMForDestination(destination) }); req.destination = destination; } @@ -105,5 +104,4 @@ }; return new CallInterface(); }); - -})(); \ No newline at end of file +})(); From 04a23f0096d69178487ec885d818501cbfd5413e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Thu, 20 Jun 2013 14:06:02 +0200 Subject: [PATCH 05/10] Move ModuleController from incubator The Notification dependency is removed and uses exceptions now, otherwise it's mostly the same like in the incubator refs #4092 --- application/controllers/ModulesController.php | 47 ++++++++++++++ application/views/scripts/inline.phtml | 3 + .../views/scripts/mixedPagination.phtml | 61 +++++++++++++++++++ .../views/scripts/modules/overview.phtml | 38 ++++++++++++ application/views/scripts/modules/show.phtml | 24 ++++++++ .../Application/ApplicationBootstrap.php | 3 +- .../Icinga/Application/Modules/Manager.php | 54 +++++++++++----- public/js/icinga/container.js | 7 ++- public/js/icinga/util/async.js | 4 +- 9 files changed, 221 insertions(+), 20 deletions(-) create mode 100755 application/controllers/ModulesController.php create mode 100644 application/views/scripts/inline.phtml create mode 100755 application/views/scripts/mixedPagination.phtml create mode 100644 application/views/scripts/modules/overview.phtml create mode 100644 application/views/scripts/modules/show.phtml diff --git a/application/controllers/ModulesController.php b/application/controllers/ModulesController.php new file mode 100755 index 000000000..f8f40a3b3 --- /dev/null +++ b/application/controllers/ModulesController.php @@ -0,0 +1,47 @@ +manager = Icinga::app()->moduleManager(); + } + + public function indexAction() + { + $this->view->modules = $this->manager->select() + ->from('modules') + ->order('name'); + $this->render('overview'); + } + + public function overviewAction() + { + $this->indexAction(); + + } + + public function enableAction() + { + $this->manager->enableModule($this->_getParam('name')); + $this->redirectNow('modules/overview?_render=body'); + } + + public function disableAction() + { + $this->manager->disableModule($this->_getParam('name')); + $this->redirectNow('modules/overview?_render=body'); + } + +} diff --git a/application/views/scripts/inline.phtml b/application/views/scripts/inline.phtml new file mode 100644 index 000000000..24e5f00ce --- /dev/null +++ b/application/views/scripts/inline.phtml @@ -0,0 +1,3 @@ +layout()->moduleStart ?> +layout()->content ?> +layout()->moduleEnd ?> diff --git a/application/views/scripts/mixedPagination.phtml b/application/views/scripts/mixedPagination.phtml new file mode 100755 index 000000000..7b2b23570 --- /dev/null +++ b/application/views/scripts/mixedPagination.phtml @@ -0,0 +1,61 @@ +pageCount > 1): ?> +preserve)) { + $preserve = $this->preserve; +} else { + $preserve = array(); +} + +$fromto = t('%d to %d of %d'); +$total = $this->totalItemCount; +?> + + diff --git a/application/views/scripts/modules/overview.phtml b/application/views/scripts/modules/overview.phtml new file mode 100644 index 000000000..cc651c78f --- /dev/null +++ b/application/views/scripts/modules/overview.phtml @@ -0,0 +1,38 @@ +modules->limit(10); +$modules = $this->modules->paginate(); + +?> +

    Installed Modules

    +paginationControl($modules, null, null, array( + 'preserve' => $this->preserve +)); ?> + + + + + + + + +
    +enabled): ?> + qlink( + $module->name, + 'modules/disable', + array('name' => $module->name), + array('target'=>'body') + ) ?> + + qlink( + $module->name, + 'modules/enable', + array('name' => $module->name), + array('target'=>'body') + ) ?> + + (enabled + ? ($module->loaded ? 'enabled' : 'failed') + : 'disabled' + ?>)
    diff --git a/application/views/scripts/modules/show.phtml b/application/views/scripts/modules/show.phtml new file mode 100644 index 000000000..38d5cf050 --- /dev/null +++ b/application/views/scripts/modules/show.phtml @@ -0,0 +1,24 @@ +container('modules-container',array( + "class" => "expanded_absolute" +))->beginContent() ?> + + + + + + + + + + modules as $module): ?> + + + + + + + + +
    ModuleTypeActive
    + + \ No newline at end of file diff --git a/library/Icinga/Application/ApplicationBootstrap.php b/library/Icinga/Application/ApplicationBootstrap.php index dc40cb646..715850ee5 100755 --- a/library/Icinga/Application/ApplicationBootstrap.php +++ b/library/Icinga/Application/ApplicationBootstrap.php @@ -18,6 +18,7 @@ use Icinga\Exception\ConfigurationError; * Usage example for CLI: * * use Icinga\Application\Cli; + * Cli::start(); * * @@ -99,7 +100,7 @@ abstract class ApplicationBootstrap public function moduleManager() { if ($this->moduleManager === null) { - $this->moduleManager = new ModuleManager($this); + $this->moduleManager = new ModuleManager($this, "/etc/icinga2/modules_enabled"); } return $this->moduleManager; } diff --git a/library/Icinga/Application/Modules/Manager.php b/library/Icinga/Application/Modules/Manager.php index 45f4c89aa..4061a2421 100644 --- a/library/Icinga/Application/Modules/Manager.php +++ b/library/Icinga/Application/Modules/Manager.php @@ -6,6 +6,7 @@ use Icinga\Application\ApplicationBootstrap; use Icinga\Data\ArrayDatasource; use Icinga\Web\Notification; use Icinga\Exception\ConfigurationError; +use Icinga\Exception\SystemPermissionException; // TODO: show whether enabling/disabling modules is allowed by checking enableDir // perms @@ -20,17 +21,20 @@ class Manager protected $enableDir; - public function __construct(ApplicationBootstrap $app) + public function __construct(ApplicationBootstrap $app, $dir = null) { $this->app = $app; - $this->prepareEssentials(); + if ($dir === null) { + $dir = $this->app->getConfig()->getConfigDir() + . '/enabledModules'; + } + $this->prepareEssentials($dir); $this->detectEnabledModules(); } - protected function prepareEssentials() + protected function prepareEssentials($moduleDir) { - $this->enableDir = $this->app->getConfig()->getConfigDir() - . '/enabledModules'; + $this->enableDir = $moduleDir; if (! file_exists($this->enableDir) || ! is_dir($this->enableDir)) { throw new ProgrammingError( @@ -99,13 +103,20 @@ class Manager $target = $this->installedBaseDirs[$name]; $link = $this->enableDir . '/' . $name; if (! is_writable($this->enableDir)) { - Notification::error("I do not have permissions to enable modules"); + throw new SystemPermissionException( + "Insufficient system permissions for enabling modules", + "write", + $this->enableDir + ); + } + if (file_exists($link) && is_link($link)) { return $this; } - if (@symlink($target, $link)) { - Notification::success("The module $name has been enabled"); - } else { - Notification::error("Enabling module $name failed"); + if (!@symlink($target, $link)) { + $error = error_get_last(); + if (strstr($error["message"], "File exists") === false) { + throw new SystemPermissionException($error["message"], "symlink", $link); + } } return $this; } @@ -116,16 +127,29 @@ class Manager return $this; } if (! is_writable($this->enableDir)) { - Notification::error("I do not have permissions to disable modules"); + throw new SystemPermissionException("Can't write the module directory", "write", $this->enableDir); return $this; } $link = $this->enableDir . '/' . $name; + if (!file_exists($link)) { + throw new ConfigurationError("The module $name could not be found, can't disable it"); + } + if (!is_link($link)) { + throw new ConfigurationError( + "The module $name can't be disabled as this would delete the whole module. ". + "It looks like you have installed this module manually and moved it to your module folder.". + "In order to dynamically enable and disable modules, you have to create a symlink to ". + "the enabled_modules folder" + ); + } + if (file_exists($link) && is_link($link)) { - if (@unlink($link)) { - Notification::success("The module $name has been disabled"); - } else { - Notification::error("Disabling module $name failed"); + if (!@unlink($link)) { + $error = error_get_last(); + throw new SystemPermissionException($error["message"], "unlink", $link); } + } else { + } return $this; } diff --git a/public/js/icinga/container.js b/public/js/icinga/container.js index a64075b94..08d60f047 100755 --- a/public/js/icinga/container.js +++ b/public/js/icinga/container.js @@ -24,7 +24,7 @@ this.updateContainer = function(id,content,req) { var target = id; if (typeof id === "string") { - target = $('div[container-id='+id+']'); + target = this.getContainer(id); } var ctrl = $('.container-controls',target); target.html(content); @@ -100,6 +100,9 @@ }; this.getContainer = function(id) { + if(id == 'body') { + return $(document.body); + } return $('div[container-id='+id+']'); }; @@ -111,4 +114,4 @@ return containerMgrInstance; }); -})(); \ No newline at end of file +})(); diff --git a/public/js/icinga/util/async.js b/public/js/icinga/util/async.js index 8e015b45a..15a6be7df 100644 --- a/public/js/icinga/util/async.js +++ b/public/js/icinga/util/async.js @@ -10,7 +10,7 @@ }; var getDOMForDestination = function(destination) { var target = destination; - if(typeof destination === "string") { + if (typeof destination === "string") { target = containerMgr.getContainer(destination)[0]; } else if(typeof destination.context !== "undefined") { target = destination[0]; @@ -89,7 +89,7 @@ if(destination) { pending.push({ request: req, - DOM: getDOMForDestination(destination) + DOM: getDOMForDestination(destination) }); req.destination = destination; } From de34c2729f53d6bb519ed123142e53054cc72bc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Thu, 20 Jun 2013 17:01:13 +0200 Subject: [PATCH 06/10] Add clearstatcache and tests for ModuleManager Added the clearstatcache call before enabling or disabling modules, as this prevents a 'File exists' error that occurs sometimes on symlink creation (even when the folder is empty). Also added tests for teh moduleManager refs #4092 --- .../Icinga/Application/Modules/Manager.php | 76 +++++++----- .../Icinga/Application/Modules/Manager.php | 111 ++++++++++++++++++ 2 files changed, 157 insertions(+), 30 deletions(-) create mode 100644 test/php/library/Icinga/Application/Modules/Manager.php diff --git a/library/Icinga/Application/Modules/Manager.php b/library/Icinga/Application/Modules/Manager.php index 4061a2421..f4612de7f 100644 --- a/library/Icinga/Application/Modules/Manager.php +++ b/library/Icinga/Application/Modules/Manager.php @@ -2,33 +2,39 @@ namespace Icinga\Application\Modules; -use Icinga\Application\ApplicationBootstrap; -use Icinga\Data\ArrayDatasource; -use Icinga\Web\Notification; use Icinga\Exception\ConfigurationError; use Icinga\Exception\SystemPermissionException; +use Icinga\Exception\ProgrammingError; // TODO: show whether enabling/disabling modules is allowed by checking enableDir // perms class Manager { - protected $installedBaseDirs; + protected $installedBaseDirs = null; protected $enabledDirs = array(); protected $loadedModules = array(); protected $index; protected $app; - protected $enableDir; - - public function __construct(ApplicationBootstrap $app, $dir = null) + protected $modulePaths = array(); + /** + * @param $app : The applicaiton bootstrap. This one needs a properly defined interface + * In order to test it correctly, the application now only requires a stdClass + * @param $dir : + **/ + public function __construct($app, $enabledDir = null, array $availableDirs = array()) { $this->app = $app; - if ($dir === null) { - $dir = $this->app->getConfig()->getConfigDir() + if (empty($availableDirs)) { + $availableDirs = array(ICINGA_APPDIR."/../modules"); + } + $this->modulePaths = $availableDirs; + if ($enabledDir === null) { + $enabledDir = $this->app->getConfig()->getConfigDir() . '/enabledModules'; } - $this->prepareEssentials($dir); + $this->prepareEssentials($enabledDir); $this->detectEnabledModules(); } @@ -46,10 +52,17 @@ class Manager } } + public function select() + { + $source = new \Icinga\Data\ArrayDataSource($this->getModuleInfo()); + return $source->select(); + } + protected function detectEnabledModules() { $fh = opendir($this->enableDir); + $this->enabledDirs = array(); while (false !== ($file = readdir($fh))) { if ($file[0] === '.') { @@ -78,12 +91,18 @@ class Manager return $this; } - public function loadModule($name) + public function loadModule($name, $moduleBase = null) { if ($this->hasLoaded($name)) { return $this; } - $module = new Module($this->app, $name, $this->getModuleDir($name)); + + $module = null; + if ($moduleBase === null) { + $module = new Module($this->app, $name, $this->getModuleDir($name)); + } else { + $module = new $moduleBase($this->app, $name, $this->getModuleDir($name)); + } $module->register(); $this->loadedModules[$name] = $module; return $this; @@ -100,6 +119,7 @@ class Manager ); return $this; } + clearstatcache(true); $target = $this->installedBaseDirs[$name]; $link = $this->enableDir . '/' . $name; if (! is_writable($this->enableDir)) { @@ -118,6 +138,7 @@ class Manager throw new SystemPermissionException($error["message"], "symlink", $link); } } + $this->enabledDirs[$name] = $link; return $this; } @@ -151,6 +172,7 @@ class Manager } else { } + unset($this->enabledDirs[$name]); return $this; } @@ -228,12 +250,6 @@ class Manager return $info; } - public function select() - { - $ds = new ArrayDatasource($this->getModuleInfo()); - return $ds->select(); - } - public function listEnabledModules() { return array_keys($this->enabledDirs); @@ -254,19 +270,19 @@ class Manager public function detectInstalledModules() { - // TODO: Allow multiple paths for installed modules (e.g. web vs pkg) - $basedir = realpath(ICINGA_APPDIR . '/../modules'); - $fh = @opendir($basedir); - if ($fh === false) { - return $this; - } - - while ($name = readdir($fh)) { - if ($name[0] === '.') { - continue; + foreach ($this->modulePaths as $basedir) { + $fh = opendir($basedir); + if ($fh === false) { + return $this; } - if (is_dir($basedir . '/' . $name)) { - $this->installedBaseDirs[$name] = $basedir . '/' . $name; + + while ($name = readdir($fh)) { + if ($name[0] === '.') { + continue; + } + if (is_dir($basedir . '/' . $name)) { + $this->installedBaseDirs[$name] = $basedir . '/' . $name; + } } } } diff --git a/test/php/library/Icinga/Application/Modules/Manager.php b/test/php/library/Icinga/Application/Modules/Manager.php new file mode 100644 index 000000000..24d2a8c25 --- /dev/null +++ b/test/php/library/Icinga/Application/Modules/Manager.php @@ -0,0 +1,111 @@ +name = $name; + $this->dir = $dir; + } + + public function register() + { + } +} + + +class ManagerTest extends \PHPUnit_Framework_TestCase +{ + const MODULE_TARGET = "/tmp"; + + protected function setUp() + { + $moduleDir = self::MODULE_TARGET; + if (!is_writable($moduleDir)) { + $this->markTestSkipped("Temporary folder not writable for this user"); + return; + } + if (is_dir($moduleDir."/enabledModules")) { + exec("rm -r $moduleDir/enabledModules"); + } + + mkdir($moduleDir."/enabledModules"); + } + + public function testDetectEnabledModules() + { + $manager = new ModuleManager(null, "/tmp/enabledModules", array("none")); + $this->assertEmpty($manager->listEnabledModules()); + + symlink(getcwd()."/res/testModules/module1", "/tmp/enabledModules/module1"); + $manager = new ModuleManager(null, "/tmp/enabledModules", array("none")); + $this->assertEquals(array("module1"), $manager->listEnabledModules()); + symlink(getcwd()."/res/testModules/module2", "/tmp/enabledModules/module2"); + symlink(getcwd()."/res/???", "/tmp/enabledModules/module3"); + $manager = new ModuleManager(null, "/tmp/enabledModules", array("none")); + $this->assertEquals(array("module1", "module2"), $manager->listEnabledModules()); + } + + public function testLoadModule() + { + $manager = new ModuleManager(null, "/tmp/enabledModules", array("./res/testModules")); + $this->assertEmpty($manager->getLoadedModules()); + $manager->loadModule("module1", "Tests\Icinga\Application\Module\Manager\ModuleMock"); + $elems = $manager->getLoadedModules(); + $this->assertNotEmpty($elems); + $this->assertTrue(isset($elems["module1"])); + // assert the changes not to be permanent: + $manager = new ModuleManager(null, "/tmp/enabledModules", array("./res/testModules")); + $this->assertEmpty($manager->getLoadedModules()); + } + + public function testEnableModule() + { + $manager = new ModuleManager(null, "/tmp/enabledModules", array(getcwd()."/res/testModules")); + $this->assertEmpty($manager->listEnabledModules()); + $manager->enableModule("module1"); + $elems = $manager->listEnabledModules(); + $this->assertNotEmpty($elems); + $this->assertEquals($elems[0], "module1"); + $this->assertTrue(is_link("/tmp/enabledModules/module1")); + // assert the changes to be permanent: + $manager = new ModuleManager(null, "/tmp/enabledModules", array("./res/testModules")); + $this->assertNotEmpty($manager->listEnabledModules()); + } + + public function testDisableModule() + { + clearstatcache(true); + symlink(getcwd()."/res/testModules/module1", "/tmp/enabledModules/module1"); + $manager = new ModuleManager(null, "/tmp/enabledModules", array(getcwd()."/res/testModules")); + $elems = $manager->listEnabledModules(); + $this->assertNotEmpty($elems); + $this->assertEquals($elems[0], "module1"); + $manager->disableModule("module1"); + $this->assertFalse(file_exists("/tmp/enabledModules/module1")); + $this->assertEmpty($manager->listEnabledModules()); + // assert the changes to be permanent: + $manager = new ModuleManager(null, "/tmp/enabledModules", array("./res/testModules")); + $this->assertEmpty($manager->listEnabledModules()); + } + + protected function tearDown() + { + $moduleDir = self::MODULE_TARGET; + exec("rm -r $moduleDir/enabledModules"); + + } +} From 04511121b1ee2c53fed2ed6fc477307529b78dba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Fri, 21 Jun 2013 09:53:37 +0200 Subject: [PATCH 07/10] Remove moved files the 'behaviour' namespace in javascript has been renamed to module refs #4092 --- public/js/icinga/behaviour/actionTable.js | 118 ---------------------- public/js/icinga/behaviour/mainDetail.js | 94 ----------------- 2 files changed, 212 deletions(-) delete mode 100644 public/js/icinga/behaviour/actionTable.js delete mode 100755 public/js/icinga/behaviour/mainDetail.js diff --git a/public/js/icinga/behaviour/actionTable.js b/public/js/icinga/behaviour/actionTable.js deleted file mode 100644 index e8e61952b..000000000 --- a/public/js/icinga/behaviour/actionTable.js +++ /dev/null @@ -1,118 +0,0 @@ -/*global Icinga:false, document: false, define:false require:false base_url:false console:false */ - -/** - * ActionTable behaviour as described in - * https://wiki.icinga.org/display/cranberry/Frontend+Components#FrontendComponents-ActionTable - * - * @TODO: Row selection - */ -define(['jquery','logging','icinga/util/async'],function($,log,async) { - "use strict"; - - var ActionTableBehaviour = function() { - var onTableHeaderClick; - - var TABLE_BASE_MATCHER = '.icinga-container table.action'; - var linksInActionTable = TABLE_BASE_MATCHER+" tbody tr > a"; - var actionTableRow = TABLE_BASE_MATCHER+" tbody tr"; - var headerRow = TABLE_BASE_MATCHER+" > th a"; - var searchField = ".icinga-container .actiontable.controls input[type=search]"; - - - onTableHeaderClick = function (ev) { - var target = ev.currentTarget, - href = $(target).attr('href'), - destination; - if ($(target).parents('.layout-main-detail').length) { - if ($(target).parents("#icinga-main").length) { - destination = 'icinga-main'; - } - else { - destination = 'icinga-detail'; - } - - } else { - destination = 'icinga-main'; - } - async.loadToTarget(destination, href); - ev.preventDefault(); - ev.stopImmediatePropagation(); - return false; - }; - - var onLinkTagClick = function(ev) { - - var target = ev.currentTarget, - href = $(target).attr('href'), - destination; - if ($(target).parents('.layout-main-detail').length) { - destination = 'icinga-detail'; - } else { - destination = 'icinga-main'; - } - async.loadToTarget(destination,href); - ev.preventDefault(); - ev.stopImmediatePropagation(); - return false; - - }; - - var onTableRowClick = function(ev) { - ev.stopImmediatePropagation(); - - var target = $(ev.currentTarget), - href = target.attr('href'), - destination; - $('tr.active',target.parent('tbody').first()).removeClass("active"); - target.addClass('active'); - - // When the tr has a href, act like it is a link - if(href) { - ev.currentTarget = target.first(); - return onLinkTagClick(ev); - } - // Try to find a designated row action - var links = $("a.row-action",target); - if(links.length) { - ev.currentTarget = links.first(); - return onLinkTagClick(ev); - } - - // otherwise use the first anchor tag - links = $("a",target); - if(links.length) { - ev.currentTarget = links.first(); - return onLinkTagClick(ev); - } - - log.debug("No target for this table row found"); - return false; - }; - - var onSearchInput = function(ev) { - ev.stopImmediatePropagation(); - var txt = $(this).val(); - }; - - this.eventHandler = {}; - this.eventHandler[linksInActionTable] = { - 'click' : onLinkTagClick - }; - this.eventHandler[actionTableRow] = { - 'click' : onTableRowClick - }; - this.eventHandler[headerRow] = { - 'click' : onTableHeaderClick - }; - this.eventHandler[searchField] = { - 'keyup' : onSearchInput - }; - - this.enable = function() { - - }; - }; - - return new ActionTableBehaviour(); - -}); \ No newline at end of file diff --git a/public/js/icinga/behaviour/mainDetail.js b/public/js/icinga/behaviour/mainDetail.js deleted file mode 100755 index 9a207c933..000000000 --- a/public/js/icinga/behaviour/mainDetail.js +++ /dev/null @@ -1,94 +0,0 @@ -/*global Icinga:false, document: false, define:false require:false base_url:false console:false */ - -/** - * Main-Detail layout behaviour as described in - * https://wiki.icinga.org/display/cranberry/Frontend+Components#FrontendComponents-Behaviour - * - */ -define(['jquery','logging','icinga/util/async'],function($,log,async) { - "use strict"; - - var MainDetailBehaviour = function() { - - var onOuterLinkClick = function(ev) { - var a = $(ev.currentTarget), - target = a.attr("target"), - href = a.attr("href"); - ev.stopImmediatePropagation(); - collapseDetailView(); - async.loadToTarget("icinga-main",href); - return false; - }; - - var onLinkTagClick = function(ev) { - - var a = $(ev.currentTarget), - target = a.attr("target"), - href = a.attr("href"); - - // check for protocol:// - if(/^[A-Z]{2,10}\:\/\//i.test(href)) { - window.open(href); - ev.stopImmediatePropagation(); - return false; - } - - // check for link in table header - if(a.parents('th').length > 0) { - ev.stopImmediatePropagation(); - return false; - } - - if(typeof target === "undefined") { - if(a.parents("#icinga-detail").length) { - async.loadToTarget("icinga-detail",href); - } else { - async.loadToTarget("icinga-main",href); - } - } else { - switch(target) { - case "main": - async.loadToTarget("icinga-main",href); - collapseDetailView(); - break; - case "detail": - async.loadToTarget("icinga-detail",href); - break; - case "popup": - async.loadToTarget(null,href); - break; - default: - return true; - } - } - ev.stopImmediatePropagation(); - return false; - }; - - var expandDetailView = function() { - $("#icinga-detail").parents(".collapsed").removeClass('collapsed'); - }; - - var collapseDetailView = function(elementInDetailView) { - $("#icinga-detail").parents(".layout-main-detail").addClass('collapsed'); - }; - - this.expandDetailView = expandDetailView; - this.collapseDetailView = collapseDetailView; - - this.eventHandler = { - '.layout-main-detail * a' : { - 'click' : onLinkTagClick - }, - 'a' : { - 'click' : onOuterLinkClick - }, - '.layout-main-detail .icinga-container#icinga-detail' : { - 'focus' : expandDetailView - } - }; - }; - - return new MainDetailBehaviour(); -}); - From 6881c8d0743bf229eee130f13dc0a8bb8db584e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Fri, 21 Jun 2013 11:36:12 +0200 Subject: [PATCH 08/10] Add javascript autoloading the js/modules/%modulename%/%file% is now mapped to the module path (if existing). To prevent name clashing, the modules folder has been renamed to components. --- application/controllers/StaticController.php | 10 ++++++---- library/Icinga/Application/Web.php | 12 ++++++++++++ .../js/icinga/{modules => components}/actionTable.js | 0 .../js/icinga/{modules => components}/mainDetail.js | 0 public/js/icinga/container.js | 10 +++------- public/js/icinga/icinga.js | 4 ++-- 6 files changed, 23 insertions(+), 13 deletions(-) rename public/js/icinga/{modules => components}/actionTable.js (100%) rename public/js/icinga/{modules => components}/mainDetail.js (100%) diff --git a/application/controllers/StaticController.php b/application/controllers/StaticController.php index bb5723a84..5704dbf4a 100644 --- a/application/controllers/StaticController.php +++ b/application/controllers/StaticController.php @@ -9,6 +9,7 @@ use Icinga\Web\ActionController; use Icinga\Application\Icinga; use Zend_Controller_Action_Exception as ActionException; +use Icinga\Application\Benchmark; /** * Class StaticController * @package Icinga\Web\Form @@ -69,7 +70,7 @@ class StaticController extends ActionController */ public function javascriptAction() { - $module = $this->_getParam('moduleName'); + $module = $this->_getParam('module_name'); $file = $this->_getParam('file'); $basedir = Icinga::app()->getModule($module)->getBaseDir(); @@ -96,15 +97,16 @@ class StaticController extends ActionController ) . ' GMT' ); + $hash = md5_file($filePath); + if ($hash === $this->getRequest()->getHeader('If-None-Match')) { $response->setHttpResponseCode(304); return; } else { readfile($filePath); } - // TODO: get rid of exit: - exit; + return; } } -// @codingStandardsIgnoreEnd \ No newline at end of file +// @codingStandardsIgnoreEnd diff --git a/library/Icinga/Application/Web.php b/library/Icinga/Application/Web.php index d034d344f..123c25acf 100644 --- a/library/Icinga/Application/Web.php +++ b/library/Icinga/Application/Web.php @@ -60,6 +60,17 @@ class Web extends ApplicationBootstrap ) ) ); + + $this->frontController->getRouter()->addRoute( + 'module_javascript', + new Route( + 'js/modules/:module_name/:file', + array( + 'controller' => 'static', + 'action' => 'javascript' + ) + ) + ); return $this; } @@ -173,6 +184,7 @@ class Web extends ApplicationBootstrap */ protected function configurePagination() { + Paginator::addScrollingStylePrefixPath( 'Icinga_Web_Paginator_ScrollingStyle', 'Icinga/Web/Paginator/ScrollingStyle' diff --git a/public/js/icinga/modules/actionTable.js b/public/js/icinga/components/actionTable.js similarity index 100% rename from public/js/icinga/modules/actionTable.js rename to public/js/icinga/components/actionTable.js diff --git a/public/js/icinga/modules/mainDetail.js b/public/js/icinga/components/mainDetail.js similarity index 100% rename from public/js/icinga/modules/mainDetail.js rename to public/js/icinga/components/mainDetail.js diff --git a/public/js/icinga/container.js b/public/js/icinga/container.js index 08d60f047..8e0891802 100755 --- a/public/js/icinga/container.js +++ b/public/js/icinga/container.js @@ -5,7 +5,7 @@ var containerMgrInstance = null; var async; - var ContainerMgr = function($,log,Widgets,SubTable,holder) { + var ContainerMgr = function($,log,Widgets,SubTable) { var enhanceDetachLinks = function() { @@ -34,14 +34,12 @@ } target.focus(); this.initializeContainers(target); - holder.run(); }; this.updateControlTargets = function(ctrl, req) { $('a',ctrl).each(function() { $(this).attr("href",req.url); }); - holder.run(); }; this.initControlBehaviour = function(root) { @@ -86,7 +84,6 @@ this.initExpandables(root); this.drawImplicitWidgets(root); this.loadAsyncContainers(root); - holder.run(); }; this.createPopupContainer = function(content,req) { @@ -96,7 +93,6 @@ .append($("
    ").addClass('modal-body').html(content)).appendTo(document.body); closeButton.on("click",function() {container.remove();}); - holder.run(); }; this.getContainer = function(id) { @@ -107,9 +103,9 @@ }; }; - define(['jquery','logging','icinga/widgets/checkIcons','icinga/widgets/subTable','Holder'],function($,log,widgets,subTable,holder) { + define(['jquery','logging','icinga/widgets/checkIcons','icinga/widgets/subTable'],function($,log,widgets,subTable) { if (containerMgrInstance === null) { - containerMgrInstance = new ContainerMgr($,log,widgets,subTable,holder); + containerMgrInstance = new ContainerMgr($,log,widgets,subTable); } return containerMgrInstance; diff --git a/public/js/icinga/icinga.js b/public/js/icinga/icinga.js index 9be6b9013..ff59321f6 100755 --- a/public/js/icinga/icinga.js +++ b/public/js/icinga/icinga.js @@ -13,7 +13,7 @@ define([ * Icinga prototype */ var Icinga = function() { - var internalModules = ['icinga/modules/actionTable','icinga/modules/mainDetail']; + var internalModules = ['icinga/components/actionTable','icinga/components/mainDetail']; this.modules = {}; var failedModules = []; @@ -37,7 +37,7 @@ define([ }; var loadModuleScript = function(name) { - moduleMgr.enableModule("modules/"+name, function(error) { + moduleMgr.enableModule("modules/"+name+"/"+name, function(error) { failedModules.push({ name: name, errorMessage: error From 01611d8a8b330f150906484337a4cdd3d8a7e072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Fri, 21 Jun 2013 11:40:25 +0200 Subject: [PATCH 09/10] Fix test for new module route The js base route is now js/modules/%modulename%/%file%. This commit reflects these changes in the testcases refs #4092 refs #3735 --- test/js/test/icinga/moduleTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/test/icinga/moduleTest.js b/test/js/test/icinga/moduleTest.js index 6a446b5f4..494d0264a 100644 --- a/test/js/test/icinga/moduleTest.js +++ b/test/js/test/icinga/moduleTest.js @@ -195,7 +195,7 @@ describe('The icinga module bootstrap', function() { var testClick = false; rjsmock.registerDependencies({ "icinga/module": module, - "modules/test" : { + "modules/test/test" : { eventHandler: { "a.test" : { click : function() { From c239117c559e3d9ce5f257bdaf17d8262976cb54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= Date: Fri, 21 Jun 2013 15:33:06 +0200 Subject: [PATCH 10/10] Add support for lazy module loading When the X-Icinga-Module-Enable header is send, the modulemanager automatically tries to load javascript files for that module. This is realized by adding the 'registerHeaderListener' method to the async manager, which allows to listen to specific headers and firing callbacks if a response with the specified header is retrieved. Also the tests have changed a bit, requireNow should be used when using the requiremock, so a require always loads files new. refs #4092 refs #3753 --- application/controllers/ModulesController.php | 6 +- public/js/icinga/icinga.js | 8 ++- public/js/icinga/util/async.js | 34 ++++++++-- test/js/test/icinga/asyncTest.js | 35 +++++++++++ test/js/test/icinga/moduleTest.js | 62 +++++++++++++++++-- test/js/testlib/asyncmock.js | 33 ++++++++++ test/js/testlib/requiremock.js | 15 ++++- 7 files changed, 179 insertions(+), 14 deletions(-) create mode 100644 test/js/test/icinga/asyncTest.js create mode 100644 test/js/testlib/asyncmock.js diff --git a/application/controllers/ModulesController.php b/application/controllers/ModulesController.php index f8f40a3b3..afd3b9178 100755 --- a/application/controllers/ModulesController.php +++ b/application/controllers/ModulesController.php @@ -35,7 +35,11 @@ class ModulesController extends ActionController public function enableAction() { $this->manager->enableModule($this->_getParam('name')); - $this->redirectNow('modules/overview?_render=body'); + $this->manager->loadModule($this->_getParam('name')); + $this->getResponse()->setHeader('X-Icinga-Enable-Module', $this->_getParam('name')); + $this->replaceLayout = true; + $this->indexAction(); + } public function disableAction() diff --git a/public/js/icinga/icinga.js b/public/js/icinga/icinga.js index ff59321f6..d538ab552 100755 --- a/public/js/icinga/icinga.js +++ b/public/js/icinga/icinga.js @@ -19,16 +19,19 @@ define([ var failedModules = []; var initialize = function () { + registerLazyModuleLoading(); enableInternalModules(); - + containerMgr.registerAsyncMgr(async); containerMgr.initializeContainers(document); log.debug("Initialization finished"); enableModules(); }; - + var registerLazyModuleLoading = function() { + async.registerHeaderListener("X-Icinga-Enable-Module", loadModuleScript, this); + }; var enableInternalModules = function() { $.each(internalModules,function(idx,module) { @@ -37,6 +40,7 @@ define([ }; var loadModuleScript = function(name) { + console.log("Loading ", name); moduleMgr.enableModule("modules/"+name+"/"+name, function(error) { failedModules.push({ name: name, diff --git a/public/js/icinga/util/async.js b/public/js/icinga/util/async.js index 15a6be7df..1b564b176 100644 --- a/public/js/icinga/util/async.js +++ b/public/js/icinga/util/async.js @@ -3,7 +3,8 @@ "use strict"; var asyncMgrInstance = null; - define(['icinga/container','logging','icinga/behaviour','jquery'],function(containerMgr,log,behaviour,$) { + define(['icinga/container','logging','jquery'],function(containerMgr,log,$) { + var headerListeners = {}; var pending = { @@ -18,12 +19,30 @@ return target; }; - var handleResponse = function(html) { + var applyHeaderListeners = function(headers) { + for (var header in headerListeners) { + if (headers.getResponseHeader(header) === null) { + // see if the browser/server converts headers to lowercase + if (headers.getResponseHeader(header.toLowerCase()) === null) { + continue; + } + header = header.toLowerCase(); + } + var value = headers.getResponseHeader(header); + var listeners = headerListeners[header]; + for (var i=0;i")); $('a.test').click(); @@ -223,4 +226,55 @@ describe('The icinga module bootstrap', function() { should.equal(icinga.getFailedModules()[0].name, "test2"); tearDownTestDOM(); }); + + it("Should load modules lazily when discovering a X-Icinga-Enable-Module header", function() { + rjsmock.purgeDependencies(); + + requireNew("icinga/util/async.js"); + var async = rjsmock.getDefine(); + + rjsmock.registerDependencies({ + "icinga/module": module, + "icinga/util/async": async, + "modules/test/test" : { + eventHandler: { + "a.test" : { + click : function() { + testClick = true; + } + } + } + }, + "icinga/container" : { + registerAsyncMgr: function() {}, + initializeContainers: function() {} + }, + "modules/list" : [ + ] + }); + + tearDownTestDOM(); + + requireNew("icinga/icinga.js"); + var icinga = rjsmock.getDefine(); + + var testClick = false; + // The module shouldn't be loaded + $('body').append($("")); + $('a.test').click(); + should.equal(testClick, false, "Unregistered module was loaded"); + + asyncMock.setNextAsyncResult(async,"result", false, { + "X-Icinga-Enable-Module" : "test" + }); + async.createRequest(); + // The module shouldn't be loaded + $('body').append($("")); + $('a.test').click(); + should.equal(testClick, true, "Module wasn't automatically loaded on header!"); + + + tearDownTestDOM(); + + }); }); diff --git a/test/js/testlib/asyncmock.js b/test/js/testlib/asyncmock.js new file mode 100644 index 000000000..aad6d5eea --- /dev/null +++ b/test/js/testlib/asyncmock.js @@ -0,0 +1,33 @@ +/** + * Helper for mocking $.async's XHR requests + * + */ + + +var getCallback = function(empty, response, succeed, headers) { + if (empty) + return function() {}; + return function(callback) { + callback(response, succeed, { + getAllResponseHeaders: function() { + return headers; + }, + getResponseHeader: function(header) { + return headers[header] || null; + } + }); + }; +}; + +module.exports = { + setNextAsyncResult: function(async, response, fails, headers) { + headers = headers || {}; + var succeed = fails ? "fail" : "success"; + async.__internalXHRImplementation = function(config) { + return { + done: getCallback(fails, response, succeed, headers), + fail: getCallback(!fails, response, succeed, headers) + }; + }; + } +}; diff --git a/test/js/testlib/requiremock.js b/test/js/testlib/requiremock.js index 8181092da..e61b08694 100644 --- a/test/js/testlib/requiremock.js +++ b/test/js/testlib/requiremock.js @@ -13,6 +13,7 @@ * to console. * **/ +var path = require('path'); var registeredDependencies = {}; /** @@ -21,11 +22,13 @@ var registeredDependencies = {}; * in dependencies and calls fn with them as the parameter * **/ +var debug = false; var requireJsMock = function(dependencies, fn) { var fnArgs = []; for (var i=0;i