dev/core#1717 - Fix SMTP failure involving disconnect() method

[seamuslee001]

Overview

v5.24 introduced a new Mailer utility, CRM_Utils_Mail_FilteredPearMailer, which is essentially a wrapper around Mail_Smtp, Mail_mail and Mail_sendmail. This allowed us to inject some of our customisation on top of PEAR Mail (and remove some patch-files).

However, FilteredPearMailer did not implement the SMTP-specific disconnect method (used by some error-handling code for flexmailer and Mailing BAO - https://github.com/civicrm/civicrm-core/blob/master/CRM/Mailing/BAO/MailingJob.php#L668).

Before

Disconnect method not implemented on wrapper factory

After

Disconnect method is implemented

ping @eileenmcnaughton @totten @aydun @jaapjansma

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

• I think this makes sense. The pre-existing class hierarchy is a little off, but meh. This change is KISS. 👍
• A slightly more aggressive fix might be to delegate any other public methods that occur in the Mail class hierarchy. I grepped vendor/pear/mail, and it's a fairly short list. (Mail/smtp.php has getSMTPObject and addServiceExtensionParameter. The little/never used Mail/smtpmx.php has _getMx, _loadNetDns, and _raiseError - which appear internal anyway.) Using function __call feels maybe a little more precise. But again, meh. I grepped universe and couldn't find anything which used these. 🤷‍♂️

[eileenmcnaughton]

@seamuslee001 I'm happy for you to merge this after considering @totten's feedback

[seamuslee001]

@totten I have added the if wrapper around and made the function return True otherwise

[seamuslee001]

Merging as Tim's feedback has been incoprorated now