MDL-39846 improve validation and enable hardcoding of system context

lib/classes/event/base.php

@@ -136,6 +136,9 @@ abstract class base implements \IteratorAggregate {
         $event->data['object'] = substr($parts[2], 0, $pos);
         $event->data['action'] = substr($parts[2], $pos+1);
 
+        // Set static event data specific for child class.
+        $event->init();
+
         // Set optional data or use defaults.
         $event->data['objectid'] = isset($data['objectid']) ? $data['objectid'] : null;
         $event->data['courseid'] = isset($data['courseid']) ? $data['courseid'] : null;
@@ -143,8 +146,19 @@ abstract class base implements \IteratorAggregate {
         $event->data['other'] = isset($data['other']) ? $data['other'] : null;
         $event->data['relateduserid'] = isset($data['relateduserid']) ? $data['relateduserid'] : null;
 
-        if (!empty($data['context'])) {
+        if (isset($event->context)) {
+            if (isset($data['context']) and $event->context->id != $data['context']->id) {
+                throw new \coding_exception('Given context does not match expected context');
+            }
+            $event->data['contextid'] = $event->context->id;
+            $event->data['contextlevel'] = $event->context->contextlevel;
+            $event->data['contextinstanceid'] = $event->context->instanceid;
+
+        } else if (!empty($data['context'])) {
             $event->context = $data['context'];
+            if (isset($event->data['contextlevel']) and $event->data['contextlevel'] != $event->context->contextlevel) {
+                throw new \coding_exception('Context does not match expected context level');
+            }
             $event->data['contextid'] = $event->context->id;
             $event->data['contextlevel'] = $event->context->contextlevel;
             $event->data['contextinstanceid'] = $event->context->instanceid;
@@ -164,7 +178,7 @@ abstract class base implements \IteratorAggregate {
                 $event->data['contextinstanceid'] = $event->context->instanceid;
             }
         } else {
-            throw new \coding_exception('context (or contextid) is a required event property.');
+            throw new \coding_exception('context (or contextid) is a required event property, system context may be hardcoded in init() method.');
         }
 
         if (!isset($event->data['courseid'])) {
@@ -179,9 +193,6 @@ abstract class base implements \IteratorAggregate {
             $event->data['relateduserid'] = $event->context->instanceid;
         }
 
-        // Set static event data specific for child class, this should also validate other data.
-        $event->init();
-
         // Warn developers if they do something wrong.
         if (debugging('', DEBUG_DEVELOPER)) { // This should be replaced by new $CFG->slowdebug flag if introduced.
             static $automatickeys = array('eventname', 'component', 'action', 'object', 'timecreated');
@@ -203,6 +214,9 @@ abstract class base implements \IteratorAggregate {
             }
         }
 
+        // Let developers validate their custom data (such as $this->date['other']).
+        $event->validate_data();
+
         return $event;
     }
 
@@ -210,18 +224,28 @@ abstract class base implements \IteratorAggregate {
      * Override in subclass.
      *
      * Set all required data properties:
-     *  1/ crud - letter [crud]
-     *  2/ level - number 1...100
+     *  1/ crud - letter [crud]     TODO: MDL-37658
+     *  2/ level - number 1...100   TODO: MDL-37658
      *  3/ objecttable - name of database table if objectid specified
      *
-     * TODO: MDL-37658
-     *
-     * Optionally validate $this->data['other'].
+     * Optionally it can set:
+     * a/ fixed system context
+     * b/ required context level
      *
      * @return void
      */
     protected abstract function init();
 
+    /**
+     * Let developers validate their custom data (such as $this->date['other']).
+     *
+     * Throw \coding_exception or debugging() notice in case of any problems.
+     */
+    protected function validate_data() {
+        // Override if you want to validate event properties when
+        // creating new events.
+    }
+
     /**
      * Returns localised general event name.
      *

lib/tests/event_test.php

@@ -622,6 +622,28 @@ class core_event_testcase extends advanced_testcase {
         $this->assertDebuggingNotCalled();
         $event6->trigger();
         $this->assertDebuggingCalled();
+
+        $event = \core_tests\event\problematic_event2::create(array('context'=>\context_course::instance(1)));
+        try {
+            $event = \core_tests\event\problematic_event2::create(array('context'=>\context_system::instance()));
+            $this->fail('Exception expected when $data contains context that does not match hardcoded level');
+        } catch (\moodle_exception $e) {
+            $this->assertInstanceOf('\coding_exception', $e);
+        }
+
+        $event = \core_tests\event\problematic_event3::create(array());
+        $event = \core_tests\event\problematic_event3::create(array('context'=>\context_system::instance()));
+        try {
+            $event = \core_tests\event\problematic_event3::create(array('context'=>\context_course::instance(1)));
+            $this->fail('Exception expected when $data contains matching or no context if hardcoded in init');
+        } catch (\moodle_exception $e) {
+            $this->assertInstanceOf('\coding_exception', $e);
+        }
+
+        $event = \core_tests\event\problematic_event4::create(array('other'=>1));
+        $this->assertDebuggingNotCalled();
+        $event = \core_tests\event\problematic_event4::create(array());
+        $this->assertDebuggingCalled();
     }
 
     public function test_record_snapshots() {

lib/tests/fixtures/event_fixtures.php

@@ -162,3 +162,33 @@ class problematic_event1 extends \core\event\base {
         $this->data['level'] = 10;
     }
 }
+
+class problematic_event2 extends \core\event\base {
+    protected function init() {
+        $this->data['crud'] = 'c';
+        $this->data['level'] = 10;
+        $this->data['contextlevel'] = CONTEXT_COURSE;
+    }
+}
+
+class problematic_event3 extends \core\event\base {
+    protected function init() {
+        $this->data['crud'] = 'c';
+        $this->data['level'] = 10;
+        $this->context = \context_system::instance();
+    }
+}
+
+class problematic_event4 extends \core\event\base {
+    protected function init() {
+        $this->data['crud'] = 'c';
+        $this->data['level'] = 10;
+        $this->context = \context_system::instance();
+    }
+
+    protected function validate_data() {
+        if (empty($this->data['other'])) {
+            debugging('other is missing');
+        }
+    }
+}