From 772617f23178ca943c80e769673bf649232345dc Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 15 Jan 2024 19:52:40 +1300 Subject: [PATCH 1/7] Mark invalid targets on message send then delete on maintenance sweep --- app/config/collections.php | 11 +++++++ app/controllers/api/messaging.php | 2 +- app/init.php | 3 +- src/Appwrite/Platform/Tasks/Maintenance.php | 15 +++++++--- src/Appwrite/Platform/Tasks/ScheduleBase.php | 1 + .../Platform/Tasks/ScheduleMessages.php | 5 +++- src/Appwrite/Platform/Workers/Deletes.php | 30 ++++++++++++++----- src/Appwrite/Platform/Workers/Messaging.php | 8 +++-- 8 files changed, 58 insertions(+), 17 deletions(-) diff --git a/app/config/collections.php b/app/config/collections.php index 979a267fa4..5369025e24 100644 --- a/app/config/collections.php +++ b/app/config/collections.php @@ -1961,6 +1961,17 @@ $commonCollections = [ 'array' => false, 'filters' => [], ], + [ + '$id' => ID::custom('expired'), + 'type' => Database::VAR_BOOLEAN, + 'format' => '', + 'size' => 0, + 'signed' => true, + 'required' => false, + 'default' => false, + 'array' => false, + 'filters' => [], + ], ], 'indexes' => [ [ diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index 2471ab8b26..356ec7bcbe 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -1944,7 +1944,7 @@ App::delete('/v1/messaging/topics/:topicId') $dbForProject->deleteDocument('topics', $topicId); $queueForDeletes - ->setType(DELETE_TYPE_TOPIC) + ->setType(DELETE_TYPE_TOPICS) ->setDocument($topic); $queueForEvents diff --git a/app/init.php b/app/init.php index e08cca1954..075b79f3d5 100644 --- a/app/init.php +++ b/app/init.php @@ -170,7 +170,8 @@ const DELETE_TYPE_SESSIONS = 'sessions'; const DELETE_TYPE_CACHE_BY_TIMESTAMP = 'cacheByTimeStamp'; const DELETE_TYPE_CACHE_BY_RESOURCE = 'cacheByResource'; const DELETE_TYPE_SCHEDULES = 'schedules'; -const DELETE_TYPE_TOPIC = 'topic'; +const DELETE_TYPE_TARGETS = 'targets'; +const DELETE_TYPE_TOPICS = 'topics'; // Mail Types const MAIL_TYPE_VERIFICATION = 'verification'; const MAIL_TYPE_MAGIC_SESSION = 'magicSession'; diff --git a/src/Appwrite/Platform/Tasks/Maintenance.php b/src/Appwrite/Platform/Tasks/Maintenance.php index 58e3228d27..914e6b5651 100644 --- a/src/Appwrite/Platform/Tasks/Maintenance.php +++ b/src/Appwrite/Platform/Tasks/Maintenance.php @@ -56,6 +56,7 @@ class Maintenance extends Action $this->renewCertificates($dbForConsole, $queueForCertificates); $this->notifyDeleteCache($cacheRetention, $queueForDeletes); $this->notifyDeleteSchedules($schedulesDeletionRetention, $queueForDeletes); + $this->notifyDeleteTargets($schedulesDeletionRetention, $queueForDeletes); }, $interval); } @@ -134,8 +135,7 @@ class Maintenance extends Action private function notifyDeleteCache($interval, Delete $queueForDeletes): void { - - ($queueForDeletes) + $queueForDeletes ->setType(DELETE_TYPE_CACHE_BY_TIMESTAMP) ->setDatetime(DateTime::addSeconds(new \DateTime(), -1 * $interval)) ->trigger(); @@ -143,10 +143,17 @@ class Maintenance extends Action private function notifyDeleteSchedules($interval, Delete $queueForDeletes): void { - - ($queueForDeletes) + $queueForDeletes ->setType(DELETE_TYPE_SCHEDULES) ->setDatetime(DateTime::addSeconds(new \DateTime(), -1 * $interval)) ->trigger(); } + + private function notifyDeleteTargets($interval, Delete $queueForDeletes): void + { + $queueForDeletes + ->setType(DELETE_TYPE_TARGETS) + ->setDatetime(DateTime::addSeconds(new \DateTime(), -1 * $interval)) + ->trigger(); + } } diff --git a/src/Appwrite/Platform/Tasks/ScheduleBase.php b/src/Appwrite/Platform/Tasks/ScheduleBase.php index 1ec8e471bb..f0aa559a7f 100644 --- a/src/Appwrite/Platform/Tasks/ScheduleBase.php +++ b/src/Appwrite/Platform/Tasks/ScheduleBase.php @@ -73,6 +73,7 @@ abstract class ScheduleBase extends Action '$id' => $schedule->getId(), 'resourceId' => $schedule->getAttribute('resourceId'), 'schedule' => $schedule->getAttribute('schedule'), + 'active' => $schedule->getAttribute('active'), 'resourceUpdatedAt' => $schedule->getAttribute('resourceUpdatedAt'), 'project' => $project, // TODO: @Meldiron Send only ID to worker to reduce memory usage here 'resource' => $resource, // TODO: @Meldiron Send only ID to worker to reduce memory usage here diff --git a/src/Appwrite/Platform/Tasks/ScheduleMessages.php b/src/Appwrite/Platform/Tasks/ScheduleMessages.php index 471aeb8f2a..1e71d1423e 100644 --- a/src/Appwrite/Platform/Tasks/ScheduleMessages.php +++ b/src/Appwrite/Platform/Tasks/ScheduleMessages.php @@ -33,6 +33,10 @@ class ScheduleMessages extends ScheduleBase protected function enqueueResources(Group $pools, Database $dbForConsole): void { foreach ($this->schedules as $schedule) { + if (!$schedule['active']) { + continue; + } + $now = DateTime::now(); $scheduledAt = DateTime::formatTz($schedule['scheduledAt']); @@ -59,7 +63,6 @@ class ScheduleMessages extends ScheduleBase $queueForDeletes ->setType(DELETE_TYPE_SCHEDULES) - ->setDocument($schedule) ->trigger(); $queue->reclaim(); diff --git a/src/Appwrite/Platform/Workers/Deletes.php b/src/Appwrite/Platform/Workers/Deletes.php index 6ed0c5d496..bfd4e1d223 100644 --- a/src/Appwrite/Platform/Workers/Deletes.php +++ b/src/Appwrite/Platform/Workers/Deletes.php @@ -151,7 +151,10 @@ class Deletes extends Action case DELETE_TYPE_SCHEDULES: $this->deleteSchedules($dbForConsole, $getProjectDB, $datetime, $document); break; - case DELETE_TYPE_TOPIC: + case DELETE_TYPE_TARGETS: + $this->deleteTargets($project, $getProjectDB, $document); + break; + case DELETE_TYPE_TOPICS: $this->deleteTopic($project, $getProjectDB, $document); break; default: @@ -178,7 +181,6 @@ class Deletes extends Action 'schedules', [ Query::equal('region', [App::getEnv('_APP_REGION', 'default')]), - Query::equal('resourceType', [$document->getAttribute('resourceType')]), Query::lessThanEqual('resourceUpdatedAt', $datetime), Query::equal('active', [false]), ], @@ -213,23 +215,37 @@ class Deletes extends Action ); } + private function deleteTargets(Document $project, callable $getProjectDB, Document $target) + { + $this->deleteByGroup( + 'targets', + [ + Query::equal('expired', [true]) + ], + $getProjectDB($project) + ); + } + /** * @param Document $project * @param callable $getProjectDB * @param Document $topic * @throws Exception */ - protected function deleteTopic(Document $project, callable $getProjectDB, Document $topic) + private function deleteTopic(Document $project, callable $getProjectDB, Document $topic) { if ($topic->isEmpty()) { Console::error('Failed to delete subscribers. Topic not found'); return; } - $dbForProject = $getProjectDB($project); - $this->deleteByGroup('subscribers', [ - Query::equal('topicInternalId', [$topic->getInternalId()]) - ], $dbForProject); + $this->deleteByGroup( + 'subscribers', + [ + Query::equal('topicInternalId', [$topic->getInternalId()]) + ], + $getProjectDB($project) + ); } /** diff --git a/src/Appwrite/Platform/Workers/Messaging.php b/src/Appwrite/Platform/Workers/Messaging.php index 9b7bda57bb..6535dfce3d 100644 --- a/src/Appwrite/Platform/Workers/Messaging.php +++ b/src/Appwrite/Platform/Workers/Messaging.php @@ -232,9 +232,11 @@ class Messaging extends Action Query::equal('identifier', [$detail['recipient']]) ]); - if ($target instanceof Document && !$target->isEmpty()) { - $dbForProject->deleteDocument('targets', $target->getId()); - } + $dbForProject->updateDocument( + 'targets', + $target->getId(), + $target->setAttribute('expired', true) + ); } } } catch (\Exception $e) { From 9fc68d23a00f94c63376a8272991ec4b96416c86 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 19 Jan 2024 17:23:44 +1300 Subject: [PATCH 2/7] Fix delete type for expired targets --- app/init.php | 1 + src/Appwrite/Platform/Tasks/Maintenance.php | 7 ++- src/Appwrite/Platform/Workers/Deletes.php | 47 ++++++++++++--------- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/app/init.php b/app/init.php index 2422a54ab3..5eba7ad993 100644 --- a/app/init.php +++ b/app/init.php @@ -172,6 +172,7 @@ const DELETE_TYPE_CACHE_BY_RESOURCE = 'cacheByResource'; const DELETE_TYPE_SCHEDULES = 'schedules'; const DELETE_TYPE_TOPIC = 'topic'; const DELETE_TYPE_TARGET = 'target'; +const DELETE_TYPE_EXPIRED_TARGETS = 'invalid_targets'; // Mail Types const MAIL_TYPE_VERIFICATION = 'verification'; const MAIL_TYPE_MAGIC_SESSION = 'magicSession'; diff --git a/src/Appwrite/Platform/Tasks/Maintenance.php b/src/Appwrite/Platform/Tasks/Maintenance.php index 914e6b5651..66ce40a0d2 100644 --- a/src/Appwrite/Platform/Tasks/Maintenance.php +++ b/src/Appwrite/Platform/Tasks/Maintenance.php @@ -56,7 +56,7 @@ class Maintenance extends Action $this->renewCertificates($dbForConsole, $queueForCertificates); $this->notifyDeleteCache($cacheRetention, $queueForDeletes); $this->notifyDeleteSchedules($schedulesDeletionRetention, $queueForDeletes); - $this->notifyDeleteTargets($schedulesDeletionRetention, $queueForDeletes); + $this->notifyDeleteTargets($queueForDeletes); }, $interval); } @@ -149,11 +149,10 @@ class Maintenance extends Action ->trigger(); } - private function notifyDeleteTargets($interval, Delete $queueForDeletes): void + private function notifyDeleteTargets(Delete $queueForDeletes): void { $queueForDeletes - ->setType(DELETE_TYPE_TARGETS) - ->setDatetime(DateTime::addSeconds(new \DateTime(), -1 * $interval)) + ->setType(DELETE_TYPE_EXPIRED_TARGETS) ->trigger(); } } diff --git a/src/Appwrite/Platform/Workers/Deletes.php b/src/Appwrite/Platform/Workers/Deletes.php index 11a7a69967..a98566f0f8 100644 --- a/src/Appwrite/Platform/Workers/Deletes.php +++ b/src/Appwrite/Platform/Workers/Deletes.php @@ -119,11 +119,9 @@ class Deletes extends Action break; } break; - case DELETE_TYPE_EXECUTIONS: $this->deleteExecutionLogs($dbForConsole, $getProjectDB, $datetime); break; - case DELETE_TYPE_AUDIT: if (!empty($datetime)) { $this->deleteAuditLogs($dbForConsole, $getProjectDB, $datetime); @@ -136,11 +134,9 @@ class Deletes extends Action case DELETE_TYPE_ABUSE: $this->deleteAbuseLogs($dbForConsole, $getProjectDB, $datetime); break; - case DELETE_TYPE_REALTIME: $this->deleteRealtimeUsage($dbForConsole, $datetime); break; - case DELETE_TYPE_SESSIONS: $this->deleteExpiredSessions($dbForConsole, $getProjectDB); break; @@ -156,18 +152,17 @@ class Deletes extends Action case DELETE_TYPE_SCHEDULES: $this->deleteSchedules($dbForConsole, $getProjectDB, $datetime, $document); break; - case DELETE_TYPE_TARGETS: - $this->deleteTargets($project, $getProjectDB, $document); - break; - case DELETE_TYPE_TOPICS: + case DELETE_TYPE_TOPIC: $this->deleteTopic($project, $getProjectDB, $document); break; case DELETE_TYPE_TARGET: $this->deleteTarget($project, $getProjectDB, $document); break; + case DELETE_TYPE_EXPIRED_TARGETS: + $this->deleteExpiredTargets($project, $getProjectDB); + break; default: throw new \Exception('No delete operation for type: ' . \strval($type)); - break; } } @@ -223,17 +218,6 @@ class Deletes extends Action ); } - private function deleteTargets(Document $project, callable $getProjectDB, Document $target) - { - $this->deleteByGroup( - 'targets', - [ - Query::equal('expired', [true]) - ], - $getProjectDB($project) - ); - } - /** * @param Document $project * @param callable $getProjectDB @@ -262,7 +246,7 @@ class Deletes extends Action * @param Document $target * @throws Exception */ - protected function deleteTarget(Document $project, callable $getProjectDB, Document $target) + private function deleteTarget(Document $project, callable $getProjectDB, Document $target) { /** @var Database */ $dbForProject = $getProjectDB($project); @@ -285,6 +269,27 @@ class Deletes extends Action ); } + /** + * @param Document $project + * @param callable $getProjectDB + * @param Document $target + * @return void + * @throws Exception + */ + private function deleteExpiredTargets(Document $project, callable $getProjectDB) + { + $this->deleteByGroup( + 'targets', + [ + Query::equal('expired', [true]) + ], + $getProjectDB($project), + function (Document $target) use ($getProjectDB, $project) { + $this->deleteTarget($project, $getProjectDB, $target); + } + ); + } + /** * @param Document $project * @param callable $getProjectDB From a970018b045b598e4ed82e8c3c591b8e09aaad80 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 19 Jan 2024 19:18:56 +1300 Subject: [PATCH 3/7] Fix tests --- app/controllers/api/messaging.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index 51e046a6ec..ef72be19cf 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -1931,7 +1931,7 @@ App::delete('/v1/messaging/topics/:topicId') $dbForProject->deleteDocument('topics', $topicId); $queueForDeletes - ->setType(DELETE_TYPE_TOPICS) + ->setType(DELETE_TYPE_TOPIC) ->setDocument($topic); $queueForEvents From 8fd5a336af2ec5f0da869b324e25ca03ad171191 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 19 Jan 2024 20:18:10 +1300 Subject: [PATCH 4/7] Make sure target exists before updating --- src/Appwrite/Platform/Workers/Messaging.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Messaging.php b/src/Appwrite/Platform/Workers/Messaging.php index e95d7835fd..a5c8154b91 100644 --- a/src/Appwrite/Platform/Workers/Messaging.php +++ b/src/Appwrite/Platform/Workers/Messaging.php @@ -247,11 +247,13 @@ class Messaging extends Action Query::equal('identifier', [$detail['recipient']]) ]); - $dbForProject->updateDocument( - 'targets', - $target->getId(), - $target->setAttribute('expired', true) - ); + if ($target instanceof Document && !$target->isEmpty()) { + $dbForProject->updateDocument( + 'targets', + $target->getId(), + $target->setAttribute('expired', true) + ); + } } } } catch (\Exception $e) { From e81ab1d1e0fbcfc013b2c218ceab0b0ff2a3d107 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 26 Jan 2024 00:35:04 +1300 Subject: [PATCH 5/7] Remove redundant document pass through --- src/Appwrite/Platform/Workers/Deletes.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Deletes.php b/src/Appwrite/Platform/Workers/Deletes.php index cd04eb52b2..2de55b5cc0 100644 --- a/src/Appwrite/Platform/Workers/Deletes.php +++ b/src/Appwrite/Platform/Workers/Deletes.php @@ -153,7 +153,7 @@ class Deletes extends Action $this->deleteCacheByDate($project, $getProjectDB, $datetime); break; case DELETE_TYPE_SCHEDULES: - $this->deleteSchedules($dbForConsole, $getProjectDB, $datetime, $document); + $this->deleteSchedules($dbForConsole, $getProjectDB, $datetime); break; case DELETE_TYPE_TOPIC: $this->deleteTopic($project, $getProjectDB, $document); @@ -181,7 +181,7 @@ class Deletes extends Action * @throws Structure * @throws DatabaseException */ - private function deleteSchedules(Database $dbForConsole, callable $getProjectDB, string $datetime, ?Document $document = null): void + private function deleteSchedules(Database $dbForConsole, callable $getProjectDB, string $datetime): void { $this->listByGroup( 'schedules', From 34973b8413ad0ee524afcb8151213357a4629fb8 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 26 Jan 2024 00:35:37 +1300 Subject: [PATCH 6/7] Directly delete message schedule instead of deferring to worker --- src/Appwrite/Platform/Tasks/ScheduleMessages.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Appwrite/Platform/Tasks/ScheduleMessages.php b/src/Appwrite/Platform/Tasks/ScheduleMessages.php index fef0c79bb2..cc641b434a 100644 --- a/src/Appwrite/Platform/Tasks/ScheduleMessages.php +++ b/src/Appwrite/Platform/Tasks/ScheduleMessages.php @@ -48,23 +48,17 @@ class ScheduleMessages extends ScheduleBase $queue = $pools->get('queue')->pop(); $connection = $queue->getResource(); $queueForMessaging = new Messaging($connection); - $queueForDeletes = new Delete($connection); $queueForMessaging ->setMessageId($schedule['resourceId']) ->setProject($schedule['project']) ->trigger(); - $dbForConsole->updateDocument( + $dbForConsole->deleteDocument( 'schedules', $schedule['$id'], - new Document(['active' => false]) ); - $queueForDeletes - ->setType(DELETE_TYPE_SCHEDULES) - ->trigger(); - $queue->reclaim(); unset($this->schedules[$schedule['resourceId']]); From 89343704f75e1e7aeac10d5c68d77e0493802d94 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 26 Jan 2024 00:41:05 +1300 Subject: [PATCH 7/7] Rename deleteTargetSubscribers method to make actions clearer --- src/Appwrite/Platform/Workers/Deletes.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Deletes.php b/src/Appwrite/Platform/Workers/Deletes.php index 2de55b5cc0..cd62d2e621 100644 --- a/src/Appwrite/Platform/Workers/Deletes.php +++ b/src/Appwrite/Platform/Workers/Deletes.php @@ -159,7 +159,7 @@ class Deletes extends Action $this->deleteTopic($project, $getProjectDB, $document); break; case DELETE_TYPE_TARGET: - $this->deleteTarget($project, $getProjectDB, $document); + $this->deleteTargetSubscribers($project, $getProjectDB, $document); break; case DELETE_TYPE_EXPIRED_TARGETS: $this->deleteExpiredTargets($project, $getProjectDB); @@ -249,7 +249,7 @@ class Deletes extends Action * @param Document $target * @throws Exception */ - private function deleteTarget(Document $project, callable $getProjectDB, Document $target) + private function deleteTargetSubscribers(Document $project, callable $getProjectDB, Document $target) { /** @var Database */ $dbForProject = $getProjectDB($project); @@ -288,7 +288,7 @@ class Deletes extends Action ], $getProjectDB($project), function (Document $target) use ($getProjectDB, $project) { - $this->deleteTarget($project, $getProjectDB, $target); + $this->deleteTargetSubscribers($project, $getProjectDB, $target); } ); } @@ -634,14 +634,14 @@ class Deletes extends Action ], $dbForProject); // Delete targets - $this->listByGroup( + $this->deleteByGroup( 'targets', [ Query::equal('userInternalId', [$userInternalId]) ], $dbForProject, function (Document $target) use ($getProjectDB, $project) { - $this->deleteTarget($project, $getProjectDB, $target); + $this->deleteTargetSubscribers($project, $getProjectDB, $target); } ); }