Move heavy lifting of wp_mail() to child class of PHPMailer

[stephenharris]

If a plug-in is sending an e-mail, the class PHPMailer has a lot of useful methods (e.g. addStringAttachment()), but these are not available when using wp_mail(), which is a requirement to work with numerous other plug-ins owing to the hooks it triggers.

wp_mail() does a number of things:

1. Instantiates a PHPMailer instance
2. Sets default values (e.g. "from" headers, charset, content type etc. )
3. Parses the passed arguments and feeds it to the PHPMalier instance.
4. Executes a "pre-send" routine, (e.g. triggering hooks wp_mail_from, phpmailer_init etc)
5. Sends the e-mail

The attached patch does a number of things:

1. Defines a WPMailer class ( a child of PHPMailer)
2. Defines a WPMailerFactory class which creates an instance with appropriate default values
3. Defines 'helper' methods which do the 'heavy lifting' of (3) above
4. Overrides the preSend method of PHPMailer to execute the 'pre-send routine' present in wp_mail() (i.e. (4) above)
5. Refactors wp_mail() to "operate" WPMailer() instance

The result is that developers can either use wp_mail() or $wpmailer = WPMailerFactory::getMailer() to send e-mails, and both will behave identically (in terms of default values, and hooks being triggered), while maintaining backwards compatibility.

This would also effectively close tickets #28407, #28059, #23291, #15539 , #11376 and maybe others.

Remarks

Why just not use phpmailer_init?
This hook is very useful, but offers no context in which wp_mail() is called. As an example, suppose a plug-ins sends an e-mail with an "on-the-fly" purchase receipt attached. At phpmailer_init I don't know the purchase ID from which to generate and attach the receipt.

Class/method naming standards
I've used PHPMailer's naming standards which I understand conflicts slightly with WordPress' naming standards. A future iteration of this patch could well change this if that is deemed best.

Global $phpmailer
The global $phpmailer is redundant, as the factory creates a fresh instance for each request. Or at least it would. The only reason the patch still uses this global, is that all the relevant unit tests pass without any further changes. Subject to this ticket being accepted in principle, these tests should be updated along with the patch.

Backwards compatability
Assuming wp_mail() hasn't been overriden by a plug-in/theme, then the is no change in behaviour. If it has been overridden, it's clear from the original function that the $_GLOBAL['phpmailer'] should not be expected to exist, nor even the required classes to be loaded. As such they can be expected to operate independently of the changes made here, which are non-destructive.

Uni tests
For me, the mail group unit tests pass with 1 skipped. For some reason some tests failed (e.g. Tests_DB::test_bail()), but these failed even without this patch.

[SergeyBiryukov]

#29673 was marked as a duplicate.

[wonderboymusic]

This looks interesting - if someone decides to pick this up, could be 4.1

[jorbin]

Patch needs to be refreshed. Also needs a good review including seeing if there was a good reason for not using $phpmailer more in the begining (I doubt the commit or tickets will tell us much, but at least finding them and linking them here would be helpfull).

[stephenharris]

Patch ported to 4.2

[stephenharris]

Latest patch passes all unit tests (with 47 skipped). All mail component tests pass.

@jorbin The patch still uses the global $phpmailer. I personally think its a good opportunity to get rid of a global but happy either way. I'll dig a little re. why originally a new instance wasn't created for each e-mail, but I suspect it might be concerns relating to performance. Perhaps constructing a PHPMailer instance is expensive(?). I'll do some benchmark tests.

[meitar]

IIUC, this patch will enable the use of PHPMailer::addStringAttachment() for plugins that make use of the wp_mail filter hook? That's something I need for a plugin I'm working on (​see here), so +1 from me.

[stephenharris]

@meitar Sorry, seemed to have missed the notification for you're reply. But yes, the original motivation of the ticket was to expose more PHPMailer methods.

[stephenharris]

#36751 was marked as a duplicate.

[stephenharris]

Refreshed patch

[stephenharris]

The global $phpmailer was introduced when PHPMailer was introduced in this change [3862]. I can't determine as to why it was implemented like that, but in the interest of backwards compatibility I'm tempted to say leave it.

What I think we should probably consider though, is destroying and recreating a new PHPMailer instance each time. This would fix #33972.

In addition to the tickets mentioned in the description, this would also provide a work-around for #31775

[drzraf]

Still an acute problem and the only way to process outgoing email attachments which is a major issue.

Patch does not apply anymore.

[desrosj]

The patch here needs to be refreshed.