Issue #382: Improve save and export speed on high latency file systems.

README.md

@@ -1,3 +1,15 @@
+# Catalyst fork
+
+This is a fork of the stable version of the h5p plugin. All the various extra bits
+in this fork should be in pull requests for the core plugin but are either waiting
+for review, and have been merged but have not yet been released into the stable
+branch of the plugin.
+
+To see the differences:
+
+https://github.com/h5p/moodle-mod_hvp/compare/stable...catalyst:stable-catalyst
+
+
 # H5P Moodle Plugin
 
 Create and add rich content inside your LMS for free. Some examples of what you

classes/file_storage.php

@@ -118,9 +118,8 @@ public function cloneContent($id, $newid) {
      */
     // @codingStandardsIgnoreLine
     public function getTmpPath() {
-        global $CFG;
-
-        return $CFG->tempdir . uniqid('/hvp-');
+        // Use request directory. This is generally the lowest latency available storage.
+        return get_request_storage_directory() . uniqid('/hvp-');
     }
 
     /**
@@ -494,30 +493,92 @@ public function removeContentFile($file, $content) {
      *  Path to source directory
      * @param array $options
      *  For Moodle's file record
+     * @param \ZipArchive $archive
+     *  A currently open archive to store complete libraries. Used in recursive calls.
+     * @param string $relativepath
+     *  The current relative path from the base function call. Used in recursive calls.
+     * @param bool $ziponly
+     *  Create only the zip archive, not the base files.
+     *
      * @throws \Exception Unable to copy
      */
     // @codingStandardsIgnoreLine
-    private static function readFileTree($source, $options) {
+    private static function readFileTree($source, $options, $archive = null, $relativepath = '', $ziponly = false) {
         $dir = opendir($source);
         if ($dir === false) {
             trigger_error('Unable to open directory ' . $source, E_USER_WARNING);
             throw new \Exception('unabletocopy');
         }
 
+        // Should we build a zipped copy? (Is this a lib).
+        $exportzip = $options['filearea'] === 'libraries';
+
+        // Create an empty zip to store a full lib archive as well.
+        if (empty($archive) && $exportzip) {
+            $archive = new \ZipArchive();
+            $path = tempnam(get_request_storage_directory(),'libdir');
+            $archive->open($path, \ZipArchive::CREATE || \ZipArchive::OVERWRITE);
+            // Set recursion flag.
+            $top = true;
+        } else {
+            $top = false;
+        }
+
         while (false !== ($file = readdir($dir))) {
             if (($file != '.') && ($file != '..') && $file != '.git' && $file != '.gitignore') {
                 if (is_dir($source . DIRECTORY_SEPARATOR . $file)) {
                     $suboptions = $options;
-                    $suboptions['filepath'] .= $file . '/';
-                    self::readFileTree($source . '/' . $file, $suboptions);
+                    $pathchunk = $file . '/';
+                    $suboptions['filepath'] .= $pathchunk;
+
+                    // Setup the relative path from the root dir.
+                    $origpath = $relativepath;
+                    $relativepath = !empty($relativepath) ? $relativepath . $pathchunk : $pathchunk;
+                    self::readFileTree($source . '/' . $file, $suboptions, $archive, $relativepath, $ziponly);
+                    // Reset path after recursing.
+                    $relativepath = $origpath;
                 } else {
-                    $record = $options;
-                    $record['filename'] = $file;
-                    $fs = get_file_storage();
-                    $fs->create_file_from_pathname($record, $source . '/' . $file);
+                    // Are we building the full lib?
+                    if (!$ziponly) {
+                        $record = $options;
+                        $record['filename'] = $file;
+                        $fs = get_file_storage();
+                        $fs->create_file_from_pathname($record, $source . '/' . $file);
+                    }
+
+                    $zippath = !empty($relativepath) ? $relativepath . $file : $file;
+                    // Also add file to open archive.
+                    if ($exportzip) {
+                        $archive->addFile($source . '/' . $file, $zippath);
+                    }
                 }
             }
         }
+
+        // Store the zipfile alongside the lib files.
+        if ($top) {
+            if (empty($fs)) {
+                $fs = get_file_storage();
+            }
+            $record = $options;
+
+            // Only store the lib if it doesn't already exist.
+            if (!$fs->file_exists(
+                $record['contextid'],
+                $record['component'],
+                'library_archives',
+                $record['itemid'],
+                $record['filepath'],
+                'lib-export.zip'
+            )) {
+                $record['filearea'] = 'library_archives';
+                $record['filename'] = 'lib-export.zip';
+                $archive->close();
+                $fs->create_file_from_pathname($record, $path);
+                unlink($path);
+            }
+        }
+
         closedir($dir);
     }
 
@@ -545,20 +606,53 @@ private static function exportFileTree($target, $contextid, $filearea, $filepath
         // Read source files.
         $fs = get_file_storage();
         $files = $fs->get_directory_files($contextid, 'mod_hvp', $filearea, $itemid, $filepath, true);
-
-        foreach ($files as $file) {
-            // Correct target path for file.
-            $path = $target . str_replace($filepath, '/', $file->get_filepath());
-
-            if ($file->is_directory()) {
-                // Create directory.
-                $path = rtrim($path, '/');
-                if (!file_exists($path)) {
-                    mkdir($path, 0777, true);
+        $library = $filearea === 'libraries';
+
+        if ($library && $file = $fs->get_file(
+            $contextid,
+            'mod_hvp',
+            'library_archives',
+            $itemid,
+            $filepath,
+            'lib-export.zip'
+        )) {
+            // Libraries may have a precompiled zip to extract
+            $reqpath = get_request_storage_directory() . '/libzip.zip';
+            $file->copy_content_to($reqpath);
+
+            // Extract the archive to the required dir.
+            $zip = new \ZipArchive();
+            $zip->open($reqpath);
+            $zip->extractTo($target);
+            $zip->close();
+            unlink($reqpath);
+        } else {
+            foreach ($files as $file) {
+                // Correct target path for file.
+                $path = $target . str_replace($filepath, '/', $file->get_filepath());
+
+                if ($file->is_directory()) {
+                    // Create directory.
+                    $path = rtrim($path, '/');
+                    if (!file_exists($path)) {
+                        mkdir($path, 0777, true);
+                    }
+                } else {
+                    // Copy file.
+                    $file->copy_content_to($path . $file->get_filename());
                 }
-            } else {
-                // Copy file.
-                $file->copy_content_to($path . $file->get_filename());
+            }
+
+            // Now that the filetree is exported, create a zip for future use.
+            if ($library) {
+                $options = [
+                    'contextid' => $contextid,
+                    'component' => 'mod_hvp',
+                    'filearea' => $filearea,
+                    'itemid' => $itemid,
+                    'filepath' => $filepath
+                ];
+                self::readFileTree($target, $options, null, '', true);
             }
         }
     }

classes/privacy/provider.php

@@ -24,6 +24,8 @@
 use \core_privacy\local\request\contextlist;
 use \core_privacy\local\request\approved_contextlist;
 use \core_privacy\local\request\deletion_criteria;
+use \core_privacy\local\request\approved_userlist;
+use \core_privacy\local\request\userlist;
 use \core_privacy\local\metadata\collection;
 
 defined('MOODLE_INTERNAL') || die();
@@ -34,7 +36,7 @@
 class provider implements
     // This plugin has data.
     \core_privacy\local\metadata\provider,
-
+    \core_privacy\local\request\core_userlist_provider,
     // This plugin currently implements the original plugin_provider interface.
     \core_privacy\local\request\plugin\provider {
 
@@ -443,4 +445,75 @@ public static function _delete_data_for_user(approved_contextlist $contextlist)
             'user_id' => $userid,
         ]);
     }
+
+    /**
+     * Get the list of users who have data within a context.
+     *
+     * @param   userlist    $userlist   The userlist containing the list of users who have data in this context/plugin combination.
+     */
+    public static function get_users_in_context(userlist $userlist) {
+        $context = $userlist->get_context();
+        if (!is_a($context, \context_module::class)) {
+            return;
+        }
+
+        $sql = "
+          SELECT d.user_id
+          FROM {course_modules} cm
+            INNER JOIN {modules} m ON m.id = cm.module AND m.name = 'hvp'
+            INNER JOIN {context} c ON c.instanceid = cm.id
+            INNER JOIN {hvp} h ON h.id = cm.instance
+            INNER JOIN {hvp_content_user_data} d ON h.id = d.hvp_id
+          WHERE c.contextlevel = :contextlevel AND c.id = :contextid";
+
+        $params = [
+            'contextlevel' => CONTEXT_MODULE,
+            'contextid'    => $context->id,
+        ];
+        $userlist->add_from_sql('user_id', $sql, $params);
+
+        $sql = "
+          SELECT
+            x.user_id
+          FROM {course_modules} cm
+            INNER JOIN {modules} m ON m.id = cm.module AND m.name = 'hvp'
+            INNER JOIN {context} c ON c.instanceid = cm.id
+            INNER JOIN {hvp} h ON h.id = cm.instance
+            INNER JOIN {hvp_xapi_results} x ON x.content_id = h.id
+          WHERE c.contextlevel = :contextlevel AND c.id = :contextid";
+        $userlist->add_from_sql('user_id', $sql, $params);
+    }
+
+    /**
+     * Delete multiple users within a single context.
+     *
+     * @param   approved_userlist       $userlist The approved context and user information to delete information for.
+     */
+    public static function delete_data_for_users(approved_userlist $userlist) {
+        global $DB;
+        $context = $userlist->get_context();
+        if (!is_a($context, \context_module::class)) {
+            return;
+        }
+        $cm = get_coursemodule_from_id('hvp', $context->instanceid);
+
+        // Prepare SQL to gather all completed IDs.
+        $userids = $userlist->get_userids();
+        list($insql, $inparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED);
+
+        $inparams['hvpid'] = $cm->instance;
+
+        $DB->delete_records_select(
+            'hvp_content_user_data',
+            "hvp_id = :hvpid AND user_id $insql",
+            $inparams
+        );
+
+        $DB->delete_records_select(
+            'hvp_xapi_results',
+            "hvp_id = :hvpid AND user_id $insql",
+            $inparams
+        );
+
+    }
 }