From a4f230561c3ea895090108bab136b417a2617591 Mon Sep 17 00:00:00 2001 From: waleedhassan Date: Tue, 22 Oct 2024 08:21:11 +0100 Subject: [PATCH] Task tool_emailutils\task\update_suppression_list should gracefully fail #53 Fixed an Issue raised about the aws suppression list update so that it doesn't throw any exceptions if it's not enabled and doesn't cause any noise. --- classes/task/update_suppression_list.php | 132 +++++++++++++---------- lang/en/tool_emailutils.php | 2 + settings.php | 9 ++ tests/suppressionlist_test.php | 79 +++++++++++--- 4 files changed, 153 insertions(+), 69 deletions(-) diff --git a/classes/task/update_suppression_list.php b/classes/task/update_suppression_list.php index af88f39..5eec1bb 100644 --- a/classes/task/update_suppression_list.php +++ b/classes/task/update_suppression_list.php @@ -30,8 +30,6 @@ if (!class_exists('\Aws\SesV2\SesV2Client')) { if (file_exists($CFG->dirroot . '/local/aws/sdk/aws-autoloader.php')) { require_once($CFG->dirroot . '/local/aws/sdk/aws-autoloader.php'); - } else { - throw new \Exception('AWS SDK not found.'); } } @@ -61,46 +59,59 @@ public function get_name(): string { * the local database with the fetched information. * * @return void + * @throws \dml_exception */ public function execute(): void { - global $DB; + if (!$this->is_feature_enabled()) { + return; + } - $suppressionlist = $this->fetch_aws_ses_suppression_list(); + if (!$this->check_connection()) { + debugging('Unable to connect to AWS SES. Suppression list update skipped.'); + return; + } - $DB->delete_records('tool_emailutils_suppression'); + $suppressionlist = $this->fetch_aws_ses_suppression_list(); + $this->update_local_suppression_list($suppressionlist); + } - foreach ($suppressionlist as $item) { - $record = new \stdClass(); - $record->email = $item['email']; - $record->reason = $item['reason']; - $record->created_at = $item['created_at']; - $record->timecreated = time(); - $record->timemodified = time(); + /** + * Check if the email suppression list feature is enabled. + * + * @return bool True if the feature is enabled, false otherwise. + * @throws \dml_exception + */ + protected function is_feature_enabled(): bool { + return (bool)get_config('tool_emailutils', 'enable_suppression_list'); + } - $DB->insert_record('tool_emailutils_suppression', $record); + /** + * Check the connection to AWS SES. + * + * @return bool True if the connection is successful, false otherwise. + */ + protected function check_connection(): bool { + try { + $client = $this->get_ses_client(); + $client->listSuppressedDestinations(['MaxItems' => 1]); + return true; + } catch (\Exception $e) { + return false; } } /** * Fetch the suppression list from AWS SES. * - * This method connects to AWS SES, retrieves the suppression list, - * and formats it for local storage. It includes error handling and - * retries for rate limiting. - * * @return array The fetched suppression list. */ protected function fetch_aws_ses_suppression_list(): array { - if (!$this->sesclient) { - $this->sesclient = $this->create_ses_client(); - } - try { $suppressionlist = []; $params = ['MaxItems' => 100]; do { - $result = $this->sesclient->listSuppressedDestinations($params); + $result = $this->get_ses_client()->listSuppressedDestinations($params); foreach ($result['SuppressedDestinationSummaries'] as $item) { $suppressionlist[] = [ 'email' => $item['EmailAddress'], @@ -113,55 +124,68 @@ protected function fetch_aws_ses_suppression_list(): array { return $suppressionlist; } catch (\Exception $e) { - $this->log_error('Error fetching suppression list: ' . $e->getMessage()); + debugging('Error fetching suppression list: ' . $e->getMessage()); return []; } } /** - * Create an SES client instance. + * Update the local suppression list with the fetched data. * - * @return \Aws\SesV2\SesV2Client + * @param array $suppressionlist The fetched suppression list. + * @throws \dml_exception */ - protected function create_ses_client(): \Aws\SesV2\SesV2Client { - global $CFG; + protected function update_local_suppression_list(array $suppressionlist): void { + global $DB; - $awsregion = get_config('tool_emailutils', 'aws_region'); - $awskey = get_config('tool_emailutils', 'aws_key'); - $awssecret = get_config('tool_emailutils', 'aws_secret'); + $DB->delete_records('tool_emailutils_suppression'); - if (empty($awsregion) || empty($awskey) || empty($awssecret)) { - throw new \Exception('AWS credentials are not configured.'); + foreach ($suppressionlist as $item) { + $record = new \stdClass(); + $record->email = $item['email']; + $record->reason = $item['reason']; + $record->created_at = $item['created_at']; + $record->timecreated = time(); + $record->timemodified = time(); + + $DB->insert_record('tool_emailutils_suppression', $record); } - return new \Aws\SesV2\SesV2Client([ - 'version' => 'latest', - 'region' => $awsregion, - 'credentials' => [ - 'key' => $awskey, - 'secret' => $awssecret, - ], - 'retries' => [ - 'mode' => 'adaptive', - 'max_attempts' => 10, - ], - ]); + mtrace('Suppression list updated successfully.'); } /** - * Log an error message, printing to console if running via CLI. - * - * This method logs error messages, ensuring they are visible both in - * the Moodle error log and on the console when run via CLI. + * Get or create the SES client. * - * @param string $message The error message to log. - * @return void + * @return \Aws\SesV2\SesV2Client|null + * @throws \dml_exception */ - private function log_error(string $message): void { - if (CLI_SCRIPT) { - mtrace($message); + protected function get_ses_client(): ?\Aws\SesV2\SesV2Client { + if (!$this->sesclient) { + $awsregion = get_config('tool_emailutils', 'aws_region'); + $awskey = get_config('tool_emailutils', 'aws_key'); + $awssecret = get_config('tool_emailutils', 'aws_secret'); + + if (empty($awsregion) || empty($awskey) || empty($awssecret)) { + debugging('AWS credentials are not configured.'); + return null; + } + + $this->sesclient = new \Aws\SesV2\SesV2Client([ + 'version' => 'latest', + 'region' => $awsregion, + 'credentials' => [ + 'key' => $awskey, + 'secret' => $awssecret, + ], + 'retries' => [ + 'mode' => 'adaptive', + 'max_attempts' => 10, + ], + ]); } - debugging($message); + + return $this->sesclient; } /** diff --git a/lang/en/tool_emailutils.php b/lang/en/tool_emailutils.php index ed77c90..d8b7bd2 100644 --- a/lang/en/tool_emailutils.php +++ b/lang/en/tool_emailutils.php @@ -64,6 +64,8 @@ $string['downloadsuppressionlist'] = 'Download Suppression List'; $string['enabled'] = 'Enabled'; $string['enabled_help'] = 'Allow the plugin to process incoming messages'; +$string['enable_suppression_list'] = 'Enable email suppression list'; +$string['enable_suppression_list_desc'] = 'When enabled, the system will maintain a suppression list of email addresses that should not receive emails.'; $string['event:notificationreceived'] = 'AWS SNS notification received'; $string['header'] = 'Header'; $string['header_help'] = 'HTTP Basic Auth Header'; diff --git a/settings.php b/settings.php index 9583ae5..e838bd4 100644 --- a/settings.php +++ b/settings.php @@ -78,6 +78,15 @@ new lang_string('enabled_help', 'tool_emailutils'), 0) ); + + // Add the enable_suppression_list setting. + $settings->add(new admin_setting_configcheckbox( + 'tool_emailutils/enable_suppression_list', + new lang_string('enable_suppression_list', 'tool_emailutils'), + new lang_string('enable_suppression_list_desc', 'tool_emailutils'), + 0) // Default to disabled + ); + // Auth Settings. $settings->add(new admin_setting_heading( 'authorisation', diff --git a/tests/suppressionlist_test.php b/tests/suppressionlist_test.php index 0ad61eb..ab6bff5 100644 --- a/tests/suppressionlist_test.php +++ b/tests/suppressionlist_test.php @@ -35,24 +35,17 @@ final class suppressionlist_test extends \advanced_testcase { /** - * Test the update of the suppression list and the generation of the CSV file. - * - * This test checks the following: - * 1. The suppression list is properly updated in the database from the mock AWS SES response. - * 2. The correct number of records (2 in this case) is added to the suppression table. - * 3. Each record has the correct email and reason as per the mock data. - * 4. A CSV file is generated with the correct headers and content matching the database. + * Set up the test environment and return a configured task. * - * @covers \tool_emailutils\task\update_suppression_list::execute - * @covers \tool_emailutils\suppression_list::generate_csv - * - * @return void + * @param bool $enablefeature Whether to enable the suppression list feature. + * @return \tool_emailutils\task\update_suppression_list */ - public function test_suppression_list_update_and_export(): void { - global $DB; - + protected function setup_test_environment(bool $enablefeature): \tool_emailutils\task\update_suppression_list { $this->resetAfterTest(true); + // Set the suppression list feature configuration. + set_config('enable_suppression_list', $enablefeature ? 1 : 0, 'tool_emailutils'); + // Set up a user with necessary permissions. $this->setAdminUser(); @@ -83,8 +76,36 @@ public function test_suppression_list_update_and_export(): void { $task = new \tool_emailutils\task\update_suppression_list(); $task->set_ses_client($mockclient); - // Execute the task. + return $task; + } + + /** + * Test the update of the suppression list and the generation of the CSV file. + * + * This test checks the following: + * 1. The suppression list is properly updated in the database from the mock AWS SES response. + * 2. The correct number of records (2 in this case) is added to the suppression table. + * 3. Each record has the correct email and reason as per the mock data. + * 4. A CSV file is generated with the correct headers and content matching the database. + * + * @covers \tool_emailutils\task\update_suppression_list::execute + * @covers \tool_emailutils\suppression_list::generate_csv + * + * @return void + * @throws \dml_exception + */ + public function test_suppression_list_update_and_export(): void { + global $DB; + + $task = $this->setup_test_environment(true); + + // Capture the output. + ob_start(); $task->execute(); + $output = ob_get_clean(); + + // Assert that the expected string is in the output. + $this->assertStringContainsString('Suppression list updated successfully.', $output); // Verify that the suppression list was updated in the database. $records = $DB->get_records('tool_emailutils_suppression'); @@ -116,4 +137,32 @@ public function test_suppression_list_update_and_export(): void { $this->assertStringContainsString('test2@example.com', $lines[2]); $this->assertStringContainsString('COMPLAINT', $lines[2]); } + + /** + * Test the update of the suppression list when the feature is disabled. + * + * This test checks the following: + * 1. The suppression list is not updated in the database. + * 2. The suppression list table is empty. + * + * @return void + * @throws \dml_exception + */ + public function test_suppression_list_update_when_disabled(): void { + global $DB; + + $task = $this->setup_test_environment(false); + + // Capture the output. + ob_start(); + $task->execute(); + $output = ob_get_clean(); + + // Assert that there is no output. + $this->assertEmpty($output); + + // Verify that the suppression list table is empty. + $records = $DB->get_records('tool_emailutils_suppression'); + $this->assertEmpty($records); + } }