Skip to content

Commit dfe006f

Browse files
matthewhiltonmatthewhilton
committed
WIP more tagging implementation
1 parent e5b94bf commit dfe006f

17 files changed

+274
-87
lines changed

‎classes/local/store/object_client.php

+4
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ public function proxy_range_request(\stored_file $file, $ranges);
137137
*/
138138
public function test_range_request($filesystem);
139139

140+
public function set_object_tags(string $contenthash, array $tags);
141+
142+
public function supports_object_tagging(): bool;
143+
140144
}
141145

142146

‎classes/local/store/object_client_base.php

+8
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,12 @@ public function test_connection() {
187187
public function test_permissions($testdelete) {
188188
return (object)['success' => false, 'details' => ''];
189189
}
190+
191+
public function set_object_tags(string $contenthash, array $tags) {
192+
return;
193+
}
194+
195+
public function supports_object_tagging(): bool {
196+
return false;
197+
}
190198
}

‎classes/local/store/object_file_system.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use file_storage;
3838
use BlobRestProxy;
3939
use tool_objectfs\local\manager;
40+
use tool_objectfs\local\tag\tag_manager;
4041

4142
defined('MOODLE_INTERNAL') || die();
4243

@@ -352,7 +353,6 @@ public function copy_object_from_local_to_external_by_hash($contenthash, $object
352353
$finallocation = $initiallocation;
353354

354355
if ($initiallocation === OBJECT_LOCATION_LOCAL) {
355-
356356
$success = $this->copy_from_local_to_external($contenthash);
357357

358358
if ($success) {
@@ -1154,4 +1154,8 @@ private function update_object(array $result): array {
11541154

11551155
return $result;
11561156
}
1157+
1158+
public function update_external_object_tags($contenthash, $tags) {
1159+
return;
1160+
}
11571161
}

‎classes/local/store/s3/client.php

+27
Original file line numberDiff line numberDiff line change
@@ -866,4 +866,31 @@ public function test_range_request($filesystem) {
866866
}
867867
return (object)['result' => false, 'error' => get_string('fixturefilemissing', 'tool_objectfs')];
868868
}
869+
870+
public function set_object_tags(string $contenthash, array $tags) {
871+
// First convert PHP array tags to XML string.
872+
$s3tags = [];
873+
874+
foreach($tags as $key => $value) {
875+
$s3tags[] = [
876+
'Key' => $key,
877+
'Value' => $value,
878+
];
879+
}
880+
881+
$key = $this->bucketkeyprefix . $this->get_filepath_from_hash($contenthash);
882+
883+
// Then put onto object.
884+
$this->client->putObjectTagging([
885+
'Bucket' => $this->bucket,
886+
'Key' => $key,
887+
'Tagging' => [
888+
'TagSet' => $s3tags
889+
]
890+
]);
891+
}
892+
893+
public function supports_object_tagging(): bool {
894+
return true;
895+
}
869896
}

‎classes/local/store/s3/file_system.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public function copy_from_local_to_external($contenthash) {
9696
return false;
9797
}
9898
}
99-
99+
100100
/**
101101
* supports_presigned_urls
102102
* @return bool

‎classes/local/tag/file_type_source.php

-7
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
namespace tool_objectfs\local\tag;
44

5-
use stored_file;
6-
75
class file_type_source implements tag_source {
86
public const TYPE_STANDARD = 'standard';
97

@@ -13,11 +11,6 @@ public static function get_identifer(): string {
1311
return 'type';
1412
}
1513

16-
public function get_value_for_stored_file(stored_file $file): ?string {
17-
// TODO implement types e.g. is backup, etc..
18-
return self::TYPE_STANDARD;
19-
}
20-
2114
public function get_value_for_contenthash(string $contenthash): ?string {
2215
// TODO implement types e.g. is backup, etc..
2316
return self::TYPE_STANDARD;

‎classes/local/tag/tag_manager.php

+130-64
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,20 @@
22

33
namespace tool_objectfs\local\tag;
44

5-
use stored_file;
5+
use coding_exception;
6+
use core\lock\lock_config;
7+
use Exception;
8+
use tool_objectfs\local\manager;
9+
10+
defined('MOODLE_INTERNAL') || die();
11+
require_once($CFG->dirroot . '/admin/tool/objectfs/lib.php');
612

713
class tag_manager {
814

15+
public const SYNC_STATUS_NEEDS_SYNC = 0;
16+
17+
public const SYNC_STATUS_SYNC_NOT_REQUIRED = 1;
18+
919
/**
1020
* Returns an array of tag_source instances that are currently defined.
1121
* @return array
@@ -23,106 +33,162 @@ public static function get_defined_tag_sources(): array {
2333
* @return bool
2434
*/
2535
public static function is_tagging_enabled(): bool {
26-
return false; // TODO config.
36+
$enabledinconfig = !empty(get_config('tool_objectfs', 'taggingenabled'));
37+
$client = manager::get_client(manager::get_objectfs_config());
38+
$supportedbyfs = $client->supports_object_tagging();
39+
40+
return $enabledinconfig && $supportedbyfs;
2741
}
2842

2943
/**
30-
* Gathers the tags for a given content hash
44+
* Gathers the tag values for a given content hash
3145
* @param string $contenthash
3246
* @return array array of key=>value pairs, the tags for the given file.
3347
*/
34-
private function gather_tags(string $contenthash): array {
48+
private static function gather_tags(string $contenthash): array {
3549
$tags = [];
36-
foreach(self::get_defined_tag_sources() as $source) {
37-
$tags[$source->get_identifer()] = $source->get_value_for_contenthash($contenthash);
50+
foreach (self::get_defined_tag_sources() as $source) {
51+
$val = $source->get_value_for_contenthash($contenthash);
52+
53+
// Null means not set for this object.
54+
if (is_null($val)) {
55+
continue;
56+
}
57+
58+
$tags[$source->get_identifer()] = $val;
3859
}
3960
return $tags;
4061
}
4162

4263
/**
4364
* Returns the tags stored locally in the DB for the given file contenthash
44-
* @param string $contenthash
65+
* @param int $objectid
4566
* @return array array of key=>value pairs, the tags for the given file that are stored in the database.
4667
*/
47-
private function query_local_tags(string $contenthash): array {
48-
// TODO.
49-
return [];
50-
}
68+
public static function query_local_tags(string $contenthash): array {
69+
global $DB;
70+
$records = $DB->get_records('tool_objectfs_object_tags', ['contenthash' => $contenthash], '', 'tagkey,tagvalue');
5171

52-
/**
53-
* Returns the tags attached to an object in the external storage.
54-
* @param string $contenthash
55-
* @return array array of key=>value pairs, the tags for the given file that are attached to the external object.
56-
*/
57-
private function query_external_tags(string $contenthash): array {
58-
// TODO.
59-
return [];
72+
$tags = [];
73+
foreach ($records as $record) {
74+
$tags[$record->tagkey] = $record->tagvalue;
75+
}
76+
return $tags;
6077
}
6178

6279
/**
6380
* Synchronises the tags for a given file.
6481
*
6582
* @param string $contenthash file to update tags for
66-
* @param bool $force By default, will not update stored tags unless the gathered tags differ from the locally stored tags.
67-
* Setting this to true will force it to always update external object tags.
83+
* @param bool $force By default, will not update stored tags unless the gathered tags differ from the locally stored tags. If this is true, it will always store the newest tags.
6884
*/
69-
public function sync_tags(string $contenthash, bool $force = false) {
70-
// Gather local and compare against local db.
71-
$gatheredtags = $this->gather_tags($contenthash);
72-
$localtags = $this->query_local_tags($contenthash);
85+
public static function update_tags_if_necessary(string $contenthash, bool $force = false) {
86+
// Gather the exact current and compare against local db.
87+
$gatheredtags = self::gather_tags($contenthash);
88+
$localtags = self::query_local_tags($contenthash);
7389

7490
// Update if forced to, or if what we just gathered is not the same as what is stored in the database.
75-
$shouldupdate = $force || $this->are_tags_different($gatheredtags, $localtags);
91+
$shouldupdate = $force || self::are_tags_different($gatheredtags, $localtags);
7692

7793
if ($shouldupdate) {
78-
$this->store_tags($contenthash, $gatheredtags);
94+
self::store_tags($contenthash, $gatheredtags);
7995
}
8096
}
8197

8298
/**
8399
* Updates the tags for a given file. Updates both local db and external.
84-
* @param string $contenthash
100+
* @param int $objectid
85101
* @param array $tags
86-
* @param bool $updateexternal If the external tags should be updated.
87-
* Unless you are uploading the file (and uploading tags along with it), this should always be true to keep the local and external values in sync.
88102
*/
89-
private function store_tags(string $contenthash, array $tags, bool $updateexternal = true) {
103+
private static function store_tags(string $contenthash, array $tags) {
104+
global $DB;
105+
106+
// Get a lock, since we are modifying the tags in the DB.
107+
$lock = self::get_object_tag_lock($contenthash);
108+
109+
if (!$lock) {
110+
throw new coding_exception("Unable to obtain lock for object " . $contenthash); // TODO different ex type ?
111+
}
112+
113+
try {
114+
// Purge any existing tags for this object.
115+
$DB->delete_records('tool_objectfs_object_tags', ['contenthash' => $contenthash]);
116+
117+
// Record time in var, so that they all have the same time.
118+
$timemodified = time();
119+
120+
// Store new records.
121+
$recordstostore = [];
122+
foreach ($tags as $key => $value) {
123+
$recordstostore[] = [
124+
'contenthash' => $contenthash,
125+
'tagkey' => $key,
126+
'tagvalue' => $value,
127+
'timemodified' => $timemodified
128+
];
129+
}
130+
$DB->insert_records('tool_objectfs_object_tags', $recordstostore);
131+
132+
// We always mark them needing sync after changing the values.
133+
self::mark_object_tag_status($contenthash, self::SYNC_STATUS_NEEDS_SYNC);
134+
} catch (Exception $e) {
135+
// Failed - release lock and return false.
136+
$lock->release();
137+
throw $e;
138+
}
139+
}
140+
141+
public static function get_objects_needing_sync(int $limit) {
90142
global $DB;
91-
// TODO store locally.
92-
// TODO store externally.
143+
144+
// Find object records where the status is NEEDS_SYNC and is replicated.
145+
[$insql, $inparams] = $DB->get_in_or_equal([OBJECT_LOCATION_DUPLICATED, OBJECT_LOCATION_EXTERNAL, OBJECT_LOCATION_ORPHANED], SQL_PARAMS_NAMED);
146+
$inparams['syncstatus'] = self::SYNC_STATUS_NEEDS_SYNC;
147+
148+
$records = $DB->get_records_select('tool_objectfs_objects', 'tagsyncstatus = :syncstatus AND location ' . $insql, $inparams, '', 'contenthash', 0, $limit);
149+
return array_column($records, 'contenthash');
93150
}
94151

95-
private function are_tags_different(array $a, array $b) {
96-
return true; // TODO compare the keys and values to ensure they are exactly the same!
152+
public static function replicate_local_to_external_tags_for_object(string $contenthash) {
153+
global $DB;
154+
155+
// Take lock, to ensure tags are not updated while we sync them.
156+
$lock = self::get_object_tag_lock($contenthash);
157+
158+
if (!$lock) {
159+
throw new coding_exception("Unable to obtain lock for object " . $contenthash); // TODO different ex type ?
160+
}
161+
162+
try {
163+
164+
// TODO refactor so all func use contenthash, no objectid.
165+
// Then also probably remove from DB.
166+
$tags = self::query_local_tags($contenthash);
167+
168+
// Get current client.
169+
$client = manager::get_client(manager::get_objectfs_config());
170+
$client->set_object_tags($contenthash, $tags);
171+
172+
// Mark as synced, this stops it from re-syncing in the future.
173+
self::mark_object_tag_status($contenthash, self::SYNC_STATUS_SYNC_NOT_REQUIRED);
174+
} catch (Exception $e) {
175+
$lock->release();
176+
throw $e;
177+
}
178+
}
179+
180+
private static function get_object_tag_lock(string $contenthash, int $timeout = 5) {
181+
$factory = lock_config::get_lock_factory('tool_objectfs_object_tags');
182+
return $factory->get_lock($contenthash, $timeout);
97183
}
98184

99-
// private function query_tags_for_contenthash(string $contenthash): array {
100-
// // TODO query from the db, the current db values.
101-
// }
102-
103-
// public function update_object_tags_by_contenthash(string $contenthash, bool $skipifsame = true) {
104-
// $shouldupdateexternal = true;
105-
// $tags = $this->build_tags_for_contenthash($contenthash);
106-
107-
108-
// foreach ($tags as $key => $value) {
109-
// $this->upsert_tag_value($contenthash, $key, $value);
110-
// }
111-
// $this->store_local_tags_for_contenthash($contenthash);
112-
// }
113-
114-
// private function store_local_tags_for_contenthash(string $contenthash) {
115-
// }
116-
117-
// private function upsert_tag_value(string $contenthash, string $tagkey, string $tagvalue) {
118-
// // TODO upsert this in the DB.
119-
// global $DB;
120-
// // TODO.
121-
// }
122-
123-
// public function sync_external_tags_by_contenthash(string $contenthash) {
124-
// // TODO call external api to sync.
125-
// global $DB;
126-
// $tags =
127-
// }
185+
private static function mark_object_tag_status(string $contenthash, int $status) {
186+
global $DB;
187+
// TODO ensure valid status.
188+
$DB->set_field('tool_objectfs_objects', 'tagsyncstatus', $status, ['contenthash' => $contenthash]);
189+
}
190+
191+
private static function are_tags_different(array $a, array $b) {
192+
return true; // TODO compare the keys and values to ensure they are exactly the same!
193+
}
128194
}

‎classes/local/tag/tag_source.php

-8
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,6 @@ interface tag_source {
1414
*/
1515
public static function get_identifer(): string;
1616

17-
/**
18-
* Returns the value of this tag for the given stored file.
19-
* This must be deterministic, and should never exceed 256 chars. TODO unit test this.
20-
* @param stored_file $file
21-
* @return string
22-
*/
23-
public function get_value_for_stored_file(stored_file $file): ?string;
24-
2517
/**
2618
* Returns the value of this tag for the file with the given content hash.
2719
* This must be deterministic, and should never exceed 256 chars. TODO unit test this.

0 commit comments

Comments
 (0)