From 62bac8a864343852e4265e344eeec89bed0fe2f0 Mon Sep 17 00:00:00 2001 From: waleedhassan Date: Mon, 7 Oct 2024 17:34:36 +0100 Subject: [PATCH] WR #439133: PR changes and updated the tests to be more meaningful Added changes asked by Mark on PR and also made some changes to the code to have more meaningful unit tests --- classes/suppression_list.php | 60 +++++++ classes/task/update_suppression_list.php | 119 +++++++------ db/install.xml | 6 +- db/tasks.php | 4 +- db/upgrade.php | 18 +- lang/en/tool_emailutils.php | 6 +- settings.php | 34 ++-- suppression_list.php | 47 ++--- tests/suppressionlist_test.php | 213 ++++++++--------------- 9 files changed, 247 insertions(+), 260 deletions(-) create mode 100644 classes/suppression_list.php diff --git a/classes/suppression_list.php b/classes/suppression_list.php new file mode 100644 index 0000000..38dde3e --- /dev/null +++ b/classes/suppression_list.php @@ -0,0 +1,60 @@ +. + +namespace tool_emailutils; + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->libdir . '/csvlib.class.php'); + +/** + * Class for handling suppression list operations. + * + * @package tool_emailutils + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class suppression_list { + + /** + * Generate CSV content for the suppression list. + * + * @return string The CSV content. + * @throws \dml_exception + */ + public static function generate_csv(): string { + global $DB; + + $csvexport = new \csv_export_writer('comma'); + + // Add CSV headers. + $csvexport->add_data(['Email', 'Reason', 'Created At']); + + // Fetch suppression list from database. + $suppressionlist = $DB->get_records('tool_emailutils_suppression'); + + // Add suppression list data to CSV. + foreach ($suppressionlist as $item) { + $csvexport->add_data([ + $item->email, + $item->reason, + $item->created_at, + ]); + } + + return $csvexport->print_csv_data(true); + } +} diff --git a/classes/task/update_suppression_list.php b/classes/task/update_suppression_list.php index 2efcc7b..26122c8 100644 --- a/classes/task/update_suppression_list.php +++ b/classes/task/update_suppression_list.php @@ -18,14 +18,13 @@ * Scheduled task for updating the email suppression list. * * @package tool_emailutils - * @copyright 2019 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @author Waleed ul hassan */ namespace tool_emailutils\task; -defined('MOODLE_INTERNAL') || die(); /** * Scheduled task class for updating the email suppression list. @@ -34,12 +33,15 @@ * the local database with this information. */ class update_suppression_list extends \core\task\scheduled_task { + /** @var \Aws\SesV2\SesV2Client|null */ + protected $sesclient = null; + /** * Return the task's name as shown in admin screens. * * @return string The name of the task. */ - public function get_name() { + public function get_name(): string { return get_string('task_update_suppression_list', 'tool_emailutils'); } @@ -51,7 +53,7 @@ public function get_name() { * * @return void */ - public function execute() { + public function execute(): void { global $DB; $suppressionlist = $this->fetch_aws_ses_suppression_list(); @@ -79,62 +81,17 @@ public function execute() { * * @return array The fetched suppression list. */ - protected function fetch_aws_ses_suppression_list() { - global $CFG; - require_once($CFG->dirroot . '/local/aws/sdk/aws-autoloader.php'); - require_once($CFG->dirroot . '/local/aws/sdk/Aws/SesV2/SesV2Client.php'); - - $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)) { - $this->log_error('AWS credentials are not configured. Please set them in the plugin settings.'); - return []; + protected function fetch_aws_ses_suppression_list(): array { + if (!$this->sesclient) { + $this->sesclient = $this->create_ses_client(); } try { - $sesv2 = new \Aws\SesV2\SesV2Client([ - 'version' => 'latest', - 'region' => $awsregion, - 'credentials' => [ - 'key' => $awskey, - 'secret' => $awssecret, - ], - 'retries' => [ - 'mode' => 'adaptive', - 'max_attempts' => 10, - ], - ]); - $suppressionlist = []; - $params = ['MaxItems' => 100]; // Reduced from 1000 to 100 to lower the chance of rate limiting. + $params = ['MaxItems' => 100]; do { - $retries = 0; - $maxretries = 5; - $delay = 1; - - while ($retries < $maxretries) { - try { - $result = $sesv2->listSuppressedDestinations($params); - break; // If successful, exit the retry loop. - } catch (\Aws\Exception\AwsException $e) { - if ($e->getAwsErrorCode() === 'TooManyRequestsException') { - $retries++; - if ($retries >= $maxretries) { - $this->log_error('Max retries reached for AWS SES API call: ' . $e->getMessage()); - return []; // Return empty array after max retries. - } - $this->log_error("Rate limit hit, retrying in {$delay} seconds..."); - sleep($delay); // Wait before retrying. - $delay *= 2; // Exponential backoff. - } else { - $this->log_error('AWS SES Error: ' . $e->getMessage()); - return []; // Return empty array for other AWS exceptions. - } - } - } + $result = $this->sesclient->listSuppressedDestinations($params); foreach ($result['SuppressedDestinationSummaries'] as $item) { $suppressionlist[] = [ 'email' => $item['EmailAddress'], @@ -142,17 +99,54 @@ protected function fetch_aws_ses_suppression_list() { 'created_at' => $item['LastUpdateTime']->format('Y-m-d H:i:s'), ]; } - $params['NextToken'] = $result['NextToken'] ?? null; } while ($params['NextToken']); return $suppressionlist; } catch (\Exception $e) { - $this->log_error('Unexpected error: ' . $e->getMessage()); + $this->log_error('Error fetching suppression list: ' . $e->getMessage()); return []; } } + /** + * Create an SES client instance. + * + * @return \Aws\SesV2\SesV2Client + */ + protected function create_ses_client(): \Aws\SesV2\SesV2Client { + global $CFG; + + 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.'); + } + } + + $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)) { + throw new \Exception('AWS credentials are not configured.'); + } + + return new \Aws\SesV2\SesV2Client([ + 'version' => 'latest', + 'region' => $awsregion, + 'credentials' => [ + 'key' => $awskey, + 'secret' => $awssecret, + ], + 'retries' => [ + 'mode' => 'adaptive', + 'max_attempts' => 10, + ], + ]); + } + /** * Log an error message, printing to console if running via CLI. * @@ -162,10 +156,19 @@ protected function fetch_aws_ses_suppression_list() { * @param string $message The error message to log. * @return void */ - private function log_error($message) { + private function log_error(string $message): void { if (CLI_SCRIPT) { mtrace($message); } debugging($message); } -} \ No newline at end of file + + /** + * Set the SES client (for testing purposes). + * + * @param \Aws\SesV2\SesV2Client $client + */ + public function set_ses_client(\Aws\SesV2\SesV2Client $client): void { + $this->sesclient = $client; + } +} diff --git a/db/install.xml b/db/install.xml index aa541f1..d640f8a 100644 --- a/db/install.xml +++ b/db/install.xml @@ -37,16 +37,14 @@ - + - - - diff --git a/db/tasks.php b/db/tasks.php index bcbc82b..26a1f94 100644 --- a/db/tasks.php +++ b/db/tasks.php @@ -18,7 +18,7 @@ * Definition of Email utils scheduled tasks. * * @package tool_emailutils - * @copyright 2019 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @author Waleed ul hassan */ @@ -29,7 +29,7 @@ [ 'classname' => 'tool_emailutils\task\update_suppression_list', 'blocking' => 0, - 'minute' => '0', + 'minute' => 'R', 'hour' => '3', 'day' => '*', 'month' => '*', diff --git a/db/upgrade.php b/db/upgrade.php index b69ac3d..15e4e93 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -18,13 +18,22 @@ * Email utils plugin upgrade code. * * @package tool_emailutils - * @copyright 2019 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @author Waleed ul hassan */ -defined('MOODLE_INTERNAL') || die(); - +/** + * Upgrade script for the email utilities tool plugin. + * + * This function is executed during the plugin upgrade process. + * It checks the current version of the plugin and applies + * necessary upgrades, such as creating new database tables + * or modifying existing structures. + * + * @param int $oldversion The version we are upgrading from. + * @return bool Always returns true. + */ function xmldb_tool_emailutils_upgrade($oldversion) { global $DB; $dbman = $DB->get_manager(); @@ -45,9 +54,6 @@ function xmldb_tool_emailutils_upgrade($oldversion) { // Adding keys to table tool_emailutils_suppression. $table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']); - // Adding indexes to table tool_emailutils_suppression. - $table->add_index('email', XMLDB_INDEX_UNIQUE, ['email']); - // Conditionally launch create table for tool_emailutils_suppression. if (!$dbman->table_exists($table)) { $dbman->create_table($table); diff --git a/lang/en/tool_emailutils.php b/lang/en/tool_emailutils.php index b3f058e..fbc8fbd 100644 --- a/lang/en/tool_emailutils.php +++ b/lang/en/tool_emailutils.php @@ -73,16 +73,16 @@ $string['password'] = 'Password'; $string['password_help'] = 'HTTP Basic Auth Password - Leave empty if you\'re not changing the password'; $string['pluginname'] = 'Email utilities'; -$string['postmastertools'] = 'Post master tools'; $string['postmastergoogletoken'] = 'Post master tools google-site-verification'; $string['postmastergoogletoken_help'] = 'This is the entire TXT DNS record and looks similar to google-site-verification=abcdef123456789abcdef'; +$string['postmastertools'] = 'Post master tools'; $string['privacy:metadata:tool_emailutils_list'] = 'Information.'; $string['privacy:metadata:tool_emailutils_list:updatedid'] = 'The ID of updated user.'; $string['privacy:metadata:tool_emailutils_list:userid'] = 'The ID of the user.'; $string['resetbounces'] = 'Reset the number of bounces'; -$string['selectoractive'] = 'Active selector'; $string['selectoractivate'] = 'Activate key pair'; $string['selectoractivateconfirm'] = 'This will set $CFG->emaildkimselector to this selector and it will be used for signing outgoing emails.'; +$string['selectoractive'] = 'Active selector'; $string['selectoractivated'] = 'Selector was activated'; $string['selectorcreate'] = 'Create a new domain:selector certificate pair'; $string['selectorcreated'] = 'A new certificate pair has been created'; @@ -102,4 +102,4 @@ $string['suppressionlistdesc'] = 'Download the email suppression list from AWS SES for your account.'; $string['task_update_suppression_list'] = 'Update email suppression list'; $string['username'] = 'Username'; -$string['username_help'] = 'HTTP Basic Auth Username'; \ No newline at end of file +$string['username_help'] = 'HTTP Basic Auth Username'; diff --git a/settings.php b/settings.php index e40ffba..9583ae5 100644 --- a/settings.php +++ b/settings.php @@ -114,20 +114,26 @@ // Add AWS credentials settings. - $settings->add(new admin_setting_configtext('tool_emailutils/aws_region', - get_string('aws_region', 'tool_emailutils'), - get_string('aws_region_desc', 'tool_emailutils'), - '', PARAM_TEXT)); - - $settings->add(new admin_setting_configtext('tool_emailutils/aws_key', - get_string('aws_key', 'tool_emailutils'), - get_string('aws_key_desc', 'tool_emailutils'), - '', PARAM_TEXT)); - - $settings->add(new admin_setting_configpasswordunmask('tool_emailutils/aws_secret', - get_string('aws_secret', 'tool_emailutils'), - get_string('aws_secret_desc', 'tool_emailutils'), - '')); + $settings->add(new admin_setting_configtext( + 'tool_emailutils/aws_region', + new lang_string('aws_region', 'tool_emailutils'), + new lang_string('aws_region_desc', 'tool_emailutils'), + '', PARAM_TEXT) + ); + + $settings->add(new admin_setting_configtext( + 'tool_emailutils/aws_key', + new lang_string('aws_key', 'tool_emailutils'), + new lang_string('aws_key_desc', 'tool_emailutils'), + '', PARAM_TEXT) + ); + + $settings->add(new admin_setting_configpasswordunmask( + 'tool_emailutils/aws_secret', + new lang_string('aws_secret', 'tool_emailutils'), + new lang_string('aws_secret_desc', 'tool_emailutils'), + '') + ); $ADMIN->add('tool_emailutils', $settings); } diff --git a/suppression_list.php b/suppression_list.php index 0634731..d28a7cc 100644 --- a/suppression_list.php +++ b/suppression_list.php @@ -18,7 +18,7 @@ * Email suppression list download page. * * @package tool_emailutils - * @copyright 2019 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @author Waleed ul hassan */ @@ -26,15 +26,23 @@ require_once(__DIR__ . '/../../../config.php'); require_once($CFG->libdir . '/adminlib.php'); -admin_externalpage_setup('toolemailutilssuppressionlist'); +// Ensure the user is logged in and has the necessary permissions. +require_login(); +require_capability('moodle/site:config', context_system::instance()); $action = optional_param('action', '', PARAM_ALPHA); if ($action === 'download') { - // Generate and download CSV file. - generate_suppression_list_csv(); + $content = \tool_emailutils\suppression_list::generate_csv(); + $filename = 'email_suppression_list_' . date('Y-m-d') . '.csv'; + header('Content-Type: text/csv'); + header('Content-Disposition: attachment; filename="' . $filename . '"'); + header('Content-Length: ' . strlen($content)); + echo $content; + exit; } else { // Display the download page. + admin_externalpage_setup('toolemailutilssuppressionlist'); echo $OUTPUT->header(); echo $OUTPUT->heading(get_string('suppressionlist', 'tool_emailutils')); @@ -49,34 +57,3 @@ echo $OUTPUT->footer(); } - -/** - * Generate and download the suppression list CSV file. - */ -function generate_suppression_list_csv() { - global $CFG, $DB; - require_once($CFG->libdir . '/csvlib.class.php'); - - $filename = 'email_suppression_list_' . date('Y-m-d') . '.csv'; - $csvexport = new csv_export_writer(); - $csvexport->set_filename($filename); - - // Add CSV headers. - $csvexport->add_data(['Email', 'Reason', 'Created At']); - - // Fetch suppression list from database. - $suppressionlist = $DB->get_records('tool_emailutils_suppression'); - - // Add suppression list data to CSV. - foreach ($suppressionlist as $item) { - $csvexport->add_data([ - $item->email, - $item->reason, - $item->created_at, - ]); - } - - $csvexport->download_file(); - exit; -} - diff --git a/tests/suppressionlist_test.php b/tests/suppressionlist_test.php index a642136..86b0868 100644 --- a/tests/suppressionlist_test.php +++ b/tests/suppressionlist_test.php @@ -14,161 +14,98 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +namespace tool_emailutils; + + /** - * Unit tests for suppression list functionality. + * Test case for suppression list functionality. * * @package tool_emailutils - * @copyright 2019 onwards Catalyst IT {@link http://www.catalyst-eu.net/} + * @copyright 2024 onwards Catalyst IT {@link http://www.catalyst-eu.net/} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - * @author Waleed ul hassan */ - -defined('MOODLE_INTERNAL') || die(); - -global $CFG; -require_once($CFG->dirroot . '/admin/tool/emailutils/classes/task/update_suppression_list.php'); - -/** - * Test cases for suppression list functionality. - */ -class suppressionlist_test extends advanced_testcase { +final class suppressionlist_test extends \advanced_testcase { /** - * Test the creation and retrieval of suppression list entries. + * 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. * - * This test covers the basic CRUD (Create, Read, Update, Delete) operations - * for the suppression list database table. + * @covers \tool_emailutils\task\update_suppression_list::execute + * @covers \tool_emailutils\suppression_list::generate_csv * + * @return void */ - public function test_suppression_list_crud() { + public function test_suppression_list_update_and_export(): void { global $DB; $this->resetAfterTest(true); - // Create some test data. - $testdata = [ - ['email' => 'test1@example.com', 'reason' => 'Bounce', 'created_at' => '2024-03-01 10:00:00'], - ['email' => 'test2@example.com', 'reason' => 'Complaint', 'created_at' => '2024-03-02 11:00:00'], - ]; - - // Insert test data. - foreach ($testdata as $data) { - $record = new stdClass(); - $record->email = $data['email']; - $record->reason = $data['reason']; - $record->created_at = $data['created_at']; - $record->timecreated = time(); - $record->timemodified = time(); - - $DB->insert_record('tool_emailutils_suppression', $record); - } - - // Test retrieval. + // Set up a user with necessary permissions. + $this->setAdminUser(); + + // Create a mock command result. + $mockresult = new \Aws\Result([ + 'SuppressedDestinationSummaries' => [ + [ + 'EmailAddress' => 'test1@example.com', + 'Reason' => 'BOUNCE', + 'LastUpdateTime' => new \DateTime('2024-03-01 10:00:00'), + ], + [ + 'EmailAddress' => 'test2@example.com', + 'Reason' => 'COMPLAINT', + 'LastUpdateTime' => new \DateTime('2024-03-02 11:00:00'), + ], + ], + 'NextToken' => null, + ]); + + // Create a mock SES client. + $mockclient = $this->createMock(\Aws\SesV2\SesV2Client::class); + $mockclient->method('__call') + ->with($this->equalTo('listSuppressedDestinations'), $this->anything()) + ->willReturn($mockresult); + + // Create the task and set the mock client. + $task = new \tool_emailutils\task\update_suppression_list(); + $task->set_ses_client($mockclient); + + // Execute the task. + $task->execute(); + + // Verify that the suppression list was updated in the database. $records = $DB->get_records('tool_emailutils_suppression'); $this->assertCount(2, $records); - // Test specific record retrieval. - $record = $DB->get_record('tool_emailutils_suppression', ['email' => 'test1@example.com']); - $this->assertEquals('Bounce', $record->reason); - - // Test update. - $record->reason = 'Updated Reason'; - $DB->update_record('tool_emailutils_suppression', $record); - $updatedrecord = $DB->get_record('tool_emailutils_suppression', ['email' => 'test1@example.com']); - $this->assertEquals('Updated Reason', $updatedrecord->reason); - - // Test delete. - $DB->delete_records('tool_emailutils_suppression', ['email' => 'test2@example.com']); - $remainingrecords = $DB->get_records('tool_emailutils_suppression'); - $this->assertCount(1, $remainingrecords); - } - - /** - * Test the scheduled task for updating the suppression list. - * - * This test creates a mock task that overrides the AWS SES client - * to return predefined test data. It then executes the task and - * verifies that the suppression list in the database is updated correctly. - * - * @return void - * @covers \tool_emailutils\task\update_suppression_list::execute - * @covers \tool_emailutils\task\update_suppression_list::fetch_aws_ses_suppression_list - * @throws dml_exception - */ - public function test_update_suppression_list_task(): void { - global $DB; - $this->resetAfterTest(true); - - // Set up mock AWS credentials (these won't be used, but let's set them anyway). - set_config('aws_region', 'eu-west-2', 'tool_emailutils'); - set_config('aws_key', 'testkey', 'tool_emailutils'); - set_config('aws_secret', 'testsecret', 'tool_emailutils'); - - $mocktask = new class extends \tool_emailutils\task\update_suppression_list { - /** - * Override the execute method to use our mocked data. - * - * @return void - */ - public function execute(): void { - global $DB; - $suppressionlist = $this->fetch_aws_ses_suppression_list(); - $DB->delete_records('tool_emailutils_suppression'); - 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); - } - } - - /** - * Override the fetch method to return mock data. - * - * @return array Mock suppression list data. - */ - protected function fetch_aws_ses_suppression_list(): array { - return [ - [ - 'email' => 'suppressed@example.com', - 'reason' => 'BOUNCE', - 'created_at' => '2024-03-03 12:00:00', - ], - ]; + // Check if the records exist and have the correct data. + $foundtest1 = false; + $foundtest2 = false; + foreach ($records as $record) { + if ($record->email === 'test1@example.com') { + $this->assertEquals('BOUNCE', $record->reason); + $foundtest1 = true; + } else if ($record->email === 'test2@example.com') { + $this->assertEquals('COMPLAINT', $record->reason); + $foundtest2 = true; } - }; - - // Execute the mock task. - $mocktask->execute(); - - // Check if the suppression list was updated in the database. - $record = $DB->get_record('tool_emailutils_suppression', ['email' => 'suppressed@example.com']); - $this->assertNotFalse($record); - $this->assertEquals('BOUNCE', $record->reason); - $this->assertEquals('2024-03-03 12:00:00', $record->created_at); - } - - /** - * Test AWS credentials configuration. - * - * This test verifies that AWS credentials can be correctly set and retrieved - * from the Moodle configuration. - * - */ - public function test_aws_credentials_config() { - $this->resetAfterTest(true); - - // Set test configuration. - set_config('aws_region', 'eu-west-2', 'tool_emailutils'); - set_config('aws_key', 'testkey', 'tool_emailutils'); - set_config('aws_secret', 'testsecret', 'tool_emailutils'); - - // Verify the configuration. - $this->assertEquals('eu-west-2', get_config('tool_emailutils', 'aws_region')); - $this->assertEquals('testkey', get_config('tool_emailutils', 'aws_key')); - $this->assertEquals('testsecret', get_config('tool_emailutils', 'aws_secret')); + } + $this->assertTrue($foundtest1, 'test1@example.com not found in the database'); + $this->assertTrue($foundtest2, 'test2@example.com not found in the database'); + + // Now test the CSV file generation. + $csvcontent = \tool_emailutils\suppression_list::generate_csv(); + + // Verify the CSV content. + $lines = explode("\n", trim($csvcontent)); + $this->assertEquals('Email,Reason,"Created At"', $lines[0]); + $this->assertStringContainsString('test1@example.com', $lines[1]); + $this->assertStringContainsString('BOUNCE', $lines[1]); + $this->assertStringContainsString('test2@example.com', $lines[2]); + $this->assertStringContainsString('COMPLAINT', $lines[2]); } }