diff --git a/lib/classes/event/manager.php b/lib/classes/event/manager.php index d384029dc7e..400723d181b 100644 --- a/lib/classes/event/manager.php +++ b/lib/classes/event/manager.php @@ -97,6 +97,7 @@ class manager { protected static function process_buffers() { global $DB, $CFG; + self::init_all_observers(); while (self::$buffer or self::$extbuffer) { @@ -116,45 +117,49 @@ class manager { return; } - $observers = self::get_event_observers('\\'.get_class($event)); - - foreach ($observers as $observer) { - if ($observer->internal) { - if ($fromextbuffer) { - // Do not send buffered external events to internal handlers, - // they processed them already. - continue; - } - } else { - if ($DB->is_transaction_started()) { + $observingclasses = self::get_observing_classes($event); + foreach ($observingclasses as $observingclass) { + if (!isset(self::$allobservers[$observingclass])) { + continue; + } + foreach (self::$allobservers[$observingclass] as $observer) { + if ($observer->internal) { if ($fromextbuffer) { - // Weird! + // Do not send buffered external events to internal handlers, + // they processed them already. continue; } - // Do not notify external observers while in DB transaction. - if (!$addedtoextbuffer) { - self::$extbuffer[] = $event; - $addedtoextbuffer = true; + } else { + if ($DB->is_transaction_started()) { + if ($fromextbuffer) { + // Weird! + continue; + } + // Do not notify external observers while in DB transaction. + if (!$addedtoextbuffer) { + self::$extbuffer[] = $event; + $addedtoextbuffer = true; + } + continue; } - continue; } - } - if (isset($observer->includefile) and file_exists($observer->includefile)) { - include_once($observer->includefile); - } - if (is_callable($observer->callable)) { - try { - call_user_func($observer->callable, $event); - } catch (\Exception $e) { - // Observers are notified before installation and upgrade, this may throw errors. - if (empty($CFG->upgraderunning)) { - // Ignore errors during upgrade, otherwise warn developers. - debugging("Exception encountered in event observer '$observer->callable': ".$e->getMessage(), DEBUG_DEVELOPER, $e->getTrace()); - } + if (isset($observer->includefile) and file_exists($observer->includefile)) { + include_once($observer->includefile); + } + if (is_callable($observer->callable)) { + try { + call_user_func($observer->callable, $event); + } catch (\Exception $e) { + // Observers are notified before installation and upgrade, this may throw errors. + if (empty($CFG->upgraderunning)) { + // Ignore errors during upgrade, otherwise warn developers. + debugging("Exception encountered in event observer '$observer->callable': ".$e->getMessage(), DEBUG_DEVELOPER, $e->getTrace()); + } + } + } else { + debugging("Can not execute event observer '$observer->callable'"); } - } else { - debugging("Can not execute event observer '$observer->callable'"); } } @@ -163,22 +168,25 @@ class manager { } /** - * Returns list of event observers. - * @param string $classname + * Returns list of classes related to this event. + * @param \core\event\base $event * @return array */ - protected static function get_event_observers($classname) { - self::init_all_observers(); + protected static function get_observing_classes(\core\event\base $event) { + $observers = array('\core\event\base', '\\'.get_class($event)); + return $observers; - if (isset(self::$allobservers[$classname])) { - return self::$allobservers[$classname]; + // Note if we ever decide to observe events by parent class name use the following code instead. + /* + $classname = get_class($event); + $observers = array('\\'.$classname); + while ($classname = get_parent_class($classname)) { + $observers[] = '\\'.$classname; } + $observers = array_reverse($observers, false); - if (isset(self::$allobservers['*'])) { - return self::$allobservers['*']; - } - - return array(); + return $observers; + */ } /** @@ -246,7 +254,10 @@ class manager { debugging("Invalid 'eventname' detected in $file observer definition", DEBUG_DEVELOPER); continue; } - if ($observer['eventname'] !== '*' and strpos($observer['eventname'], '\\') !== 0) { + if ($observer['eventname'] === '*') { + $observer['eventname'] = '\core\event\base'; + } + if (strpos($observer['eventname'], '\\') !== 0) { $observer['eventname'] = '\\'.$observer['eventname']; } if (empty($observer['callback'])) { @@ -285,22 +296,7 @@ class manager { * Reorder observers to allow quick lookup of observer for each event. */ protected static function order_all_observers() { - $catchall = array(); - if (isset(self::$allobservers['*'])) { - $catchall = self::$allobservers['*']; - unset(self::$allobservers['*']); // Move it to the end. - \core_collator::asort_objects_by_property($catchall, 'priority', \core_collator::SORT_NUMERIC); - $catchall = array_reverse($catchall); - self::$allobservers['*'] = $catchall; - } foreach (self::$allobservers as $classname => $observers) { - if ($classname === '*') { - continue; - } - if ($catchall) { - $observers = array_merge($observers, $catchall); - } - \core_collator::asort_objects_by_property($observers, 'priority', \core_collator::SORT_NUMERIC); self::$allobservers[$classname] = array_reverse($observers); } diff --git a/lib/tests/event_test.php b/lib/tests/event_test.php index 8d9393307c4..74e037892d4 100644 --- a/lib/tests/event_test.php +++ b/lib/tests/event_test.php @@ -92,6 +92,10 @@ class core_event_testcase extends advanced_testcase { public function test_observers_parsing() { $observers = array( + array( + 'eventname' => '*', + 'callback' => array('\core_tests\event\unittest_observer', 'observe_all_alt'), + ), array( 'eventname' => '\core_tests\event\unittest_executed', 'callback' => '\core_tests\event\unittest_observer::observe_one', @@ -102,7 +106,7 @@ class core_event_testcase extends advanced_testcase { 'callback' => array('\core_tests\event\unittest_observer', 'observe_all'), 'includefile' => null, 'internal' => 1, - 'priority' => 9999, + 'priority' => 10, ), array( 'eventname' => '\core\event\unknown_executed', @@ -118,58 +122,49 @@ class core_event_testcase extends advanced_testcase { ); $result = \core\event\manager::phpunit_replace_observers($observers); - $this->assertCount(3, $result); - end($result); - $this->assertSame('*', key($result)); $expected = array(); $observer = new stdClass(); - $observer->callable = array('\core_tests\event\unittest_observer', 'observe_all'); - $observer->priority = 9999; - $observer->internal = true; - $observer->includefile = null; - $expected[0] = $observer; - $observer = new stdClass(); $observer->callable = '\core_tests\event\unittest_observer::external_observer'; $observer->priority = 200; $observer->internal = false; $observer->includefile = null; - $expected[1] = $observer; + $expected[0] = $observer; $observer = new stdClass(); $observer->callable = '\core_tests\event\unittest_observer::observe_one'; $observer->priority = 0; $observer->internal = true; $observer->includefile = 'lib/tests/fixtures/event_fixtures.php'; - $expected[2] = $observer; + $expected[1] = $observer; $this->assertEquals($expected, $result['\core_tests\event\unittest_executed']); $expected = array(); $observer = new stdClass(); - $observer->callable = array('\core_tests\event\unittest_observer', 'observe_all'); - $observer->priority = 9999; - $observer->internal = true; - $observer->includefile = null; - $expected[0] = $observer; - $observer = new stdClass(); $observer->callable = '\core_tests\event\unittest_observer::broken_observer'; $observer->priority = 100; $observer->internal = true; $observer->includefile = null; - $expected[1] = $observer; + $expected[0] = $observer; $this->assertEquals($expected, $result['\core\event\unknown_executed']); $expected = array(); $observer = new stdClass(); $observer->callable = array('\core_tests\event\unittest_observer', 'observe_all'); - $observer->priority = 9999; + $observer->priority = 10; $observer->internal = true; $observer->includefile = null; $expected[0] = $observer; + $observer = new stdClass(); + $observer->callable = array('\core_tests\event\unittest_observer', 'observe_all_alt'); + $observer->priority = 0; + $observer->internal = true; + $observer->includefile = null; + $expected[1] = $observer; - $this->assertEquals($expected, $result['*']); + $this->assertEquals($expected, $result['\core\event\base']); // Now test broken stuff... diff --git a/lib/tests/fixtures/event_fixtures.php b/lib/tests/fixtures/event_fixtures.php index c8a287fb1d4..93a4a357058 100644 --- a/lib/tests/fixtures/event_fixtures.php +++ b/lib/tests/fixtures/event_fixtures.php @@ -76,20 +76,25 @@ class unittest_observer { self::$event[] = $event; } - public static function external_observer(\core\event\base $event) { + public static function external_observer(unittest_executed $event) { self::$info[] = 'external_observer-'.$event->courseid; self::$event[] = $event; } - public static function broken_observer(\core\event\base $event) { + public static function broken_observer(unittest_executed $event) { self::$info[] = 'broken_observer-'.$event->courseid; self::$event[] = $event; throw new \Exception('someerror'); } - public static function observe_all(unittest_executed $event) { + public static function observe_all(\core\event\base $event) { + if (!($event instanceof unittest_executed)) { + self::$info[] = 'observe_all-unknown'; + self::$event[] = $event; + return; + } self::$event[] = $event; - if ($event->nest) { + if (!empty($event->nest)) { self::$info[] = 'observe_all-nesting-'.$event->courseid; unittest_executed::create(array('courseid'=>3, 'context'=>\context_system::instance(), 'other'=>array('sample'=>666, 'xx'=>666)))->trigger(); } else { @@ -97,6 +102,11 @@ class unittest_observer { } } + public static function observe_all_alt(\core\event\base $event) { + self::$info[] = 'observe_all_alt'; + self::$event[] = $event; + } + public static function legacy_handler($data) { self::$info[] = 'legacy_handler-'.$data[0]; self::$event[] = $data;