From 9ede00db1858503ee8735abb2237859acae2ff86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0koda?= Date: Fri, 7 Mar 2014 15:07:33 +0800 Subject: [PATCH] MDL-44500 detect context-courseid inconsistencies in new events --- lib/classes/event/base.php | 7 ++ .../event/mnet_access_control_created.php | 2 +- .../event/mnet_access_control_updated.php | 2 +- lib/tests/event_test.php | 66 +++++++++---------- lib/tests/fixtures/event_fixtures.php | 12 ++-- mnet/lib.php | 2 - 6 files changed, 48 insertions(+), 43 deletions(-) diff --git a/lib/classes/event/base.php b/lib/classes/event/base.php index 6cdb8e179e8..0050d963642 100644 --- a/lib/classes/event/base.php +++ b/lib/classes/event/base.php @@ -229,6 +229,13 @@ abstract class base implements \IteratorAggregate { debugging("Data key '$key' does not exist in \\core\\event\\base"); } } + $expectedcourseid = 0; + if ($coursecontext = $event->context->get_course_context(false)) { + $expectedcourseid = $coursecontext->instanceid; + } + if ($expectedcourseid != $event->data['courseid']) { + debugging("Inconsistent courseid - context combination detected.", DEBUG_DEVELOPER); + } } // Let developers validate their custom data (such as $this->data['other'], contextlevel, etc.). diff --git a/lib/classes/event/mnet_access_control_created.php b/lib/classes/event/mnet_access_control_created.php index 7f5baf199c0..3bf5979f1fc 100644 --- a/lib/classes/event/mnet_access_control_created.php +++ b/lib/classes/event/mnet_access_control_created.php @@ -76,7 +76,7 @@ class mnet_access_control_created extends base { $mnetaccesscontrol = $this->get_record_snapshot('mnet_sso_access_control', $this->objectid); $mnethost = $this->get_record_snapshot('mnet_host', $mnetaccesscontrol->mnet_host_id); - return array($this->courseid, 'admin/mnet', 'add', 'admin/mnet/access_control.php', 'SSO ACL: ' . + return array(SITEID, 'admin/mnet', 'add', 'admin/mnet/access_control.php', 'SSO ACL: ' . $mnetaccesscontrol->accessctrl . ' user \'' . $mnetaccesscontrol->username . '\' from ' . $mnethost->name); } diff --git a/lib/classes/event/mnet_access_control_updated.php b/lib/classes/event/mnet_access_control_updated.php index 3e4151e70a2..88e309d6495 100644 --- a/lib/classes/event/mnet_access_control_updated.php +++ b/lib/classes/event/mnet_access_control_updated.php @@ -76,7 +76,7 @@ class mnet_access_control_updated extends base { $mnetaccesscontrol = $this->get_record_snapshot('mnet_sso_access_control', $this->objectid); $mnethost = $this->get_record_snapshot('mnet_host', $mnetaccesscontrol->mnet_host_id); - return array($this->courseid, 'admin/mnet', 'update', 'admin/mnet/access_control.php', 'SSO ACL: ' . + return array(SITEID, 'admin/mnet', 'update', 'admin/mnet/access_control.php', 'SSO ACL: ' . $mnetaccesscontrol->accessctrl . ' user \'' . $mnetaccesscontrol->username . '\' from ' . $mnethost->name); } diff --git a/lib/tests/event_test.php b/lib/tests/event_test.php index fdfa34fc167..28f761bce68 100644 --- a/lib/tests/event_test.php +++ b/lib/tests/event_test.php @@ -33,7 +33,7 @@ class core_event_testcase extends advanced_testcase { global $USER; $system = \context_system::instance(); - $event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>$system, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10))); + $event = \core_tests\event\unittest_executed::create(array('context'=>$system, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10))); $this->assertSame('\core_tests\event\unittest_executed', $event->eventname); $this->assertSame('core_tests', $event->component); @@ -49,7 +49,7 @@ class core_event_testcase extends advanced_testcase { $this->assertSame($system->instanceid, $event->contextinstanceid); $this->assertSame($USER->id, $event->userid); - $this->assertSame(1, $event->courseid); + $this->assertSame(0, $event->courseid); $this->assertNull($event->relateduserid); $this->assertFalse(isset($event->relateduserid)); @@ -74,7 +74,7 @@ class core_event_testcase extends advanced_testcase { $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))); + $event2 = \core_tests\event\unittest_executed::create(array('contextid'=>$system->id, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10))); $this->assertEquals($event->get_context(), $event2->get_context()); } @@ -284,7 +284,7 @@ class core_event_testcase extends advanced_testcase { \core\event\manager::phpunit_replace_observers($observers); \core_tests\event\unittest_observer::reset(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $event1->nest = 1; $this->assertFalse($event1->is_triggered()); $this->assertFalse($event1->is_dispatched()); @@ -294,23 +294,23 @@ class core_event_testcase extends advanced_testcase { $this->assertTrue($event1->is_dispatched()); $this->assertFalse($event1->is_restored()); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>2, 'context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); $event1->trigger(); $this->assertSame( - array('observe_all-nesting-1', 'observe_one-1', 'observe_all-3', 'observe_one-3', 'observe_all-2', 'observe_one-2'), + array('observe_all-nesting-1', 'observe_one-1', 'observe_all-666', 'observe_one-666', 'observe_all-2', 'observe_one-2'), \core_tests\event\unittest_observer::$info); } public function test_event_sink() { $sink = $this->redirectEvents(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $event1->trigger(); $this->assertSame(1, $sink->count()); $retult = $sink->get_events(); $this->assertSame($event1, $retult[0]); - $event2 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); + $event2 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); $event2->trigger(); $this->assertSame(2, $sink->count()); $retult = $sink->get_events(); @@ -321,14 +321,14 @@ class core_event_testcase extends advanced_testcase { $this->assertSame(0, $sink->count()); $this->assertSame(array(), $sink->get_events()); - $event3 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>3, 'xx'=>10))); + $event3 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>3, 'xx'=>10))); $event3->trigger(); $this->assertSame(1, $sink->count()); $retult = $sink->get_events(); $this->assertSame($event3, $retult[0]); $sink->close(); - $event4 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>4, 'xx'=>10))); + $event4 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>4, 'xx'=>10))); $event4->trigger(); $this->assertSame(1, $sink->count()); $retult = $sink->get_events(); @@ -353,11 +353,11 @@ class core_event_testcase extends advanced_testcase { \core\event\manager::phpunit_replace_observers($observers); \core_tests\event\unittest_observer::reset(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $event1->trigger(); $this->assertDebuggingCalled(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>2, 'context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); $event1->trigger(); $this->assertDebuggingCalled(); @@ -389,9 +389,9 @@ class core_event_testcase extends advanced_testcase { \core\event\manager::phpunit_replace_observers($observers); \core_tests\event\unittest_observer::reset(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $event1->trigger(); - $event2 = \core_tests\event\unittest_executed::create(array('courseid'=>2, 'context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); + $event2 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); $event2->trigger(); $this->assertSame( @@ -405,9 +405,9 @@ class core_event_testcase extends advanced_testcase { $trans = $DB->start_delegated_transaction(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $event1->trigger(); - $event2 = \core_tests\event\unittest_executed::create(array('courseid'=>2, 'context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); + $event2 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); $event2->trigger(); $this->assertSame( @@ -423,10 +423,10 @@ class core_event_testcase extends advanced_testcase { \core\event\manager::phpunit_replace_observers($observers); \core_tests\event\unittest_observer::reset(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $event1->trigger(); $trans = $DB->start_delegated_transaction(); - $event2 = \core_tests\event\unittest_executed::create(array('courseid'=>2, 'context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); + $event2 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>2, 'xx'=>10))); $event2->trigger(); try { $trans->rollback(new \moodle_exception('xxx')); @@ -486,27 +486,27 @@ class core_event_testcase extends advanced_testcase { \core\event\manager::phpunit_replace_observers($observers); \core_tests\event\unittest_observer::reset(); - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>5, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>5, 'xx'=>10))); $event1->trigger(); - $event2 = \core_tests\event\unittest_executed::create(array('courseid'=>2, 'context'=>\context_system::instance(), 'other'=>array('sample'=>6, 'xx'=>11))); + $event2 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>6, 'xx'=>11))); $event2->nest = true; $event2->trigger(); $this->assertSame( - array('observe_all-1', 'observe_one-1', 'legacy_handler-1', 'observe_all-nesting-2', 'legacy_handler-3', 'observe_one-2', 'observe_all-3', 'observe_one-3', 'legacy_handler-2'), + array('observe_all-5', 'observe_one-5', 'legacy_handler-0', 'observe_all-nesting-6', 'legacy_handler-0', 'observe_one-6', 'observe_all-666', 'observe_one-666', 'legacy_handler-0'), \core_tests\event\unittest_observer::$info); $this->assertSame($event1, \core_tests\event\unittest_observer::$event[0]); $this->assertSame($event1, \core_tests\event\unittest_observer::$event[1]); - $this->assertSame(array(1, 5), \core_tests\event\unittest_observer::$event[2]); + $this->assertSame(array(0, 5), \core_tests\event\unittest_observer::$event[2]); $logs = $DB->get_records('log', array(), 'id ASC'); $this->assertCount(0, $logs); } public function test_restore_event() { - $event1 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event1 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $data1 = $event1->get_data(); $event2 = \core\event\base::restore($data1, array('origin'=>'clid')); @@ -542,7 +542,7 @@ class core_event_testcase extends advanced_testcase { public function test_trigger_problems() { $this->resetAfterTest(true); - $event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>5, 'xx'=>10))); + $event = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>5, 'xx'=>10))); $event->trigger(); try { $event->trigger(); @@ -563,7 +563,7 @@ class core_event_testcase extends advanced_testcase { $this->assertInstanceOf('coding_exception', $e); } - $event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>5, 'xx'=>10))); + $event = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>5, 'xx'=>10))); try { \core\event\manager::dispatch($event); $this->fail('Exception expected on manual event dispatching'); @@ -576,7 +576,7 @@ class core_event_testcase extends advanced_testcase { $this->resetAfterTest(true); try { - $event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'other'=>array('sample'=>5, 'xx'=>10))); + $event = \core_tests\event\unittest_executed::create(array('other'=>array('sample'=>5, 'xx'=>10))); $this->fail('Exception expected when context and contextid missing'); } catch (\moodle_exception $e) { $this->assertInstanceOf('coding_exception', $e); @@ -663,7 +663,7 @@ class core_event_testcase extends advanced_testcase { $this->assertDebuggingCalled(); // Check that whole float numbers do not trigger debugging messages. - $event7 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), + $event7 = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other' => array('wholenumber' => 90.0000, 'numberwithdecimals' => 54.7656, 'sample' => 1))); $event7->trigger(); $this->assertDebuggingNotCalled(); @@ -684,13 +684,13 @@ class core_event_testcase extends advanced_testcase { $this->resetAfterTest(true); - $event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $course1 = $DB->get_record('course', array('id'=>1)); $this->assertNotEmpty($course1); $event->add_record_snapshot('course', $course1); - $result = $event->get_record_snapshot('course', 1); + $result = $event->get_record_snapshot('course', $course1->id); // Convert to arrays because record snapshot returns a clone of the object. $this->assertSame((array)$course1, (array)$result); @@ -709,7 +709,7 @@ class core_event_testcase extends advanced_testcase { $event2 = \core_tests\event\unittest_executed::restore($event->get_data(), array()); try { - $event2->get_record_snapshot('course', 1, $course1); + $event2->get_record_snapshot('course', $course1->id); $this->fail('Reading of snapshots from restored events is not ok');; } catch (\moodle_exception $e) { $this->assertInstanceOf('\coding_exception', $e); @@ -717,12 +717,12 @@ class core_event_testcase extends advanced_testcase { } public function test_get_name() { - $event = \core_tests\event\noname_event::create(array('courseid' => 1, 'other' => array('sample' => 1, 'xx' => 10))); + $event = \core_tests\event\noname_event::create(array('other' => array('sample' => 1, 'xx' => 10))); $this->assertEquals("core_tests: noname event", $event->get_name()); } public function test_iteration() { - $event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); + $event = \core_tests\event\unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>1, 'xx'=>10))); $data = array(); foreach ($event as $k => $v) { @@ -736,7 +736,7 @@ class core_event_testcase extends advanced_testcase { * @expectedException PHPUnit_Framework_Error_Notice */ public function test_context_not_used() { - $event = \core_tests\event\context_used_in_event::create(array('courseid' => 1, 'other' => array('sample' => 1, 'xx' => 10))); + $event = \core_tests\event\context_used_in_event::create(array('other' => array('sample' => 1, 'xx' => 10))); $this->assertEventContextNotUsed($event); $eventcontext = phpunit_event_mock::testable_get_event_context($event); diff --git a/lib/tests/fixtures/event_fixtures.php b/lib/tests/fixtures/event_fixtures.php index 576a73bff15..9d8e5168081 100644 --- a/lib/tests/fixtures/event_fixtures.php +++ b/lib/tests/fixtures/event_fixtures.php @@ -72,17 +72,17 @@ class unittest_observer { } public static function observe_one(unittest_executed $event) { - self::$info[] = 'observe_one-'.$event->courseid; + self::$info[] = 'observe_one-'.$event->other['sample']; self::$event[] = $event; } public static function external_observer(unittest_executed $event) { - self::$info[] = 'external_observer-'.$event->courseid; + self::$info[] = 'external_observer-'.$event->other['sample']; self::$event[] = $event; } public static function broken_observer(unittest_executed $event) { - self::$info[] = 'broken_observer-'.$event->courseid; + self::$info[] = 'broken_observer-'.$event->other['sample']; self::$event[] = $event; throw new \Exception('someerror'); } @@ -95,10 +95,10 @@ class unittest_observer { } self::$event[] = $event; 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(); + self::$info[] = 'observe_all-nesting-'.$event->other['sample']; + unittest_executed::create(array('context'=>\context_system::instance(), 'other'=>array('sample'=>666, 'xx'=>666)))->trigger(); } else { - self::$info[] = 'observe_all-'.$event->courseid; + self::$info[] = 'observe_all-'.$event->other['sample']; } } diff --git a/mnet/lib.php b/mnet/lib.php index 62c8befc5a0..4e1ad1d8135 100644 --- a/mnet/lib.php +++ b/mnet/lib.php @@ -447,7 +447,6 @@ function mnet_update_sso_access_control($username, $mnet_host_id, $accessctrl) { // Trigger access control updated event. $params = array( 'objectid' => $aclrecord->id, - 'courseid' => SITEID, 'context' => context_system::instance() ); $event = \core\event\mnet_access_control_updated::create($params); @@ -464,7 +463,6 @@ function mnet_update_sso_access_control($username, $mnet_host_id, $accessctrl) { // Trigger access control created event. $params = array( 'objectid' => $aclrecord->id, - 'courseid' => SITEID, 'context' => context_system::instance() ); $event = \core\event\mnet_access_control_created::create($params);