From 7d9ff4e0bbb1bccfc2218c6833ee5522dddac5f1 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 7 Oct 2020 15:20:49 +0800 Subject: [PATCH 1/1] MDL-68665 feedback --- mod/assign/feedback/editpdf/lib.php | 44 +++--- mod/assign/feedback/editpdf/locallib.php | 166 +++++++++++++++-------- 2 files changed, 133 insertions(+), 77 deletions(-) diff --git a/mod/assign/feedback/editpdf/lib.php b/mod/assign/feedback/editpdf/lib.php index 7e1d143a5b3..701d39414ef 100644 --- a/mod/assign/feedback/editpdf/lib.php +++ b/mod/assign/feedback/editpdf/lib.php @@ -24,6 +24,9 @@ defined('MOODLE_INTERNAL') || die(); +global $CFG; +require_once($CFG->dirroot . '/mod/assign/locallib.php'); + /** * Serves assignment feedback and other files. * @@ -36,27 +39,28 @@ defined('MOODLE_INTERNAL') || die(); * @param array $options - List of options affecting file serving. * @return bool false if file not found, does not return if found - just send the file */ -function assignfeedback_editpdf_pluginfile($course, - $cm, - context $context, - $filearea, - $args, - $forcedownload, - array $options=array()) { - global $USER, $DB, $CFG; - - require_once($CFG->dirroot . '/mod/assign/locallib.php'); - - if ($filearea === 'stamps' && $context->contextlevel === CONTEXT_SYSTEM) { - - $itemid = (int)array_shift($args); - - $relativepath = implode('/', $args); +function assignfeedback_editpdf_pluginfile( + $course, + $cm, + context $context, + $filearea, + $args, + $forcedownload, + array $options = array() +) { + global $DB; + if ($filearea === 'systemstamps') { + + if ($context->contextlevel !== CONTEXT_SYSTEM) { + return false; + } - $fullpath = "/{$context->id}/assignfeedback_editpdf/$filearea/$itemid/$relativepath"; + $filename = array_pop($args); + $filepath = '/' . implode('/', $args) . '/'; $fs = get_file_storage(); - if (!$file = $fs->get_file_by_hash(sha1($fullpath)) or $file->is_directory()) { + $file = $fs->get_file($context->id, 'assignfeedback_editpdf', $filearea, 0, $filepath, $filename); + if (!$file) { return false; } @@ -64,7 +68,9 @@ function assignfeedback_editpdf_pluginfile($course, $options['immutable'] = true; send_stored_file($file, 0, 0, true, $options); - } else if ($context->contextlevel == CONTEXT_MODULE) { + } + + if ($context->contextlevel == CONTEXT_MODULE) { require_login($course, false, $cm); $itemid = (int)array_shift($args); diff --git a/mod/assign/feedback/editpdf/locallib.php b/mod/assign/feedback/editpdf/locallib.php index 2f039b20152..d575db0ebff 100644 --- a/mod/assign/feedback/editpdf/locallib.php +++ b/mod/assign/feedback/editpdf/locallib.php @@ -72,69 +72,82 @@ class assign_feedback_editpdf extends assign_feedback_plugin { $systemfiles = array(); $fs = get_file_storage(); $syscontext = context_system::instance(); + $asscontext = $this->assignment->get_context(); + + // Three file areas are used for stamps. + // Current stamps are those configured as a site administration setting to be available for new uses. + // When a stamp is removed from this filearea it is no longer available for new grade items. + $currentstamps = $fs->get_area_files($syscontext->id, 'assignfeedback_editpdf', 'stamps', 0, "filename", false); + + // Grade stamps are those which have been assigned for a specific grade item. + // The stamps associated with a grade item are always used for that grade item, even if the stamp is removed + // from the list of current stamps. + $gradestamps = $fs->get_area_files($asscontext->id, 'assignfeedback_editpdf', 'stamps', $grade->id, "filename", false); + + // The system stamps are perpetual and always exist. + // They allow Moodle to serve a common URL for all users for any possible combination of stamps. + // Files in the perpetual stamp filearea are within the system context, in itemid 0, and use the original stamps + // contenthash as a folder name. This ensures that the combination of stamp filename, and stamp file content is + // unique. + $systemstamps = $fs->get_area_files($syscontext->id, 'assignfeedback_editpdf', 'systemstamps', 0, "filename", false); + + // First check that all current stamps are listed in the grade stamps. + foreach ($currentstamps as $stamp) { + // Ensure that the current stamp is in the list of perpetual stamps. + $systempathnamehash = $this->get_system_stamp_path($stamp); + if (!array_key_exists($systempathnamehash, $systemstamps)) { + $filerecord = (object) [ + 'filearea' => 'systemstamps', + 'filepath' => '/' . $stamp->get_contenthash() . '/', + ]; + $newstamp = $fs->create_file_from_storedfile($filerecord, $stamp); + $systemstamps[$newstamp->get_pathnamehash()] = $newstamp; + } - // Copy any new stamps to this instance. - if ($files = $fs->get_area_files($syscontext->id, - 'assignfeedback_editpdf', - 'stamps', - 0, - "filename", - false)) { - foreach ($files as $file) { - $filename = $file->get_filename(); - if ($filename !== '.') { - $systemfiles[] = $filename; - - $existingfile = $fs->file_exists( - $this->assignment->get_context()->id, - 'assignfeedback_editpdf', - 'stamps', - $grade->id, - '/', - $file->get_filename() - ); - - if (!$existingfile) { - $newrecord = new stdClass(); - $newrecord->contextid = $this->assignment->get_context()->id; - $newrecord->itemid = $grade->id; - $fs->create_file_from_storedfile($newrecord, $file); - } - } + // Ensure that the current stamp is in the list of stamps for the current grade item. + $gradeitempathhash = $this->get_assignment_stamp_path($stamp, $grade->id); + if (!array_key_exists($gradeitempathhash, $gradestamps)) { + $filerecord = (object) [ + 'contextid' => $asscontext->id, + 'filearea' => 'stamps', + 'itemid' => $grade->id, + ]; + $newstamp = $fs->create_file_from_storedfile($filerecord, $stamp); + $gradestamps[$newstamp->get_pathnamehash()] = $newstamp; } } - // Now get the full list of stamp files for this instance. - if ($files = $fs->get_area_files($this->assignment->get_context()->id, - 'assignfeedback_editpdf', - 'stamps', - $grade->id, - "filename", - false)) { - foreach ($files as $file) { - $filename = $file->get_filename(); - if ($filename !== '.') { - - // Check to see if the file exists in system context. - $insystemfiles = in_array($filename, $systemfiles); - - // If stamp is available in the system context, use that copy. - // If not then fall back to file saved in the files table. - $context = $insystemfiles ? $syscontext->id : $this->assignment->get_context()->id; - $itemid = $insystemfiles ? 0 : $grade->id; - - $url = moodle_url::make_pluginfile_url( - $context, - 'assignfeedback_editpdf', - 'stamps', - $itemid, - '/', - $file->get_filename(), - false - ); - array_push($stampfiles, $url->out()); - } + foreach ($gradestamps as $stamp) { + // All gradestamps should be available in the systemstamps filearea, but some legacy stamps may not be. + // These need to be copied over. + // Note: This should ideally be performed as an upgrade step, but there may be other cases that these do not + // exist, for example restored backups. + // In any case this is a cheap operation as it is solely performing an array lookup. + $systempathnamehash = $this->get_system_stamp_path($stamp); + if (!array_key_exists($systempathnamehash, $systemstamps)) { + $filerecord = (object) [ + 'contextid' => $syscontext->id, + 'itemid' => 0, + 'filearea' => 'systemstamps', + 'filepath' => '/' . $stamp->get_contenthash() . '/', + ]; + $systemstamp = $fs->create_file_from_storedfile($filerecord, $stamp); + $systemstamps[$systemstamp->get_pathnamehash()] = $systemstamp; } + + // Always serve the perpetual system stamp. + // This ensures that the stamp is highly cached and reduces the hit on the application server. + $gradestamp = $systemstamps[$systempathnamehash]; + $url = moodle_url::make_pluginfile_url( + $gradestamp->get_contextid(), + $gradestamp->get_component(), + $gradestamp->get_filearea(), + false, + $gradestamp->get_filepath(), + $gradestamp->get_filename(), + false + ); + array_push($stampfiles, $url->out()); } $url = false; @@ -161,6 +174,43 @@ class assign_feedback_editpdf extends assign_feedback_plugin { return $widget; } + /** + * Get the pathnamehash for the specified stamp if in the system stamps. + * + * @param stored_file $file + * @return string + */ + protected function get_system_stamp_path(stored_file $stamp): string { + $systemcontext = context_system::instance(); + + return file_storage::get_pathname_hash( + $systemcontext->id, + 'assignfeedback_editpdf', + 'systemstamps', + 0, + '/' . $stamp->get_contenthash() . '/', + $stamp->get_filename() + ); + } + + /** + * Get the pathnamehash for the specified stamp if in the current assignment stamps. + * + * @param stored_file $file + * @param int $gradeid + * @return string + */ + protected function get_assignment_stamp_path(stored_file $stamp, int $gradeid): string { + return file_storage::get_pathname_hash( + $this->assignment->get_context()->id, + 'assignfeedback_editpdf', + 'stamps', + $gradeid, + $stamp->get_filepath(), + $stamp->get_filename() + ); + } + /** * Get form elements for grading form * -- 2.26.2