🇨🇭Switzerland @znerol

I found one more instance with the following regex(es):

git grep -h -o ' \\Drupal\\[A-Z][\\A-Za-z0-9]*' | grep -v '\\Drupal\\Core' | grep -v '\\Drupal\\Component' | grep -v '\\Drupal\\Composer' | grep -v '\\Drupal\\[^\\]*Test[^\\]'

znerol → created an issue.

znerol → created an issue.

There is a config schema for mailer_dsn. The use of TLS depends on the options. The options correspond directly with the ones provided by the Symfony Mailer component.

I expect that we will add a proper admin UI for transport configuration before the new mailer in Drupal gets stable.

Can somebody please rebase the MR? The spell check job fails, and that happens sometimes if it is too far behind 11.x.

In the CR example "For authenticated SMTP", what is the default port? Should this be mentioned somewhere?

Interesting question. There is not one default port but two (25 for SMTP and 465 for SMTPS). This is analogous to HTTP (port 80) and HTTPS (port 443). My recommendation would be to just leave out the port unless its a custom one (e.g., most useful example is mailpit). Although, some mail providers require port 587 to send out authenticated mail and only allow port 25 for incoming messages only.

That said, I do not think that the CR should go into these kind details. Imagine we'd be replacing guzzle with some other HTTP client library. Would you expect the CR to discuss port numbers?

Updated the existing CR.

There is a coding standard error which needs fixing. Therefore still needs work.

FILE: /builds/core/lib/Drupal/Core/File/FileSystem.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 206 | WARNING | Line exceeds 80 characters; contains 81 characters
     |         | (Drupal.Files.LineLength.TooLong)
--------------------------------------------------------------------------------

Note to self: add a section to the existing mailer module CR → .

Left comments in the MR. Also it seems that the test run did not pass.

Had to revert the changes in core/tests/Drupal/Tests/Component/Annotation/Doctrine/Fixtures. These @var annotations are intentionally invalid.

Thanks, I kicked off the test-only pipeline (results).

How about this for the @todo:

diff --git a/core/modules/update/update.module b/core/modules/update/update.module
index 1a217fa54da..b492d4f6ba3 100644
--- a/core/modules/update/update.module
+++ b/core/modules/update/update.module
@@ -214,6 +214,9 @@ function _update_message_text($msg_type, $msg_reason, $langcode = NULL) {
     case UpdateFetcherInterface::NOT_CHECKED:
     case UpdateFetcherInterface::NOT_FETCHED:
     case UpdateFetcherInterface::FETCH_PENDING:
+      // @todo Conditionally generate absolute URL when used in email context
+      // and pass language option to Url::fromRoute().
+      // @see https://drupal.org/i/3553063
       if ($msg_type == 'core') {
         $text = t('There was a problem checking <a href=":update-report">available updates</a> for Drupal.', [':update-report' => Url::fromRoute('update.status')->toString()], ['langcode' => $langcode]);
       }

znerol → created an issue.

znerol → created an issue.

Similar discussion over in 📌 Extract _user_mail_notify() into a user MailHandler Active . I do not have much of an opinion, but we should do it consistently here and in the user module.

Also in the other issue we went without an interface.

Thank you @dcam.

Thank you @xjm

I'd prefer to resolve 📌 Resolve potential namespace conflict with contrib mailer module Active before the module is part of a release.

znerol → created an issue.

Thanks @zengenuity, whats missing is ✨ [PP-1] Ensure origin headers of mails sent using the mailer.transport service comply to RFC5322 Postponed .

Symfony\Component\Mailer\SentMessage is not quite friendly for asserting HTML emails. In functional tests you will likely fetch the original message. I wonder if it is better to capture the original message instead?

True. This MR allows access to either of them. getEmails() returns the original Email objects, while getCapturedMessages() returns a list of SentMessage.

Thank you @dcam.

Thanks for #44, it puts the change nicely into a broader perspective.

The front controller (especially the main entry point index.php) could be viewed as application code. It would be great if subsequent refactoring could be made without changing it anymore. It might be possible to achieve this by using the following pattern:

return static function (AppContext $context) {
  return new DrupalKernel($context, require 'autoload.php');
};

The AppContext value object could start out as a very basic class only containing $environment = 'prod'. The DrupalKernel constructor can be changed to accept string|AppContext as its first parameter. If the caller passes the environment string, it is used as is, if the caller passes AppContext, the environment string is taken from there. Nothing else.

Then we can figure out the rest in subsequent refactorings without having to touch the front controllers anymore.

Regarding #45 and putenv: If that code path is called exclusively on the CLI, then this is fine.

Regarding $_SERVER and $_ENV: if it is unacceptable to drop $_ENV, then at least ensure that $_SERVER is used as the primary data source and use $_ENV only as a fallback (like its done in the symfony/runtime component):

% git grep '\$_SERVER.*??=' src/Symfony/Component/Runtime/
src/Symfony/Component/Runtime/Internal/autoload_runtime.template:if (is_string($_SERVER['APP_RUNTIME_OPTIONS'] ??= $_ENV['APP_RUNTIME_OPTIONS'] ?? [])) {
src/Symfony/Component/Runtime/Internal/autoload_runtime.template:$_SERVER['APP_RUNTIME'] ??= $_ENV['APP_RUNTIME'] ?? %runtime_class%;
src/Symfony/Component/Runtime/SymfonyRuntime.php:            $_SERVER[$envKey] ??= $_ENV[$envKey] ?? 'dev';
src/Symfony/Component/Runtime/SymfonyRuntime.php:            $_SERVER[$debugKey] ??= $_ENV[$debugKey] ?? !\in_array($_SERVER[$envKey], (array) ($options['prod_envs'] ?? ['prod']), true);
src/Symfony/Component/Runtime/Tests/phpt/autoload.php:$_SERVER['APP_RUNTIME_OPTIONS'] ??= [];

The php ini value for variables_order is set to GPCS by default in many (most?) Linux distros. That means that environment variables are not parsed into the $_ENV superglobal in many cases. They are still available from $_SERVER though.

The getenv() and putenv() C functions used by the PHP function with the same name are not thread-safe. As a result, concurrent access in multi-threaded environments may lead to hard to debug problems. There have been reports about this in the past (e.g. vlucas/phpdotenv#76 or laravel/framework#7354).

I suggest to try to avoid $_ENV, getenv and putenv and exclusively rely on $_SERVER as an environment variable source - at least initially.

Made the follow-up: #3553063: _update_message_text() calls Url::fromRoute without options → .

znerol → created an issue.

Rerolled.

Thank you. I left a comment in the merge request. There is also an unresolved suggestion by @smustgrave from a couple of weeks ago. That should be resolved as well.

Setting to needs work for the @todo and the follow-up (see MR comments).

znerol → created an issue.

Merge conflicts resolved.

znerol → created an issue.

Thanks for taking a look @zengenuity.

There are some docs in the MR (mailer.api.php).

The 3539651-email-content-yaml-plugin-core-emails branch shows how the core emails call sites and implementations might look like with this approach applied.

But neither of those really explain the design choices very well.

Pivoting to an approach without controller.

I added the issue link to the mail capturing section.

Adding dependency evaluation → section and required issue tags.

znerol → created an issue.

Rerolled.

znerol → changed the visibility of the branch 3539651-all-in-one-integration to hidden.

znerol → changed the visibility of the branch 3539651-core-emails to hidden.

znerol → changed the visibility of the branch 3539651-introduce-email-yaml to hidden.

I think that this happens only on routes where the default (html) format is disabled.

I am seeing this with (custom) content negotiation. I think a request is affected when it goes through some code path which calls Request::setRequestFormat() (see NegotiationMiddleware::handle()).

I'm seeing that as well.

I think it isn't necessary to use reflection in this case. Instead I suggest to following:

• Add the authentication_collector as an additional dependency to the authentication_subscriber
• Loop through getSortedProviders() and call applies() on each of them
• Return the provider id of the first match

I pushed a test which asserts that http_kernel.basic is not initialized when http_kernel is retrieved from the container.

Also rebased and converted phpunit annotations to attributes.

I took the opportunity to host a spontaneous bar camp session at DrupalCamp Ruhr. The idea was that participants talk about their projects and tell their success stories and pain points with the current mix of core, contrib and custom extensions around email. Around 15 people took part in the session. Notes by @znerol.

Session format: You talk, I write.

Highlights

• One person operates a big multi-tenant system featuring the DSM+ and the SM module. This project is using the DSM+ mailer policy module and also hooks/event subscribers for routing and customization. Another useful extension in the mix is the Symfony Mailer Log module. It logs each success or failure and also allows to review the rendered emails.
• One person mentiones the Mailsystem module since it permits flexible routing, theming, sending with multiple accounts.
• One person prefers to apply customization directly in the TWIG template for custom projects (including translation).

Pain Points

• Infamous "Recursive rendering detected" in simplenews. ✨ Add setting to disable per-user rendering Active
  This project might just be switched to external newsletter software to avoid this kind of issues.
• Sending mails with attachments.
• Multiple smtp accounts. User emails from normal mail account. Newsletter email from scalable infrastructure. Multi tenancy.
• Competing solutions (DSM+ vs. DSML vs. swiftmailer)
• Configuration (for users its config, for other type of mail its different)
• hook_mail: What to put here? The bare minimum (like commerce) or most of the logic (like contact).
• LocalGov Drupal: Complicated templating, notifications, newsletters, tokens without knowing what the underlying transport will be (DSM+ vs DSML).
• Rendering URLs outside a request-response cycle - needs crude hacks when mails are rendered asynchronously (e.g. inside symfony messenger queue).
• Tokens in mails seem strange and dated.
• User password-reset tokens are implemented in a weird way.

Ideas

• Mailframe module: Receiving (imap) with multiple accounts. Bounce management, Ticketing system (should be solved with Mailframe).
• Mail composing (templates) mailjet language mjml, inki (foundation for email).
• Read receipt handling: E.g., ensure that a contact mail is received by staff.

Questions

• What is the trajectory of core mail?
• What is the plan to deprecate hook_mail / hook_mail_alter? When and how?

This isn't relevant anymore. There is some test coverage in Drupal\Tests\Core\UnroutedUrlTest and the external option was completely removed from Drupal\Core\Routing\UrlGenerator in #2575869: Remove all remaining usage of deprecated UrlGeneratorInterface::generateFromPath() and delete it → .

Reviewed this in context. Adapted the issue title and the CR to match the actual implementation.

There is a slight risk that the newly added create() method in PluginBasebreaks existing contrib/custom plugins with an incompatible create() method in the same way as MenuLinkMock in this issue. Authors of affected plugin classes may rebase onto Drupal\Component\Plugin\PluginBase in that case.

I'm okay with #54 as well.

#52 is fine with me.

• We are trying to avoid a namespace conflict with an existing contrib module. Claiming the namespace of another existing contrib module doesn't seem to make much sense.
• The core mailer module isn't supposed to stay in core. It is a temporary container for a bunch of service definitions. The service implementations themselves are added to Drupal\Core\Mailer namespace directly. When enough of the implementation is stable, the service definitions will be moved to core.services.yml and the core mailer module goes straight from experimental to deprecated.

So, I'd suggest to either agree on a namespace which is available or just not do anything at all.

I like the suggestions in #50. I'd probably prefix the method with create which is common for factory methods. E.g., createAutowired() or createAutowiredInstance().

Mailer module namespace conflict

The experimental core mailer module namespace conflicts with a preexisting contrib mailer project. The participants expect that the transition from experimental to a stable implementation (making the module superfluous) could take a couple of releases. The current draft MR which proposes to introduce a mailer_experiment module isn’t ideal, especially when the module approaches its end state. Members of the group proposed mails or email as an alternative. The group decided to try it with mailer_symfony instead.

• 📌 Resolve potential namespace conflict with contrib mailer module Active

It was briefly discussed how the transition from experimental to stable could be performed. One way would be to introduce a new mailer_legacy module containing legacy services (mail manager) in the same release the new mailer becomes stable (and default). Existing sites which haven’t enabled mailer_symfony yet will enable mailer_legacy during the upgrade and continue to use the old API for core mail.

Backwards compatibility

The group briefly touched on backwards compatibility. When the core mailer becomes stable, a lot of modules either need a complete rewrite or will become obsolete. In order to help with that, it's important to provide specific migration documentation. E.g., modules which implemented hook_mail_alter before in order to add a note to every outgoing mail should implement hook/event/whatever. etc.

Also when the core mailer becomes stable, multiple contrib Symfony Mailer integration modules will become obsolete. It is also important to provide documentation on how to migrate from them.

Sending emails defined by another module

In some version of the MR in the template/controller draft issue, the call site looks quite complex. Members of the group point out that this could be a problem. E.g., A custom project which needs a button which (re)sends a password reset or an order receipt, would need to copy and maintain the call site. If that piece is complex, then that means more technical debt for a custom project.

The group discussed whether sending emails defined by another module really should be a supported use case. Projects like ECA would benefit from that.

On the other hand, if the module itself decides whether or not to provide a dedicated API for notifications, then that could include other channels as well. E.g., users might want to receive password resets via an instant messaging app.

There are also types of mails where it isn’t obvious why other modules would want to trigger them. One example is the update status notification. Exposing that as a public API doesn’t seem desirable.

The group doesn’t take a decision whether to go one or the other way. Whether or not to expose triggering an email as a public API may end up being the decision of the email sending module.

• 📌 Introduce email yaml plugin and an email building pattern known from controllers Active

Altering emails defined by another module

The group agrees that it is desirable to provide a registry of all emails available on a site. This should also include very simple emails. In that case, the altering module might not see any parameters but only that a message (subject, (un-)rendered body) of a certain module/key combination is about to be sent.

Access to the Symfony Email object

The group discussed which role the Symfony Email object should be playing in the upcoming API. Several members pointed out that Symfony has a history of introducing breaking changes in a rhythm which is difficult to keep up in Drupal projects (this is especially true for contrib modules which are expected to support multiple core versions).

On the other hand, direct access to the Symfony Email object allows for advanced use cases.

One option to tackle this dilemma is to ensure that developers do not need to interact with the Symfony Email object when performing standard email building activities.

Next meeting:

Next planning meeting takes place October 1st at 4PM UTC+2.

I honestly think that the current way routes are defined is really not great DX. [...] I think we shouldn't try to reuse terminology like defaults and controller for this. IMHO, it makes it harder to use, not easier.

This is good to know. I cannot really tell.

plugins are pretty much expected to have a class property in the definition.

True. In a previous version, the class was set to the Symfony\Component\Mime\Email. We do this for constraints, but I guess that this architecture isn't something we want to push more. It also required a custom factory - and I removed it again. The Symfony Email is a value object, it doesn't really map well to a plugin instance.

I think there is a way how plugins could have an interface for callers and still support dynamic arguments for the implementation. The methods which currently make out the controllers could be folded into plugin implementations. The argument resolver could be moved from the renderer to a shared plugin base. I guess I need to try that out next.

I do not think that mail is abandoned. At least, the mail.info.yml declares compatibility with Drupal 10.

https://git.drupalcode.org/project/mail/-/blob/8.x-1.x/mail.info.yml?ref...

What about Core Mailer Preview?

Without rename:

Experimental Mailer:

Core Mailer Preview:

My intention was to only change the module namespace. But of course, if there are two modules available with the same human readable name, that would be confusing.

Thank you! I left one small suggestion.

The CR → needs minor updates when this gets committed.

znerol → created an issue.

znerol → created an issue.

znerol → created an issue.

I do not need help with that. Thank you.

znerol → created an issue.

Added a draft MR with an Password Argon2 module. This is implementing the idea from #32.

Thank you.

I was wondering why only the content block was converted and found the answer in the issue comments. Added that info to the issue summary.

Left a question and a suggestion in MR comments.