mirror of
https://github.com/moodle/moodle.git
synced 2025-04-22 17:02:03 +02:00
MDL-40943 use observer priority only inside group of observers
This makes the '*' observers to be executed always first. This change allows us to implement parent event class catching if we ever decide we need it without breaking BC. This change includes some more unit test fixes.
This commit is contained in:
parent
a31e811563
commit
2d1884d987
@ -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);
|
||||
}
|
||||
|
@ -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...
|
||||
|
||||
|
18
lib/tests/fixtures/event_fixtures.php
vendored
18
lib/tests/fixtures/event_fixtures.php
vendored
@ -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;
|
||||
|
Loading…
x
Reference in New Issue
Block a user