tests and docs

Author: matthewhilton

TAGGING.md

@@ -1,39 +1,76 @@
 # Tagging
+Tagging allows extra metadata about your files to be send to the external object store. These sources are defined in code, and currently cannot be configured on/off from the UI.
 
-todo blurb about tagging
+Currently, this is only implemented for the S3 file system client. [See the S3 docs for more information](https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-tagging.html).
 
-You probably don't need this unless you have a lot of objects!
+**Tagging vs metadata**
+Note object tags are different from object metadata.
 
-Support level: currenly only implemented for AWS/S3
+Object metadata is immutable, and attached to the object on upload. With metadata, if you wish to update it (for example during a migration, or the sources changed), you have to copy the object with the new metadata, and delete the old object. This is problematic, since deletion is optional in objectfs.
 
-Note extra cost, extra api calls + tagging cost per object per month
+Object tags are more suitable, since their permissions can be managed separately (e.g. a client can be allowed to modify tags, but not delete objects).
 
 ## Sources
+The following sources are implemented currently:
+### Environment
+What environment the file was uploaded in. Configure the environment using `$CFG->objectfs_environment_name`
 
-Constraints:
-- Key - max 128 chars (aws + azure)
-- Value - max 128 chars (actual max is 256, but reserving half for future use e.g. versioning)
-- Deterministic
+### Mimetype
+What mimetype the file is stored as under the `mdl_files` table.
 
-Implemented:
-- Mimetype 
-- Env - defined by <insert cfg here>
+## Multi environment bucket
+It is possible you are using objectfs with multiple environments (e.g. prod, staging) that both point to the same bucket. Since files are referenced by contenthash, it generally does not matter where they come from s
+If this is the case, for tagging to work as expected, you must ensure object override is turned off for every environment except production.
 
-## Multi env
-- Turn off object override on every env except prod
-- TODO diagram here
+This means that staging is unable to overwrite objects (thus overwriting tags) for files uploaded elsewhere. However, files uploaded elsewhere can be overwitten (hence overwriting tags) by production. This allows 
 
-## Reporting
-TODO blurb about reporting
-Note reporting quirk because of multi env setup and what each env knows about.
+See this diagram for a visual explanation:
 
-## Migration
+```mermaid
+graph LR
+    subgraph S3
+        Object("`**Object**
+        contenthash: xyz
+        tags: env=prod`")
+    end
+    subgraph Prod
+        UploadObjectProd["`**Upload object**
+        contenthash: xyz
+        tags: env=prod`"] --> Object
+    end
+    subgraph Staging
+        UploadObjectStaging["`**Upload object**
+        contenthash: xyz
+        tags: env=staging`"]
+    end
+    Blocked["Blocked - does not have permissions\nto overwrite existing objects"]
+    UploadObjectStaging --- Blocked
+    Blocked -.-> Object
+
+    style Object fill:#ffcd75,stroke:#ffa812
+    style S3 fill:#ffd68f,stroke:#ffa812
+    style Prod fill:#c2ffcc,stroke:#26ff4a
+    style UploadObjectProd fill:#c2ffcc,stroke:#26ff4a
+    style Staging fill:#ddd9ff,stroke:#978aff
+    style UploadObjectStaging fill:#ddd9ff,stroke:#978aff
+    style Blocked fill:#ff6161,stroke:#ff0000
+```
 
-TODO
-If you change any of the tags e.g. the way they work and want to re-update tags, update tag status to needs sync and queue adhoc task to re-sync.
+## Reporting
+// TODO.
 
+## Migration
+If the way a tag was calculated has changed, or new tags are added (or removed) or this feature was turned on for the first time (or turned on after being off), you must do the following: 
+- Manually run `trigger_update_object_tags` scheduled task from the UI, or call the CLI to queue a `update_object_tags` adhoc task.
 ## For developers
 
 ### Adding a new source
-- Add to <insert place in tag manager here>
-- Run migration steps (TODO link to above)
+Note the rules about sources:
+- Identifier must be < 128 chars long.
+- Value must be < 128 chars long. Note we intentionally reserve 128 chars of extra space for later use.
+
+To add a new source:
+- Implement `tag_source`
+- Add to the `tag_manager` class
+- As part of an upgrade step, mark all objects as needing tag sync (using `tag_manager` class)
+- As part of an upgrade step, queue a `update_object_tags` adhoc task to process the tag migration.

classes/local/manager.php

@@ -389,9 +389,10 @@ public static function get_filename_from_header($header) {
      * @return bool
      */
     public static function can_override_existing_objects(): bool {
+        // This is either "1" or "0"
         $config = get_config('tool_objectfs', 'canoverrideobjects');
 
-        // Default true for backwards compatibility.
-        return empty($config) ? true : $config;
+        // Default to true if not set.
+        return is_string($config) ? (bool) $config : true;
     }
 }

classes/local/store/object_file_system.php

@@ -1165,12 +1165,12 @@ private function update_object(array $result): array {
      * @param string $contenthash file to sync tags for
      */
     public function sync_object_tags(string $contenthash) {
-        if (!$this->get_external_client()->is_tagging_supported()) {
+        if (!$this->get_external_client()->supports_object_tagging()) {
             throw new coding_exception("Cannot sync tags, external client does not support tagging.");
         }
 
         // Get a lock before syncing, to ensure other parts of objectfs are not moving/interacting with this object.
-        $lock = $this->acquire_object_lock($contenthash, 5);
+        $lock = $this->acquire_object_lock($contenthash, 10);
 
         // No lock - just skip it.
         if (!$lock) {

classes/tests/test_client.php

@@ -34,6 +34,11 @@ class test_client extends object_client_base {
      */
     private $bucketpath;
 
+    /**
+     * @var array in-memory tags used for unit tests
+     */
+    public $tags;
+
     /**
      * string
      * @param \stdClass $config
@@ -158,6 +163,20 @@ public function get_maximum_upload_size() {
         return $this->maxupload;
     }
 
+    /**
+     * Sets object tags - uses in-memory store for unit tests
+     */
+    public function set_object_tags(string $contenthash, array $tags) {
+        $this->tags[$contenthash] = $tags;
+    }
+
+    /**
+     * Gets object tags - uses in-memory store for unit tests
+     */
+    public function get_object_tags(string $contenthash): array {
+        return $this->tags[$contenthash] ?? [];
+    }
+
     /**
      * Object tagging support, for unit testing
      * @return bool

classes/tests/testcase.php

@@ -30,6 +30,10 @@
  * @package tool_objectfs
  */
 abstract class testcase extends \advanced_testcase {
+    /**
+     * @var test_file_system test file system
+     */
+    protected $filesystem;
 
     /**
      * setUp
@@ -42,6 +46,7 @@ protected function setUp(): void {
         $CFG->objectfs_environment_name = 'test';
         $this->filesystem = new test_file_system();
         $this->logger = new \tool_objectfs\log\null_logger();
+
         $this->resetAfterTest(true);
     }
 

tests/object_file_system_test.php

@@ -16,8 +16,10 @@
 
 namespace tool_objectfs;
 
+use coding_exception;
 use tool_objectfs\local\store\object_file_system;
 use tool_objectfs\local\manager;
+use tool_objectfs\local\tag\tag_manager;
 use tool_objectfs\tests\test_file_system;
 
 /**
@@ -1017,5 +1019,104 @@ public function test_add_file_from_string_update_object_fail() {
         $this->assertTrue($result[2]);
     }
 
-    // TODO test tag syncing somehow ? Using a test FS ? Maybe test FS can store them in memory ?
+    /**
+     * Test syncing tags throws exception when client does not support tagging.
+     */
+    public function test_sync_object_tags_not_supported() {
+        global $CFG;
+        $CFG->phpunit_objectfs_supports_object_tagging = false;	
+        $this->expectException(coding_exception::class);
+        $this->expectExceptionMessage('Cannot sync tags, external client does not support tagging');
+        $this->filesystem->sync_object_tags('123');
+    }
+
+    /**
+     * Tests syncing object tags where the file is not replicated.
+     */
+    public function test_sync_object_tags_object_not_replicated() {
+        global $CFG, $DB;
+        $CFG->phpunit_objectfs_supports_object_tagging = true;	
+
+        // Create object - not replicated to 'external' store yet.
+        $object = $this->create_local_object('test syncing local');
+
+        // Sync, this should do nothing but change sync status - cannot sync object tags
+        // where the object is not replicated.
+        $this->filesystem->sync_object_tags($object->contenthash);
+        $object = $DB->get_record('tool_objectfs_objects', ['contenthash' => $object->contenthash]);
+        $this->assertEquals($object->tagsyncstatus, tag_manager::SYNC_STATUS_SYNC_NOT_REQUIRED);
+    }
+
+    /**
+     * Provides values to sync_object_tags_replicated
+     * @return array
+     */
+    public static function sync_object_tags_replicated_provider(): array {
+        return [
+            'can override' => [
+                'can override' => true,
+            ],
+            'cannot override' => [
+                'cannot override' => false,
+            ],
+        ];
+    }
+
+    /**
+     * Tests sync_object_tags when the object is replicated.
+     * @dataProvider sync_object_tags_replicated_provider
+     */
+    public function test_sync_object_tags_replicated(bool $canoverride) {
+        global $CFG, $DB;
+        $CFG->phpunit_objectfs_supports_object_tagging = true;	
+
+        set_config('canoverrideobjects', $canoverride, 'tool_objectfs');
+        $this->assertEquals($canoverride, manager::can_override_existing_objects());
+
+        $object = $this->create_duplicated_object('test syncing replicated');
+
+        $testtags = [
+            'test' => 123,
+            'test2' => 123,
+            'test3' => 123,
+            'test4' => 123,
+        ];
+
+        // Fake set the tags in the external store.
+        $this->filesystem->get_external_client()->tags[$object->contenthash] = $testtags;
+
+        // Ensure tags are set 'externally'.
+        $tags = $this->filesystem->get_external_client()->get_object_tags($object->contenthash);
+        $this->assertCount(count($testtags), $tags);
+
+        // But tags will not be stored locally (yet).
+        $localtags = $DB->get_records('tool_objectfs_object_tags', ['contenthash' => $object->contenthash]);
+        $this->assertCount(0, $localtags);
+
+        // Sync the file.
+        $this->filesystem->sync_object_tags($object->contenthash);
+
+        // Tags should now be replicated locally.
+        $localtags = $DB->get_records('tool_objectfs_object_tags', ['contenthash' => $object->contenthash]);
+        $externaltags = $this->filesystem->get_external_client()->get_object_tags($object->contenthash);
+
+        if ($canoverride) {
+            // If can override, we expect it to be overwritten by the tags defined in the sources.
+            $expectednum = count(tag_manager::get_defined_tag_sources());
+            $this->assertCount($expectednum, $localtags);
+
+            // Also expect the external store to be updated.
+            $this->assertCount($expectednum, $externaltags);
+        } else {
+            // If cannot override, we expect the tags to match what was in the 'external' store.
+            $this->assertCount(count($testtags), $localtags);
+
+            // External store should not be updated.
+            $this->assertCount(count($testtags), $externaltags);
+        }
+
+        // Ensure status changed to not needing sync.
+        $object = $DB->get_record('tool_objectfs_objects', ['contenthash' => $object->contenthash]);
+        $this->assertEquals($object->tagsyncstatus, tag_manager::SYNC_STATUS_SYNC_NOT_REQUIRED);
+    }
 }