From dee6e58227627540766b1ea80abba046987e7776 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 10 Feb 2020 17:48:50 -0800 Subject: [PATCH] (REF) CRM_Utils_Mail - Split LoggingMailer into FilteredPearMailer and Logger 1. Captures more context (i.e. the original driver and params) 2. Changes various property names to avoid potential for conflict with delegated properties 3. Adds `addFilter($id, $func)` so that one can move more filters in here 4. Consolidates the various references ot CIVICRM_MAIL_LOG* into one file. --- CRM/Utils/Mail.php | 43 ++----- CRM/Utils/Mail/FilteredPearMailer.php | 112 ++++++++++++++++++ CRM/Utils/Mail/Logger.php | 76 ++++++++++++ CRM/Utils/Mail/LoggingMailer.php | 67 ----------- .../CRM/Utils/Mail/FilteredPearMailerTest.php | 54 +++++++++ 5 files changed, 255 insertions(+), 97 deletions(-) create mode 100644 CRM/Utils/Mail/FilteredPearMailer.php create mode 100644 CRM/Utils/Mail/Logger.php delete mode 100644 CRM/Utils/Mail/LoggingMailer.php create mode 100644 tests/phpunit/CRM/Utils/Mail/FilteredPearMailerTest.php diff --git a/CRM/Utils/Mail.php b/CRM/Utils/Mail.php index c6a982c587..02dc32cd31 100644 --- a/CRM/Utils/Mail.php +++ b/CRM/Utils/Mail.php @@ -123,10 +123,17 @@ class CRM_Utils_Mail { } else { $mailer = Mail::factory($driver, $params); - // Previously, CiviCRM bundled patches to change the behavior of these three classes. Use a decorator to avoid patching. - if ($mailer instanceof Mail_smtp || $mailer instanceof Mail_mail || $mailer instanceof Mail_sendmail) { - $mailer = new CRM_Utils_Mail_LoggingMailer($mailer); - } + } + + // Previously, CiviCRM bundled patches to change the behavior of 3 specific drivers. Use wrapper/filters to avoid patching. + $mailer = new CRM_Utils_Mail_FilteredPearMailer($driver, $params, $mailer); + if (in_array($driver, ['smtp', 'mail', 'sendmail'])) { + $mailer->addFilter('2000_log', ['CRM_Utils_Mail_Logger', 'filter']); + $mailer->addFilter('2100_validate', function ($mailer, &$recipients, &$headers, &$body) { + if (!is_array($headers)) { + return PEAR::raiseError('$headers must be an array'); + } + }); } CRM_Utils_Hook::alterMailer($mailer, $driver, $params); return $mailer; @@ -330,34 +337,10 @@ class CRM_Utils_Mail { * @param $to * @param $headers * @param $message + * @deprecated */ public static function logger(&$to, &$headers, &$message) { - if (is_array($to)) { - $toString = implode(', ', $to); - $fileName = $to[0]; - } - else { - $toString = $fileName = $to; - } - $content = "To: " . $toString . "\n"; - foreach ($headers as $key => $val) { - $content .= "$key: $val\n"; - } - $content .= "\n" . $message . "\n"; - - if (is_numeric(CIVICRM_MAIL_LOG)) { - $config = CRM_Core_Config::singleton(); - // create the directory if not there - $dirName = $config->configAndLogDir . 'mail' . DIRECTORY_SEPARATOR; - CRM_Utils_File::createDir($dirName); - $fileName = md5(uniqid(CRM_Utils_String::munge($fileName))) . '.txt'; - file_put_contents($dirName . $fileName, - $content - ); - } - else { - file_put_contents(CIVICRM_MAIL_LOG, $content, FILE_APPEND); - } + CRM_Utils_Mail_Logger::log($to, $headers, $message); } /** diff --git a/CRM/Utils/Mail/FilteredPearMailer.php b/CRM/Utils/Mail/FilteredPearMailer.php new file mode 100644 index 0000000000..1c11e23d16 --- /dev/null +++ b/CRM/Utils/Mail/FilteredPearMailer.php @@ -0,0 +1,112 @@ +_driver = $driver; + $this->_params = $params; + $this->_delegate = $mailer; + } + + public function send($recipients, $headers, $body) { + $filterArgs = [$this, &$recipients, &$headers, &$body]; + foreach ($this->_filters as $filter) { + $result = call_user_func_array($filter, $filterArgs); + if ($result !== NULL) { + return $result; + } + } + + return $this->_delegate->send($recipients, $headers, $body); + } + + /** + * @param string $id + * Unique ID for this filter. Filters are sorted by ID. + * Suggestion: '{nnnn}_{name}', where '{nnnn}' is a number. + * Filters are sorted and executed in order. + * @param callable $func + * function(FilteredPearMailer $mailer, mixed $recipients, array $headers, string $body). + * The return value should generally be null/void. However, if you wish to + * short-circuit execution of the filters, then return a concrete value. + * @return static + */ + public function addFilter($id, $func) { + $this->_filters[$id] = $func; + ksort($this->_filters); + return $this; + } + + /** + * @return string + * Ex: 'smtp', 'sendmail', 'mail'. + */ + public function getDriver() { + return $this->_driver; + } + + public function &__get($name) { + return $this->_delegate->{$name}; + } + + public function __set($name, $value) { + return $this->_delegate->{$name} = $value; + } + + public function __isset($name) { + return isset($this->_delegate->{$name}); + } + + public function __unset($name) { + unset($this->_delegate->{$name}); + } + +} diff --git a/CRM/Utils/Mail/Logger.php b/CRM/Utils/Mail/Logger.php new file mode 100644 index 0000000000..453554eddd --- /dev/null +++ b/CRM/Utils/Mail/Logger.php @@ -0,0 +1,76 @@ + $val) { + $content .= "$key: $val\n"; + } + $content .= "\n" . $message . "\n"; + + if (is_numeric(CIVICRM_MAIL_LOG)) { + $config = CRM_Core_Config::singleton(); + // create the directory if not there + $dirName = $config->configAndLogDir . 'mail' . DIRECTORY_SEPARATOR; + CRM_Utils_File::createDir($dirName); + $fileName = md5(uniqid(CRM_Utils_String::munge($fileName))) . '.txt'; + file_put_contents($dirName . $fileName, + $content + ); + } + else { + file_put_contents(CIVICRM_MAIL_LOG, $content, FILE_APPEND); + } + } + +} diff --git a/CRM/Utils/Mail/LoggingMailer.php b/CRM/Utils/Mail/LoggingMailer.php deleted file mode 100644 index 3f7b59b899..0000000000 --- a/CRM/Utils/Mail/LoggingMailer.php +++ /dev/null @@ -1,67 +0,0 @@ -delegate = $delegate; - } - - public function send($recipients, $headers, $body) { - if (defined('CIVICRM_MAIL_LOG')) { - CRM_Utils_Mail::logger($recipients, $headers, $body); - if (!defined('CIVICRM_MAIL_LOG_AND_SEND') && !defined('CIVICRM_MAIL_LOG_AND SEND')) { - return TRUE; - } - } - - if (!is_array($headers)) { - return PEAR::raiseError('$headers must be an array'); - } - - return $this->delegate->send($recipients, $headers, $body); - } - - public function &__get($name) { - return $this->delegate->{$name}; - } - - public function __set($name, $value) { - return $this->delegate->{$name} = $value; - } - - public function __isset($name) { - return isset($this->delegate->{$name}); - } - - public function __unset($name) { - unset($this->delegate->{$name}); - } - -} diff --git a/tests/phpunit/CRM/Utils/Mail/FilteredPearMailerTest.php b/tests/phpunit/CRM/Utils/Mail/FilteredPearMailerTest.php new file mode 100644 index 0000000000..56b257c9d6 --- /dev/null +++ b/tests/phpunit/CRM/Utils/Mail/FilteredPearMailerTest.php @@ -0,0 +1,54 @@ +buf['recipients'] = $recipients; + $this->buf['headers'] = $headers; + $this->buf['body'] = $body; + return 'all the fruits in the basket'; + } + + }; + + $fm = new CRM_Utils_Mail_FilteredPearMailer('mock', [], $mock); + $fm->addFilter('1000_apple', function ($mailer, &$recipients, &$headers, &$body) { + $body .= ' with apples!'; + }); + $fm->addFilter('1000_banana', function ($mailer, &$recipients, &$headers, &$body) { + $headers['Banana'] = 'Cavendish'; + }); + $r = $fm->send(['recip'], ['Subject' => 'Fruit loops'], 'body'); + + $this->assertEquals('Fruit loops', $mock->buf['headers']['Subject']); + $this->assertEquals('Cavendish', $mock->buf['headers']['Banana']); + $this->assertEquals('body with apples!', $mock->buf['body']); + $this->assertEquals('all the fruits in the basket', $r); + } + + public function testFilter_shortCircuit() { + $mock = new class() extends \Mail { + + public function send($recipients, $headers, $body) { + return 'all the fruits in the basket'; + } + + }; + + $fm = new CRM_Utils_Mail_FilteredPearMailer('mock', [], $mock); + $fm->addFilter('1000_short_circuit', function ($mailer, &$recipients, &$headers, &$body) { + return 'the triumph of veggies over fruits'; + }); + $r = $fm->send(['recip'], ['Subject' => 'Fruit loops'], 'body'); + $this->assertEquals('the triumph of veggies over fruits', $r); + } + +} -- 2.25.1