MDL-44500 detect context-courseid inconsistencies in new events

lib/classes/event/base.php

@@ -229,6 +229,13 @@ public static final function create(array $data = null) {
                     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.).

lib/classes/event/mnet_access_control_created.php

@@ -76,7 +76,7 @@ protected function get_legacy_logdata() {
         $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);
     }

lib/classes/event/mnet_access_control_updated.php

@@ -76,7 +76,7 @@ protected function get_legacy_logdata() {
         $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);
     }

lib/tests/event_test.php

@@ -33,7 +33,7 @@ public function test_event_properties() {
         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 @@ public function test_event_properties() {
         $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 @@ public function test_event_properties() {
             $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 @@ public function test_normal_dispatching() {
         \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 @@ public function test_normal_dispatching() {
         $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 @@ public function test_event_sink() {
         $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 @@ public function test_ignore_exceptions() {
         \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 @@ public function test_external_buffer() {
         \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 @@ public function test_external_buffer() {
 
         $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 @@ public function test_external_buffer() {
         \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 @@ public function test_legacy() {
         \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 @@ public function test_restore_event() {
     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 @@ public function test_trigger_problems() {
             $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 @@ public function test_bad_events() {
         $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 @@ public function test_problematic_events() {
         $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 @@ public function test_record_snapshots() {
 
         $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,20 +709,20 @@ public function test_record_snapshots() {
 
         $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);
         }
     }
 
     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 @@ public function test_iteration() {
      * @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);

lib/tests/fixtures/event_fixtures.php

@@ -72,17 +72,17 @@ public static function reset() {
     }
 
     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 @@ public static function observe_all(\core\event\base $event) {
         }
         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'];
         }
     }