(REF) Move CIVICRM_MAIL_LOG logic from patch-files to wrapper-class

Overview
--------

This aims to improve maintainablity and deployment workflows by reduing the
scope of the needed files.

It is an alternative approach for #16481.

Before
------

`civicrm-core` included patch files for `pear/mail`.  These files modified
the behavior of `Mail_sendmail`, `Mail_smtp`, and `Mail_mail` to ensure that
the `send()` function would abide by the `CIVICRM_MAIL_LOG` constant.

After
-----

`civicrm-core` includes a wrapper class `LoggingMailer`.  It takes an
instance of `Mail_sendmail`, `Mail_smtp`, or `Mail_mail` and mixes-in the
`CIVICRM_MAIL_LOG` logic.

Technical Details
-----------------

There is a `hook_civicrm_alterMailer`, for which consumers could potentially
see a change in behavior (because the concrete-class of `$mailer` may
change and support different properties). Mitigating factors:

* There is only one public method in the `Mail` contract: `send()`.
* The only example of `alterMailer` in the documentation suggests
  that you wholly replace the object with your own implementation.
  This was the original purpose. If you use it this way, then it doesn't
  matter what's happening inside the old object.
  https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_alterMailer/
* In public `universe`, there is only one extension (Cividesk's Sparkpost)
  which uses this hook. And it does swap in that expected way.
* Just in case... the `LoggingMailer` passes-through all calls to read/write properties.
  You can see this in action in a specific configuration, e.g.
  ```
  ## Before patch
  $ cv ev '$m=Civi::service("pear_mail"); return [get_class($m), $m->host];'
  [
      "Mail_smtp",
      "127.0.0.1"
  ]

  ## After patch
  $ cv ev '$m=Civi::service("pear_mail"); return [get_class($m), $m->host];'
  [
      "CRM_Utils_Mail_LoggingMailer",
      "127.0.0.1"
  ]
  ```

diff --git a/CRM/Utils/Mail.php b/CRM/Utils/Mail.php
index b9aa6ef617e2662fd423a5872899e63d73cecffd..c6a982c587bb9d4ad361da13e626f39f24abe22c 100644 (file)
--- a/CRM/Utils/Mail.php
+++ b/CRM/Utils/Mail.php
@@ -123,6 +123,10 @@ 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);
+      }
     }
     CRM_Utils_Hook::alterMailer($mailer, $driver, $params);
     return $mailer;

diff --git a/CRM/Utils/Mail/LoggingMailer.php b/CRM/Utils/Mail/LoggingMailer.php
new file mode 100644 (file)
index 0000000..3f7b59b
--- /dev/null
+++ b/CRM/Utils/Mail/LoggingMailer.php
@@ -0,0 +1,67 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ * The logging-mailer is a utility to wrap an existing PEAR Mail class
+ * and apply extra logging functionality.
+ *
+ * It replaces a set of patches which had been previously applied directly
+ * to a few specific PEAR Mail classes.
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+class CRM_Utils_Mail_LoggingMailer extends Mail {
+
+  /**
+   * @var Mail
+   */
+  protected $delegate;
+
+  /**
+   * @param Mail $delegate
+   */
+  public function __construct($delegate) {
+    $this->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/tools/scripts/composer/patches/pear-mail.patch.txt b/tools/scripts/composer/patches/pear-mail.patch.txt
index e2f488dd33105a782adbba3ac24052c790aab623..23fa638c9d40e9a2c186211ec8ef29faa4256c67 100644 (file)
--- a/tools/scripts/composer/patches/pear-mail.patch.txt
+++ b/tools/scripts/composer/patches/pear-mail.patch.txt
@@ -17,21 +17,6 @@ diff --git a/Mail/mail.php b/Mail/mail.php
 index ee1ecef..ae6e2e8 100644
 --- a/Mail/mail.php
 +++ b/Mail/mail.php
-@@ -114,6 +114,14 @@ class Mail_mail extends Mail {
-      */
-     public function send($recipients, $headers, $body)
-     {
-+        if (defined('CIVICRM_MAIL_LOG')) {
-+            CRM_Utils_Mail::logger($recipients, $headers, $body);
-+            // Note: "CIVICRM_MAIL_LOG_AND SEND" (space not underscore) was a typo that existed for some years, so kept here for compatibility, but it should not be used.
-+            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');
-         }
 @@ -145,7 +153,12 @@ class Mail_mail extends Mail {
          if (is_a($headerElements, 'PEAR_Error')) {
              return $headerElements;
@@ -46,41 +31,3 @@ index ee1ecef..ae6e2e8 100644
  
          // We only use mail()'s optional fifth parameter if the additional
          // parameters have been provided and we're not running in safe mode.
-diff --git a/Mail/sendmail.php b/Mail/sendmail.php
-index 7e8f804..e0300a0 100644
---- a/Mail/sendmail.php
-+++ b/Mail/sendmail.php
-@@ -132,6 +132,14 @@ class Mail_sendmail extends Mail {
-      */
-     public function send($recipients, $headers, $body)
-     {
-+        if (defined('CIVICRM_MAIL_LOG')) {
-+            CRM_Utils_Mail::logger($recipients, $headers, $body);
-+            // Note: "CIVICRM_MAIL_LOG_AND SEND" (space not underscore) was a typo that existed for some years, so kept here for compatibility, but it should not be used.
-+            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');
-         }
-diff --git a/Mail/smtp.php b/Mail/smtp.php
-index 5e698fe..5f057e2 100644
---- a/Mail/smtp.php
-+++ b/Mail/smtp.php
-@@ -255,6 +255,14 @@ class Mail_smtp extends Mail {
-      */
-     public function send($recipients, $headers, $body)
-     {
-+        if (defined('CIVICRM_MAIL_LOG')) {
-+            CRM_Utils_Mail::logger($recipients, $headers, $body);
-+            // Note: "CIVICRM_MAIL_LOG_AND SEND" (space not underscore) was a typo that existed for some years, so kept here for compatibility, but it should not be used.
-+            if (!defined('CIVICRM_MAIL_LOG_AND_SEND') && !defined('CIVICRM_MAIL_LOG_AND SEND')) {
-+                return true;
-+            }
-+        }
-+
-         $result = $this->send_or_fail($recipients, $headers, $body);
- 
-         /* If persistent connections are disabled, destroy our SMTP object. */