diff --git a/library/Icinga/Web/Hook.php b/library/Icinga/Web/Hook.php index 915597f86..6f3ffc2f0 100644 --- a/library/Icinga/Web/Hook.php +++ b/library/Icinga/Web/Hook.php @@ -29,9 +29,9 @@ namespace Icinga\Web; +use Exception; use Icinga\Logger\Logger; use Icinga\Exception\ProgrammingError; -use stdClass; /** * Icinga Web Hook registry @@ -79,9 +79,9 @@ class Hook /** * Whether someone registered itself for the given hook name * - * @param string $name One of the predefined hook names + * @param string $name One of the predefined hook names * - * @return bool + * @return bool */ public static function has($name) { @@ -89,72 +89,29 @@ class Hook } /** - * Get the first registered instance for the given hook name + * Create or return an instance of a given hook * - * TODO: Multiple instances are not handled yet * TODO: Should return some kind of a hook interface * - * @param string $name One of the predefined hook names - * - * @return mixed - * @throws ProgrammingError - */ - public static function get($name) - { - if (! self::has($name)) { - return null; - } - if (! array_key_exists($name, self::$instances)) { - $class = self::$hooks[$name]; - try { - $obj = new $class(); - } catch (\Exception $e) { - // TODO: Persist unloading for "some time" or "current session" - Logger::debug( - 'Hook "%s" (%s) failed, will be unloaded: %s', - $name, - $class, - $e->getMessage() - ); - unset(self::$hooks[$name]); - return null; - } - $base_class = 'Icinga\\Web\\Hook\\' . ucfirst($name); - if (! $obj instanceof $base_class) { - throw new ProgrammingError( - sprintf( - '%s is not an instance of %s', - get_class($obj), - $base_class - ) - ); - } - self::$instances[$name] = $obj; - - } - return self::$instances[$name]; - } - - /** - * Create or return an instance of the hook - * - * @param string $name - * @param string $key - * @return stdClass + * @param string $name One of the predefined hook names + * @param string $key The identifier of a specific subtype * + * @return mixed */ public static function createInstance($name, $key) { if (!self::has($name, $key)) { return null; } + if (isset(self::$instances[$name][$key])) { return self::$instances[$name][$key]; } + $class = self::$hooks[$name][$key]; try { $instance = new $class(); - } catch (\Exception $e) { + } catch (Exception $e) { Logger::debug( 'Hook "%s" (%s) (%s) failed, will be unloaded: %s', $name, @@ -162,9 +119,11 @@ class Hook $class, $e->getMessage() ); + // TODO: Persist unloading for "some time" or "current session" unset(self::$hooks[$name][$key]); return null; } + self::assertValidHook($instance, $name); self::$instances[$name][$key] = $instance; return $instance; @@ -173,11 +132,12 @@ class Hook /** * Test for a valid class name * - * @param stdClass $instance - * @param string $name - * @throws ProgrammingError + * @param mixed $instance + * @param string $name + * + * @throws ProgrammingError */ - private static function assertValidHook(&$instance, $name) + private static function assertValidHook($instance, $name) { $base_class = self::$BASE_NS . ucfirst($name); if (!$instance instanceof $base_class) { @@ -194,43 +154,45 @@ class Hook /** * Return all instances of a specific name * - * @param string $name + * @param string $name One of the predefined hook names * - * @return array + * @return array */ public static function all($name) { if (!self::has($name)) { return array(); } + foreach (self::$hooks[$name] as $key => $hook) { if (self::createInstance($name, $key) === null) { return array(); } } + return self::$instances[$name]; } /** * Get the first hook * - * @param string $name + * @param string $name One of the predefined hook names * - * @return stdClass + * @return null|mixed */ public static function first($name) { - return self::createInstance($name, key(self::$hooks[$name])); + if (self::has($name)) { + return self::createInstance($name, key(self::$hooks[$name])); + } } /** * Register your hook * - * @param string $name One of the predefined hook names - * @param string $key - * @param string $class Your class name, must inherit one of the classes - * in the Icinga/Web/Hook folder + * Alias for Hook::registerClass() * + * @see Hook::registerClass() */ public static function register($name, $key, $class) { @@ -240,12 +202,17 @@ class Hook /** * Register a class * - * @param string $name - * @param string $key - * @param string $class + * @param string $name One of the predefined hook names + * @param string $key The identifier of a specific subtype + * @param string $class Your class name, must inherit one of the + * classes in the Icinga/Web/Hook folder */ public static function registerClass($name, $key, $class) { + if (!class_exists($class)) { + throw new ProgrammingError('"' . $class . '" is not an existing class'); + } + if (!isset(self::$hooks[$name])) { self::$hooks[$name] = array(); } @@ -256,23 +223,23 @@ class Hook /** * Register an object * - * @param string $name - * @param string $key - * @param object $object + * @param string $name One of the predefined hook names + * @param string $key The identifier of a specific subtype + * @param object $object The instantiated hook to register * - * @throws ProgrammingError + * @throws ProgrammingError */ public static function registerObject($name, $key, $object) { if (!is_object($object)) { - throw new ProgrammingError('object is not an instantiated class'); + throw new ProgrammingError('"' . $object . '" is not an instantiated class'); } if (!isset(self::$instances[$name])) { self::$instances[$name] = array(); } - self::$instances[$name][$key] =& $object; + self::$instances[$name][$key] = $object; self::registerClass($name, $key, get_class($object)); } } diff --git a/library/Icinga/Web/Hook/Configuration/ConfigurationTabBuilder.php b/library/Icinga/Web/Hook/Configuration/ConfigurationTabBuilder.php index ae772f68e..dad7a37f0 100644 --- a/library/Icinga/Web/Hook/Configuration/ConfigurationTabBuilder.php +++ b/library/Icinga/Web/Hook/Configuration/ConfigurationTabBuilder.php @@ -90,7 +90,7 @@ class ConfigurationTabBuilder { /** @var ConfigurationTab $configTab */ $configTab = null; - foreach (Hook::get(self::HOOK_NAMESPACE) as $configTab) { + foreach (Hook::all(self::HOOK_NAMESPACE) as $configTab) { if (!$configTab instanceof ConfigurationTabInterface) { throw new ProgrammingError('tab not instance of ConfigTabInterface'); } diff --git a/library/Icinga/Web/Widget/FilterBox.php b/library/Icinga/Web/Widget/FilterBox.php index 3059197d1..4395129ff 100644 --- a/library/Icinga/Web/Widget/FilterBox.php +++ b/library/Icinga/Web/Widget/FilterBox.php @@ -110,7 +110,6 @@ EOT; $query = $form->getElement('query')->setDecorators(array('ViewHelper')); $badges = new FilterBadgeRenderer($this->initialFilter); - $form->setIgnoreChangeDiscarding(true); return '
' . $badges->render($view) . '
' . $form; $html = str_replace('{{FORM}}', $form->render($view), self::$TPL); $html = '
' . $html . '
'; diff --git a/modules/monitoring/application/controllers/ListController.php b/modules/monitoring/application/controllers/ListController.php index 5e26d3654..8005e8713 100644 --- a/modules/monitoring/application/controllers/ListController.php +++ b/modules/monitoring/application/controllers/ListController.php @@ -74,7 +74,7 @@ class Monitoring_ListController extends Controller public function init() { $this->backend = Backend::createBackend($this->_getParam('backend')); - $this->view->grapher = Hook::get('grapher'); + $this->view->grapher = Hook::first('grapher'); $this->createTabs(); $this->view->activeRowHref = $this->getParam('detail'); $this->view->compact = ($this->_request->getParam('view') === 'compact'); diff --git a/modules/monitoring/application/views/helpers/CommandForm.php b/modules/monitoring/application/views/helpers/CommandForm.php index d3124019d..7ebc788c6 100644 --- a/modules/monitoring/application/views/helpers/CommandForm.php +++ b/modules/monitoring/application/views/helpers/CommandForm.php @@ -56,7 +56,6 @@ class Zend_View_Helper_CommandForm extends Zend_View_Helper_Abstract { $form = new Form(); $form->setAttrib('class', 'inline'); - $form->setIgnoreChangeDiscarding(true); $form->setRequest(Zend_Controller_Front::getInstance()->getRequest()); // Filter work only from get parts. Put important diff --git a/modules/monitoring/library/Monitoring/Environment.php b/modules/monitoring/library/Monitoring/Environment.php index a1e41d732..0213118a2 100644 --- a/modules/monitoring/library/Monitoring/Environment.php +++ b/modules/monitoring/library/Monitoring/Environment.php @@ -37,12 +37,12 @@ class Environment public static function grapher($env = null) { - return Hook::get('grapher', null, self::config($env, 'grapher')); + return Hook::createInstance('grapher', null, self::config($env, 'grapher')); } public static function configBackend($env = null) { - return Hook::get( + return Hook::createInstance( 'configBackend', null, self::config($env, 'configBackend') diff --git a/test/php/library/Icinga/Web/HookTest.php b/test/php/library/Icinga/Web/HookTest.php index 9c14f1f62..9e82ad90d 100644 --- a/test/php/library/Icinga/Web/HookTest.php +++ b/test/php/library/Icinga/Web/HookTest.php @@ -4,27 +4,16 @@ namespace Tests\Icinga\Web; -use \Mockery; +use Mockery; +use Exception; use Icinga\Web\Hook; use Icinga\Test\BaseTestCase; -class Base -{ -} - -class TestHookImplementation extends Base -{ -} - -class TestBadHookImplementation -{ -} - class ErrorProneHookImplementation { public function __construct() { - throw new \Exception("HOOK ERROR"); + throw new Exception(); } } @@ -42,103 +31,182 @@ class HookTest extends BaseTestCase Hook::clean(); } - public function testHas() + public function testWhetherHasReturnsTrueIfGivenAKnownHook() { - $this->assertFalse(Hook::has("a")); - $this->assertFalse(Hook::has("a","b")); + Hook::registerClass('TestHook', __FUNCTION__, get_class(Mockery::mock(Hook::$BASE_NS . 'TestHook'))); - Hook::registerClass("a","b","c"); - $this->assertTrue(Hook::has("a")); - $this->assertTrue(Hook::has("a","b")); + $this->assertTrue(Hook::has('TestHook'), 'Hook::has does not return true if given a known hook'); } - public function testCreateInstance() + public function testWhetherHasReturnsFalseIfGivenAnUnknownHook() { - Hook::$BASE_NS = "Tests\\Icinga\\Web\\"; - Hook::registerClass("Base","b","Tests\\Icinga\\Web\\TestHookImplementation"); - $this->assertInstanceOf("Tests\\Icinga\\Web\\TestHookImplementation",Hook::createInstance("Base","b")); - Hook::clean(); + $this->assertFalse(Hook::has('not_existing'), 'Hook::has does not return false if given an unknown hook'); } - public function testCreateInvalidInstance1() + public function testWhetherHooksCanBeRegisteredWithRegisterClass() { - $this->setExpectedException('\Icinga\Exception\ProgrammingError'); - Hook::$BASE_NS = "Tests\\Icinga\\Web\\"; - Hook::registerClass("Base","b","Tests\\Icinga\\Web\\TestBadHookImplementation"); - Hook::createInstance("Base","b"); - Hook::clean(); + Hook::registerClass('TestHook', __FUNCTION__, get_class(Mockery::mock(Hook::$BASE_NS . 'TestHook'))); + + $this->assertTrue(Hook::has('TestHook'), 'Hook::registerClass does not properly register a given hook'); } - public function testCreateInvalidInstance2() + /** + * @depends testWhetherHooksCanBeRegisteredWithRegisterClass + */ + public function testWhetherMultipleHooksOfTheSameTypeCanBeRegisteredWithRegisterClass() { - Hook::$BASE_NS = "Tests\\Icinga\\Web\\"; - $test = Hook::createInstance("Base","NOTEXIST"); - $this->assertNull($test); - } + $firstHook = Mockery::mock(Hook::$BASE_NS . 'TestHook'); + $secondHook = Mockery::mock('overload:' . get_class($firstHook)); - public function testCreateInvalidInstance3() - { - Hook::$BASE_NS = "Tests\\Icinga\\Web\\"; - Hook::register("Base","ErrorProne","Tests\\Icinga\\Web\\ErrorProneHookImplementation"); - $test = Hook::createInstance("Base","ErrorProne"); - $this->assertNull($test); - } - - public function testAll() - { - Hook::$BASE_NS = "Tests\\Icinga\\Web\\"; - Hook::registerClass("Base","a","Tests\\Icinga\\Web\\TestHookImplementation"); - Hook::registerClass("Base","b","Tests\\Icinga\\Web\\TestHookImplementation"); - Hook::registerClass("Base","c","Tests\\Icinga\\Web\\TestHookImplementation"); - $this->assertCount(3,Hook::all("Base")); - foreach(Hook::all("Base") as $instance) { - $this->assertInstanceOf("Tests\\Icinga\\Web\\TestHookImplementation",$instance); - } - } - - public function testFirst() - { - Hook::$BASE_NS = "Tests\\Icinga\\Web\\"; - Hook::registerClass("Base","a","Tests\\Icinga\\Web\\TestHookImplementation"); - Hook::registerClass("Base","b","Tests\\Icinga\\Web\\TestHookImplementation"); - Hook::registerClass("Base","c","Tests\\Icinga\\Web\\TestHookImplementation"); - - $this->assertInstanceOf("Tests\\Icinga\\Web\\TestHookImplementation",Hook::first("Base")); - } - - public function testRegisterObject() - { - $o1 = Mockery::mock('Some\\Name\\Space\\ObjectHook'); - $o1->test = '$123123'; - $o2 = Mockery::mock('Some\\Name\\Space\\ObjectHook'); - $o2->test = '#456456'; - - Hook::registerObject('Test', 'o1', $o1); - Hook::registerObject('Test', 'o2', $o2); - - $this->assertInstanceOf('Some\\Name\\Space\\ObjectHook', Hook::createInstance('Test', 'o1')); - $this->assertInstanceOf('Some\\Name\\Space\\ObjectHook', Hook::createInstance('Test', 'o2')); - - $string = ""; - foreach (Hook::all('Test') as $hook) { - $string .= $hook->test; - } - $this->assertEquals('$123123#456456', $string); + Hook::registerClass('TestHook', 'one', get_class($firstHook)); + Hook::registerClass('TestHook', 'two', get_class($secondHook)); + $this->assertInstanceOf( + get_class($secondHook), + Hook::createInstance('TestHook', 'two'), + 'Hook::registerClass is not able to register different hooks of the same type' + ); } /** * @expectedException Icinga\Exception\ProgrammingError - * @expectedExceptionMessage object is not an instantiated class */ - public function testErrorObjectRegistration() + public function testWhetherOnlyClassesCanBeRegisteredAsHookWithRegisterClass() { - Hook::registerObject('Test', 'e1', 'STRING'); + Hook::registerClass('TestHook', __FUNCTION__, 'nope'); } - public function testGetZeroHooks() + public function testWhetherHooksCanBeRegisteredWithRegisterObject() { - $nh = Hook::all('DOES_NOT_EXIST'); - $this->assertInternalType('array', $nh); - $this->assertCount(0, $nh); + Hook::registerObject('TestHook', __FUNCTION__, Mockery::mock(Hook::$BASE_NS . 'TestHook')); + + $this->assertTrue(Hook::has('TestHook'), 'Hook::registerObject does not properly register a given hook'); + } + + /** + * @depends testWhetherHooksCanBeRegisteredWithRegisterObject + */ + public function testWhetherMultipleHooksOfTheSameTypeCanBeRegisteredWithRegisterObject() + { + $firstHook = Mockery::mock(Hook::$BASE_NS . 'TestHook'); + $secondHook = Mockery::mock('overload:' . get_class($firstHook)); + + Hook::registerObject('TestHook', 'one', $firstHook); + Hook::registerObject('TestHook', 'two', $secondHook); + $this->assertInstanceOf( + get_class($secondHook), + Hook::createInstance('TestHook', 'two'), + 'Hook::registerObject is not able to register different hooks of the same type' + ); + } + + /** + * @expectedException Icinga\Exception\ProgrammingError + */ + public function testWhetherOnlyObjectsCanBeRegisteredAsHookWithRegisterObject() + { + Hook::registerObject('TestHook', __FUNCTION__, 'nope'); + } + + public function testWhetherCreateInstanceReturnsNullIfGivenAnUnknownHookName() + { + $this->assertNull( + Hook::createInstance('not_existing', __FUNCTION__), + 'Hook::createInstance does not return null if given an unknown hook' + ); + } + + /** + * @depends testWhetherHooksCanBeRegisteredWithRegisterClass + */ + public function testWhetherCreateInstanceInitializesHooksInheritingFromAPredefinedAbstractHook() + { + $baseHook = Mockery::mock(Hook::$BASE_NS . 'TestHook'); + Hook::registerClass( + 'TestHook', + __FUNCTION__, + get_class(Mockery::mock('overload:' . get_class($baseHook))) + ); + + $this->assertInstanceOf( + get_class($baseHook), + Hook::createInstance('TestHook', __FUNCTION__), + 'Hook::createInstance does not initialize hooks inheriting from a predefined abstract hook' + ); + } + + /** + * @depends testWhetherHooksCanBeRegisteredWithRegisterClass + */ + public function testWhetherCreateInstanceDoesNotInitializeMultipleHooksForASpecificIdentifier() + { + Hook::registerClass('TestHook', __FUNCTION__, get_class(Mockery::mock(Hook::$BASE_NS . 'TestHook'))); + $secondHook = Hook::createInstance('TestHook', __FUNCTION__); + $thirdHook = Hook::createInstance('TestHook', __FUNCTION__); + + $this->assertSame( + $secondHook, + $thirdHook, + 'Hook::createInstance initializes multiple hooks for a specific identifier' + ); + } + + /** + * @depends testWhetherHooksCanBeRegisteredWithRegisterClass + */ + public function testWhetherCreateInstanceReturnsNullIfHookCannotBeInitialized() + { + Hook::registerClass('TestHook', __FUNCTION__, 'Tests\Icinga\Web\ErrorProneHookImplementation'); + + $this->assertNull(Hook::createInstance('TestHook', __FUNCTION__)); + } + + /** + * @expectedException Icinga\Exception\ProgrammingError + * @depends testWhetherHooksCanBeRegisteredWithRegisterClass + */ + public function testWhetherCreateInstanceThrowsAnErrorIfGivenAHookNotInheritingFromAPredefinedAbstractHook() + { + Mockery::mock(Hook::$BASE_NS . 'TestHook'); + Hook::registerClass('TestHook', __FUNCTION__, get_class(Mockery::mock('TestHook'))); + + Hook::createInstance('TestHook', __FUNCTION__); + } + + /** + * @depends testWhetherHooksCanBeRegisteredWithRegisterObject + */ + public function testWhetherAllReturnsAllRegisteredHooks() + { + $hook = Mockery::mock(Hook::$BASE_NS . 'TestHook'); + Hook::registerObject('TestHook', 'one', $hook); + Hook::registerObject('TestHook', 'two', $hook); + Hook::registerObject('TestHook', 'three', $hook); + + $this->assertCount(3, Hook::all('TestHook'), 'Hook::all does not return all registered hooks'); + } + + public function testWhetherAllReturnsNothingIfGivenAnUnknownHookName() + { + $this->assertEmpty( + Hook::all('not_existing'), + 'Hook::all does not return an empty array if given an unknown hook' + ); + } + + /** + * @depends testWhetherHooksCanBeRegisteredWithRegisterObject + */ + public function testWhetherFirstReturnsTheFirstRegisteredHook() + { + $firstHook = Mockery::mock(Hook::$BASE_NS . 'TestHook'); + $secondHook = Mockery::mock(Hook::$BASE_NS . 'TestHook'); + Hook::registerObject('TestHook', 'first', $firstHook); + Hook::registerObject('TestHook', 'second', $secondHook); + + $this->assertSame( + $firstHook, + Hook::first('TestHook'), + 'Hook::first does not return the first registered hook' + ); } }