Convert drupal_mail_system() to a MailFactory service to allow more flexible replacements

It sounds like you're referring to #1135262: drupal_mail() should support using different MailSystemInterface classes for format() and mail() and maybe #1089956: Provide Administrative UI and Developers API for safely setting the mail_system variable..

I'm thinking, at this point, those two are probably out of scope for 8.x.

This is the minimal implementation that doesn't introduce any API changes.

Completely untested :)

Will update the issue summary to explain why this is useful.

Updating title for the updated focus of this issue

@@ -200,98 +200,21 @@ function drupal_mail($module, $key, $to, $langcode, $params = array(), $from = N
+  Drupal::service('mail.factory')->get($module, $key);
 }
 
 /**

@@ -0,0 +1,134 @@
+    $this->instances = &drupal_static(__FUNCTION__, array());

Do we still want to have a Drupal::static? All you probably need is the property on the factory object ... Hopefully we don't consider drupal_static_reset('drupal_mail_system') as an API.

@@ -0,0 +1,134 @@
+   * An implementation needs to implement the following methods:
+   * - format: Allows to preprocess, format, and postprocess a mail
+   *   message before it is passed to the sending system. By default, all messages
+   *   may contain HTML and are converted to plain-text by the
+   *   Drupal\Core\Mail\PhpMail implementation. For example, an alternative
+   *   implementation could override the default implementation and additionally
+   *   sanitize the HTML for usage in a MIME-encoded e-mail, but still invoking
+   *   the Drupal\Core\Mail\PhpMail implementation to generate an alternate
+   *   plain-text version for sending.
+   * - mail: Sends a message through a custom mail sending engine.
+   *   By default, all messages are sent via PHP's mail() function by the
+   *   Drupal\Core\Mail\PhpMail implementation.

Yes this is just moved, but that is kind of a redundant information as it is part of the interface. There you seem to have way less detailed descriptions.

@@ -0,0 +1,134 @@
+
+} ¶

Additional empty line and whitespace :p

Resetting @Berdir's title change.

Thanks for the review.

- Removed the drupal_static(), that was a left-over
- Changed the Mail Factory to store the config object and not the actual configuration, that allows to change it while the factory is already instantiated
- Added the missing return in drupal_mail_system(). Going to be hard without that :p
- Moved the format documentation to MailInterface::format(), removed the mail part, not really relevant for either the interface or the factory to document what the default implementation is and does?

This looks really good for me.

@@ -214,7 +214,7 @@ function drupal_mail($module, $key, $to, $langcode, $params = array(), $from = N
 function drupal_mail_system($module, $key) {
-  Drupal::service('mail.factory')->get($module, $key);
+  return Drupal::service('mail.factory')->get($module, $key);
 }

That was just an implementation detail

The patch looks good to me.

Needs a reroll

git ac https://drupal.org/files/mailfactory-1889644-7.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 10089  100 10089    0     0  24819      0 --:--:-- --:--:-- --:--:-- 29500
error: patch failed: core/includes/mail.inc:200
error: core/includes/mail.inc: patch does not apply

Really? ;)

mail.inc didn't change in *5 month* before that config() stuff ;)

Just conflicted due to config()/Drupal::config() in the removed code. So I think it's ok to move it back to RTBC.

+1

1. +++ b/core/lib/Drupal/Core/Mail/MailFactory.php
   @@ -0,0 +1,120 @@
   + * Contains Drupal\Core\Mail\MailFactory.
   

   Theoretically we just add a "\" here :(

2. +++ b/core/lib/Drupal/Core/Mail/MailFactory.php
   @@ -0,0 +1,120 @@
   +  public function __construct(ConfigFactory $configFactory) {
   

   Needs docs :(

Fixed those docs.

Thanks

Committed 4d783b7 and pushed to 8.x. Thanks!

Let's can a change notice to announce the new possibilities this patch permits.

Created https://drupal.org/node/2073215.