Skip to content

Commit 8e1b1cd

Browse files
matthewhiltonmatthewhilton
committed
WIP big refactor of tagging to support multi env
1 parent 29fefe0 commit 8e1b1cd

10 files changed

+172
-148
lines changed

‎classes/local/manager.php

+1
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ public static function get_objectfs_config() {
7979
$config->s3_bucket = '';
8080
$config->s3_region = 'us-east-1';
8181
$config->s3_base_url = '';
82+
$config->s3_canoverrideobjects = true;
8283
$config->key_prefix = '';
8384

8485
// Digital ocean file system.

‎classes/local/store/object_client.php

+7
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ public function test_set_object_tag(): stdClass;
152152
*/
153153
public function set_object_tags(string $contenthash, array $tags);
154154

155+
/**
156+
* Returns given objects tags queried from the external store. If object does not exist in store, return null.
157+
* @param string $contenthash file content has
158+
* @param array array of key=>value tag pairs, or null if object does not exist.
159+
*/
160+
public function get_object_tags(string $contenthash): ?array;
161+
155162
/**
156163
* If the client supports object tagging feature.
157164
* @return bool true if supports, else false

‎classes/local/store/object_client_base.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,16 @@ public function test_set_object_tag(): stdClass {
204204
* @param array $tags array of key=>value pairs to set as tags.
205205
*/
206206
public function set_object_tags(string $contenthash, array $tags) {
207-
return;
207+
return [];
208+
}
209+
210+
/**
211+
* Returns given objects tags queried from the external store. If object does not exist in store, return null.
212+
* @param string $contenthash file content has
213+
* @param array array of key=>value tag pairs, or null if object does not exist.
214+
*/
215+
public function get_object_tags(string $contenthash): ?array {
216+
return [];
208217
}
209218

210219
/**

‎classes/local/store/object_file_system.php

+3
Original file line numberDiff line numberDiff line change
@@ -1155,4 +1155,7 @@ private function update_object(array $result): array {
11551155

11561156
return $result;
11571157
}
1158+
1159+
public function sync_tags(string $contenthash) {
1160+
}
11581161
}

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

+80-7
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ class client extends object_client_base {
6262
*/
6363
private $signingmethod;
6464

65+
protected $bucketkeyprefix;
66+
67+
protected $allowoverride;
68+
6569
/**
6670
* construct
6771
* @param mixed $config
@@ -82,6 +86,7 @@ public function __construct($config) {
8286
$this->enablepresignedurls = $config->enablepresignedurls;
8387
$this->signingmethod = $config->signingmethod;
8488
$this->bucketkeyprefix = $config->key_prefix;
89+
$this->allowoverride = $config->s3_allowoverride;
8590
$this->set_client($config);
8691
} else {
8792
parent::__construct($config);
@@ -483,6 +488,10 @@ public function define_client_section($settings, $config) {
483488
$settings->add(new \admin_setting_configtext('tool_objectfs/key_prefix',
484489
new \lang_string('settings:aws:key_prefix', 'tool_objectfs'),
485490
new \lang_string('settings:aws:key_prefix_help', 'tool_objectfs'), ''));
491+
492+
$settings->add(new \admin_setting_configcheckbox('tool_objectfs/s3_overrideobjects',
493+
new \lang_string('settings:aws:overrideobjects', 'tool_objectfs'),
494+
new \lang_string('settings:aws:overrideobjects_help', 'tool_objectfs'), 0));
486495

487496
return $settings;
488497
}
@@ -492,10 +501,11 @@ public function define_client_section($settings, $config) {
492501
*
493502
* @param string $localpath Path to a local file.
494503
* @param string $contenthash Content hash of the file.
504+
* @param array $tags array of tags to set. If tagging is not enabled, this will simply be empty.
495505
*
496506
* @throws \Exception if fails.
497507
*/
498-
public function upload_to_s3($localpath, $contenthash) {
508+
public function upload_to_s3($localpath, $contenthash, array $tags) {
499509
$filehandle = fopen($localpath, 'rb');
500510

501511
if (!$filehandle) {
@@ -504,10 +514,13 @@ public function upload_to_s3($localpath, $contenthash) {
504514

505515
try {
506516
$externalpath = $this->get_filepath_from_hash($contenthash);
517+
518+
// TODO upload with tags.
507519
$uploader = new \Aws\S3\ObjectUploader(
508520
$this->client, $this->bucket,
509521
$this->bucketkeyprefix . $externalpath,
510-
$filehandle
522+
$filehandle,
523+
'private',
511524
);
512525
$uploader->upload();
513526
fclose($filehandle);
@@ -910,13 +923,22 @@ public function test_set_object_tag(): stdClass {
910923
return (object) ['success' => true, 'details' => ''];
911924
}
912925

926+
private function convert_array_to_s3_tag_format(array $tags): array {
927+
928+
return [
929+
'TagSet' => $s3tags
930+
];
931+
}
932+
913933
/**
914934
* Set the given objects tags in the external store.
915935
* @param string $contenthash file content hash
916936
* @param array $tags array of key=>value pairs to set as tags.
917937
*/
918938
public function set_object_tags(string $contenthash, array $tags) {
919-
// Convert tags into format S3 client expects.
939+
$key = $this->bucketkeyprefix . $this->get_filepath_from_hash($contenthash);
940+
941+
// Convert to S3 format.
920942
$s3tags = [];
921943
foreach($tags as $key => $value) {
922944
$s3tags[] = [
@@ -925,16 +947,37 @@ public function set_object_tags(string $contenthash, array $tags) {
925947
];
926948
}
927949

928-
$key = $this->bucketkeyprefix . $this->get_filepath_from_hash($contenthash);
929-
930950
// Then put onto object.
931951
$this->client->putObjectTagging([
932952
'Bucket' => $this->bucket,
933953
'Key' => $key,
934954
'Tagging' => [
935-
'TagSet' => $s3tags
936-
]
955+
'TagSet' => $s3tags,
956+
],
957+
]);
958+
}
959+
960+
/**
961+
* Returns given objects tags queried from the external store. Object must exist.
962+
* @param string $contenthash file content has
963+
* @param array array of key=>value tag pairs
964+
*/
965+
public function get_object_tags(string $contenthash): array {
966+
$key = $this->bucketkeyprefix . $this->get_filepath_from_hash($contenthash);
967+
968+
// Query from S3.
969+
$tags = $this->client->getObjectTagging([
970+
'Bucket' => $this->bucket,
971+
'Key' => $key,
937972
]);
973+
974+
// Convert from S3 format to key=>value format.
975+
$tagkv = [];
976+
foreach($tags['Tagging']['TagSet'] as $tag) {
977+
$tagkv[$tag['Key']] = $tag['Value'];
978+
}
979+
980+
return $tagkv;
938981
}
939982

940983
/**
@@ -944,4 +987,34 @@ public function set_object_tags(string $contenthash, array $tags) {
944987
public function supports_object_tagging(): bool {
945988
return true;
946989
}
990+
991+
// TODO maybe this moves to 'global' objectfs config ?
992+
public function can_override_objects(): bool {
993+
return $this->allowoverride;
994+
}
995+
996+
public function object_exists(string $contenthash): bool {
997+
$key = $this->bucketkeyprefix . $this->get_filepath_from_hash($contenthash);
998+
999+
// Query from S3.
1000+
try {
1001+
$this->client->headObject([
1002+
'Bucket' => $this->bucket,
1003+
'Key' => $key,
1004+
]);
1005+
1006+
// Exists, headObject succeeded.
1007+
return true;
1008+
} catch (\AWS\Exception\AwsException $e) {
1009+
$is404 = false; // TOOD determine if was 404, i.e. missing object
1010+
1011+
// Object does not exist.
1012+
if($is404) {
1013+
return false;
1014+
}
1015+
1016+
// Other error - rethrow.
1017+
throw $e;
1018+
}
1019+
}
9471020
}

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

+20-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
use tool_objectfs\local\manager;
3434
use tool_objectfs\local\store\object_file_system;
35+
use tool_objectfs\local\tag\tag_manager;
3536

3637
require_once($CFG->dirroot . '/admin/tool/objectfs/lib.php');
3738

@@ -87,7 +88,24 @@ public function copy_from_local_to_external($contenthash) {
8788
$localpath = $this->get_local_path_from_hash($contenthash);
8889

8990
try {
90-
$this->get_external_client()->upload_to_s3($localpath, $contenthash);
91+
// If can't override, check if it exists already, since upload object will override.
92+
if (!manager::can_override_existing_objects() && $this->get_external_client()->object_exists($contenthash)) {
93+
// TODO somehow log it was skipped here?
94+
return true;
95+
}
96+
97+
// Gather tags (if enabled).
98+
$taggingenabled = tag_manager::is_tagging_enabled_and_supported();
99+
$tags = $taggingenabled ? tag_manager::gather_object_tags_for_upload($contenthash) : [];
100+
101+
$this->get_external_client()->upload_to_s3($localpath, $contenthash, $tags);
102+
103+
// Tag post-upload actions (if enabled).
104+
if ($taggingenabled) {
105+
tag_manager::store_tags_locally($contenthash, $tags);
106+
tag_manager::mark_object_tag_sync_status($contenthash, tag_manager::SYNC_STATUS_SYNC_NOT_REQUIRED);
107+
}
108+
91109
return true;
92110
} catch (\Exception $e) {
93111
$this->get_logger()->error_log(
@@ -104,4 +122,5 @@ public function copy_from_local_to_external($contenthash) {
104122
public function supports_presigned_urls() {
105123
return true;
106124
}
125+
107126
}

‎classes/local/tag/tag_manager.php

+31-72
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public static function can_override_existing_tags(): bool {
8888
* @param string $contenthash
8989
* @return array array of key=>value pairs, the tags for the given file.
9090
*/
91-
private static function gather_tags(string $contenthash): array {
91+
public static function gather_object_tags_for_upload(string $contenthash): array {
9292
$tags = [];
9393
foreach (self::get_defined_tag_sources() as $source) {
9494
$val = $source->get_value_for_contenthash($contenthash);
@@ -108,7 +108,7 @@ private static function gather_tags(string $contenthash): array {
108108
* @param int $objectid
109109
* @param array $tags
110110
*/
111-
private static function store_tags(string $contenthash, array $tags) {
111+
public static function store_tags_locally(string $contenthash, array $tags) {
112112
global $DB;
113113

114114
// Purge any existing tags for this object.
@@ -148,86 +148,45 @@ public static function get_objects_needing_sync(int $limit) {
148148
}
149149

150150
/**
151-
* Gathers the tag values for a given object contenthash, stores the values in the DB and then replicates these to the external store.
152-
* Will mark the object sync status to sync status not required.
151+
* Marks a given object as the given status.
153152
* @param string $contenthash
153+
* @param int $status one of SYNC_STATUS_* constants
154154
*/
155-
public static function calculate_tags_store_locally_and_replicate_to_external(string $contenthash) {
156-
// TODO someway to genericise this, so we can call it with and without replicating ,etc...
157-
158-
// Take lock, to ensure tags are not updated while we sync them.
159-
$lock = self::get_object_tag_lock($contenthash);
160-
161-
if (!$lock) {
162-
throw new coding_exception("Unable to obtain lock for object " . $contenthash); // TODO different ex type ?
155+
public static function mark_object_tag_sync_status(string $contenthash, int $status) {
156+
global $DB;
157+
if (!in_array($status, self::SYNC_STATUSES)) {
158+
throw new coding_exception("Invalid object tag sync status " . $status);
163159
}
160+
$DB->set_field('tool_objectfs_objects', 'tagsyncstatus', $status, ['contenthash' => $contenthash]);
161+
}
164162

165-
try {
166-
// Get current client, ensure it is configured correctly.
167-
$client = manager::get_client(manager::get_objectfs_config());
168-
169-
if (empty($client)) {
170-
throw new coding_exception("No client set, cannot replicate tags"); // TODO moodle ex ?
171-
}
163+
public static function sync_object_tags_post_upload(string $contenthash) {
164+
// First get object lock
165+
// TODO
172166

173-
// Sanity check, ensure supports object tagging.
174-
if (!$client->supports_object_tagging()) {
175-
throw new coding_exception("Client does not support object tagging"); // TODO different ex ?
176-
}
167+
$client = manager::get_client(manager::get_objectfs_config());
177168

178-
// Gather tags from sources & store locally.
179-
$gatheredtags = self::gather_tags($contenthash);
180-
self::store_tags($contenthash, $gatheredtags);
181-
182-
// Replicate to external.
183-
$client->set_object_tags($contenthash, $gatheredtags);
184-
185-
// Mark as synced, this stops it from re-syncing in the future.
186-
self::mark_object_tag_sync_status($contenthash, self::SYNC_STATUS_SYNC_NOT_REQUIRED);
187-
} catch (Exception $e) {
188-
$lock->release();
189-
throw $e;
169+
// If object does not exist, cannot sync tags, abort.
170+
if (!$client->object_exists($contenthash)) {
171+
return;
190172
}
191-
}
192-
193-
/**
194-
* This should only ever be called as part of uploading an object for the first time.
195-
*/
196-
public static function calculate_tags_and_store_locally_and_return(string $contenthash): array {
197-
// TODO.
198-
}
199173

200-
private static function should_set_object_tags(string $contenthash) {
201-
// If can override, always return true.
202-
if (self::can_override_existing_tags()) {
203-
return true;
174+
// Cannot override and object exists.
175+
if (!$client->can_override_objects()) {
176+
// Query existing tags and store them.
177+
$existingtags = $client->get_object_tags($contenthash);
178+
self::store_tags_locally($contenthash, $existingtags);
179+
180+
// Else can override, upload new.
181+
} else {
182+
$tags = self::gather_object_tags_for_upload($contenthash);
183+
$client->set_object_tags($contenthash, $tags);
184+
self::store_tags_locally($contenthash, $tags);
204185
}
205186

206-
// Else lookup if object already has tags, or if it even exists.
207-
// TODO.
208-
}
187+
// Either way, it has synced.
188+
self::mark_object_tag_sync_status($contenthash, self::SYNC_STATUS_SYNC_NOT_REQUIRED);
209189

210-
/**
211-
* Gets a tag writing lock on the given object, ensuring tags are not written/updated while being synced, for example.
212-
* @param string $contenthash
213-
* @param int $timeout lock wait timeout in seconds
214-
* @return lock
215-
*/
216-
private static function get_object_tag_lock(string $contenthash, int $timeout = 5): lock {
217-
$factory = lock_config::get_lock_factory('tool_objectfs_object_tags');
218-
return $factory->get_lock($contenthash, $timeout);
219-
}
220-
221-
/**
222-
* Marks a given object as the given status.
223-
* @param string $contenthash
224-
* @param int $status one of SYNC_STATUS_* constants
225-
*/
226-
private static function mark_object_tag_sync_status(string $contenthash, int $status) {
227-
global $DB;
228-
if (!in_array($status, self::SYNC_STATUSES)) {
229-
throw new coding_exception("Invalid object tag sync status " . $status);
230-
}
231-
$DB->set_field('tool_objectfs_objects', 'tagsyncstatus', $status, ['contenthash' => $contenthash]);
190+
// TODO release object lock.
232191
}
233192
}

0 commit comments

Comments
 (0)