diff --git a/admin/tool/log/backup/moodle2/restore_tool_log_logstore_subplugin.class.php b/admin/tool/log/backup/moodle2/restore_tool_log_logstore_subplugin.class.php index c48c2c86c24..1e2ceb8fb10 100644 --- a/admin/tool/log/backup/moodle2/restore_tool_log_logstore_subplugin.class.php +++ b/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 @@ abstract class restore_tool_log_logstore_subplugin extends restore_subplugin { // 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,8 +138,6 @@ abstract class restore_tool_log_logstore_subplugin extends restore_subplugin { } } } - // 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); @@ -146,6 +145,13 @@ abstract class restore_tool_log_logstore_subplugin extends restore_subplugin { } } + // 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; } } diff --git a/admin/tool/log/store/database/backup/moodle2/restore_logstore_database_subplugin.class.php b/admin/tool/log/store/database/backup/moodle2/restore_logstore_database_subplugin.class.php index 9eca1bb813d..06c6fe20092 100644 --- a/admin/tool/log/store/database/backup/moodle2/restore_logstore_database_subplugin.class.php +++ b/admin/tool/log/store/database/backup/moodle2/restore_logstore_database_subplugin.class.php @@ -93,7 +93,7 @@ class restore_logstore_database_subplugin extends restore_tool_log_logstore_subp 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); diff --git a/admin/tool/log/store/standard/backup/moodle2/restore_logstore_standard_subplugin.class.php b/admin/tool/log/store/standard/backup/moodle2/restore_logstore_standard_subplugin.class.php index ac041e46de2..beeec88b6d6 100644 --- a/admin/tool/log/store/standard/backup/moodle2/restore_logstore_standard_subplugin.class.php +++ b/admin/tool/log/store/standard/backup/moodle2/restore_logstore_standard_subplugin.class.php @@ -60,7 +60,7 @@ class restore_logstore_standard_subplugin extends restore_tool_log_logstore_subp 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); diff --git a/admin/tool/log/store/standard/tests/fixtures/event.php b/admin/tool/log/store/standard/tests/fixtures/event.php index efeac2e8c9d..3f5490ec67b 100644 --- a/admin/tool/log/store/standard/tests/fixtures/event.php +++ b/admin/tool/log/store/standard/tests/fixtures/event.php @@ -44,4 +44,14 @@ class unittest_executed extends \core\event\base { 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 []; + } } diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php index 25963afed1c..1492718b0fb 100644 --- a/admin/tool/log/store/standard/tests/store_test.php +++ b/admin/tool/log/store/standard/tests/store_test.php @@ -395,6 +395,138 @@ class logstore_standard_store_testcase extends advanced_testcase { ]; } + /** + * 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. */