diff --git a/admin/settings/courses.php b/admin/settings/courses.php index ace90d3f057..2525f224769 100644 --- a/admin/settings/courses.php +++ b/admin/settings/courses.php @@ -206,6 +206,11 @@ if ($hassiteconfig or has_any_capability($capabilities, $systemcontext)) { $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_role_assignments', new lang_string('generalroleassignments','backup'), new lang_string('configgeneralroleassignments','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_activities', new lang_string('generalactivities','backup'), new lang_string('configgeneralactivities','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_blocks', new lang_string('generalblocks','backup'), new lang_string('configgeneralblocks','backup'), array('value'=>1, 'locked'=>0))); + $temp->add(new admin_setting_configcheckbox_with_lock( + 'backup/backup_general_files', + new lang_string('generalfiles', 'backup'), + new lang_string('configgeneralfiles', 'backup'), + array('value' => '1', 'locked' => 0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_filters', new lang_string('generalfilters','backup'), new lang_string('configgeneralfilters','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_comments', new lang_string('generalcomments','backup'), new lang_string('configgeneralcomments','backup'), array('value'=>1, 'locked'=>0))); $temp->add(new admin_setting_configcheckbox_with_lock('backup/backup_general_badges', new lang_string('generalbadges','backup'), new lang_string('configgeneralbadges','backup'), array('value'=>1,'locked'=>0))); @@ -341,6 +346,10 @@ if ($hassiteconfig or has_any_capability($capabilities, $systemcontext)) { $temp->add(new admin_setting_configcheckbox('backup/backup_auto_role_assignments', new lang_string('generalroleassignments','backup'), new lang_string('configgeneralroleassignments','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_activities', new lang_string('generalactivities','backup'), new lang_string('configgeneralactivities','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_blocks', new lang_string('generalblocks','backup'), new lang_string('configgeneralblocks','backup'), 1)); + $temp->add(new admin_setting_configcheckbox( + 'backup/backup_auto_files', + new lang_string('generalfiles', 'backup'), + new lang_string('configgeneralfiles', 'backup'), '1')); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_filters', new lang_string('generalfilters','backup'), new lang_string('configgeneralfilters','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_comments', new lang_string('generalcomments','backup'), new lang_string('configgeneralcomments','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_badges', new lang_string('generalbadges','backup'), new lang_string('configgeneralbadges','backup'), 1)); diff --git a/admin/settings/server.php b/admin/settings/server.php index a869ce6294d..cfb2f21acb6 100644 --- a/admin/settings/server.php +++ b/admin/settings/server.php @@ -172,7 +172,10 @@ $temp->add(new admin_setting_configselect('tempdatafoldercleanup', new lang_stri $ADMIN->add('server', $temp); - + $temp->add(new admin_setting_configduration('filescleanupperiod', + new lang_string('filescleanupperiod', 'admin'), + new lang_string('filescleanupperiod_help', 'admin'), + 86400)); $ADMIN->add('server', new admin_externalpage('environment', new lang_string('environment','admin'), "$CFG->wwwroot/$CFG->admin/environment.php")); $ADMIN->add('server', new admin_externalpage('phpinfo', new lang_string('phpinfo'), "$CFG->wwwroot/$CFG->admin/phpinfo.php")); diff --git a/admin/tool/recyclebin/classes/category_bin.php b/admin/tool/recyclebin/classes/category_bin.php index 6477c4d25c2..b5bde99297f 100644 --- a/admin/tool/recyclebin/classes/category_bin.php +++ b/admin/tool/recyclebin/classes/category_bin.php @@ -109,14 +109,15 @@ class category_bin extends base_bin { require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php'); // As far as recycle bin is using MODE_AUTOMATED, it observes the backup_auto_storage - // setting (storing backups @ real location. For recycle bin we want to ensure that - // backup files are always stored in Moodle file area. In order to achieve that, we - // hack the setting here via $CFG->forced_plugin_settings, so it won't interfere other - // operations. See MDL-65218 for more information. + // settings (storing backups @ real location and potentially not including files). + // For recycle bin we want to ensure that backup files are always stored in Moodle file + // area and always contain the users' files. In order to achieve that, we hack the + // setting here via $CFG->forced_plugin_settings, so it won't interfere other operations. + // See MDL-65218 and MDL-35773 for more information. // This hack will be removed once recycle bin switches to use its own backup mode, with - // own preferences and 100% appart from MODLE_AUTOMATED. + // own preferences and 100% separate from MOODLE_AUTOMATED. // TODO: Remove this as part of MDL-65228. - $CFG->forced_plugin_settings['backup'] = ['backup_auto_storage' => 0]; + $CFG->forced_plugin_settings['backup'] = ['backup_auto_storage' => 0, 'backup_auto_files' => 1]; // Backup the course. $user = get_admin(); diff --git a/admin/tool/recyclebin/classes/course_bin.php b/admin/tool/recyclebin/classes/course_bin.php index deec65c1cb2..0353c2c49ef 100644 --- a/admin/tool/recyclebin/classes/course_bin.php +++ b/admin/tool/recyclebin/classes/course_bin.php @@ -113,14 +113,15 @@ class course_bin extends base_bin { } // As far as recycle bin is using MODE_AUTOMATED, it observes the backup_auto_storage - // setting (storing backups @ real location. For recycle bin we want to ensure that - // backup files are always stored in Moodle file area. In order to achieve that, we - // hack the setting here via $CFG->forced_plugin_settings, so it won't interfere other - // operations. See MDL-65218 for more information. + // settings (storing backups @ real location and potentially not including files). + // For recycle bin we want to ensure that backup files are always stored in Moodle file + // area and always contain the users' files. In order to achieve that, we hack the + // setting here via $CFG->forced_plugin_settings, so it won't interfere other operations. + // See MDL-65218 and MDL-35773 for more information. // This hack will be removed once recycle bin switches to use its own backup mode, with - // own preferences and 100% appart from MODLE_AUTOMATED. + // own preferences and 100% separate from MOODLE_AUTOMATED. // TODO: Remove this as part of MDL-65228. - $CFG->forced_plugin_settings['backup'] = ['backup_auto_storage' => 0]; + $CFG->forced_plugin_settings['backup'] = ['backup_auto_storage' => 0, 'backup_auto_files' => 1]; // Backup the activity. $user = get_admin(); diff --git a/backup/backup.php b/backup/backup.php index e0606d1b384..5090f391e7e 100644 --- a/backup/backup.php +++ b/backup/backup.php @@ -150,6 +150,12 @@ if (!async_helper::is_async_pending($id, 'course', 'backup')) { $loghtml = ''; if ($backup->get_stage() == backup_ui::STAGE_FINAL) { + // Before we perform the backup check settings to see if user + // or setting defaults are set to exclude files from the backup. + if ($backup->get_setting_value('files') == 0) { + $renderer->set_samesite_notification(); + } + if ($backupmode != backup::MODE_ASYNC) { // Synchronous backup handling. @@ -180,6 +186,7 @@ if (!async_helper::is_async_pending($id, 'course', 'backup')) { // Hide the progress display and first backup step bar (the 'finished' step will show next). echo html_writer::end_div(); echo html_writer::script('document.getElementById("executionprogress").style.display = "none";'); + } else { // Async backup handling. $backup->get_controller()->finish_ui(); @@ -203,6 +210,7 @@ if (!async_helper::is_async_pending($id, 'course', 'backup')) { 'restoreurl' => $restoreurl->out(), 'headingident' => 'backup' ); + echo $renderer->render_from_template('core/async_backup_status', $progresssetup); } diff --git a/backup/controller/backup_controller.class.php b/backup/controller/backup_controller.class.php index 9ac5f5b831f..cf5fcf96622 100644 --- a/backup/controller/backup_controller.class.php +++ b/backup/controller/backup_controller.class.php @@ -271,6 +271,37 @@ class backup_controller extends base_controller { return $this->includefiles; } + /** + * Returns the default value for $this->includefiles before we consider any settings. + * + * @return bool + * @throws dml_exception + */ + protected function get_include_files_default() : bool { + // We normally include files. + $includefiles = true; + + // In an import, we don't need to include files. + if ($this->get_mode() === backup::MODE_IMPORT) { + $includefiles = false; + } + + // When a backup is intended for the same site, we don't need to include the files. + // Note, this setting is only used for duplication of an entire course. + if ($this->get_mode() === backup::MODE_SAMESITE) { + $includefiles = false; + } + + // If backup is automated and we have set auto backup config to exclude + // files then set them to be excluded here. + $backupautofiles = (bool) get_config('backup', 'backup_auto_files'); + if ($this->get_mode() === backup::MODE_AUTOMATED && !$backupautofiles) { + $includefiles = false; + } + + return $includefiles; + } + public function get_operation() { return $this->operation; } @@ -326,6 +357,12 @@ class backup_controller extends base_controller { // Basic/initial prevention against time/memory limits core_php_time_limit::raise(1 * 60 * 60); // 1 hour for 1 course initially granted raise_memory_limit(MEMORY_EXTRA); + + // 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()); + } + // If this is not a course backup, or single activity backup (e.g. duplicate) inform the plan we are not // including all the activities for sure. This will affect any // task/step executed conditionally to stop including information @@ -386,35 +423,19 @@ class backup_controller extends base_controller { $this->log('applying plan defaults', backup::LOG_DEBUG); backup_controller_dbops::apply_config_defaults($this); $this->set_status(backup::STATUS_CONFIGURED); - $this->set_include_files(); + $this->set_include_files($this->get_include_files_default()); } /** * Set the initial value for the include_files setting. * + * @param bool $includefiles * @see backup_controller::get_include_files for further information on the purpose of this setting. - * @return int Indicates whether files should be included in backups. */ - protected function set_include_files() { - // We normally include files. - $includefiles = true; - - // In an import, we don't need to include files. - if ($this->get_mode() === backup::MODE_IMPORT) { - $includefiles = false; - } - - // When a backup is intended for the same site, we don't need to include the files. - // Note, this setting is only used for duplication of an entire course. - if ($this->get_mode() === backup::MODE_SAMESITE) { - $includefiles = false; - } - - $this->includefiles = (int) $includefiles; + protected function set_include_files(bool $includefiles) { $this->log("setting file inclusion to {$this->includefiles}", backup::LOG_DEBUG); - return $this->includefiles; + $this->includefiles = (int) $includefiles; } - } /* diff --git a/backup/moodle2/backup_root_task.class.php b/backup/moodle2/backup_root_task.class.php index 03e989da279..ee434c1e9d3 100644 --- a/backup/moodle2/backup_root_task.class.php +++ b/backup/moodle2/backup_root_task.class.php @@ -110,6 +110,12 @@ class backup_root_task extends backup_task { $this->add_setting($blocks); $this->converter_deps($blocks, $converters); + // Define files. + $files = new backup_generic_setting('files', base_setting::IS_BOOLEAN, true); + $files->set_ui(new backup_setting_ui_checkbox($files, get_string('rootsettingfiles', 'backup'))); + $this->add_setting($files); + $this->converter_deps($files, $converters); + // Define filters $filters = new backup_generic_setting('filters', base_setting::IS_BOOLEAN, true); $filters->set_ui(new backup_setting_ui_checkbox($filters, get_string('rootsettingfilters', 'backup'))); diff --git a/backup/util/dbops/backup_plan_dbops.class.php b/backup/util/dbops/backup_plan_dbops.class.php index 615f802fc6c..d3a650bf91e 100644 --- a/backup/util/dbops/backup_plan_dbops.class.php +++ b/backup/util/dbops/backup_plan_dbops.class.php @@ -201,7 +201,8 @@ abstract class backup_plan_dbops extends backup_dbops { * @param bool $useidonly only use the ID in the file name * @return string The filename to use */ - public static function get_default_backup_filename($format, $type, $id, $users, $anonymised, $useidonly = false) { + public static function get_default_backup_filename($format, $type, $id, $users, $anonymised, + $useidonly = false, $files = true) { global $DB; // Calculate backup word @@ -251,6 +252,11 @@ abstract class backup_plan_dbops extends backup_dbops { $info = '-an'; } + // Indicate if backup doesn't contain files. + if (!$files) { + $info .= '-nf'; + } + return $backupword . '-' . $format . '-' . $type . '-' . $name . '-' . $date . $info . '.mbz'; } diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index 7ef9117e230..0911c68f0be 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -1054,17 +1054,40 @@ abstract class restore_dbops { // Create the file in the filepool if it does not exist yet. if (!$fs->file_exists($newcontextid, $component, $filearea, $rec->newitemid, $file->filepath, $file->filename)) { - // Even if a file has been deleted since the backup was made, the file metadata will remain in the - // files table, and the file will not be moved to the trashdir. - // Files are not cleared from the files table by cron until several days after deletion. + // Even if a file has been deleted since the backup was made, the file metadata may remain in the + // files table, and the file will not yet have been moved to the trashdir. e.g. a draft file version. + // Try to recover from file table first. if ($foundfiles = $DB->get_records('files', array('contenthash' => $file->contenthash), '', '*', 0, 1)) { // Only grab one of the foundfiles - the file content should be the same for all entries. $foundfile = reset($foundfiles); $fs->create_file_from_storedfile($file_record, $foundfile->id); } else { - // A matching existing file record was not found in the database. - $results[] = self::get_missing_file_result($file); - continue; + $filesystem = $fs->get_file_system(); + $restorefile = $file; + $restorefile->contextid = $newcontextid; + $restorefile->itemid = $rec->newitemid; + $storedfile = new stored_file($fs, $restorefile); + + // Ok, let's try recover this file. + // 1. We check if the file can be fetched locally without attempting to fetch + // from the trash. + // 2. We check if we can get the remote filepath for the specified stored file. + // 3. We check if the file can be fetched from the trash. + // 4. All failed, say we couldn't find it. + if ($filesystem->is_file_readable_locally_by_storedfile($storedfile)) { + $localpath = $filesystem->get_local_path_from_storedfile($storedfile); + $fs->create_file_from_pathname($file, $localpath); + } else if ($filesystem->is_file_readable_remotely_by_storedfile($storedfile)) { + $url = $filesystem->get_remote_path_from_storedfile($storedfile); + $fs->create_file_from_url($file, $url); + } else if ($filesystem->is_file_readable_locally_by_storedfile($storedfile, true)) { + $localpath = $filesystem->get_local_path_from_storedfile($storedfile, true); + $fs->create_file_from_pathname($file, $localpath); + } else { + // A matching file was not found. + $results[] = self::get_missing_file_result($file); + continue; + } } } } diff --git a/backup/util/helper/backup_cron_helper.class.php b/backup/util/helper/backup_cron_helper.class.php index 980baf4457c..68ff8222b6b 100644 --- a/backup/util/helper/backup_cron_helper.class.php +++ b/backup/util/helper/backup_cron_helper.class.php @@ -415,8 +415,9 @@ abstract class backup_cron_automated_helper { $id = $bc->get_id(); $users = $bc->get_plan()->get_setting('users')->get_value(); $anonymised = $bc->get_plan()->get_setting('anonymize')->get_value(); + $incfiles = (bool)$config->backup_auto_files; $bc->get_plan()->get_setting('filename')->set_value(backup_plan_dbops::get_default_backup_filename($format, $type, - $id, $users, $anonymised)); + $id, $users, $anonymised, false, $incfiles)); $bc->set_status(backup::STATUS_AWAITING); diff --git a/backup/util/helper/backup_helper.class.php b/backup/util/helper/backup_helper.class.php index fa3bc79cf7c..167eae7bbae 100644 --- a/backup/util/helper/backup_helper.class.php +++ b/backup/util/helper/backup_helper.class.php @@ -287,7 +287,15 @@ abstract class backup_helper { $config = get_config('backup'); $dir = $config->backup_auto_destination; if ($config->backup_auto_storage == 1 and $dir and is_dir($dir) and is_writable($dir)) { - $filedest = $dir.'/'.backup_plan_dbops::get_default_backup_filename($format, $backuptype, $courseid, $hasusers, $isannon, !$config->backup_shortname); + $filedest = $dir.'/' + .backup_plan_dbops::get_default_backup_filename( + $format, + $backuptype, + $courseid, + $hasusers, + $isannon, + !$config->backup_shortname, + (bool)$config->backup_auto_files); // first try to move the file, if it is not possible copy and delete instead if (@rename($filepath, $filedest)) { return null; diff --git a/backup/util/ui/backup_ui_stage.class.php b/backup/util/ui/backup_ui_stage.class.php index 1e1caf8833f..aff941b9542 100644 --- a/backup/util/ui/backup_ui_stage.class.php +++ b/backup/util/ui/backup_ui_stage.class.php @@ -155,7 +155,9 @@ class backup_ui_stage_initial extends backup_ui_stage { $this->ui->get_type(), $this->ui->get_controller_id(), $this->ui->get_setting_value('users'), - $this->ui->get_setting_value('anonymize') + $this->ui->get_setting_value('anonymize'), + false, + (bool)$this->ui->get_setting_value('files') ); $setting->set_value($filename); } @@ -457,7 +459,16 @@ class backup_ui_stage_confirmation extends backup_ui_stage { $id = $this->ui->get_controller_id(); $users = $this->ui->get_setting_value('users'); $anonymised = $this->ui->get_setting_value('anonymize'); - $setting->set_value(backup_plan_dbops::get_default_backup_filename($format, $type, $id, $users, $anonymised)); + $files = (bool)$this->ui->get_setting_value('files'); + $filename = backup_plan_dbops::get_default_backup_filename( + $format, + $type, + $id, + $users, + $anonymised, + false, + $files); + $setting->set_value($filename); } $form->add_setting($setting, $task); break; @@ -628,6 +639,7 @@ class backup_ui_stage_complete extends backup_ui_stage_final { if (!empty($this->results['missing_files_in_pool'])) { $output .= $renderer->notification(get_string('missingfilesinpool', 'backup'), 'notifyproblem'); } + $output .= $renderer->get_samesite_notification(); $output .= $renderer->notification(get_string('executionsuccess', 'backup'), 'notifysuccess'); $output .= $renderer->continue_button($restorerul); $output .= $renderer->box_end(); diff --git a/backup/util/ui/renderer.php b/backup/util/ui/renderer.php index 4ee83e966f5..a46499f5400 100644 --- a/backup/util/ui/renderer.php +++ b/backup/util/ui/renderer.php @@ -43,6 +43,13 @@ require_once($CFG->dirroot . '/backup/moodle2/backup_plan_builder.class.php'); */ class core_backup_renderer extends plugin_renderer_base { + /** + * Same site notification display. + * + * @var string + */ + private $samesitenotification = ''; + /** * Renderers a progress bar for the backup or restore given the items that make it up. * @@ -80,6 +87,22 @@ class core_backup_renderer extends plugin_renderer_base { return $out; } + /** + * Set the same site backup notification. + * + */ + public function set_samesite_notification() { + $this->samesitenotification = $this->output->notification(get_string('samesitenotification', 'backup'), 'info'); + } + + /** + * Get the same site backup notification. + * + */ + public function get_samesite_notification() { + return $this->samesitenotification; + } + /** * Prints a dependency notification * diff --git a/lang/en/admin.php b/lang/en/admin.php index 1a0a8882904..0f8ba1703ba 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -564,6 +564,8 @@ $string['experimentalsettings'] = 'Experimental settings'; $string['extendedusernamechars'] = 'Allow extended characters in usernames'; $string['extramemorylimit'] = 'Extra PHP memory limit'; $string['fatalsessionautostart'] = '

Serious configuration error detected, please notify server administrator.

To operate properly, Moodle requires that administrator changes PHP settings.

session.auto_start must be set to off.

This setting is controlled by editing php.ini, Apache/IIS
configuration or .htaccess file on the server.

'; +$string['filescleanupperiod'] = 'Clean trash pool files'; +$string['filescleanupperiod_help'] = 'How often trash files are removed. These are files that are associated with a context that no longer exists'; $string['fileconversioncleanuptask'] = 'Cleanup of temporary records for file conversions.'; $string['filecreated'] = 'New file created'; $string['filestoredin'] = 'Save file into folder :'; diff --git a/lang/en/backup.php b/lang/en/backup.php index de681c62f9a..9cfe05e1b17 100644 --- a/lang/en/backup.php +++ b/lang/en/backup.php @@ -127,6 +127,7 @@ $string['configgeneralblocks'] = 'Sets the default for including blocks in a bac $string['configgeneralcalendarevents'] = 'Sets the default for including calendar events in a backup.'; $string['configgeneralcomments'] = 'Sets the default for including comments in a backup.'; $string['configgeneralcompetencies'] = 'Sets the default for including competencies in a backup.'; +$string['configgeneralfiles'] = 'Sets the default for including files in a backup.'; $string['configgeneralfilters'] = 'Sets the default for including filters in a backup.'; $string['configgeneralhistories'] = 'Sets the default for including user history within a backup.'; $string['configgenerallogs'] = 'If enabled logs will be included in backups by default.'; @@ -202,6 +203,7 @@ $string['generalcalendarevents'] = 'Include calendar events'; $string['generalcomments'] = 'Include comments'; $string['generalcompetencies'] = 'Include competencies'; $string['generalenrolments'] = 'Include enrolment methods'; +$string['generalfiles'] = 'Include files'; $string['generalfilters'] = 'Include filters'; $string['generalhistories'] = 'Include histories'; $string['generalgradehistories'] = 'Include histories'; @@ -328,6 +330,7 @@ $string['rootsettingbadges'] = 'Include badges'; $string['rootsettingblocks'] = 'Include blocks'; $string['rootsettingcompetencies'] = 'Include competencies'; $string['rootsettingfilters'] = 'Include filters'; +$string['rootsettingfiles'] = 'Include files'; $string['rootsettingcomments'] = 'Include comments'; $string['rootsettingcalendarevents'] = 'Include calendar events'; $string['rootsettinguserscompletion'] = 'Include user completion details'; @@ -337,6 +340,7 @@ $string['rootsettinggradehistories'] = 'Include grade history'; $string['rootsettinggroups'] = 'Include groups and groupings'; $string['rootsettingimscc1'] = 'Convert to IMS Common Cartridge 1.0'; $string['rootsettingimscc11'] = 'Convert to IMS Common Cartridge 1.1'; +$string['samesitenotification'] = 'This backup was created with only references to files, not the files themselves. Restoring will only work on this site.'; $string['sitecourseformatwarning'] = 'This is a front page backup, note that they can only be restored on the front page'; $string['storagecourseonly'] = 'Course backup filearea'; $string['storagecourseandexternal'] = 'Course backup filearea and the specified directory'; diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index b297159ab64..743beb98b39 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -2241,7 +2241,8 @@ class file_storage { // remove trash pool files once a day // if you want to disable purging of trash put $CFG->fileslastcleanup=time(); into config.php - if (empty($CFG->fileslastcleanup) or $CFG->fileslastcleanup < time() - 60*60*24) { + $filescleanupperiod = empty($CFG->filescleanupperiod) ? 86400 : $CFG->filescleanupperiod; + if (empty($CFG->fileslastcleanup) || ($CFG->fileslastcleanup < time() - $filescleanupperiod)) { require_once($CFG->libdir.'/filelib.php'); // Delete files that are associated with a context that no longer exists. mtrace('Cleaning up files from deleted contexts... ', ''); diff --git a/lib/filestorage/file_system.php b/lib/filestorage/file_system.php index ba6b8be8f6b..3f7d6f0cd7c 100644 --- a/lib/filestorage/file_system.php +++ b/lib/filestorage/file_system.php @@ -80,7 +80,7 @@ abstract class file_system { * @param bool $fetchifnotfound Whether to attempt to fetch from the remote path if not found. * @return string full path to pool file with file content */ - protected function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { + public function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { return $this->get_local_path_from_hash($file->get_contenthash(), $fetchifnotfound); } @@ -94,7 +94,7 @@ abstract class file_system { * @param stored_file $file The file to serve. * @return string full path to pool file with file content */ - protected function get_remote_path_from_storedfile(stored_file $file) { + public function get_remote_path_from_storedfile(stored_file $file) { return $this->get_remote_path_from_hash($file->get_contenthash(), false); } diff --git a/lib/filestorage/file_system_filedir.php b/lib/filestorage/file_system_filedir.php index f3840ca9ca0..103dcca41e5 100644 --- a/lib/filestorage/file_system_filedir.php +++ b/lib/filestorage/file_system_filedir.php @@ -119,7 +119,7 @@ class file_system_filedir extends file_system { * @param bool $fetchifnotfound Whether to attempt to fetch from the remote path if not found. * @return string The full path to the content file */ - protected function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { + public function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { $filepath = $this->get_local_path_from_hash($file->get_contenthash(), $fetchifnotfound); // Try content recovery. @@ -136,7 +136,7 @@ class file_system_filedir extends file_system { * @param stored_file $file The file to serve. * @return string full path to pool file with file content */ - protected function get_remote_path_from_storedfile(stored_file $file) { + public function get_remote_path_from_storedfile(stored_file $file) { return $this->get_local_path_from_storedfile($file, false); } diff --git a/lib/filestorage/tests/file_storage_test.php b/lib/filestorage/tests/file_storage_test.php index c667040cfce..30c15ad4181 100644 --- a/lib/filestorage/tests/file_storage_test.php +++ b/lib/filestorage/tests/file_storage_test.php @@ -74,10 +74,8 @@ class core_files_file_storage_testcase extends advanced_testcase { $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash))); - $method = new ReflectionMethod('file_system', 'get_local_path_from_storedfile'); - $method->setAccessible(true); $filesystem = $fs->get_file_system(); - $location = $method->invokeArgs($filesystem, array($file, true)); + $location = $filesystem->get_local_path_from_storedfile($file, true); $this->assertFileExists($location); @@ -149,10 +147,8 @@ class core_files_file_storage_testcase extends advanced_testcase { $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash))); - $method = new ReflectionMethod('file_system', 'get_local_path_from_storedfile'); - $method->setAccessible(true); $filesystem = $fs->get_file_system(); - $location = $method->invokeArgs($filesystem, array($file, true)); + $location = $filesystem->get_local_path_from_storedfile($file, true); $this->assertFileExists($location); diff --git a/lib/filestorage/tests/file_system_filedir_test.php b/lib/filestorage/tests/file_system_filedir_test.php index 76f75a294e2..14421a09a07 100644 --- a/lib/filestorage/tests/file_system_filedir_test.php +++ b/lib/filestorage/tests/file_system_filedir_test.php @@ -257,9 +257,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase { ->with($this->equalTo($file)); $file = $this->get_stored_file('example content'); - $method = new ReflectionMethod(file_system_filedir::class, 'get_local_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, array($file, true)); + $result = $fs->get_local_path_from_storedfile($file, true); $this->assertEquals($filepath, $result); } @@ -287,9 +285,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase { ->method('recover_file'); $file = $this->get_stored_file('example content'); - $method = new ReflectionMethod(file_system_filedir::class, 'get_local_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, array($file, false)); + $result = $fs->get_local_path_from_storedfile($file, false); $this->assertEquals($filepath, $result); } diff --git a/lib/filestorage/tests/file_system_test.php b/lib/filestorage/tests/file_system_test.php index fbcba725c88..8508a677b27 100644 --- a/lib/filestorage/tests/file_system_test.php +++ b/lib/filestorage/tests/file_system_test.php @@ -250,9 +250,7 @@ class core_files_file_system_testcase extends advanced_testcase { $file = $this->get_stored_file($filecontent); - $method = new ReflectionMethod(file_system::class, 'get_local_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, array_merge([$file], $args)); + $result = $fs->get_local_path_from_storedfile($file, $fetch); $this->assertEquals($filepath, $result); } @@ -280,9 +278,7 @@ class core_files_file_system_testcase extends advanced_testcase { $file = $this->get_stored_file($filecontent); - $method = new ReflectionMethod(file_system::class, 'get_remote_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, [$file]); + $result = $fs->get_remote_path_from_storedfile($file); $this->assertEquals($filepath, $result); } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 420edb4d9fa..49c6214c69e 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -20,6 +20,8 @@ information provided here is intended especially for developers. at least a single checkbox item is selected or not. * Final deprecation (removal) of the core/modal_confirm dialogue. * Upgrade scssphp to v1.0.2, This involves renaming classes from Leafo => ScssPhp as the repo has changed. +* The methods get_local_path_from_storedfile and get_remote_path_from_storedfile in lib/filestore/file_system.php + are now public. If you are overriding these then you will need to change your methods to public in your class. === 3.7 === diff --git a/version.php b/version.php index 105d8d4b469..c109bc801e9 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019072500.01; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019072500.02; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.