From f237e863de13a7888b87e0e9a16146bf98a486c2 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 12 Feb 2024 16:55:00 +1300 Subject: [PATCH 1/2] Throw on updating sent, processing, failed message --- app/config/errors.php | 10 ++++++++++ app/controllers/api/messaging.php | 27 +++++++++++++++++++++------ src/Appwrite/Extend/Exception.php | 2 ++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/app/config/errors.php b/app/config/errors.php index 79a092ec99..a8e7daf749 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -867,6 +867,16 @@ return [ 'description' => 'Message with the requested ID has already been sent.', 'code' => 400, ], + Exception::MESSAGE_ALREADY_PROCESSING => [ + 'name' => Exception::MESSAGE_ALREADY_PROCESSING, + 'description' => 'Message with the requested ID is already being processed.', + 'code' => 400, + ], + Exception::MESSAGE_ALREADY_FAILED => [ + 'name' => Exception::MESSAGE_ALREADY_FAILED, + 'description' => 'Message with the requested ID has already failed.', + 'code' => 400, + ], Exception::MESSAGE_ALREADY_SCHEDULED => [ 'name' => Exception::MESSAGE_ALREADY_SCHEDULED, 'description' => 'Message with the requested ID has already been scheduled for delivery.', diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index 25260e1071..8ff8d834dc 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -3077,8 +3077,13 @@ App::patch('/v1/messaging/messages/email/:messageId') throw new Exception(Exception::MESSAGE_NOT_FOUND); } - if ($message->getAttribute('status') === MessageStatus::SENT) { - throw new Exception(Exception::MESSAGE_ALREADY_SENT); + switch ($message->getAttribute('status')) { + case MessageStatus::PROCESSING: + throw new Exception(Exception::MESSAGE_ALREADY_PROCESSING); + case MessageStatus::SENT: + throw new Exception(Exception::MESSAGE_ALREADY_SENT); + case MessageStatus::FAILED: + throw new Exception(Exception::MESSAGE_ALREADY_FAILED); } if (!\is_null($message->getAttribute('scheduledAt')) && $message->getAttribute('scheduledAt') < new \DateTime()) { @@ -3207,8 +3212,13 @@ App::patch('/v1/messaging/messages/sms/:messageId') throw new Exception(Exception::MESSAGE_NOT_FOUND); } - if ($message->getAttribute('status') === 'sent') { - throw new Exception(Exception::MESSAGE_ALREADY_SENT); + switch ($message->getAttribute('status')) { + case MessageStatus::PROCESSING: + throw new Exception(Exception::MESSAGE_ALREADY_PROCESSING); + case MessageStatus::SENT: + throw new Exception(Exception::MESSAGE_ALREADY_SENT); + case MessageStatus::FAILED: + throw new Exception(Exception::MESSAGE_ALREADY_FAILED); } if (!is_null($message->getAttribute('scheduledAt')) && $message->getAttribute('scheduledAt') < new \DateTime()) { @@ -3329,8 +3339,13 @@ App::patch('/v1/messaging/messages/push/:messageId') throw new Exception(Exception::MESSAGE_NOT_FOUND); } - if ($message->getAttribute('status') === 'sent') { - throw new Exception(Exception::MESSAGE_ALREADY_SENT); + switch ($message->getAttribute('status')) { + case MessageStatus::PROCESSING: + throw new Exception(Exception::MESSAGE_ALREADY_PROCESSING); + case MessageStatus::SENT: + throw new Exception(Exception::MESSAGE_ALREADY_SENT); + case MessageStatus::FAILED: + throw new Exception(Exception::MESSAGE_ALREADY_FAILED); } if (!is_null($message->getAttribute('scheduledAt')) && $message->getAttribute('scheduledAt') < new \DateTime()) { diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index bd1397bc20..6a52e97b23 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -264,6 +264,8 @@ class Exception extends \Exception public const MESSAGE_NOT_FOUND = 'message_not_found'; public const MESSAGE_MISSING_TARGET = 'message_missing_target'; public const MESSAGE_ALREADY_SENT = 'message_already_sent'; + public const MESSAGE_ALREADY_PROCESSING = 'message_already_processing'; + public const MESSAGE_ALREADY_FAILED = 'message_already_failed'; public const MESSAGE_ALREADY_SCHEDULED = 'message_already_scheduled'; public const MESSAGE_TARGET_NOT_EMAIL = 'message_target_not_email'; public const MESSAGE_TARGET_NOT_SMS = 'message_target_not_sms'; From 3ba6652be8e5cb157e35e3de632ee758c630ff10 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 12 Feb 2024 16:55:34 +1300 Subject: [PATCH 2/2] Only set schedule active is status is scheduled --- app/controllers/api/messaging.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/api/messaging.php b/app/controllers/api/messaging.php index 8ff8d834dc..5e6634a136 100644 --- a/app/controllers/api/messaging.php +++ b/app/controllers/api/messaging.php @@ -3141,7 +3141,7 @@ App::patch('/v1/messaging/messages/email/:messageId') 'resourceUpdatedAt' => DateTime::now(), 'projectId' => $project->getId(), 'schedule' => $scheduledAt, - 'active' => $status === 'processing', + 'active' => $status === MessageStatus::SCHEDULED, ])); $message->setAttribute('scheduleId', $schedule->getId()); @@ -3155,7 +3155,7 @@ App::patch('/v1/messaging/messages/email/:messageId') $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) ->setAttribute('schedule', $scheduledAt) - ->setAttribute('active', $status === 'processing'); + ->setAttribute('active', $status === MessageStatus::SCHEDULED); $dbForConsole->updateDocument('schedules', $schedule->getId(), $schedule); } @@ -3260,7 +3260,7 @@ App::patch('/v1/messaging/messages/sms/:messageId') 'resourceUpdatedAt' => DateTime::now(), 'projectId' => $project->getId(), 'schedule' => $scheduledAt, - 'active' => $status === 'processing', + 'active' => $status === MessageStatus::SCHEDULED, ])); $message->setAttribute('scheduleId', $schedule->getId()); @@ -3274,7 +3274,7 @@ App::patch('/v1/messaging/messages/sms/:messageId') $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) ->setAttribute('schedule', $scheduledAt) - ->setAttribute('active', $status === 'processing'); + ->setAttribute('active', $status === MessageStatus::SCHEDULED); $dbForConsole->updateDocument('schedules', $schedule->getId(), $schedule); } @@ -3284,7 +3284,7 @@ App::patch('/v1/messaging/messages/sms/:messageId') $message = $dbForProject->updateDocument('messages', $message->getId(), $message); - if ($status === 'processing' && \is_null($message->getAttribute('scheduledAt'))) { + if ($status === MessageStatus::PROCESSING) { $queueForMessaging ->setMessageId($message->getId()) ->trigger(); @@ -3419,7 +3419,7 @@ App::patch('/v1/messaging/messages/push/:messageId') 'resourceUpdatedAt' => DateTime::now(), 'projectId' => $project->getId(), 'schedule' => $scheduledAt, - 'active' => $status === 'processing', + 'active' => $status === MessageStatus::SCHEDULED, ])); $message->setAttribute('scheduleId', $schedule->getId()); @@ -3433,7 +3433,7 @@ App::patch('/v1/messaging/messages/push/:messageId') $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) ->setAttribute('schedule', $scheduledAt) - ->setAttribute('active', $status === 'processing'); + ->setAttribute('active', $status === MessageStatus::SCHEDULED); $dbForConsole->updateDocument('schedules', $schedule->getId(), $schedule); } @@ -3443,7 +3443,7 @@ App::patch('/v1/messaging/messages/push/:messageId') $message = $dbForProject->updateDocument('messages', $message->getId(), $message); - if ($status === 'processing' && \is_null($message->getAttribute('scheduledAt'))) { + if ($status === MessageStatus::PROCESSING) { $queueForMessaging ->setMessageId($message->getId()) ->trigger();