From ea84bd8ba0ed93df88e55b4c51a3bf93fd936aaf Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Thu, 28 Nov 2024 16:08:56 +1000 Subject: [PATCH] Store notifications in emailutils log --- classes/helper.php | 76 ++++++++++++++++++++++++++++++++++++ classes/sns_notification.php | 17 +++++++- db/install.xml | 4 +- db/upgrade.php | 24 ++++++++++++ tests/sns_client_test.php | 22 +++++------ version.php | 4 +- 6 files changed, 131 insertions(+), 16 deletions(-) diff --git a/classes/helper.php b/classes/helper.php index 30a7637..c74947b 100644 --- a/classes/helper.php +++ b/classes/helper.php @@ -151,4 +151,80 @@ public static function get_bounce_config(): array { self::get_bounce_ratio(), ]; } + + /** + * Get the most recent bounce recorded for an email address + * + * @param string $email + * @return \stdClass|false + */ + public static function get_most_recent_bounce(string $email): mixed { + global $DB; + + $params = [ + 'email' => $email, + ]; + $sql = "SELECT * + FROM {tool_emailutils_log} + WHERE type = 'Bounce' AND email = :email + ORDER BY time DESC + LIMIT 1"; + return $DB->get_record_sql($sql, $params); + } + + /** + * Gets the sum of the send count for multiple users + * @param array $users array of users or userids + * @return int sum of send count + */ + public static function get_sum_send_count(array $users): int { + $sendcount = 0; + foreach ($users as $user) { + $sendcount += get_user_preferences('email_send_count', 0, $user); + } + return $sendcount; + } + + /** + * Gets the max bounce count from a group of users + * @param array $users array of users or userids + * @return int max bounce count + */ + public static function get_max_bounce_count(array $users): int { + $maxbounces = 0; + foreach ($users as $user) { + $maxbounces = max($maxbounces, get_user_preferences('email_bounce_count', 0, $user)); + } + return $maxbounces; + } + + /** + * Checks whether bounces are likely to be consecutive, and if not reset the bounce count. + * This is a fallback in case delivery notifications are not enabled. + * + * @param string $email + * @param array $users + * @return void + */ + public static function check_consecutive_bounces(string $email, array $users): void { + $recentbounce = self::get_most_recent_bounce($email); + if (empty($recentbounce)) { + // No data on the previous bounce. + return; + } + + $sendcount = self::get_sum_send_count($users); + $prevsendcount = $recentbounce->sendcount ?? 0; + $sincelastbounce = $sendcount - $prevsendcount; + + // The only things we can compare are the previous send count and time. + // A direct comparison in sendcount isn't accurate because notifications may be delayed, so use a buffer. + // Minbounces is ideal since future notifications would push it over the threshold. + $buffer = min(self::get_min_bounces(), 5); + if ($sincelastbounce >= $buffer) { + foreach ($users as $user) { + self::reset_bounce_count($user); + } + } + } } diff --git a/classes/sns_notification.php b/classes/sns_notification.php index bfd4b41..2536900 100644 --- a/classes/sns_notification.php +++ b/classes/sns_notification.php @@ -285,8 +285,7 @@ protected function process_bounce_notification(\stdClass $user): void { if ($this->should_block_immediately()) { // User should only be able to recover from this if they change their email or have their bounces reset. - // This sets the bounce ratio to 1 to improve visibility when something is a hard bounce. - $bouncecount = max($sendcount, helper::get_min_bounces()); + $bouncecount = helper::get_min_bounces(); set_user_preference('email_bounce_count', $bouncecount, $user); } else if ($this->should_block_softly()) { // Swap back to set_bounce_count($user) once MDL-73798 is integrated. @@ -344,12 +343,26 @@ public function process_notification(): void { if ($this->is_bounce()) { // Ideally bounce handling would be tracked per email instead of user. + if (helper::use_consecutive_bounces()) { + helper::check_consecutive_bounces($this->get_destination(), $users); + } foreach ($users as $user) { $this->process_bounce_notification($user); } } // TODO: Implement complaint handling. + + // Save to emailutils log. This should be done last as processing may increase send count. + // Time should represent when this was processed - send time will be in message if needed. + $DB->insert_record('tool_emailutils_log', [ + 'time' => time(), + 'type' => $this->get_type(), + 'subtypes' => $this->get_subtypes(), + 'email' => $this->get_destination(), + 'message' => $this->messageraw, + 'sendcount' => helper::get_sum_send_count($users), + ]); } /** diff --git a/db/install.xml b/db/install.xml index 3d98ded..fc05989 100644 --- a/db/install.xml +++ b/db/install.xml @@ -1,5 +1,5 @@ - @@ -9,8 +9,10 @@ + + diff --git a/db/upgrade.php b/db/upgrade.php index 2fbeefe..b049d5c 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -78,5 +78,29 @@ function xmldb_tool_emailutils_upgrade($oldversion) { // Emailutils savepoint reached. upgrade_plugin_savepoint(true, 2024111800, 'tool', 'emailutils'); } + + if ($oldversion < 2024112801) { + + // Define field subtypes to be added to tool_emailutils_log. + $table = new xmldb_table('tool_emailutils_log'); + $field = new xmldb_field('subtypes', XMLDB_TYPE_CHAR, '32', null, null, null, null, 'type'); + + // Conditionally launch add field subtypes. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Define field sendcount to be added to tool_emailutils_log. + $field = new xmldb_field('sendcount', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'message'); + + // Conditionally launch add field sendcount. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Emailutils savepoint reached. + upgrade_plugin_savepoint(true, 2024112801, 'tool', 'emailutils'); + } + return true; } diff --git a/tests/sns_client_test.php b/tests/sns_client_test.php index f04dc11..d324c49 100644 --- a/tests/sns_client_test.php +++ b/tests/sns_client_test.php @@ -145,7 +145,7 @@ private function get_mock_bounce_notification(string $bouncetype, string $bounce public function bounce_processing_provider(): array { // To be tested with minbounces of 3 and bounceratio of -1. return [ - 'Block immediately with low send count' => [ + 'Block immediately' => [ 'type' => 'Permanent', 'subtype' => 'General', 'notifications' => 1, @@ -153,14 +153,6 @@ public function bounce_processing_provider(): array { 'expectedbounces' => 3, 'overthreshold' => true, ], - 'Block immediately with high send count' => [ - 'type' => 'Permanent', - 'subtype' => 'General', - 'notifications' => 1, - 'sendcount' => 100, - 'expectedbounces' => 100, - 'overthreshold' => true, - ], 'Block softly' => [ 'type' => 'Transient', 'subtype' => 'General', @@ -218,7 +210,7 @@ public function bounce_processing_provider(): array { **/ public function test_bounce_processing(string $type, string $subtype, int $notifications, int $sendcount, int $expectedbounces, bool $overthreshold): void { - global $CFG; + global $CFG, $DB; // Setup config and users. $this->resetAfterTest(); @@ -249,6 +241,10 @@ public function test_bounce_processing(string $type, string $subtype, int $notif $bounceevents = 2 * (int) $overthreshold; $this->assertCount($notifications + $bounceevents, $events); + // Confirm bounce notification stored in emailutils log table. + $records = $DB->get_records('tool_emailutils_log', null, 'id ASC'); + $this->assertCount($notifications, $records); + // Confirm that shared email addresses have the same status. $this->assertSame($overthreshold, over_bounce_threshold($user2)); } @@ -259,7 +255,7 @@ public function test_bounce_processing(string $type, string $subtype, int $notif * @covers \tool_emailutils\sns_notification::process_notification() **/ public function test_delivery_processing(): void { - global $CFG; + global $CFG, $DB; // Setup config and users. $this->resetAfterTest(); @@ -297,6 +293,10 @@ public function test_delivery_processing(): void { // This also confirms the third user didn't have their count reset. $this->assertCount(2, $events); + // Confirm delivery notification isn't stored in emailutils log table. + $records = $DB->get_records('tool_emailutils_log'); + $this->assertCount(0, $records); + // Ensure bounces aren't reset when bounce ratio config is positive. $CFG->bounceratio = 0.5; $this->assertFalse(helper::use_consecutive_bounces()); diff --git a/version.php b/version.php index 1e075a7..335cf76 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024112200; -$plugin->release = 2024112200; +$plugin->version = 2024112801; +$plugin->release = 2024112801; $plugin->requires = 2024042200; $plugin->component = 'tool_emailutils'; $plugin->maturity = MATURITY_STABLE;