WIP more cleanups and refactor

Author: matthewhilton

classes/local/manager.php

@@ -79,7 +79,6 @@ public static function get_objectfs_config() {
         $config->s3_bucket = '';
         $config->s3_region = 'us-east-1';
         $config->s3_base_url = '';
-        $config->s3_canoverrideobjects = true;
         $config->key_prefix = '';
 
         // Digital ocean file system.
@@ -385,7 +384,11 @@ public static function get_filename_from_header($header) {
         return $filename;
     }
 
-    public static function can_override_existing_objects() {
+    /**
+     * If is able to override existing objects in the external store.
+     * @return bool
+     */
+    public static function can_override_existing_objects(): bool {
         return !empty(get_config('tool_objectfs', 'canoverrideobjects'));
     }
 }

classes/local/store/object_client.php

@@ -153,11 +153,11 @@ public function test_set_object_tag(): stdClass;
     public function set_object_tags(string $contenthash, array $tags);
 
     /**
-     * Returns given objects tags queried from the external store. If object does not exist in store, return null.
+     * Returns given objects tags queried from the external store. External object must exist.
      * @param string $contenthash file content has
-     * @param array array of key=>value tag pairs, or null if object does not exist.
+     * @param array array of key=>value tag pairs
      */
-    public function get_object_tags(string $contenthash): ?array;
+    public function get_object_tags(string $contenthash): array;
 
     /**
      * If the client supports object tagging feature.

classes/local/store/object_client_base.php

@@ -208,11 +208,11 @@ public function set_object_tags(string $contenthash, array $tags) {
     }
 
     /**
-     * Returns given objects tags queried from the external store. If object does not exist in store, return null.
+     * Returns given objects tags queried from the external store. External object must exist.
      * @param string $contenthash file content has
-     * @param array array of key=>value tag pairs, or null if object does not exist.
+     * @param array array of key=>value tag pairs
      */
-    public function get_object_tags(string $contenthash): ?array {
+    public function get_object_tags(string $contenthash): array {
         return [];
     }
 

classes/local/store/s3/client.php

@@ -511,11 +511,10 @@ public function upload_to_s3($localpath, $contenthash, array $tags) {
 
         try {
             $externalpath = $this->get_filepath_from_hash($contenthash);
-
             $uploader = new \Aws\S3\ObjectUploader(
                 $this->client, $this->bucket,
                 $this->bucketkeyprefix . $externalpath,
-                $filehandle,
+                $filehandle
             );
             $uploader->upload();
             fclose($filehandle);
@@ -893,7 +892,6 @@ public function test_set_object_tag(): stdClass {
             // First ensure a test object exists to put tags on.	
             // Note this will override the existing object if exists.
             $key = $this->bucketkeyprefix . 'tagging_check_file';
-
             $this->client->putObject([
                 'Bucket' => $this->bucket,
                 'Key' => $key, 
@@ -925,7 +923,12 @@ public function test_set_object_tag(): stdClass {
         return (object) ['success' => true, 'details' => ''];
     }
 
-    private function convert_tags_to_s3_format(array $tags) {
+    /**
+     * Convert key=>value to s3 tag format
+     * @param array $tags
+     * @return array tags in s3 format.
+     */
+    private function convert_tags_to_s3_format(array $tags): array {
         foreach($tags as $key => $value) {
             $s3tags[] = [
                 'Key' => $key,

classes/local/store/s3/file_system.php

@@ -88,9 +88,9 @@ public function copy_from_local_to_external($contenthash) {
         $localpath = $this->get_local_path_from_hash($contenthash);
 
         try {
-            // If can't override, check if it exists already, since upload object will override.
+            // If cannot override, and already exists, skip.
+            // We don't want to upload and potentially override.
             if (!manager::can_override_existing_objects() && $this->is_file_readable_externally_by_hash($contenthash)) {
-                // TODO somehow log it was skipped here?
                 return true;
             }
 
@@ -122,5 +122,4 @@ public function copy_from_local_to_external($contenthash) {
     public function supports_presigned_urls() {
         return true;
     }
-
 }

classes/local/tag/environment_source.php

@@ -0,0 +1,53 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace tool_objectfs\local\tag;
+
+/**
+ * Provides environment a file was uploaded in.
+ *
+ * @package   tool_objectfs
+ * @author    Matthew Hilton <[email protected]>
+ * @copyright Catalyst IT
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class environment_source implements tag_source {
+    /**
+     * Identifier used in tagging file. Is the 'key' of the tag.
+     * @return string
+     */
+    public static function get_identifier(): string {
+        return 'environment';
+    }
+
+    public static function get_description(): string {
+        return get_string('tagsource:environment', 'tool_objectfs', self::get_env());
+    }
+
+    private static function get_env(): ?string {
+        global $CFG;
+        return $CFG->objectfs_environment_name ?? null;
+    }
+
+    /**
+     * Returns the tag value for the given file contenthash
+     * @param string $contenthash
+     * @return string|null mime type for file.
+     */
+    public function get_value_for_contenthash(string $contenthash): ?string {
+        return self::get_env();
+    }
+}

classes/local/tag/file_type_source.php classes/local/tag/mime_type_source.php

@@ -17,47 +17,39 @@
 namespace tool_objectfs\local\tag;
 
 /**
- * File type tag source.
- * Provides tag values for files based on their 'type' - these are categories (not necessarily equivalent to fileareas)
- * that make it possible to apply different permissions to. For example, move backups to a cheaper storage class but not general files.
+ * Provides mime type of file.
  *
  * @package   tool_objectfs
  * @author    Matthew Hilton <[email protected]>
  * @copyright Catalyst IT
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class file_type_source implements tag_source {
-    /**
-     * @var string Unknown/uncategorised
-     */
-    public const TYPE_UNCATEGORISED = 'uncategorised';
-
-    /**
-     * @var string A course backup
-     */
-    public const TYPE_COURSE_BACKUP = 'course_backup';
-
+class mime_type_source implements tag_source {
     /**
      * Identifier used in tagging file. Is the 'key' of the tag.
      * @return string
      */
     public static function get_identifier(): string {
-        return 'type';
+        return 'mimetype';
+    }
+
+    public static function get_description(): string {
+        return get_string('tagsource:mimetype', 'tool_objectfs');
     }
 
     /**
      * Returns the tag value for the given file contenthash
      * @param string $contenthash
-     * @return string|null the category of the file, one of TYPE_*
+     * @return string|null mime type for file.
      */
     public function get_value_for_contenthash(string $contenthash): ?string {
         global $DB;
-        // TODO are there other unknown course backup files ?
-        $iscoursebackup = $DB->record_exists('files', ['contenthash' => $contenthash, 'component' => 'backup', 'filearea' => 'course']);
-        if ($iscoursebackup) {
-            return self::TYPE_COURSE_BACKUP;
+        $mime = $DB->get_field('files', 'mimetype', ['contenthash' => $contenthash]);
+
+        if(empty($mime)) {
+            return null;
         }
 
-        return self::TYPE_UNCATEGORISED;
+        return $mime;
     }
 }

classes/local/tag/tag_manager.php

@@ -17,9 +17,7 @@
 namespace tool_objectfs\local\tag;
 
 use coding_exception;
-use core\lock\lock;
-use core\lock\lock_config;
-use Exception;
+use html_writer;
 use tool_objectfs\local\manager;
 use tool_objectfs\local\store\object_file_system;
 
@@ -62,7 +60,8 @@ public static function get_defined_tag_sources(): array {
         // All possible tag sources should be defined here.
         // Note this should be a maximum of 10 sources, as this is an AWS limit.
         return [
-            new file_type_source()
+            new mime_type_source(),
+            new environment_source(),
         ];
     }
 
@@ -161,21 +160,25 @@ public static function mark_object_tag_sync_status(string $contenthash, int $sta
         $DB->set_field('tool_objectfs_objects', 'tagsyncstatus', $status, ['contenthash' => $contenthash]);
     }
 
+    /**
+     * Syncs an objects tags after they are uploaded.
+     * Note - you should take out the object lock before calling this function to avoid race conditions.
+     * @param string $contenthash
+     * @param object_file_system $fs
+     */
     public static function sync_object_tags_post_upload(string $contenthash, object_file_system $fs) {
-        // First get object lock
-        // TODO
-
-        $client = manager::get_client(manager::get_objectfs_config());
+        $client = $fs->get_external_client();
+        $objectexists = $fs->is_file_readable_externally_by_hash($contenthash);
 
         // If object does not exist, cannot sync tags to nothing, abort.
-        if (!$fs->is_file_readable_externally_by_hash($contenthash)) {
+        if (!$objectexists) {
             // TODO maybe this should be a failed status?
             self::mark_object_tag_sync_status($contenthash, self::SYNC_STATUS_SYNC_NOT_REQUIRED);
             return;
         }
 
         // Cannot override and object exists, query existing and store.
-        if (!manager::can_override_existing_objects()) {
+        if (!manager::can_override_existing_objects() && $objectexists) {
             // Query existing tags and store them.
             $existingtags = $client->get_object_tags($contenthash);
             self::store_tags_locally($contenthash, $existingtags);
@@ -189,7 +192,19 @@ public static function sync_object_tags_post_upload(string $contenthash, object_
 
         // Either way, it has synced.
         self::mark_object_tag_sync_status($contenthash, self::SYNC_STATUS_SYNC_NOT_REQUIRED);
+    }
 
-        // TODO release object lock.
+    /**
+     * Returns a simple list of all the sources and their descriptions.
+     * @return string html string
+     */
+    public static function get_tag_summary_html(): string {
+        $sources = self::get_defined_tag_sources();
+        $html = '';
+
+        foreach ($sources as $source) {
+            $html .= $source->get_identifier() . ': ' . $source->get_description() . '<br />';
+        }
+        return $html;
     }
 }

classes/local/tag/tag_source.php

@@ -34,6 +34,12 @@ interface tag_source {
      */
     public static function get_identifier(): string;
 
+    /**
+     * Description for source displayed in the admin settings.
+     * @return string
+     */
+    public static function get_description(): string;
+
     /**
      * Returns the value of this tag for the file with the given content hash.
      * This must be deterministic, and should never exceed 256 chars.

classes/task/trigger_update_object_tags.php

@@ -0,0 +1,45 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace tool_objectfs\task;
+
+use core\task\manager;
+use core\task\scheduled_task;
+
+/**
+ * Queues update_object_tags adhoc task periodically, or manually from the frontend.
+ *
+ * @package   tool_objectfs
+ * @author    Matthew Hilton <[email protected]>
+ * @copyright Catalyst IT
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class trigger_update_object_tags extends scheduled_task {
+    /**
+     * Task name
+     */
+    public function get_name() {
+        return get_string('task:triggerupdateobjecttags', 'tool_objectfs');
+    }
+    /**
+     * Execute task
+     */
+    public function execute() {
+        // Queue adhoc task, nothing else. 
+        $task = new update_object_tags();
+        manager::queue_adhoc_task($task, true);
+    }
+}

classes/task/update_object_tags.php

@@ -16,11 +16,8 @@
 
 namespace tool_objectfs\task;
 
-use coding_exception;
 use core\task\adhoc_task;
-use Exception;
 use Throwable;
-use tool_objectfs\local\manager;
 use tool_objectfs\local\tag\tag_manager;
 
 /**

db/tasks.php

@@ -107,5 +107,16 @@
         'dayofweek' => '*',
         'month'     => '*'
     ),
+    array(
+        'classname' => 'tool_objectfs\task\trigger_update_object_tags',
+        'blocking'  => 0,
+        'minute'    => 'R',
+        'hour'      => '*',
+        'day'       => '*',
+        'dayofweek' => '*',
+        'month'     => '*',
+        // Default disabled - intended to be manually run.
+        'disabled' => true,
+    ),
 );
 

lang/en/tool_objectfs.php

@@ -280,3 +280,8 @@
 $string['settings:maxtaggingperrun:desc'] = 'The maximum number of objects to sync tags for per migration adhoc task iteration.';
 $string['settings:overrideobjects'] = 'Allow object override';
 $string['settings:overrideobjects:desc'] = 'If disabled, ObjectFS will check that an object does not exist before uploading.';
+$string['tagsource:environment'] = 'Environment defined by $CFG->objectfs_environment_name, currently: "{$a}".';
+$string['tagsource:mimetype'] = 'File mimetype as stored in {files} table';
+$string['settings:tagsources'] = 'Tag sources';
+$string['task:triggerupdateobjectags'] = 'Queue adhoc task to update object tags';
+$string['settings:tagging:help'] = 'Object tagging allows extra metadata to be attached to objects in the external store. Please read TAGGING.md in the plugin Github repository for detailed setup and considerations. This is currently only supported by the S3 external client.';