Merge branch 'MDL-65375-master' of https://github.com/sammarshallou/m…

…oodle

admin/tool/log/backup/moodle2/restore_tool_log_logstore_subplugin.class.php

@@ -42,10 +42,11 @@ abstract class restore_tool_log_logstore_subplugin extends restore_subplugin {
      * discard or save every log entry.
      *
      * @param array $data log entry.
+     * @param bool $jsonformat If true, uses JSON format for the 'other' field
      * @return object|null $dataobject A data object with values for one or more fields in the record,
      *  or null if we are not going to process the log.
      */
-    protected function process_log($data) {
+    protected function process_log($data, bool $jsonformat = false) {
         $data = (object) $data;
 
         // Complete the information that does not come from backup.
@@ -87,7 +88,7 @@ protected function process_log($data) {
         // There is no need to roll dates. Logs are supposed to be immutable. See MDL-44961.
 
         // Revert other to its original php way.
-        $data->other = unserialize(base64_decode($data->other));
+        $data->other = \tool_log\helper\reader::decode_other(base64_decode($data->other));
 
         // Arrived here, we have both 'objectid' and 'other' to be converted. This is the tricky part.
         // Both are pointing to other records id, but the sources are not identified in the
@@ -137,15 +138,20 @@ protected function process_log($data) {
                         }
                     }
                 }
-                // Now we want to serialize it so we can store it in the DB.
-                $data->other = serialize($data->other);
             } else {
                 $message = "Event class not found: \"$eventclass\". Skipping log record.";
                 $this->log($message, backup::LOG_DEBUG);
                 return; // No such class, can not restore.
             }
         }
 
+        // Serialize 'other' field so we can store it in the DB.
+        if ($jsonformat) {
+            $data->other = json_encode($data->other);
+        } else {
+            $data->other = serialize($data->other);
+        }
+
         return $data;
     }
 }

admin/tool/log/store/database/backup/moodle2/restore_logstore_database_subplugin.class.php

@@ -93,7 +93,7 @@ public function process_logstore_database_log($data) {
             return;
         }
 
-        $data = $this->process_log($data);
+        $data = $this->process_log($data, get_config('logstore_database', 'jsonformat'));
 
         if ($data) {
             self::$extdb->insert_record(self::$extdbtablename, $data);

admin/tool/log/store/standard/backup/moodle2/restore_logstore_standard_subplugin.class.php

@@ -60,7 +60,7 @@ protected function define_logstore_subplugin_structure() {
     public function process_logstore_standard_log($data) {
         global $DB;
 
-        $data = $this->process_log($data);
+        $data = $this->process_log($data, get_config('logstore_standard', 'jsonformat'));
 
         if ($data) {
             $DB->insert_record('logstore_standard_log', $data);

admin/tool/log/store/standard/tests/fixtures/event.php

@@ -44,4 +44,14 @@ protected function init() {
     public function get_url() {
         return new \moodle_url('/somepath/somefile.php', array('id' => $this->data['other']['sample']));
     }
+
+    /**
+     * The 'other' fields for this event do not need to mapped during backup and restore as they
+     * only contain test values, not IDs for anything on the course.
+     *
+     * @return array Empty array
+     */
+    public static function get_other_mapping(): array {
+        return [];
+    }
 }

admin/tool/log/store/standard/tests/store_test.php

@@ -395,6 +395,138 @@ public function test_decode_other_provider(): array {
         ];
     }
 
+    /**
+     * Checks that backup and restore of log data works correctly.
+     *
+     * @param bool $jsonformat True to test with JSON format
+     * @dataProvider test_log_writing_provider
+     * @throws moodle_exception
+     */
+    public function test_backup_restore(bool $jsonformat) {
+        global $DB;
+        $this->resetAfterTest();
+        $this->preventResetByRollback();
+
+        // Enable logging plugin.
+        set_config('enabled_stores', 'logstore_standard', 'tool_log');
+        set_config('buffersize', 0, 'logstore_standard');
+        $manager = get_log_manager(true);
+
+        // User must be enrolled in course.
+        $generator = $this->getDataGenerator();
+        $course = $generator->create_course();
+        $user = $generator->create_user();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
+        $this->setUser($user);
+
+        // Apply JSON format system setting.
+        set_config('jsonformat', $jsonformat ? 1 : 0, 'logstore_standard');
+
+        // Create some log data in a course - one with other data, one without.
+        \logstore_standard\event\unittest_executed::create([
+                'context' => context_course::instance($course->id),
+                'other' => ['sample' => 5, 'xx' => 10]])->trigger();
+        $this->waitForSecond();
+        \logstore_standard\event\unittest_executed::create([
+                'context' => context_course::instance($course->id)])->trigger();
+
+        $records = array_values($DB->get_records('logstore_standard_log',
+                ['courseid' => $course->id, 'target' => 'unittest'], 'timecreated'));
+        $this->assertCount(2, $records);
+
+        // Work out expected 'other' values based on JSON format.
+        $expected0 = [
+            false => 'a:2:{s:6:"sample";i:5;s:2:"xx";i:10;}',
+            true => '{"sample":5,"xx":10}'
+        ];
+        $expected1 = [
+            false => 'N;',
+            true => 'null'
+        ];
+
+        // Backup the course twice, including log data.
+        $this->setAdminUser();
+        $backupid1 = $this->backup($course);
+        $backupid2 = $this->backup($course);
+
+        // Restore it with same jsonformat.
+        $newcourseid = $this->restore($backupid1, $course, '_A');
+
+        // Check entries are correctly encoded.
+        $records = array_values($DB->get_records('logstore_standard_log',
+                ['courseid' => $newcourseid, 'target' => 'unittest'], 'timecreated'));
+        $this->assertCount(2, $records);
+        $this->assertEquals($expected0[$jsonformat], $records[0]->other);
+        $this->assertEquals($expected1[$jsonformat], $records[1]->other);
+
+        // Change JSON format to opposite value and restore again.
+        set_config('jsonformat', $jsonformat ? 0 : 1, 'logstore_standard');
+        $newcourseid = $this->restore($backupid2, $course, '_B');
+
+        // Check entries are correctly encoded in other format.
+        $records = array_values($DB->get_records('logstore_standard_log',
+                ['courseid' => $newcourseid, 'target' => 'unittest'], 'timecreated'));
+        $this->assertEquals($expected0[!$jsonformat], $records[0]->other);
+        $this->assertEquals($expected1[!$jsonformat], $records[1]->other);
+    }
+
+    /**
+     * Backs a course up to temp directory.
+     *
+     * @param stdClass $course Course object to backup
+     * @return string ID of backup
+     */
+    protected function backup($course): string {
+        global $USER, $CFG;
+        require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
+
+        // Turn off file logging, otherwise it can't delete the file (Windows).
+        $CFG->backup_file_logger_level = backup::LOG_NONE;
+
+        // Do backup with default settings. MODE_IMPORT means it will just
+        // create the directory and not zip it.
+        $bc = new backup_controller(backup::TYPE_1COURSE, $course->id,
+                backup::FORMAT_MOODLE, backup::INTERACTIVE_NO, backup::MODE_IMPORT,
+                $USER->id);
+        $bc->get_plan()->get_setting('users')->set_status(backup_setting::NOT_LOCKED);
+        $bc->get_plan()->get_setting('users')->set_value(true);
+        $bc->get_plan()->get_setting('logs')->set_value(true);
+        $backupid = $bc->get_backupid();
+
+        $bc->execute_plan();
+        $bc->destroy();
+        return $backupid;
+    }
+
+    /**
+     * Restores a course from temp directory.
+     *
+     * @param string $backupid Backup id
+     * @param \stdClass $course Original course object
+     * @param string $suffix Suffix to add after original course shortname and fullname
+     * @return int New course id
+     * @throws restore_controller_exception
+     */
+    protected function restore(string $backupid, $course, string $suffix): int {
+        global $USER, $CFG;
+        require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php');
+
+        // Do restore to new course with default settings.
+        $newcourseid = restore_dbops::create_new_course(
+                $course->fullname . $suffix, $course->shortname . $suffix, $course->category);
+        $rc = new restore_controller($backupid, $newcourseid,
+                backup::INTERACTIVE_NO, backup::MODE_GENERAL, $USER->id,
+                backup::TARGET_NEW_COURSE);
+        $rc->get_plan()->get_setting('logs')->set_value(true);
+        $rc->get_plan()->get_setting('users')->set_value(true);
+
+        $this->assertTrue($rc->execute_precheck());
+        $rc->execute_plan();
+        $rc->destroy();
+
+        return $newcourseid;
+    }
+
     /**
      * Disable the garbage collector if it's enabled to ensure we don't adjust memory statistics.
      */