Skip to content

Commit

Permalink
Add more specific bounce handling
Browse files Browse the repository at this point in the history
  • Loading branch information
bwalkerl committed Nov 25, 2024
1 parent 8475b2e commit ef19320
Show file tree
Hide file tree
Showing 11 changed files with 389 additions and 51 deletions.
4 changes: 1 addition & 3 deletions classes/check/bounces.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ public function get_result() : result {
FROM {user_preferences} up
LEFT JOIN {user_preferences} up2 ON up2.name = 'email_send_count' AND up.userid = up2.userid
WHERE up.name = 'email_bounce_count' AND CAST(up.value AS INTEGER) > :threshold";
// Start with a threshold of 1 as that is the default for manually created users.
// TODO: Update when core is fixed.
$bounces = $DB->get_records_sql($sql, ['threshold' => 1]);
$bounces = $DB->get_records_sql($sql, ['threshold' => 0]);
$userswithbounces = count($bounces);

// Split bounces into 3 groups based on whether they meet bounce threshold criteria.
Expand Down
9 changes: 3 additions & 6 deletions classes/event/notification_received.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
*/
namespace tool_emailutils\event;

use tool_emailutils;

/**
* Event
*/
Expand All @@ -34,18 +32,17 @@ class notification_received extends \core\event\base {
/**
* Initialise required event data properties.
*/
protected function init() {
protected function init(): void {
$this->data['crud'] = 'u';
$this->data['edulevel'] = self::LEVEL_OTHER;
$this->data['objecttable'] = 'user';
}

/**
* Returns localised event name.
*
* @return string
*/
public static function get_name() {
public static function get_name(): string {
return get_string('event:notificationreceived', 'tool_emailutils');
}

Expand All @@ -54,7 +51,7 @@ public static function get_name() {
*
* @return string
*/
public function get_description() {
public function get_description(): mixed {
return $this->other;
}
}
57 changes: 57 additions & 0 deletions classes/event/over_bounce_threshold.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* Over bounce threshold event
*
* @package tool_emailutils
* @author Benjamin Walker ([email protected])
* @copyright 2024 Catalyst IT
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace tool_emailutils\event;

/**
* Event
*/
class over_bounce_threshold extends \core\event\base {

/**
* Initialise required event data properties.
*/
protected function init(): void {
$this->data['crud'] = 'u';
$this->data['edulevel'] = self::LEVEL_OTHER;
}

/**
* Returns localised event name.
*
* @return string
*/
public static function get_name(): string {
return get_string('event:overbouncethreshold', 'tool_emailutils');
}

/**
* Returns non-localised description of what happened.
*
* @return string
*/
public function get_description(): string {
return "The user with id '$this->relateduserid' is now over the bounce threshold.";
}
}
46 changes: 45 additions & 1 deletion classes/helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public static function get_username_fields(string $tablealias = ''): string {
*/
public static function get_min_bounces(): int {
global $CFG;
return $CFG->minbounce ?? self::DEFAULT_MIN_BOUNCES;
return $CFG->minbounces ?? self::DEFAULT_MIN_BOUNCES;
}

/**
Expand All @@ -79,6 +79,50 @@ public static function get_bounce_ratio(): float {
return $CFG->bounceratio ?? self::DEFAULT_BOUNCE_RATIO;
}

/**
* Gets the rolling total based on bounce ratio.
* This is used to ignore the bounce ratio in favor of a rolling metric.
* @return int
*/
public static function get_rolling_total(): int {
return (int) floor(self::get_min_bounces() / self::get_bounce_ratio());
}

/**
* Calculates the rolling bounce count
* @param int $bouncecount previous bounce count
* @param int $sincelastbounce emails send since the last bounce
* @return int
*/
public static function get_rolling_bounce_count(int $bouncecount, int $sincelastbounce): int {
if (empty($sincelastbounce)) {
return $bouncecount;
}

$rollingtotal = self::get_rolling_total();
if ($sincelastbounce >= $rollingtotal) {
// All bounces are old.
return 0;
}

// We know there's at least 1 recent bounce.
$reducemax = $bouncecount - 1;
$percentclear = $sincelastbounce / ($rollingtotal - 1);

// We know the bouncecount and numbers of emails since the last bounce, but not when the others were.
// So the best we can do is reduce the bounces by an approximation.
$reducetotal = $reducemax * $percentclear;
$reducecount = floor($reducetotal);

// Handle the remainder with probability.
$remainder = $reducetotal - $reducecount;
if (!empty($remainder) && mt_rand() / mt_getrandmax() < $remainder) {
$reducecount++;
}

return $bouncecount - $reducecount;
}

/**
* Gets the bounce config, or the defaults.
* @return array [handlebounces, minbounces, bounceratio]
Expand Down
22 changes: 14 additions & 8 deletions classes/sns_client.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ class sns_client {
/** Bounce */
const BOUNCE_TYPE = 'Bounce';

/** Delivery */
const DELIVERY_TYPE = 'Delivery';

/**
* SNS Message Object
* @var \Aws\Sns\Message
Expand Down Expand Up @@ -182,25 +185,28 @@ public function is_authorised() {
* Validates the incoming message and sets up the sns_notification message.
* This accepts message types of 'Notification'.
*
* @param Message|null $message message injector for unit testing
* @return bool Successful validtion
*/
public function process_message() {
public function process_message(?Message $message = null): bool {
$this->validator = new MessageValidator();
$this->client = new Client();
$this->notification = new sns_notification();

// Get the message from the POST data.
$this->message = Message::fromRawPostData();
$this->message = !PHPUNIT_TEST ? Message::fromRawPostData() : $message;

$isvalidmessage = false;

// Validate the incoming message.
try {
$this->validator->validate($this->message);
} catch (InvalidSnsMessageException $e) {
// Message not valid!
http_response_code(400); // Bad request.
return $isvalidmessage;
if (!PHPUNIT_TEST) {
try {
$this->validator->validate($this->message);
} catch (InvalidSnsMessageException $e) {
// Message not valid!
http_response_code(400); // Bad request.
return $isvalidmessage;
}
}

// Process the message depending on its type.
Expand Down
126 changes: 122 additions & 4 deletions classes/sns_notification.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,31 @@

namespace tool_emailutils;

use tool_emailutils\event\notification_received;
use tool_emailutils\event\over_bounce_threshold;

/**
* Amazon SNS Notification Class
*
* Parses Amazon SES complaints and bounces contained in SNS Notification messages.
*/
class sns_notification {

/** Bounce subtypes that should be blocked immediately */
const BLOCK_IMMEDIATELY = [
'Permanent:General',
'Permanent:NoEmail',
'Permanent:Suppressed',
'Permanent:OnAccountSuppressionList',
];

/** Bounce subtypes that should be blocked after a few failures */
const BLOCK_SOFTLY = [
'Undetermined:Undetermined',
'Transient:General',
'Transient:MailboxFull',
];

/**
* SNS Message
* @var mixed
Expand Down Expand Up @@ -173,9 +191,9 @@ public function get_complaintsubtype(): string {

/**
* Returns all the message subtypes as an array
* @return string subtypes as a array
* @return string subtypes as a string, split by ':'
*/
protected function get_subtypes(): array {
protected function get_subtypes(): string {
$subtypes = [];
if ($this->is_complaint()) {
$subtypes = [
Expand All @@ -188,7 +206,7 @@ protected function get_subtypes(): array {
$this->get_bouncesubtype(),
];
}
return array_filter($subtypes);
return implode(':', array_filter($subtypes));
}

/**
Expand All @@ -207,6 +225,106 @@ public function is_bounce(): bool {
return $this->get_type() === sns_client::BOUNCE_TYPE;
}

/**
* Is the message about a delivery?
* @return bool Is delivery?
*/
public function is_delivery(): bool {
return $this->get_type() === sns_client::DELIVERY_TYPE;
}

/**
* Does this bounce type imply this should be blocked immediately?
* @return bool block immediately?
*/
public function should_block_immediately(): bool {
return in_array($this->get_subtypes(), self::BLOCK_IMMEDIATELY);
}

/**
* Does this bounce type imply this should be blocked softly?
* @return bool block softly?
*/
public function should_block_softly(): bool {
return in_array($this->get_subtypes(), self::BLOCK_SOFTLY);
}

/**
* Processes a bounce notification based on the subtype
* @param \stdClass $user
* @return void
*/
protected function process_bounce_notification(\stdClass $user): void {
$startingthreshold = over_bounce_threshold($user);
$sendcount = get_user_preferences('email_send_count', 0, $user);
$bouncecount = get_user_preferences('email_bounce_count', 0, $user);
$rollingtotal = helper::get_rolling_total();
if ($bouncecount && $sendcount > $rollingtotal) {
// Use the send count to extract the number of emails that have been sent without bouncing.
// This assumes tool_emailutils has set rolling totals, but at worst would reset historical bounces.
// The main potential issue is with delayed notifications if multiple emails were sent at the same time.
$sincelastbounce = $sendcount - $rollingtotal - 1;
$bouncecount = helper::get_rolling_bounce_count($bouncecount, $sincelastbounce);
}

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.
set_user_preference('email_send_count', $rollingtotal, $user);
set_user_preference('email_bounce_count', $rollingtotal, $user);
} else if ($this->should_block_softly()) {
set_user_preference('email_send_count', $rollingtotal, $user);
set_user_preference('email_bounce_count', $bouncecount + 1, $user);
}

if (!$startingthreshold && over_bounce_threshold($user)) {
$event = over_bounce_threshold::create([
'relateduserid' => $user->id,
'context' => \context_system::instance(),
]);
$event->trigger();
}
}

/**
* Processes a notification based on the type
* @return void
*/
public function process_notification(): void {
global $DB;

if ($this->is_delivery()) {
// Not currently processing delivery information.
return;
}

if (!$this->is_complaint() && !$this->is_bounce()) {
// Invalid request. We should never be here.
http_response_code(400);
return;
}

$user = $DB->get_record('user', ['email' => $this->get_destination()], 'id, email');

// Log all complaints and bounces as an event, even if user is invalid.
$event = notification_received::create([
'relateduserid' => $user->id ?? null,
'context' => \context_system::instance(),
'other' => $this->get_messageasstring(),
]);
$event->trigger();

if (empty($user)) {
return;
}

if ($this->is_bounce()) {
$this->process_bounce_notification($user);
}

// TODO: Implement complaint handling.
}

/**
* Return the message as a string
* Eg. "Type about x from y"
Expand All @@ -215,7 +333,7 @@ public function is_bounce(): bool {
public function get_messageasstring(): string {
if ($this->is_complaint() || $this->is_bounce()) {
$subtypes = $this->get_subtypes();
$subtypestring = !empty($subtypes) ? ' (' . implode(':', $subtypes) . ')' : '';
$subtypestring = !empty($subtypes) ? " ($subtypes)" : '';
$type = $this->get_type() . $subtypestring;
return $type . ' about ' . $this->get_source_email() . ' from ' . $this->get_destination();
} else {
Expand Down
Loading

0 comments on commit ef19320

Please sign in to comment.