diff --git a/classes/check/tagging_status.php b/classes/check/tagging_status.php index bfdacbd9..83e375db 100644 --- a/classes/check/tagging_status.php +++ b/classes/check/tagging_status.php @@ -29,7 +29,15 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class tagging_status extends check { - // TODO action link. + /** + * Link to ObjectFS settings page. + * + * @return \action_link|null + */ + public function get_action_link(): ?\action_link { + $url = new \moodle_url('/admin/category.php', ['category' => 'tool_objectfs']); + return new \action_link($url, get_string('pluginname', 'tool_objectfs')); + } /** * Get result @@ -46,11 +54,9 @@ public function get_result(): result { $result = $client->test_set_object_tag(); if ($result->success) { - // TODO lang. - return new result(result::OK, 'tagging success', $result->details); + return new result(result::OK, get_string('check:tagging:ok', 'tool_objectfs'), $result->details); } else { - // TODO lang. - return new result(result::ERROR, 'tagging failed', $result->details); + return new result(result::ERROR, get_string('check:tagging:error', 'tool_objectfs'), $result->details); } } } diff --git a/classes/local/store/s3/client.php b/classes/local/store/s3/client.php index 704be023..1e72e8a3 100644 --- a/classes/local/store/s3/client.php +++ b/classes/local/store/s3/client.php @@ -916,9 +916,8 @@ public function test_set_object_tag(): stdClass { * @param array $tags array of key=>value pairs to set as tags. */ public function set_object_tags(string $contenthash, array $tags) { - // First convert PHP array tags to XML string. + // Convert tags into format S3 client expects. $s3tags = []; - foreach($tags as $key => $value) { $s3tags[] = [ 'Key' => $key, diff --git a/classes/local/tag/tag_manager.php b/classes/local/tag/tag_manager.php index 1f89ec99..47db73bb 100644 --- a/classes/local/tag/tag_manager.php +++ b/classes/local/tag/tag_manager.php @@ -45,6 +45,9 @@ class tag_manager { */ public const SYNC_STATUS_SYNC_NOT_REQUIRED = 1; + /** + * @var array All possible tag sync statuses. + */ public const SYNC_STATUSES = [ self::SYNC_STATUS_NEEDS_SYNC, self::SYNC_STATUS_SYNC_NOT_REQUIRED @@ -131,7 +134,7 @@ public static function update_tags_if_necessary(string $contenthash, bool $force } /** - * Updates the tags for a given file. Updates both local db and external. + * Updates the local db tag records, and marks object as needing sync to sync local tags to external. * @param int $objectid * @param array $tags */ @@ -202,7 +205,6 @@ public static function replicate_local_to_external_tags_for_object(string $conte } try { - // TODO refactor so all func use contenthash, no objectid. // Then also probably remove from DB. $tags = self::query_local_tags($contenthash); @@ -249,12 +251,15 @@ private static function mark_object_tag_sync_status(string $contenthash, int $st } /** - * Returns true if the tags given are different - * @param array $a - * @param array $b + * Returns true if the tags given are different. They are different when their keys or values do not match, order does not matter. + * @param array $a array of key=>value where keys and values are both strings + * @param array $b array of key=>value where keys and values are both strings * @return bool */ private static function are_tags_different(array $a, array $b): bool { + // Order the keys in both arrays, then do a simple equality check. + ksort($a); + ksort($b); return $a !== $b; } } diff --git a/classes/task/queue_objects_needing_tags.php b/classes/task/queue_objects_needing_tags.php index 597d5dfb..e1704181 100644 --- a/classes/task/queue_objects_needing_tags.php +++ b/classes/task/queue_objects_needing_tags.php @@ -46,8 +46,8 @@ public function execute() { return; } - // TODO configurable limit per run. - $contenthashes = tag_manager::get_objects_needing_sync(1000); + $limit = get_config('tool_objectfs', 'maxtaggingperrun'); + $contenthashes = tag_manager::get_objects_needing_sync($limit); mtrace("Found " . count($contenthashes) . " objects needing tag sync"); foreach ($contenthashes as $contenthash) { diff --git a/classes/task/update_object_tags.php b/classes/task/update_object_tags.php index b4119b4d..5f3117ce 100644 --- a/classes/task/update_object_tags.php +++ b/classes/task/update_object_tags.php @@ -33,7 +33,7 @@ class update_object_tags extends adhoc_task { */ public function execute() { if (!tag_manager::is_tagging_enabled_and_supported()) { - mtrace("Tagging feature not enabled or supported by filesystem, exiting early."); + mtrace("Tagging feature not enabled or supported by filesystem, exiting."); return; } @@ -41,12 +41,14 @@ public function execute() { $contenthash = $this->get_custom_data()->contenthash ?? null; if (empty($contenthash)) { - mtrace("No content hash given, invalid customdata, exiting"); + mtrace("No contenthash given, invalid customdata, exiting."); return; } + // Update local tags in db. tag_manager::update_tags_if_necessary($contenthash); - // TODO if this fails because of 404, just ignore since file is clearly not replicated yet + + // Sync these to external store. tag_manager::replicate_local_to_external_tags_for_object($contenthash); } } diff --git a/db/upgrade.php b/db/upgrade.php index f9126580..76476360 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -171,7 +171,41 @@ function xmldb_tool_objectfs_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2023013100, 'tool', 'objectfs'); } - // TODO db update step for db. + if ($oldversion < 2023051704) { + + // Define table tool_objectfs_object_tags to be created. + $table = new xmldb_table('tool_objectfs_object_tags'); + + // Adding fields to table tool_objectfs_object_tags. + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('contenthash', XMLDB_TYPE_CHAR, '40', null, XMLDB_NOTNULL, null, null); + $table->add_field('tagkey', XMLDB_TYPE_CHAR, '15', null, XMLDB_NOTNULL, null, null); + $table->add_field('tagvalue', XMLDB_TYPE_CHAR, '20', null, XMLDB_NOTNULL, null, null); + $table->add_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null); + + // Adding keys to table tool_objectfs_object_tags. + $table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']); + + // Adding indexes to table tool_objectfs_object_tags. + $table->add_index('objecttagkey_idx', XMLDB_INDEX_UNIQUE, ['contenthash', 'tagkey']); + + // Conditionally launch create table for tool_objectfs_object_tags. + if (!$dbman->table_exists($table)) { + $dbman->create_table($table); + } + + // Define field tagsyncstatus to be added to tool_objectfs_objects. + $table = new xmldb_table('tool_objectfs_objects'); + $field = new xmldb_field('tagsyncstatus', XMLDB_TYPE_INTEGER, '2', null, XMLDB_NOTNULL, null, '0', 'filesize'); + + // Conditionally launch add field tagsyncstatus. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Objectfs savepoint reached. + upgrade_plugin_savepoint(true, 2023051704, 'tool', 'objectfs'); + } return true; } diff --git a/lang/en/tool_objectfs.php b/lang/en/tool_objectfs.php index 4437600b..bddd08ad 100644 --- a/lang/en/tool_objectfs.php +++ b/lang/en/tool_objectfs.php @@ -274,3 +274,8 @@ $string['checkproxy_range_request'] = 'Pre-signed URL range request proxy'; $string['task:queueobjectsneedingtags'] = 'Queue objects needing tagging'; +$string['checktagging_status'] = 'Object tagging'; +$string['check:tagging:ok'] = 'Object tagging ok'; +$string['check:tagging:error'] = 'Error trying to tag object'; +$string['settings:maxtaggingperrun'] = 'Max objects tagged per run'; +$string['settings:maxtaggingperrun:desc'] = 'The maximum number of objects to be queued for tag updates per run of the queue_objects_needing_tags scheduled task.'; \ No newline at end of file diff --git a/settings.php b/settings.php index d864fcf5..7b0f358a 100644 --- a/settings.php +++ b/settings.php @@ -250,6 +250,7 @@ $settings->add(new admin_setting_configcheckbox('tool_objectfs/preferexternal', new lang_string('settings:preferexternal', 'tool_objectfs'), '', '')); + // Tagging settings. $settings->add(new admin_setting_heading('tool_objectfs/taggingsettings', new lang_string('settings:taggingheader', 'tool_objectfs'), '')); @@ -257,4 +258,10 @@ $settings->add(new admin_setting_configcheckbox('tool_objectfs/taggingenabled', new lang_string('settings:taggingenabled', 'tool_objectfs'), '', '')); + + $settings->add(new admin_setting_configtext('tool_objectfs/maxtaggingperrun', + new lang_string('settings:maxtaggingperrun', 'tool_objectfs'), + get_string('settings:maxtaggingperrun:desc', 'tool_objectfs'), + 10000, + PARAM_INT)); } diff --git a/tests/local/tagging_test.php b/tests/local/tagging_test.php index 03555df5..00fd8ce9 100644 --- a/tests/local/tagging_test.php +++ b/tests/local/tagging_test.php @@ -84,6 +84,9 @@ public function test_tag_sources_value(tag_source $source) { $this->assertGreaterThan(0, $count); } + // TODO ensure all sources have unique keys. + // TODO ensure keys and values are all strings. + /** * Provides values to test_is_tagging_enabled_and_supported * @return array