Merge branch 'MDL-68665-assign-stamp-cache' of https://github.com/jamie-catalyst...
authorAndrew Nicols <andrew@nicols.co.uk>
Thu, 8 Oct 2020 00:17:01 +0000 (08:17 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Thu, 8 Oct 2020 00:17:01 +0000 (08:17 +0800)
mod/assign/feedback/editpdf/lib.php
mod/assign/feedback/editpdf/locallib.php

index 143acff..701d394 100644 (file)
@@ -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,16 +39,36 @@ 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');
+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;
+        }
+
+        $filename = array_pop($args);
+        $filepath = '/' . implode('/', $args) . '/';
+
+        $fs = get_file_storage();
+        $file = $fs->get_file($context->id, 'assignfeedback_editpdf', $filearea, 0, $filepath, $filename);
+        if (!$file) {
+            return false;
+        }
+
+        $options['cacheability'] = 'public';
+        $options['immutable'] = true;
+
+        send_stored_file($file, 0, 0, true, $options);
+    }
 
     if ($context->contextlevel == CONTEXT_MODULE) {
 
index 03f9f26..84f7b23 100644 (file)
@@ -69,56 +69,85 @@ class assign_feedback_editpdf extends assign_feedback_plugin {
                                                                  $attempt);
 
         $stampfiles = array();
+        $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 !== '.') {
-
-                    $existingfile = $fs->get_file($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 !== '.') {
-                    $url = moodle_url::make_pluginfile_url($this->assignment->get_context()->id,
-                                                   'assignfeedback_editpdf',
-                                                   'stamps',
-                                                   $grade->id,
-                                                   '/',
-                                                   $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(),
+                null,
+                $gradestamp->get_filepath(),
+                $gradestamp->get_filename(),
+                false
+            );
+            array_push($stampfiles, $url->out());
         }
 
         $url = false;
@@ -145,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
      *