Permalink
Browse files

MDL-44141 Completion: System updates data during restore

The completion system has code which is supposed to prevent it
updating completion status while grades are being restored. This
code was a nasty hack and had not worked for some time. As a result
completion dates were restored incorrectly.

This commit implements a 'proper' way to find out if a restore is
currently running, replacing the previous hack.
  • Loading branch information...
1 parent 872f4e2 commit 0f2929fab1246b4c68d13d72c63da4f2c1ade5d2 @sammarshallou sammarshallou committed Feb 26, 2014
View
24 backup/controller/restore_controller.class.php
@@ -57,6 +57,9 @@ class restore_controller extends base_controller {
protected $checksum; // Cache @checksumable results for lighter @is_checksum_correct() uses
+ /** @var int Number of restore_controllers that are currently executing */
+ protected static $executing = 0;
+
/**
* Constructor.
*
@@ -325,7 +328,26 @@ public function execute_plan() {
$this->log('notifying plan about excluded activities by type', backup::LOG_DEBUG);
$this->plan->set_excluding_activities();
}
- return $this->plan->execute();
+ self::$executing++;
+ try {
+ $this->plan->execute();
+ } catch (Exception $e) {
+ self::$executing--;
+ throw $e;
+ }
+ self::$executing--;
+ }
+
+ /**
+ * Checks whether restore is currently executing. Certain parts of code that
+ * is called during restore, but not directly part of the restore system, may
+ * need to behave differently during restore (e.g. do not bother resetting a
+ * cache because we know it will be reset at end of operation).
+ *
+ * @return bool True if any restore is currently executing
+ */
+ public static function is_executing() {
+ return self::$executing > 0;
}
/**
View
68 backup/controller/tests/controller_test.php
@@ -26,6 +26,7 @@
// Include all the needed stuff
global $CFG;
require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
+require_once($CFG->dirroot . '/backup/util/includes/restore_includes.php');
/*
* controller tests (all)
@@ -101,6 +102,73 @@ public function test_backup_controller_include_files() {
backup::INTERACTIVE_NO, backup::MODE_IMPORT, $this->userid);
$this->assertEquals($bc->get_include_files(), 0);
}
+
+ /**
+ * Tests the restore_controller.
+ */
+ public function test_restore_controller_is_executing() {
+ global $CFG;
+
+ // Make a backup.
+ check_dir_exists($CFG->tempdir . '/backup');
+ $bc = new backup_controller(backup::TYPE_1ACTIVITY, $this->moduleid, backup::FORMAT_MOODLE,
+ backup::INTERACTIVE_NO, backup::MODE_IMPORT, $this->userid);
+ $backupid = $bc->get_backupid();
+ $bc->execute_plan();
+ $bc->destroy();
+
+ // The progress class will get called during restore, so we can use that
+ // to check the executing flag is true.
+ $progress = new core_backup_progress_restore_is_executing();
+
+ // Set up restore.
+ $rc = new restore_controller($backupid, $this->courseid,
+ backup::INTERACTIVE_NO, backup::MODE_SAMESITE, $this->userid,
+ backup::TARGET_EXISTING_ADDING);
+ $this->assertTrue($rc->execute_precheck());
+
+ // Check restore is NOT executing.
+ $this->assertFalse(restore_controller::is_executing());
+
+ // Execute restore.
+ $rc->set_progress($progress);
+ $rc->execute_plan();
+
+ // Check restore is NOT executing afterward either.
+ $this->assertFalse(restore_controller::is_executing());
+ $rc->destroy();
+
+ // During restore, check that executing was true.
+ $this->assertTrue(count($progress->executing) > 0);
+ $alltrue = true;
+ foreach ($progress->executing as $executing) {
+ if (!$executing) {
+ $alltrue = false;
+ break;
+ }
+ }
+ $this->assertTrue($alltrue);
+
+ // Avoid test warning.
+ set_time_limit(0);
+ }
+}
+
+
+/**
+ * Progress class that records the result of restore_controller::is_executing calls.
+ *
+ * @package core_backup
+ * @copyright 2014 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class core_backup_progress_restore_is_executing extends core_backup_progress {
+ /** @var array Array of results from calling function */
+ public $executing = array();
+
+ public function update_progress() {
+ $this->executing[] = restore_controller::is_executing();
+ }
}
View
17 lib/grade/grade_grade.php
@@ -774,14 +774,6 @@ public function update($source=null) {
function notify_changed($deleted) {
global $CFG;
- // Ignore during restore
- // TODO There should be a proper way to determine when we are in restore
- // so that this hack looking for a $restore global is not needed.
- global $restore;
- if (!empty($restore->backup_unique_code)) {
- return;
- }
-
// Inform conditionlib since it may cache the grades for conditional availability of modules or sections.
if (!empty($CFG->enableavailability)) {
require_once($CFG->libdir.'/conditionlib.php');
@@ -791,11 +783,18 @@ function notify_changed($deleted) {
require_once($CFG->libdir.'/completionlib.php');
// Bail out immediately if completion is not enabled for site (saves loading
- // grade item below)
+ // grade item & requiring the restore stuff).
if (!completion_info::is_enabled_for_site()) {
return;
}
+ // Ignore during restore, as completion data will be updated anyway and
+ // doing it now will result in incorrect dates (it will say they got the
+ // grade completion now, instead of the correct time).
+ if (class_exists('restore_controller', false) && restore_controller::is_executing()) {
+ return;
+ }
+
// Load information about grade item
$this->load_grade_item();

0 comments on commit 0f2929f

Please sign in to comment.