MDL-42387 standardise file lifetime handling

blocks/html/lib.php

@@ -81,8 +81,10 @@ function block_html_pluginfile($course, $birecord_or_cm, $context, $filearea, $a
         $forcedownload = true;
     }
 
+    // NOTE: it woudl be nice to have file revisions here, for now rely on standard file lifetime,
+    //       do not lower it because the files are dispalyed very often.
     \core\session\manager::write_close();
-    send_stored_file($file, 60*60, 0, $forcedownload, $options);
+    send_stored_file($file, null, 0, $forcedownload, $options);
 }
 
 /**

config-dist.php

@@ -201,7 +201,7 @@ $CFG->admin = 'admin';
 //
 // Seconds for files to remain in caches. Decrease this if you are worried
 // about students being served outdated versions of uploaded files.
-//     $CFG->filelifetime = 86400;
+//     $CFG->filelifetime = 60*60*6;
 //
 // Some web servers can offload the file serving from PHP process,
 // comment out one the following options to enable it in Moodle:

file.php

@@ -36,12 +36,6 @@ define('NO_DEBUG_DISPLAY', true);
 require_once('config.php');
 require_once('lib/filelib.php');
 
-if (!isset($CFG->filelifetime)) {
-    $lifetime = 86400;     // Seconds for files to remain in caches
-} else {
-    $lifetime = $CFG->filelifetime;
-}
-
 $relativepath  = get_file_argument();
 $forcedownload = optional_param('forcedownload', 0, PARAM_BOOL);
 
@@ -112,6 +106,6 @@ if ($file->get_filename() == '.') {
 // finally send the file
 // ========================================
 \core\session\manager::write_close(); // Unlock session during file serving.
-send_stored_file($file, $lifetime, $CFG->filteruploadedfiles, $forcedownload);
+send_stored_file($file, null, $CFG->filteruploadedfiles, $forcedownload);
 
 

lib/filelib.php

@@ -2226,7 +2226,7 @@ function send_temp_file_finished($path) {
  * @category files
  * @param string $path Path of file on disk (including real filename), or actual content of file as string
  * @param string $filename Filename to send
- * @param int $lifetime Number of seconds before the file should expire from caches (default 24 hours)
+ * @param int $lifetime Number of seconds before the file should expire from caches (null means $CFG->filelifetime)
  * @param int $filter 0 (default)=no filtering, 1=all files, 2=html files only
  * @param bool $pathisstring If true (default false), $path is the content to send and not the pathname
  * @param bool $forcedownload If true (default false), forces download of file rather than view in browser/plugin
@@ -2237,20 +2237,15 @@ function send_temp_file_finished($path) {
  *                        and should not be reopened.
  * @return null script execution stopped unless $dontdie is true
  */
-function send_file($path, $filename, $lifetime = 'default' , $filter=0, $pathisstring=false, $forcedownload=false, $mimetype='', $dontdie=false) {
+function send_file($path, $filename, $lifetime = null , $filter=0, $pathisstring=false, $forcedownload=false, $mimetype='', $dontdie=false) {
     global $CFG, $COURSE;
 
     if ($dontdie) {
         ignore_user_abort(true);
     }
 
-    // MDL-11789, apply $CFG->filelifetime here
-    if ($lifetime === 'default') {
-        if (!empty($CFG->filelifetime)) {
-            $lifetime = $CFG->filelifetime;
-        } else {
-            $lifetime = 86400;
-        }
+    if ($lifetime === 'default' or is_null($lifetime)) {
+        $lifetime = $CFG->filelifetime;
     }
 
     \core\session\manager::write_close(); // Unlock session during file serving.
@@ -2356,13 +2351,13 @@ function send_file($path, $filename, $lifetime = 'default' , $filter=0, $pathiss
  *
  * @category files
  * @param stored_file $stored_file local file object
- * @param int $lifetime Number of seconds before the file should expire from caches (default 24 hours)
+ * @param int $lifetime Number of seconds before the file should expire from caches (null means $CFG->filelifetime)
  * @param int $filter 0 (default)=no filtering, 1=all files, 2=html files only
  * @param bool $forcedownload If true (default false), forces download of file rather than view in browser/plugin
  * @param array $options additional options affecting the file serving
  * @return null script execution stopped unless $options['dontdie'] is true
  */
-function send_stored_file($stored_file, $lifetime=86400 , $filter=0, $forcedownload=false, array $options=array()) {
+function send_stored_file($stored_file, $lifetime=null, $filter=0, $forcedownload=false, array $options=array()) {
     global $CFG, $COURSE;
 
     if (empty($options['filename'])) {
@@ -2377,6 +2372,10 @@ function send_stored_file($stored_file, $lifetime=86400 , $filter=0, $forcedownl
         $dontdie = true;
     }
 
+    if ($lifetime === 'default' or is_null($lifetime)) {
+        $lifetime = $CFG->filelifetime;
+    }
+
     if (!empty($options['preview'])) {
         // replace the file with its preview
         $fs = get_file_storage();
@@ -4556,10 +4555,8 @@ function file_pluginfile($relativepath, $forcedownload, $preview = null) {
                 send_file_not_found();
             }
 
-            $lifetime = isset($CFG->filelifetime) ? $CFG->filelifetime : 86400;
-
             // finally send the file
-            send_stored_file($file, $lifetime, 0, false, array('preview' => $preview));
+            send_stored_file($file, null, 0, false, array('preview' => $preview));
         }
 
         $filefunction = $component.'_pluginfile';

lib/setup.php

@@ -766,6 +766,11 @@ if (!PHPUNIT_TEST and !defined('BEHAT_TEST')) {
     $USER    =& $_SESSION['USER'];
 }
 
+// Initialise some variables that are supposed to be set in config.php only.
+if (!isset($CFG->filelifetime)) {
+    $CFG->filelifetime = 60*60*6;
+}
+
 // Late profiling, only happening if early one wasn't started
 if (!empty($CFG->profilingenabled)) {
     require_once($CFG->libdir . '/xhprof/xhprof_moodle.php');

lib/upgrade.txt

@@ -52,6 +52,8 @@ information provided here is intended especially for developers.
 * New class introduced to help auto generate zIndex values for modal dialogues. Class "moodle-has-zindex"
   should set on any element which uses a non-default zindex and needs to ensure it doesn't show above a
   dialogue.
+* $CFG->filelifetime is now used consistently for most file serving operations, the default was lowered
+  to 6 hours from 24 hours because etags and x-sendfile support should make file serving less expensive.
 
 DEPRECATIONS:
 Various previously deprecated functions have now been altered to throw DEBUG_DEVELOPER debugging notices

mod/assign/feedback/editpdf/classes/pdf.php

@@ -530,7 +530,7 @@ class pdf extends \FPDI {
 
         $testimagefolder = \make_temp_directory('assignfeedback_editpdf_test');
         $testimage = $testimagefolder.'/image_page0.png';
-        send_file($testimage, basename($testimage));
+        send_file($testimage, basename($testimage), 0);
         die();
     }
 

mod/assignment/type/online/assignment.class.php

@@ -400,7 +400,7 @@ class assignment_online extends assignment_base {
     }
 
     public function send_file($filearea, $args, $forcedownload, array $options=array()) {
-        global $USER;
+        global $USER, $CFG;
         require_capability('mod/assignment:view', $this->context);
 
         $fullpath = "/{$this->context->id}/mod_assignment/$filearea/".implode('/', $args);
@@ -416,7 +416,14 @@ class assignment_online extends assignment_base {
 
         \core\session\manager::write_close(); // Unlock session during file serving.
 
-        send_stored_file($file, 60*60, 0, true, $options);
+        // Make the lifetime significantly shorter,
+        // it would be better to have file revision numbers.
+        $lifetime = $CFG->filelifetime;
+        if ($lifetime > 60*6) {
+            $lifetime = 60*6;
+        }
+
+        send_stored_file($file, $lifetime, 0, true, $options);
     }
 
     /**

mod/book/lib.php

@@ -442,8 +442,14 @@ function book_pluginfile($course, $cm, $context, $filearea, $args, $forcedownloa
         return false;
     }
 
+    // Nasty hack because we do not have file revisions in book yet.
+    $lifetime = $CFG->filelifetime;
+    if ($lifetime > 60*10) {
+        $lifetime = 60*10;
+    }
+
     // finally send the file
-    send_stored_file($file, 360, 0, $forcedownload, $options);
+    send_stored_file($file, $lifetime, 0, $forcedownload, $options);
 }
 
 /**

mod/folder/lib.php

@@ -317,7 +317,7 @@ function folder_pluginfile($course, $cm, $context, $filearea, $args, $forcedownl
 
     // finally send the file
     // for folder module, we force download file all the time
-    send_stored_file($file, 86400, 0, true, $options);
+    send_stored_file($file, 0, 0, true, $options);
 }
 
 /**

mod/imscp/lib.php

@@ -356,7 +356,7 @@ function imscp_pluginfile($course, $cm, $context, $filearea, $args, $forcedownlo
         }
 
         // finally send the file
-        send_stored_file($file, 86400, 0, $forcedownload, $options);
+        send_stored_file($file, null, 0, $forcedownload, $options);
 
     } else if ($filearea === 'backup') {
         if (!has_capability('moodle/course:managefiles', $context)) {
@@ -372,7 +372,7 @@ function imscp_pluginfile($course, $cm, $context, $filearea, $args, $forcedownlo
         }
 
         // finally send the file
-        send_stored_file($file, 86400, 0, $forcedownload, $options);
+        send_stored_file($file, null, 0, $forcedownload, $options);
 
     } else {
         return false;

mod/page/lib.php

@@ -405,7 +405,7 @@ function page_pluginfile($course, $cm, $context, $filearea, $args, $forcedownloa
         }
 
         // finally send the file
-        send_stored_file($file, 86400, 0, $forcedownload, $options);
+        send_stored_file($file, null, 0, $forcedownload, $options);
     }
 }
 

mod/resource/lib.php

@@ -432,7 +432,7 @@ function resource_pluginfile($course, $cm, $context, $filearea, $args, $forcedow
     }
 
     // finally send the file
-    send_stored_file($file, 86400, $filter, $forcedownload, $options);
+    send_stored_file($file, null, $filter, $forcedownload, $options);
 }
 
 /**

mod/scorm/lib.php

@@ -942,7 +942,7 @@ function scorm_pluginfile($course, $cm, $context, $filearea, $args, $forcedownlo
 
     require_login($course, true, $cm);
 
-    $lifetime = isset($CFG->filelifetime) ? $CFG->filelifetime : 86400;
+    $lifetime = null;
 
     if ($filearea === 'content') {
         $revision = (int)array_shift($args); // prevents caching problems - ignored here

mod/wiki/lib.php

@@ -464,9 +464,7 @@ function wiki_pluginfile($course, $cm, $context, $filearea, $args, $forcedownloa
             return false;
         }
 
-        $lifetime = isset($CFG->filelifetime) ? $CFG->filelifetime : 86400;
-
-        send_stored_file($file, $lifetime, 0, $options);
+        send_stored_file($file, null, 0, $options);
     }
 }
 

mod/workshop/lib.php

@@ -1230,10 +1230,8 @@ function workshop_pluginfile($course, $cm, $context, $filearea, array $args, $fo
             send_file_not_found();
         }
 
-        $lifetime = isset($CFG->filelifetime) ? $CFG->filelifetime : 86400;
-
         // finally send the file
-        send_stored_file($file, $lifetime, 0, $forcedownload, $options);
+        send_stored_file($file, null, 0, $forcedownload, $options);
 
     } else if ($filearea === 'instructreviewers') {
         array_shift($args); // itemid is ignored here
@@ -1245,10 +1243,8 @@ function workshop_pluginfile($course, $cm, $context, $filearea, array $args, $fo
             send_file_not_found();
         }
 
-        $lifetime = isset($CFG->filelifetime) ? $CFG->filelifetime : 86400;
-
         // finally send the file
-        send_stored_file($file, $lifetime, 0, $forcedownload, $options);
+        send_stored_file($file, null, 0, $forcedownload, $options);
 
     } else if ($filearea === 'conclusion') {
         array_shift($args); // itemid is ignored here
@@ -1260,10 +1256,8 @@ function workshop_pluginfile($course, $cm, $context, $filearea, array $args, $fo
             send_file_not_found();
         }
 
-        $lifetime = isset($CFG->filelifetime) ? $CFG->filelifetime : 86400;
-
         // finally send the file
-        send_stored_file($file, $lifetime, 0, $forcedownload, $options);
+        send_stored_file($file, null, 0, $forcedownload, $options);
 
     } else if ($filearea === 'submission_content' or $filearea === 'submission_attachment') {
         $itemid = (int)array_shift($args);

repository/boxnet/lib.php

@@ -337,12 +337,12 @@ class repository_boxnet extends repository {
      * Repository method to serve the referenced file
      *
      * @param stored_file $storedfile the file that contains the reference
-     * @param int $lifetime Number of seconds before the file should expire from caches (default 24 hours)
+     * @param int $lifetime Number of seconds before the file should expire from caches (null means $CFG->filelifetime)
      * @param int $filter 0 (default)=no filtering, 1=all files, 2=html files only
      * @param bool $forcedownload If true (default false), forces download of file rather than view in browser/plugin
      * @param array $options additional options affecting the file serving
      */
-    public function send_file($storedfile, $lifetime=86400 , $filter=0, $forcedownload=false, array $options = null) {
+    public function send_file($storedfile, $lifetime=null , $filter=0, $forcedownload=false, array $options = null) {
         $ref = $storedfile->get_reference();
         // Let box.net serve the file. It will return 'no such file' content if file not found
         // also if file has the different name than alias, it will be returned with the box.net filename

repository/dropbox/lib.php

@@ -669,12 +669,12 @@ class repository_dropbox extends repository {
      * serves from there.
      *
      * @param stored_file $storedfile the file that contains the reference
-     * @param int $lifetime Number of seconds before the file should expire from caches (default 24 hours)
+     * @param int $lifetime Number of seconds before the file should expire from caches (null means $CFG->filelifetime)
      * @param int $filter 0 (default)=no filtering, 1=all files, 2=html files only
      * @param bool $forcedownload If true (default false), forces download of file rather than view in browser/plugin
      * @param array $options additional options affecting the file serving
      */
-    public function send_file($storedfile, $lifetime=86400 , $filter=0, $forcedownload=false, array $options = null) {
+    public function send_file($storedfile, $lifetime=null , $filter=0, $forcedownload=false, array $options = null) {
         $ref = unserialize($storedfile->get_reference());
         if ($storedfile->get_filesize() > $this->max_cache_bytes()) {
             header('Location: '.$this->get_file_download_link($ref->url));

repository/equella/lib.php

@@ -240,12 +240,12 @@ class repository_equella extends repository {
      * Repository method to serve the referenced file
      *
      * @param stored_file $storedfile the file that contains the reference
-     * @param int $lifetime Number of seconds before the file should expire from caches (default 24 hours)
+     * @param int $lifetime Number of seconds before the file should expire from caches (null means $CFG->filelifetime)
      * @param int $filter 0 (default)=no filtering, 1=all files, 2=html files only
      * @param bool $forcedownload If true (default false), forces download of file rather than view in browser/plugin
      * @param array $options additional options affecting the file serving
      */
-    public function send_file($stored_file, $lifetime=86400 , $filter=0, $forcedownload=false, array $options = null) {
+    public function send_file($stored_file, $lifetime=null , $filter=0, $forcedownload=false, array $options = null) {
         $reference  = unserialize(base64_decode($stored_file->get_reference()));
         $url = $this->appendtoken($reference->url);
         if ($url) {

repository/filesystem/lib.php

@@ -323,12 +323,12 @@ class repository_filesystem extends repository {
      * @see send_stored_file
      *
      * @param stored_file $storedfile the file that contains the reference
-     * @param int $lifetime Number of seconds before the file should expire from caches (default 24 hours)
+     * @param int $lifetime Number of seconds before the file should expire from caches (null means $CFG->filelifetime)
      * @param int $filter 0 (default)=no filtering, 1=all files, 2=html files only
      * @param bool $forcedownload If true (default false), forces download of file rather than view in browser/plugin
      * @param array $options additional options affecting the file serving
      */
-    public function send_file($storedfile, $lifetime=86400 , $filter=0, $forcedownload=false, array $options = null) {
+    public function send_file($storedfile, $lifetime=null , $filter=0, $forcedownload=false, array $options = null) {
         $reference = $storedfile->get_reference();
         if ($reference{0} == '/') {
             $file = $this->root_path.substr($reference, 1, strlen($reference)-1);
@@ -469,7 +469,6 @@ class repository_filesystem extends repository {
         global $CFG;
         // Check if this repository is allowed to use relative linking.
         $allowlinks = $this->supports_relative_file();
-        $lifetime = isset($CFG->filelifetime) ? $CFG->filelifetime : 86400;
         if (!empty($allowlinks)) {
             // Get path to the mainfile.
             $mainfilepath = $mainfile->get_source();
@@ -482,7 +481,7 @@ class repository_filesystem extends repository {
 
             // Sanity check to make sure this path is inside this repository and the file exists.
             if (strpos($fullrelativefilepath, $this->root_path) === 0 && file_exists($fullrelativefilepath)) {
-                send_file($fullrelativefilepath, basename($relativepath), $lifetime, 0);
+                send_file($fullrelativefilepath, basename($relativepath), null, 0);
             }
         }
         send_file_not_found();
@@ -511,7 +510,7 @@ class repository_filesystem extends repository {
  * @return bool
  */
 function repository_filesystem_pluginfile($course, $cm, $context, $filearea, $args, $forcedownload, array $options=array()) {
-    global $OUTPUT;
+    global $OUTPUT, $CFG;
     // Allowed filearea is either thumb or icon - size of the thumbnail.
     if ($filearea !== 'thumb' && $filearea !== 'icon') {
         return false;
@@ -532,7 +531,12 @@ function repository_filesystem_pluginfile($course, $cm, $context, $filearea, $ar
         // Generation failed, redirect to default icon for file extension.
         redirect($OUTPUT->pix_url(file_extension_icon($file, 90)));
     }
-    send_stored_file($file, 360, 0, $forcedownload, $options);
+    // The thumbnails should not be changing much, but maybe the default lifetime is too long.
+    $lifetime = $CFG->filelifetime;
+    if ($lifetime > 60*10) {
+        $lifetime = 60*10;
+    }
+    send_stored_file($file, $lifetime, 0, $forcedownload, $options);
 }
 
 /**