From 27af3e625c488e7cab1e08cd59d47f2ad703d677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Sat, 13 Jul 2013 14:14:33 +0200 Subject: [PATCH] MDL-39846 require context when creating events No more guessing or falling back to system context. --- lib/classes/event/base.php | 39 ++++++++++++++++++++++---------------- lib/tests/event_test.php | 39 +++++++++++++++++++++++++------------- 2 files changed, 49 insertions(+), 29 deletions(-) diff --git a/lib/classes/event/base.php b/lib/classes/event/base.php index f4f73beca3e..be4710812ea 100644 --- a/lib/classes/event/base.php +++ b/lib/classes/event/base.php @@ -143,32 +143,39 @@ abstract class base { $event->data['other'] = isset($data['other']) ? $data['other'] : null; $event->data['relateduserid'] = isset($data['relateduserid']) ? $data['relateduserid'] : null; - $event->context = null; - if (isset($data['context'])) { + if (!empty($data['context'])) { $event->context = $data['context']; - } else if (isset($data['contextid'])) { - $event->context = \context::instance_by_id($data['contextid']); - } else if ($event->data['courseid']) { - $event->context = \context_course::instance($event->data['courseid']); - } else if (isset($PAGE)) { - $event->context = $PAGE->context; + $event->data['contextid'] = $event->context->id; + $event->data['contextlevel'] = $event->context->contextlevel; + $event->data['contextinstanceid'] = $event->context->instanceid; + + } else if (!empty($data['contextid'])) { + $event->context = null; + if (isset($data['contextlevel']) and isset($data['contextinstanceid'])) { + // Useful especially when deleting contexts because we can nto fetch it from DB anymore. + $event->data['contextid'] = $data['contextid']; + $event->data['contextlevel'] = $data['contextlevel']; + $event->data['contextinstanceid'] = $data['contextinstanceid']; + $event->context = \context::instance_by_id($data['contextid'], IGNORE_MISSING); + } else { + $event->context = \context::instance_by_id($data['contextid'], MUST_EXIST); + $event->data['contextid'] = $event->context->id; + $event->data['contextlevel'] = $event->context->contextlevel; + $event->data['contextinstanceid'] = $event->context->instanceid; + } + } else { + throw new \coding_exception('context (or contextid) is a required event property.'); } - if (!$event->context) { - $event->context = \context_system::instance(); - } - $event->data['contextid'] = $event->context->id; - $event->data['contextlevel'] = $event->context->contextlevel; - $event->data['contextinstanceid'] = $event->context->instanceid; if (!isset($event->data['courseid'])) { - if ($coursecontext = $event->context->get_course_context(false)) { + if ($event->context and $coursecontext = $event->context->get_course_context(false)) { $event->data['courseid'] = $coursecontext->id; } else { $event->data['courseid'] = 0; } } - if (!array_key_exists('relateduserid', $data) and $event->context->contextlevel == CONTEXT_USER) { + if (!array_key_exists('relateduserid', $data) and $event->context and $event->context->contextlevel == CONTEXT_USER) { $event->data['relateduserid'] = $event->context->instanceid; } diff --git a/lib/tests/event_test.php b/lib/tests/event_test.php index 1c32ede9197..f86f8080271 100644 --- a/lib/tests/event_test.php +++ b/lib/tests/event_test.php @@ -73,6 +73,12 @@ class core_event_testcase extends advanced_testcase { } catch (\moodle_exception $e) { $this->assertInstanceOf('coding_exception', $e); } + + $event2 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'contextid'=>$system->id, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10))); + $this->assertSame($event->get_context(), $event2->get_context()); + + $event3 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'contextid'=>999, 'contextlevel'=>CONTEXT_COURSE, 'contextinstanceid'=>4554645, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10))); + $this->assertSame(false, $event3->get_context()); } public function test_observers_parsing() { @@ -539,7 +545,14 @@ class core_event_testcase extends advanced_testcase { } public function test_bad_events() { - $event = \core_tests\event\bad_event1::create(); + try { + $event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'other'=>array('sample'=>5, 'xx'=>10))); + $this->fail('Exception expected when context and contextid missing'); + } catch (Exception $e) { + $this->assertInstanceOf('coding_exception', $e); + } + + $event = \core_tests\event\bad_event1::create(array('context'=>\context_system::instance())); try { $event->trigger(); $this->fail('Exception expected when $data not valid'); @@ -547,7 +560,7 @@ class core_event_testcase extends advanced_testcase { $this->assertInstanceOf('\coding_exception', $e); } - $event = \core_tests\event\bad_event2::create(); + $event = \core_tests\event\bad_event2::create(array('context'=>\context_system::instance())); try { $event->trigger(); $this->fail('Exception expected when $data not valid'); @@ -555,23 +568,23 @@ class core_event_testcase extends advanced_testcase { $this->assertInstanceOf('\coding_exception', $e); } - $event = \core_tests\event\bad_event3::create(); + $event = \core_tests\event\bad_event3::create(array('context'=>\context_system::instance())); @$event->trigger(); $this->assertDebuggingCalled(); - $event = \core_tests\event\bad_event4::create(); + $event = \core_tests\event\bad_event4::create(array('context'=>\context_system::instance())); @$event->trigger(); $this->assertDebuggingCalled(); - $event = \core_tests\event\bad_event5::create(); + $event = \core_tests\event\bad_event5::create(array('context'=>\context_system::instance())); @$event->trigger(); $this->assertDebuggingCalled(); - $event = \core_tests\event\bad_event6::create(); + $event = \core_tests\event\bad_event6::create(array('context'=>\context_system::instance())); $event->trigger(); $this->assertDebuggingCalled(); - $event = \core_tests\event\bad_event7::create(array('objectid'=>1)); + $event = \core_tests\event\bad_event7::create(array('objectid'=>1, 'context'=>\context_system::instance())); try { $event->trigger(); $this->fail('Exception expected when $data contains objectid by objecttable not specified'); @@ -582,30 +595,30 @@ class core_event_testcase extends advanced_testcase { public function test_problematic_events() { global $CFG; - $event1 = \core_tests\event\problematic_event1::create(); + $event1 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance())); $this->assertDebuggingNotCalled(); $this->assertNull($event1->xxx); $this->assertDebuggingCalled(); - $event2 = \core_tests\event\problematic_event1::create(array('xxx'=>0)); + $event2 = \core_tests\event\problematic_event1::create(array('xxx'=>0, 'context'=>\context_system::instance())); $this->assertDebuggingCalled(); $CFG->debug = 0; - $event3 = \core_tests\event\problematic_event1::create(array('xxx'=>0)); + $event3 = \core_tests\event\problematic_event1::create(array('xxx'=>0, 'context'=>\context_system::instance())); $this->assertDebuggingNotCalled(); $CFG->debug = E_ALL | E_STRICT; - $event4 = \core_tests\event\problematic_event1::create(array('other'=>array('a'=>1))); + $event4 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance(), 'other'=>array('a'=>1))); $event4->trigger(); $this->assertDebuggingNotCalled(); - $event5 = \core_tests\event\problematic_event1::create(array('other'=>(object)array('a'=>1))); + $event5 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance(), 'other'=>(object)array('a'=>1))); $this->assertDebuggingNotCalled(); $event5->trigger(); $this->assertDebuggingCalled(); $url = new moodle_url('/admin/'); - $event6 = \core_tests\event\problematic_event1::create(array('other'=>array('a'=>$url))); + $event6 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance(), 'other'=>array('a'=>$url))); $this->assertDebuggingNotCalled(); $event6->trigger(); $this->assertDebuggingCalled();