WIP tests and check api for tagging

Author: matthewhilton

classes/local/store/object_client.php

@@ -120,6 +120,8 @@ public function test_connection();
      */
     public function test_permissions($testdelete);
 
+	public function test_set_object_tag();
+
     /**
      * proxy_range_request
      * @param \stored_file $file

classes/local/store/object_client_base.php

@@ -188,6 +188,10 @@ public function test_permissions($testdelete) {
         return (object)['success' => false, 'details' => ''];
     }
 
+	public function test_set_object_tag() {
+		return (object)['success' => false, 'details' => ''];
+	}
+
 	public function set_object_tags(string $contenthash, array $tags) {
 		return;
 	}

classes/local/store/s3/client.php

@@ -29,6 +29,7 @@
 use tool_objectfs\local\store\object_client_base;
 use tool_objectfs\local\store\signed_url;
 use local_aws\admin_settings_aws_region;
+use Throwable;
 
 define('AWS_API_VERSION', '2006-03-01');
 define('AWS_CAN_READ_OBJECT', 0);
@@ -390,6 +391,42 @@ public function test_permissions($testdelete) {
         return $permissions;
     }
 
+	public function test_set_object_tag() {
+		try {
+			// 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, 
+				'Body' => 'test content'
+			]);
+
+			// Next try to tag it.
+			$res = $this->client->putObjectTagging([
+				'Bucket' => $this->bucket,
+				'Key' => $key,
+				'Tagging' => [
+					'TagSet' => [
+						[
+							'Key' => 'test',
+							'Value' => 'test',
+						]
+					]
+				]
+			]);
+		} catch (Throwable $e) {
+			return (object) [
+				'success' => false,
+				'details' => $e->getMessage(),
+			];	
+		}
+
+		// Success - no exceptions thrown.
+		return (object) ['success' => true, 'details' => ''];
+	}
+
     /**
      * get_exception_details
      * @param \Exception $exception

classes/local/tag/tag_manager.php

@@ -32,7 +32,7 @@ public static function get_defined_tag_sources(): array {
 	 * Is the tagging feature enabled?
 	 * @return bool
 	 */
-	public static function is_tagging_enabled(): bool {
+	public static function is_tagging_enabled_and_supported(): bool {
 		$enabledinconfig = !empty(get_config('tool_objectfs', 'taggingenabled'));
 
 		$client = manager::get_client(manager::get_objectfs_config());
@@ -133,7 +133,7 @@ private static function store_tags(string $contenthash, array $tags) {
 			// We always mark them needing sync after changing the values.
 			self::mark_object_tag_status($contenthash, self::SYNC_STATUS_NEEDS_SYNC);
 		} catch (Exception $e) {
-			// Failed - release lock and return false.
+			// Failed - release lock and rethrow.
 			$lock->release();
 			throw $e;
 		}
@@ -166,6 +166,11 @@ public static function replicate_local_to_external_tags_for_object(string $conte
 
 			// Get current client.
 			$client = manager::get_client(manager::get_objectfs_config());
+
+			if (empty($client)) {
+				throw new coding_exception("No client set, cannot replicate tags"); // TODO moodle ex ?
+			}
+
 			$client->set_object_tags($contenthash, $tags);
 
 			// Mark as synced, this stops it from re-syncing in the future.

classes/local/tag/tag_source.php

@@ -6,15 +6,15 @@ interface tag_source {
 	/**
 	 * Returns an unchanging identifier for this source.
 	 * Must never change, otherwise it will lose connection with the tags replicated to objects.
-	 * If it ever must change, a migration step must be completed to trigger all objects to recalculate the value under the new tag name.
-	 * Must not exceed 128 chars. TODO unit test this.
+	 * If it ever must change, a migration step must be completed to trigger all objects to recalculate their tags.
+	 * Must not exceed 128 chars.
 	 * @return string
 	 */
 	public static function get_identifier(): 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. TODO unit test this.
+	 * This must be deterministic, and should never exceed 256 chars.
 	 * @param string $contenthash
 	 * @return string
 	 */

classes/task/queue_objects_needing_tags.php

@@ -12,7 +12,7 @@ public function get_name() {
 	}
 
 	public function execute() {
-		if (!tag_manager::is_tagging_enabled()) {
+		if (!tag_manager::is_tagging_enabled_and_supported()) {
 			mtrace("Tagging feature not available or supported, ignoring.");
 			return;
 		}

classes/task/update_object_tags.php

@@ -7,7 +7,7 @@
 
 class update_object_tags extends adhoc_task {
 	public function execute() {
-		if (!tag_manager::is_tagging_enabled()) {
+		if (!tag_manager::is_tagging_enabled_and_supported()) {
 			mtrace("Tagging feature not enabled or supported by filesystem, exiting early.");
 			return;
 		}

lib.php

@@ -24,6 +24,7 @@
  */
 
 use tool_objectfs\local\object_manipulator\manipulator_builder;
+use tool_objectfs\local\tag\tag_manager;
 
 define('OBJECTFS_PLUGIN_NAME', 'tool_objectfs');
 
@@ -120,11 +121,13 @@ function tool_objectfs_pluginfile($course, $cm, context $context, $filearea, arr
  * @return array
  */
 function tool_objectfs_status_checks() {
+	$checks = [
+		new tool_objectfs\check\tagging_status(),
+	];
+
     if (get_config('tool_objectfs', 'proxyrangerequests')) {
-        return [
-            new tool_objectfs\check\proxy_range_request()
-        ];
+		$checks[] = new tool_objectfs\check\proxy_range_request();
     }
 
-    return [];
+    return $checks;
 }

tests/local/tagging_test.php

@@ -40,7 +40,7 @@ public function test_tag_sources_identifier(tag_source $source) {
 		$this->assertGreaterThan(0, $count);
 	}
 
-	public static function is_tagging_enabled_provider(): array {
+	public static function is_tagging_enabled_and_supported_provider(): array {
 		return [
 			'neither config nor fs supports' => [
 				'enabledinconfig' => false,
@@ -61,9 +61,9 @@ public static function is_tagging_enabled_provider(): array {
 	}
 
 	/**
-	 * @dataProvider is_tagging_enabled_provider
+	 * @dataProvider is_tagging_enabled_and_supported_provider
 	 */
-	public function test_is_tagging_enabled(bool $enabledinconfig, bool $supportedbyfs, bool $expected) {
+	public function test_is_tagging_enabled_and_supported(bool $enabledinconfig, bool $supportedbyfs, bool $expected) {
 		global $CFG;
 		// Set config.
 		set_config('taggingenabled', $enabledinconfig, 'tool_objectfs');
@@ -76,7 +76,7 @@ public function test_is_tagging_enabled(bool $enabledinconfig, bool $supportedby
         manager::set_objectfs_config($config);
 		$CFG->phpunit_objectfs_supports_object_tagging = $supportedbyfs;	
 
-		$this->assertEquals($expected, tag_manager::is_tagging_enabled());
+		$this->assertEquals($expected, tag_manager::is_tagging_enabled_and_supported());
 	}
 
 	public function test_query_local_tags() {
@@ -210,4 +210,24 @@ public function test_get_objects_needing_sync_limit() {
 		$this->assertCount(2, tag_manager::get_objects_needing_sync(2));
 		$this->assertCount(1, tag_manager::get_objects_needing_sync(1));
 	}
+	
+	public function test_replicate_local_to_external_tags_for_object() {
+		global $DB;
+
+		// Enable tagging, setup test fs.
+        $config = manager::get_objectfs_config();
+		$config->taggingenabled = true;
+        $config->enabletasks = true;
+        $config->filesystem = '\\tool_objectfs\\tests\\test_file_system';
+        manager::set_objectfs_config($config);
+
+		// Setup object, mark as needing sync.
+		$object = $this->create_remote_object();
+		$DB->set_field('tool_objectfs_objects', 'tagsyncstatus', tag_manager::SYNC_STATUS_NEEDS_SYNC, ['id' => $object->id]);
+		tag_manager::replicate_local_to_external_tags_for_object($object->contenthash);
+
+		// Ensure status is now set as not needing sync.
+		$status = $DB->get_field('tool_objectfs_objects', 'tagsyncstatus', ['id' => $object->id]);
+		$this->assertEquals(tag_manager::SYNC_STATUS_SYNC_NOT_REQUIRED, $status);
+	}
 }