Skip to content

Commit

Permalink
MDL-64310 backup: allow session to be released during backup/restore
Browse files Browse the repository at this point in the history
Amended to fix a wrong call to backup_controller() constructor,
originally not passing USER->id
  • Loading branch information
rhell4 authored and stronk7 committed Jan 14, 2020
1 parent 8d529d0 commit 383b0f9
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 4 deletions.
5 changes: 5 additions & 0 deletions backup/backup.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ abstract class backup implements checksumable {
const INTERACTIVE_YES = true;
const INTERACTIVE_NO = false;

/** Release the session during backup/restore */
const RELEASESESSION_YES = true;
/** Don't release the session during backup/restore */
const RELEASESESSION_NO = false;

// Predefined modes (purposes) of the backup
const MODE_GENERAL = 10;

Expand Down
2 changes: 1 addition & 1 deletion backup/backup.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@

if (!($bc = backup_ui::load_controller($backupid))) {
$bc = new backup_controller($type, $id, backup::FORMAT_MOODLE,
backup::INTERACTIVE_YES, $backupmode, $USER->id);
backup::INTERACTIVE_YES, $backupmode, $USER->id, backup::RELEASESESSION_YES);
// The backup id did not relate to a valid controller so we made a new controller.
// Now we need to reset the backup id to match the new controller.
$backupid = $bc->get_backupid();
Expand Down
9 changes: 8 additions & 1 deletion backup/controller/backup_controller.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,17 @@ class backup_controller extends base_controller {
* @param bool $interactive Whether this backup will require user interaction; backup::INTERACTIVE_YES or INTERACTIVE_NO
* @param int $mode One of backup::MODE_GENERAL, MODE_IMPORT, MODE_SAMESITE, MODE_HUB, MODE_AUTOMATED
* @param int $userid The id of the user making the backup
* @param bool $releasesession Should release the session? backup::RELEASESESSION_YES or backup::RELEASESESSION_NO
*/
public function __construct($type, $id, $format, $interactive, $mode, $userid){
public function __construct($type, $id, $format, $interactive, $mode, $userid, $releasesession = backup::RELEASESESSION_NO) {
$this->type = $type;
$this->id = $id;
$this->courseid = backup_controller_dbops::get_courseid_from_type_id($this->type, $this->id);
$this->format = $format;
$this->interactive = $interactive;
$this->mode = $mode;
$this->userid = $userid;
$this->releasesession = $releasesession;

// Apply some defaults
$this->operation = backup::OPERATION_BACKUP;
Expand Down Expand Up @@ -359,6 +361,11 @@ public function execute_plan() {
core_php_time_limit::raise(1 * 60 * 60); // 1 hour for 1 course initially granted
raise_memory_limit(MEMORY_EXTRA);

// Release the session so other tabs in the same session are not blocked.
if ($this->get_releasesession() === backup::RELEASESESSION_YES) {
\core\session\manager::write_close();
}

// If the controller has decided that we can include files, then check the setting, otherwise do not include files.
if ($this->get_include_files()) {
$this->set_include_files((bool) $this->get_plan()->get_setting('files')->get_value());
Expand Down
13 changes: 13 additions & 0 deletions backup/controller/base_controller.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ abstract class base_controller extends backup implements loggable {
*/
protected $logger;

/** @var bool Whether this backup should release the session. */
protected $releasesession = backup::RELEASESESSION_NO;

/**
* Gets the progress reporter, which can be used to report progress within
* the backup or restore process.
Expand Down Expand Up @@ -82,4 +85,14 @@ public function add_logger(base_logger $logger) {
public function log($message, $level, $a = null, $depth = null, $display = false) {
backup_helper::log($message, $level, $a, $depth, $display, $this->logger);
}

/**
* Returns the set value of releasesession.
* This is used to indicate if the session should be closed during the backup/restore.
*
* @return bool Indicates whether the session should be released.
*/
public function get_releasesession() {
return $this->releasesession;
}
}
9 changes: 8 additions & 1 deletion backup/controller/restore_controller.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,17 @@ class restore_controller extends base_controller {
* @param int $userid
* @param int $target backup::TARGET_[ NEW_COURSE | CURRENT_ADDING | CURRENT_DELETING | EXISTING_ADDING | EXISTING_DELETING ]
* @param \core\progress\base $progress Optional progress monitor
* @param bool $releasesession Should release the session? backup::RELEASESESSION_YES or backup::RELEASESESSION_NO
*/
public function __construct($tempdir, $courseid, $interactive, $mode, $userid, $target,
\core\progress\base $progress = null) {
\core\progress\base $progress = null, $releasesession = backup::RELEASESESSION_NO) {
$this->tempdir = $tempdir;
$this->courseid = $courseid;
$this->interactive = $interactive;
$this->mode = $mode;
$this->userid = $userid;
$this->target = $target;
$this->releasesession = $releasesession;

// Apply some defaults
$this->type = '';
Expand Down Expand Up @@ -357,6 +359,11 @@ public function execute_plan() {
core_php_time_limit::raise(1 * 60 * 60); // 1 hour for 1 course initially granted
raise_memory_limit(MEMORY_EXTRA);

// Release the session so other tabs in the same session are not blocked.
if ($this->get_releasesession() === backup::RELEASESESSION_YES) {
\core\session\manager::write_close();
}

// Do course cleanup precheck, if required. This was originally in restore_ui. Moved to handle async backup/restore.
if ($this->get_target() == backup::TARGET_CURRENT_DELETING || $this->get_target() == backup::TARGET_EXISTING_DELETING) {
$options = array();
Expand Down
2 changes: 1 addition & 1 deletion backup/restore.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
$restore = restore_ui::engage_independent_stage($stage/2, $contextid);
if ($restore->process()) {
$rc = new restore_controller($restore->get_filepath(), $restore->get_course_id(), backup::INTERACTIVE_YES,
$backupmode, $USER->id, $restore->get_target());
$backupmode, $USER->id, $restore->get_target(), null, backup::RELEASESESSION_YES);
}
}
if ($rc) {
Expand Down

0 comments on commit 383b0f9

Please sign in to comment.