MDL-79381 core: Refactor MoodleNet controller

Including in this commit:
 - Moved the backup controller instantiation out of the constructor
 - Created a new get_backup_controller() method to retreive the controller when needed
 - Removed some constructor-promoted properties
 - Added resourcefilename to resource_packager constructor
 - Made resource_packager to abstract class (cannot be instantiated)
 - Removed constructor-promoted properties and declare them "normally"
 - Backup controller is called and destroyed in get_package() method.
   Only PHPUnit tests will need to play with the destroy method because
   they use the reflection method
 - Made course_partial_packager extend course_packager and removed constructor-promoted properties
 - Removed the get_all_task_settings() method and replaced it with get_all_activity_tasks() method
 - Fixed bugs related to Partial course sharing
This commit is contained in:
Eloy Lafuente (stronk7) 2023-09-29 18:03:43 +02:00 committed by Huong Nguyen
parent 1724362afc
commit ace315370f
No known key found for this signature in database
GPG Key ID: 40D88AB693A3E72A
4 changed files with 97 additions and 68 deletions

View File

@ -40,24 +40,30 @@ class activity_packager extends resource_packager {
* @param int $userid The ID of the user performing the packaging.
*/
public function __construct(
protected cm_info $cminfo,
protected int $userid,
cm_info $cminfo,
int $userid,
) {
// Check backup/restore support.
if (!plugin_supports('mod', $cminfo->modname , FEATURE_BACKUP_MOODLE2)) {
throw new \coding_exception("Cannot backup module $cminfo->modname. This module doesn't support the backup feature.");
}
parent::__construct($cminfo, $userid);
parent::__construct($cminfo, $userid, $cminfo->modname);
}
$this->controller = new backup_controller(
/**
* Get the backup controller for the activity.
*
* @return backup_controller the backup controller for the activity.
*/
protected function get_backup_controller(): backup_controller {
return new backup_controller(
backup::TYPE_1ACTIVITY,
$cminfo->id,
$this->cminfo->id,
backup::FORMAT_MOODLE,
backup::INTERACTIVE_NO,
backup::MODE_GENERAL,
$userid
$this->userid
);
$this->resourcefilename = $this->cminfo->modname;
}
}

View File

@ -40,20 +40,25 @@ class course_packager extends resource_packager {
* @param int $userid The ID of the user performing the packaging
*/
public function __construct(
protected stdClass $course,
protected int $userid,
stdClass $course,
int $userid,
) {
parent::__construct($course, $userid);
parent::__construct($course, $userid, $course->shortname);
}
$this->controller = new backup_controller(
/**
* Get the backup controller for the course.
*
* @return backup_controller the backup controller for the course.
*/
protected function get_backup_controller(): backup_controller {
return new backup_controller(
backup::TYPE_1COURSE,
$course->id,
$this->course->id,
backup::FORMAT_MOODLE,
backup::INTERACTIVE_NO,
backup::MODE_GENERAL,
$userid
$this->userid,
);
$this->resourcefilename = $this->course->shortname;
}
}

View File

@ -16,7 +16,6 @@
namespace core\moodlenet;
use backup;
use backup_activity_task;
use backup_controller;
use stdClass;
@ -29,10 +28,12 @@ use stored_file;
* @copyright 2023 Huong Nguyen <huongnv13@gmail.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class course_partial_packager extends resource_packager {
class course_partial_packager extends course_packager {
/** @var array $partialsharingtasks List of partial sharing tasks. */
private array $partialsharingtasks = [];
/**
* @var int[] $cmids List of course module ids of selected activities.
*/
protected array $cmids;
/**
* Constructor for course partial packager.
@ -42,39 +43,32 @@ class course_partial_packager extends resource_packager {
* @param int $userid The ID of the user performing the packaging
*/
public function __construct(
protected stdClass $course,
protected array $cmids,
protected int $userid,
stdClass $course,
array $cmids,
int $userid,
) {
$this->cmids = $cmids;
parent::__construct($course, $userid);
$this->controller = new backup_controller(
backup::TYPE_1COURSE,
$course->id,
backup::FORMAT_MOODLE,
backup::INTERACTIVE_NO,
backup::MODE_GENERAL,
$userid
);
$this->resourcefilename = $this->course->shortname;
}
/**
* Package the resource identified by resource id into a new stored_file.
*
* @param backup_controller $controller The backup controller.
* @return stored_file
*/
protected function package(): stored_file {
$this->remove_unselected_activities();
return parent::package();
protected function package(backup_controller $controller): stored_file {
$this->remove_unselected_activities($controller);
return parent::package($controller);
}
/**
* Remove unselected activities in the course backup.
*
* @param backup_controller $controller The backup controller.
*/
protected function remove_unselected_activities(): void {
foreach ($this->partialsharingtasks as $task) {
protected function remove_unselected_activities(backup_controller $controller): void {
foreach ($this->get_all_activity_tasks($controller) as $task) {
foreach ($task->get_settings() as $setting) {
if (in_array($task->get_moduleid(), $this->cmids) &&
str_contains($setting->get_name(), '_included') !== false) {
@ -87,21 +81,19 @@ class course_partial_packager extends resource_packager {
}
/**
* Get all backup settings available for override.
* Get all the activity tasks in the controller.
*
* @return array the associative array of taskclass => settings instances.
* @param backup_controller $controller The backup controller.
* @return backup_activity_task[] Array of activity tasks.
*/
protected function get_all_task_settings(): array {
$tasksettings = [];
foreach ($this->controller->get_plan()->get_tasks() as $task) {
$taskclass = get_class($task);
$tasksettings[$taskclass] = $task->get_settings();
if ($task instanceof backup_activity_task) {
// Store partial sharing tasks.
$this->partialsharingtasks[] = $task;
protected function get_all_activity_tasks(backup_controller $controller): array {
$tasks = [];
foreach ($controller->get_plan()->get_tasks() as $task) {
if (! $task instanceof backup_activity_task) { // Only for activity tasks.
continue;
}
$tasks[] = $task;
}
return $tasksettings;
return $tasks;
}
}

View File

@ -17,10 +17,10 @@
namespace core\moodlenet;
use backup_controller;
use cm_info;
use stdClass;
use backup_root_task;
use cm_info;
use core\context\user;
use stdClass;
use stored_file;
defined('MOODLE_INTERNAL') || die();
@ -34,7 +34,7 @@ require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
* @copyright 2023 Safat Shahin <safat.shahin@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class resource_packager {
abstract class resource_packager {
/**
* @var string $resourcefilename The filename for the resource.
@ -42,9 +42,19 @@ class resource_packager {
protected string $resourcefilename = 'resource';
/**
* @var backup_controller $controller The controller object.
* @var stdClass $course The course which the resource belongs to.
*/
protected backup_controller $controller;
protected stdClass $course;
/**
* @var cm_info $cminfo The course module which the resource belongs to.
*/
protected cm_info $cminfo;
/**
* @var int $userid The ID of the user performing the packaging.
*/
protected int $userid;
/**
* Constructor for the base packager.
@ -53,17 +63,27 @@ class resource_packager {
* @param int $userid The user id
*/
public function __construct(
protected stdClass|cm_info $resource,
protected int $userid,
stdClass|cm_info $resource,
int $userid,
string $resourcefilename,
) {
if ($resource instanceof cm_info) {
$this->cminfo = $resource;
$this->course = $resource->get_course();
} else {
$this->course = $resource;
}
$this->userid = $userid;
$this->resourcefilename = $resourcefilename;
}
/**
* Destructor
* Get the backup controller for the course.
*
* @return backup_controller The backup controller instance that will be used to package the resource.
*/
public function __destruct() {
$this->controller->destroy();
}
abstract protected function get_backup_controller(): backup_controller;
/**
* Prepare the backup file using appropriate setting overrides and return relevant information.
@ -71,7 +91,8 @@ class resource_packager {
* @return stored_file
*/
public function get_package(): stored_file {
$alltasksettings = $this->get_all_task_settings();
$controller = $this->get_backup_controller();
$alltasksettings = $this->get_all_task_settings($controller);
// Override relevant settings to remove user data when packaging to share to MoodleNet.
$this->override_task_setting($alltasksettings, 'setting_root_users', 0);
@ -84,7 +105,11 @@ class resource_packager {
$this->override_task_setting($alltasksettings, 'setting_root_grade_histories', 0);
$this->override_task_setting($alltasksettings, 'setting_root_groups', 0);
return $this->package();
$storedfile = $this->package($controller);
$controller->destroy(); // We are done with the controller, destroy it.
return $storedfile;
}
/**
@ -92,9 +117,9 @@ class resource_packager {
*
* @return array the associative array of taskclass => settings instances.
*/
protected function get_all_task_settings(): array {
protected function get_all_task_settings(backup_controller $controller): array {
$tasksettings = [];
foreach ($this->controller->get_plan()->get_tasks() as $task) {
foreach ($controller->get_plan()->get_tasks() as $task) {
$taskclass = get_class($task);
$tasksettings[$taskclass] = $task->get_settings();
}
@ -125,12 +150,13 @@ class resource_packager {
/**
* Package the resource identified by resource id into a new stored_file.
*
* @param backup_controller $controller The backup controller.
* @return stored_file
*/
protected function package(): stored_file {
protected function package(backup_controller $controller): stored_file {
// Execute the backup and fetch the result.
$this->controller->execute_plan();
$result = $this->controller->get_results();
$controller->execute_plan();
$result = $controller->get_results();
if (!isset($result['backup_destination'])) {
throw new \moodle_exception('Failed to package resource.');