[PP-1] Add symfony mailer transports to DIC

Postponed on 📌 Add symfony/mailer into core RTBC

We have very similar code already in Drupal Symfony Mailer, added in ✨ Allow custom TransportFactory Fixed , see TransportFactoryManager. I believe that existing code has some advantages.

1) The symfony Transports class assumes a fixed set of transports at initialisation time. I agree this could often be true, but not always - for example a hook might wish to discard a mail by setting a null transport, however the site might not have configured one.

Also looking ahead to 🌱 [META] Adopt the symfony mailer component Needs review (where hopefully we can use this same service) it seems awkward to rely on an X-Transport header when we could have a function on the Email class.

In Drupal Symfony Mailer we have Email::getTransportDsn() and use Transport::fromString(). If we are concerned about performance, then we could add caching of transports already created. This avoids the need for a mailer.transport_configuration service.

2) Transport::getDefaultFactories() already knows about the 4 factories that you added, plus 9 more. This list will likely grow over time. We can automatically load these without needing service definitions.

3) If we end up creating a new class (which Drupal Symfony Mailer did), then we could use the service_collector tag to avoid creating a new compiler class.

for example a hook might wish to discard a mail by setting a null transport

Actually for that we have SkipMailException so maybe it's OK.

Good idea to use a service_collector here. That way we get correct behavior for the priority flag for free.

Here are my updated comments (including ones from before not yet resolved keeping the same numbers):

1) The aim here is a service mailer.transport that implements TransportInterface.

We've agreed that Core supports only a single transport, with a single config setting and no UI. So it can create a very simple class that always returns the same transport. The Transports class is complex and unhelpful - it doesn't even have a default. The mailer.transport_configuration service is again over-complex for this need.

Sites that want configuration of transports should use contrib, which will replace the service with a new class. IMHO as maintainer of Drupal Symfony Mailer, again the Transports class is not helpful. It requires a fixed list of transports at initialisation, it creates them all (even though we likely only use one). It forces use of the 'X-Transport' header when we could just call something like $email->getTransportDsn(). The mailer.transport_configuration service is similarly assuming a fixed list. So we would likely ignore both classes.

My head is spinning round trying understand all the services😃: mailer.transport_factory_collection, mailer.transport_factory (which is not an implementation of TransportFactoryInterface), mailer.transport_configuration and mailer.transports I believe it could work well combined into one simple class MailerTransport. Something like this:

function createTransportFactory() { // as existing}
function getDefaultDsn() {
  // Contrib overrides this function.
  return this->config('system.mail')->get('mailer_dsn'))
}
function initialize() {
  if (!$this->transportCollection) {
    $this->transportCollection = new Transport($this->transportFactories);
    $this->defaultTransport = $this->transportCollection->fromString($this->getDefaultDsn());
  }
}
function send(......) {
  // Contrib overrides this function.
  $this->initialize();
  return $this->defaultTransport->send();
}

What do you think?

2) Transport::getDefaultFactories() already knows about the 4 factories that you added, plus 9 more. This list will likely grow over time. We can automatically load these without needing service definitions by calling it from TransportFactoryCollection::createTransportFactory().

3) Thanks for fixing.

4) As @lucashedding has already commented, we should create our own TransportInterface to give flexibility. We might create our own Email class in ✨ Create new mailer service based on symfony Active .

Does this definitely need to be postponed? The service would be used by both ✨ Create new mailer service based on symfony Active and 📌 Add symfony/mailer into core RTBC . We'd have to go back and edit the code created there to use this service.

[I realise this is postponed, so perhaps it's not considered ready for review.]

My comments from above that are not yet done:

1) This MR apparently forces a fixed list of transports known at initialisation time. This would prevent setting of the transport programmatically - for example a module that wishes to force a null/test transport on a dev site.

We can instead

• Find a way to store the transport DSN on a email. This could be a class EmailWithDsn that extends the base Email class, adding a tranportDsn variable with accessors.
• For the mailer.transports service (or perhaps better named mailer.transport), create our own variation on the Transports class. It checks if the email contains a DSN, and uses that, otherwise uses the default. It calls Transport::fromString() with this DSN.
• Delete mailer.transport_configuration service, or simplify it to a mailer.transport_default service that returns the default DSN. Otherwise we create all possible transports but only use one of them.

2) Symfony provides Transport::getDefaultFactories() which would allow registration of 15 transport factories without writing code in a services.yml file. Core would not include the 3rd-party transports, however a site-builder would only need to add them to composer.json and then they would automatically work. Otherwise we'll need Drupal wrappers for each 3rd-party transport with just the services file.

For someone who doesn't know, what does "DIC" stand for?

what does "DIC" stand for?

Good question, let's make it clearer. I would say Dependency Injection Container

I think this issue is trying to do too much at once. Already the issue title is an important and complex topic. It would be amazing if we could get it in 10.2 because it would help a lot with DSM (Drupal Symfony Mailer), DSM-L, the upcoming Mailer Transport module (that they hopefully can share) and the end game of swiftmailer.

In addition this issue contains code for creating the email test framework for use with Symfony Mailer. This is another important and complex topic as what we do here affects the ease and efficiency of writing tests in Core and many contrib modules for years to come. As maintainer of both DSM and Simplenews this affects me a lot. We need to get it right in the beginning as it will become hard or even impossible to change later due to existing test code using it. I could easily write 20 points of feedback about it - starting with "How will it work with the Mailer Service (rather than this @Mail plug-in), because potentially we have lost access to the Drupal Email class that wrapped the Symfony Email class.

Please can we make the second part a follow-on issue? I propose roughly like this:

1. Keep TransportFactoryCollectionTest here as it tests the code of this issue
2. Keep SymfonyMailerTest here as it tests the code of 📌 Add symfony/mailer into core RTBC
3. Defer MailerTransportTest, mailer‎_transport_test‎ module, AssertMailerTransportTraitTest - which are actually general tests for sending emails, that AFAICS would (almost) work without the rest of this patch.

znerol → closed merge request !4572

znerol → reopened merge request !4572

znerol → closed merge request !4572

In #13 I said

Find a way to store the transport DSN on a email.

Maybe this part isn't needed in Core - we can just do it in Contrib.

Anyway we can discuss anything if you want, otherwise I'll wait for a NR until I look again.

I raised ✨ Framework for testing Symfony Emails Active and ✨ Page to send a verification email Active

Injected the @event_dispatcher@ into @mailer.transport_factory.abstract@, added an @OriginatorSubscriber@ which sets the @From@ and @Sender@ headers according to the rules in RFC 5322 and added tests for that. Note that message sender (i.e., the @Sender@ header in the message) is not the same as envelope sender (i.e., the FROM command in SMTP).

Still needs work because I'd like to cover FailedMessageEvent and SentMessageEvent with some tests. Also need to update the issue summary to better explain design choices. Also we need an example which demonstrates how to register a contrib or custom transport factory using a module.

Wow, that was very confusing. You committed a considerable refactoring of the service architecture between me reviewing the MR and checking it out to have a look at it a few minutes later and then somehow what I was looking at was completely different :)

I think that latest commit at least partially addresses the concerns from @AdamPS in that it simplifies creating the transport to our own TransportManager and then that just "materializes" the default transport into the mailer.transportS service. Which is confusingly named, why the plural?

Current thoughts while going through this a bit.

* Agree that this is a lot code and specifically test coverage and test implementations of things. I'm not sure yet what we can feasibly split off, but if we postpone the test trait to ✨ Framework for testing Symfony Emails Active then we could definitely also move the test coverage for it.
* The Originator event subscriber seems useful but what exactly makes specifically that necessary to be added here?
* I'm a bit unsure about the single transport thing. We did agree that core only needs a single transport, but for me, that was about the configuration aspect of the default transport. Having that baked into the service architecture as the mailer.transports service seems like it's going to make it harder for contrib to expand on that? Do we really need to do that, especially at this stage, also in combination with the next point.
* As it stands, with the public service, this pretty much gives us a working mailer system using symfony mailer. Looking at \Symfony\Component\Mailer\Mailer, if you don't set up the message bus stuff, it's just a very thin wrapper around the transport. So, what's stopping modules from using this service once it's committed now and then we risk breaking them? Anything we add in core.services.yml by default becomes a public and stable API unless noted otherwise. While there's discussion as to just how many layers we'll want to have on top of this in core, I think we all agree that we'll need something? Do we need to mark these services as internal/private/experimental?

Combining the last two points, I think one option is to not have that mailer.transports service and force direct transport calls to at least go through the transport manager which also gives us a place to document the API stability and when and when not you should use that?

Considering the naming of the services, I think there is still an option to have most of this in an experimental mailer module, we could then move the services without even renaming them to core at some point. The interfaces that you type on are trickier of course. But I'm not sure if I feel comfortable adding all this in its current state into core.services.yml.

Added a scope definition and a couple of short user stories to the issue summary to simplify the discussion a bit. Regarding the scope, my aim is to actually finish the mail delivery part of the Symfony Mailer integration. My hope is that contrib and custom modules can start relying on the service structure and as a result third-party transport integration can be shared between them.

Regarding the tests: I think it is important that we do not leak email during test execution. In order to ensure that this is working correctly, we have to cover that requirement with tests.

> I'm a bit unsure about the single transport thing.

I was pretty confused about this too, however I think Symfony forces it upon us. I guess the reason for the plural Transports (which I don't especially like either, however it comes from the Symfony class name) is that this isn't actually necessarily a single transport. Instead it will find/create the required transport and use that to send. We have to do it this way because creating the Symfony Mailer service requires injection of a class implementing TransportInterface.

Currently DSM does something different as a workaround - it creates a Symfony Mailer object for each email sent with the required single transport. However that breaks DI and prevents use of Symfony Messenger for example.

@znerol I'd like to say a big thank you for your hard work here. I can see that you are giving your time because you want to help Contrib and Custom developers.

My role is one of the major developers in this area, and so I'm grateful. Given I'm representing your intended audience, your "customers", then I hope that you will listen to some ideas about what we might actually want😃. I haven't re-reviewed the code yet, however I have some concerns from reading the IS and recent comments. Sorry @znerol this comes out as quite a long list of things that I believe need to change. In many cases I feel I'm repeating what I've already said once or twice before, but I didn't see any response so they're still relevant.

Put the necessary services in place such that contrib and custom code can start using the mail delivery part of Symfony Mailer safely in test and in production code by simply retrieving / referencing a configured transport service from / in the container.

I'm don't agree with this statement from the IS. I think that's much too big a goal for one issue. I approve of the bullet list of things you suggest to be added to the discussion of 🌱 [META] Adopt the symfony mailer component Needs review . I don't agree with them all (for example you say "share modules providing third-party transports", but actually you don't even need a Drupal module for the 3rd-party transports already known to Symfony). We don't have to do them all in this issue, we don't even necessarily have to do them all in Core. I suggest instead this issue should do what the title says - add some services for transports to the DIC.

We are still at a very early beginning stage of Symfony Mailer in Core. We can't fully predict the needs for the end goal of the new APIs and Mailer Service. I vote that everything we add here is marked as @internal and experimental. Contrib and Custom are welcome to use it, but they need to be ready for change. We are not necessarily creating something ready for production use.

Contrib code is already using Symfony Mailer😃 on 20k sites. This issue is currently in danger of breaking that - it could conflict with service names and tags already used, and it could "undo" security critical measures already taken. Therefore let's take small simple steps that we can verify the consequences of, and do each step fully without leaving important bits out. The transport factory collection definitely feels like it belongs in Core, but we need to be careful about removing features and security compared to Contrib. At very least we should ensure there is a way for Contrib to get back the features already in use with a reasonable migration path.

Regarding the tests: I think it is important that we do not leak email during test execution. In order to ensure that this is working correctly, we have to cover that requirement with tests.

I agree that is important. However I suggest it's not part of this issue as here we don't need any tests that send emails. I suggest you could add your requirements and test code to ✨ Framework for testing Symfony Emails Active . I created a detailed list of ideas and @Berdir has responded with some more good ideas. Your ideas are good too. Clearly this is a significant area for discussion and deserves a separate issue.

Still needs work because I'd like to cover FailedMessageEvent and SentMessageEvent with some tests

I see there is some discussion again about events. We had previously decided to disconnect event subscriber because we don't yet know what events/hooks/callbacks we want for Symfony Mailer in Core. This whole area is another significant discussion, and needs a separate issue. I would say it's about [PP-5]😃 as it's hard to even imagine the right events until we actually have the classes that they would live in. The current consensus is towards decorating the Symfony Email class rather than extending it, in which case these events would likely be entirely unsuitable. Sure, early adopters may listen to the Events that Symfony already provides, but it's not necessarily going to be the same when Drupal officially integrates Symfony Mailer.

using part of Symfony Mailer safely in test and in production code

I agree safety is important, so I vote we should include the fix for 📌 [PP-2] Add security checking for Symfony Mailer transports Active in this issue. Otherwise, we are encouraging Contrib to expose security bugs.

> I was pretty confused about this too, however I think Symfony forces it upon us. I guess the reason for the plural Transports (which I don't especially like either, however it comes from the Symfony class name) is that this isn't actually necessarily a single transport. Instead it will find/create the required transport and use that to send. We have to do it this way because creating the Symfony Mailer service requires injection of a class implementing TransportInterface.

The default implementation, yes, but we're not actually required to use that. With a transport injected into the mailer, it's literally a one-line wrapper around transport send, it does absolutely nothing else. It only gets a little bit more complicated when using a bus.
And the latest patch doesn't use Transports anymore, it is set as a factory from TransportManager::getTransport(). I think the S is just a leftover from that refactoring.

We can easily have our own implementation that implements the Symfony Mailer Interface and inject the TransportManager service and get the transport from that. And then we can later add a feature that gets a transport DSN from Email object or an event or whatever and pass that to TransportManager::getTransport(). Or contrib can do that in its own implementation of the Mailer. It already supports that.

As a third opinion on this discussion, I'm somewhere in the middle. I fully agree that this is too much in a single issue, it's one thing to write the code, but reviewing and approving all of this is _not_ something I feel comfortable with.

The challenge is adding an incomplete/partial API to core.services.yaml that we don't want modules to use yet. Because I really think that it is too early for that and if modules start using it now, it *will* break.

> Regarding the tests: I think it is important that we do not leak email during test execution. In order to ensure that this is working correctly, we have to cover that requirement with tests.

With this I agree, we need *some* tests for sending mails and we want to make sure that we don't leak mails in tests and verify that. But what I meant specifically is that there is the new AssertMailerTransportTrait, and a AssertMailerTransportTraitTest for it ( didn't fully grok the rest of the tests so I can't really comment on them). It's a single line of code, one that we could easily inline into those early tests and then use the test framework issue to flesh out a proper test trait and commit to that as an API.

> We are still at a very early beginning stage of Symfony Mailer in Core. We can't fully predict the needs for the end goal of the new APIs and Mailer Service. I vote that everything we add here is marked as @internal and experimental. Contrib and Custom are welcome to use it, but they need to be ready for change. We are not necessarily creating something ready for production use.

+1.

> Contrib code is already using Symfony Mailer😃 on 20k sites. This issue is currently in danger of breaking that - it could conflict with service names and tags already used, and it could "undo" security critical measures already taken.

Didn't check that closely, but IMHO, if Symfony Mailer used service names and tags that are _not_ prefixed with symfony_mailer then that's on that module. All services are prefixed with mailer and the tag is mailer.transport_factory. Unless I'm missing something, the transport discovery should then run completely parallel to symfony_mailer module and not interfere with each other? And symfony_mailer simply won't switch to the core services.

Worked on the tests in order to reduce the number of similar names (the presence of two test transport factories confused me a lot during a recent debugging session). I will try to move stuff to an experimental mailer module later. However, there are still things which need to remain in core (that obviously includes the changes to the test system which prevent leaking mails during tests).

There is an opportunity to reduce the amount of changes almost by 50% if the OriginatorSubscriber is extracted to a separate issue. However, I will rather keep the code around until after the refactoring in order to profit a little bit longer from the test coverage.

Added some clarification about the scope in the issue summary.

As a site owner I want to send emails via the SMTP protocol without installing contrib modules.

Given this product requirement, SMTP requires configuration. With the current implementation as far as I can see this will require the site owner to enter a DSN directly? Should site owners be expected to configure mail this way, or should we be providing transports via plugins with configuration forms?

I took another quick look at the code, and many of my concerns are fixed thank you.

We can easily have our own implementation that implements the Symfony Mailer Interface and inject the TransportManager service and get the transport from that. And then we can later add a feature that gets a transport DSN from Email object or an event or whatever and pass that to TransportManager::getTransport(). Or contrib can do that in its own implementation of the Mailer. It already supports that.

OK, you've lost me there. The part I'm stuck with is this. Symfony\Component\Mailer\Mailer is a final class, and it requires TransportInterface as a constructor parameter. So I agree we can override and inject anything we like, but still that code will only get the Symfony Email class in $message. I don't see how to get back to our Drupal Email class, which decorates the symfony one, rather than extending it.

Didn't check that closely, but IMHO, if Symfony Mailer used service names and tags that are _not_ prefixed with symfony_mailer then that's on that module. All services are prefixed with mailer and the tag is mailer.transport_factory. Unless I'm missing something, the transport discovery should then run completely parallel to symfony_mailer module and not interfere with each other? And symfony_mailer simply won't switch to the core services until they are fully ready.

DSM used the tag mailer.transport_factory because this is a standard indicated by Symfony mailer library for discovering transports. DSM includes a fix for 📌 [PP-2] Add security checking for Symfony Mailer transports Active but Core doesn't, so this issue as it stands could weaken security of live sites (I don't know which implementation would end up taking precedence - maybe it could even be random). Please can we get the security team to decide if the linked security issue needs fixing, and if so fix it in Core also?

There is an opportunity to reduce the amount of changes almost by 50% if the OriginatorSubscriber is extracted to a separate issue.

+1
Sorry I don't really understand autowire😃. Is this going to run automatically on all sites? If so, it could break any site already using symfony mailer library.

Given this product requirement, SMTP requires configuration. With the current implementation as far as I can see this will require the site owner to enter a DSN directly?

+1
Another problem with entering a DSN directly is there are complex escaping requirements, so it's bad UX. We have a GUI with proper forms in Contrib. I guess it will be a product management decision whether to add one to Core.

> OK, you've lost me there. The part I'm stuck with is this. Symfony\Component\Mailer\Mailer is a final class, and it requires TransportInterface as a constructor parameter. So I agree we can override and inject anything we like, but still that code will only get the Symfony Email class in $message. I don't see how to get back to our Drupal Email class, which decorates the symfony one, rather than extending it.

The Email class and how we'd use it and accessing it in (transport) events is a different concern.

I was only talking about the single-transport service/injection. Yes, the default implementation is final, but all code should work against the interface, and we could and expect will provide our own implementation of SymfonyMailer that could instead of a single Transport, work with our TransportManager. Or not even use that interface, we'll see. But it seems to be that your workaround of a one-off mailer doesn't seem necessary when all it does is $this->transport->send($message, $envelope);

I do agree that if we have our own wrapper or alternative implementations of the Email then the default transports are a problem.

On the UI: Yes, if we say that users can use SMTP without any contrib module, then we do need a UI. As we also said that core only supports a single transport (configuration) at a time, it could be fairly basic, similar to the current swiftmailer UI where you just pick one of the fixed core-provided options and if SMTP, you get the 5-6 default settings that swiftmailer also exposes (host, ip, encryption, username, password). Anything fancier, you use a contrib module. I'm also still very much open to leaving the UI entirely to contrib (if you are not comfortable setting the mailer DSN directly in settings.php or something). Having an API that standardizes HTML, attachments, headers, ... is still a _vast_ improvement to the status quo.

Added SendmailCommandValidationTransportFactory (using the same setting as Symfony Mailer Integration contrib module, thanks @AdamPS for the hint). Also added #[\SensitiveParameter] wherever the DSN is passed as a string argument.

Regarding the scope. Looking through the user story list, basically everything starting with As a site owner is probably out of scope in this issue.

> Added SendmailCommandValidationTransportFactory
Great thanks. This is looking good to me now if we defer OriginatorSubscriber. Minor comments:

1) Does this also need a change to default.settings.php? Or maybe not as it's still experimental??

2) Should mailer.transports be renamed to mailer.transport?

> But it seems to be that your workaround of a one-off mailer doesn't seem necessary
Agree - when I wrote that code I had a poor understanding of how things worked.

I was quite unhappy with TransportManager for several reasons. First and foremost the getTransport method:

  public function getTransport(#[\SensitiveParameter] ?string $dsn = NULL): TransportInterface {
    if (!isset($dsn)) {
      $dsn = $this->configFactory->get('system.mail')->get('mailer_dsn');
    }

    $transportFactory = new Transport($this->transportFactories);
    return $transportFactory->fromString($dsn);
  }

The semantics are wrong: getTransport looks like a getter, but in reality acts as a factory method. It creates an instance of type TransportInterface and returns it. However, By simply looking at the name, one could mistakenly believe that it will return the same instance (given the same params). This leads to the second thing which is utterly wrong with this method: The $dsn parameter is only used for tests. It shouldn't be used in production code.

The name of the class is weird as well. This class doesn't manage anything. Its purpose is to sit in the container and wait until something retrieves the mailer.transports service from the container. In that case, it just looks up the DSN in the config and then forwards the call to the Symfony Transport factory class. Hence, this class is not a manager, it is rather an adapter.

Also note, the public methods in the Symfony Transport class (i.e., fromDsn(), fromDsn(), fromString(), fromStrings()) are intuitively recognizable as factory methods.

All things considered, I feel that a method which simply acts as an adapter to Transport::fromString() should be named after the same pattern: I.e., fromConfig().

This is how getTransport() looks after the refactoring:

  public function fromConfig(ConfigFactoryInterface $configFactory): TransportInterface {
    $symfonyTransport = new SymfonyTransport($this->transportFactories);
    $dsn = $configFactory->get('system.mail')->get('mailer_dsn');
    return $symfonyTransport->fromString($dsn);
  }

Regarding the class name. We could call it SymfonyTransportFactoryAdapter in order to express exactly what it does. On the other hand, why not follow the pattern of Symfony Mailer in this case as well and just name it Drupal\mailer\Transport? After all, It has the same purpose as Symfony\Component\Mailer\Transport.

I opted to declare the class @internal and also made the service non-public. This clearly communicates that this class is not part of the public API of the mailer module. If something needs to customize it, I'd suggest to replace the service completely. This could be the case if a site would like to use the Transports class in order to support multiple transports. The way to do that would be to replace the mailer.transport_factory service with another definition referencing a custom class (e.g., Drupal\custom_fancymailer\Transport) implementing the following method:

  public function fromConfig(ConfigFactoryInterface $configFactory): TransportInterface {
    $symfonyTransport = new SymfonyTransport($this->transportFactories);
    $dsns = $configFactory->get('custom_fancymailer')->get('dsn_map');
    return $symfonyTransport->fromStrings($dsns);
  }

Long story short. I'm quite happy now with the service structure and also the TransportTest. Will continue to clean up the rest.

I now removed everything which isn't strictly essential for an initial patch covering the scope in the IS pretty well. I will have to work on tests a little bit. Especially, I'd like to add a functional test which ensures, that a call to \Drupal::service('mailer.transport'); results in the NullTransport by default in the child site.

Setting on NR for the overall approach.

@znerol The patches on this issue have influence from DSM, but the code here is much more beautiful and elegantly crafted into Symfony😃.

+1 from me for overall approach.
I made some detailed comments in the MR.

Given this module is experimental, I think we are not encouraging Contrib module developers to change their code yet. So perhaps we should alter the parts of the IS quoted below?

As a developer I want to start adding message event subscriber for each hook_mail_alter implementation early on so as to be prepared when users start to switch to the new mail system.

in test and in production code

Added test for the default transport in the child site of functional tests. I suggest to leave this in NR state for a couple of days so as to ensure that everybody has the chance to catch up and to read through the comments and the code.

Many thanks @AdamPS for the kind words.

Apologies, my suggestion keeps altering as I consider it more deeply😃. I'm starting to see @Berdir's point I think. I have evaluated various options and I now believe the code in the Mailer service (both Core or Contrib) will likely end up like this:

$dsn = $email->getTransportDsn();
$transport = $transport_manager->fromString($dsn);
// Then either create a Symfony\Component\Mailer\Mailer using this transport, or copy the small amount of code from that class into our own.

This means that with Core alone, it's already possible to set the transport to anything using hooks/API. It also creates the perfect base-point for Core/Contrib to add a transport GUI, even with the option for selecting different transports for different emails.

This works with a TransportManager service with a single function fromString(string $dsn = NULL). If the DSN is null, I propose that the TransportManager call into a DefaultTransport service, which has a single function getDefaultDsn(). This allows for Contrib to override the DefaultTransport service, but without needing to change the TransportManager. (Or it could be protected function getDefaultDsn() on TransportManager.)

Rejected options
Inject into the Drupal Mailer an instance of Symfony\Component\Mailer\Mailer
or of Symfony\Component\Mailer\Transport\TransportInterface. The problem is that both of these force all emails to go through a single instance of TransportInterface. This interface carries the Symfony email class, however we will decorate it - so we have no easy way to get back to the Drupal Email class. There are some awkward ways: put the transport DSN in a header, or create a class that extends the Symfony email, with a variable that points to the Drupal email.

Added the API additions to the issue summary.

I started a sandbox → with examples to better show how the API is supposed to be used in contrib / custom code.

Commenting in the issue again as it's challenging between the overlapping review-threads.

> The purpose of this class is to be used in the container to help create the mailer.transport service. Modules which like to customize the creation of the mailer.transport service are expected to swap out both service definitions (i.e., mailer.transport_factory and mailer.transport). I think that is actually the crucial thing to understand.

I understand that this would be the approach, but I don't agree that this is the correct thing to do. Maybe it is in the Symfony world, I have very little experience there working on anything but super-trivial projects. But it is IMHO not something that will work well in the Drupal world where different modules are are combined and are expected to work together.

You suggest that a module that wants to provide a more flexible and dynamic selection would implement its own factory and its own mailer.transport service, which if I understand you correctly, would then also need to implement its own fake-transport that then actually transports the mail to the chosen transport. But with its own discovery tag, that will the not be compatible with the google transport module, that just provides that transport with the core tag. So a site that wants to sometimes send mails via google API with that module can't do that.

In my opinion, Drupal core should offer a neutral transport collection and factory/selector API. All it does is give you a transport, if registered, for the requested DSN string or object. As suggested, similar to existing patterns that we have in core, that *could* in factory be a FactoryFactory pattern that implements TransportFactoryInterface.

And the decision for the chosen transport would then happen on the layer above that, for example the default core logic could be in a mailer service, and a module that wants to change that could either replace *that* or implement an event and set a header or something, like the Symfony Transport class supports.

I am finding this tricky because we seem to be going round in circles😃.

> I started a sandbox with examples to better show how the API is supposed to be used in contrib / custom code.
As I said (many times!!) IMHO this issue is not creating an API. It is adding things to the DIC. So these examples are not actually code that we'd actually expect or recommend people to write.

✨ Create new mailer service based on symfony Active would create an API which should be used in forms. The current mailer_form_example is bypassing the whole Drupal mail system, including it's templates, theming, email policy, hooks/callbacks. This would create significant problems for site builders.

I haven't yet raised an issue yet to discuss events. However I believe we would create new events for two reasons. Firstly we are likely going to decorate the email class, so we'd like events that pass the decorated class. Secondly we will likely have multiple phases in our email building pipeline that aren't possible to access using the symfony events. This means that the mailer_event_subscriber_example has several problems. First it will alter the inner (decorated) Symfony Email object - this will affect the email sent, but it won't alter the Drupal Email object. So any code that correctly uses the Drupal Email object (for example to write a log) wouldn't see the correct cc address. For this reason we might even aim to disable the symfony events. Secondly, this code acts on all mails sent, without any possibility to see the Drupal context such as module/key.

I have an example in the working which demonstrates how to decorate the mailer.transport service. The decorator will dispatch mails according to some rules dynamically and also can fall back to the inner (default) transport. Thus, we might provide a better canonical pattern instead of the replacement of mailer.transport and mailer.transport_factory.

Regarding the API: It is crucial in my point of view that there is a well known transport service in the container. This is not at least in order to support the messenger component. That one uses an event subscriber (on the messenger side) to dispatch an event from the queue directly to a transport S\C\Mailer\Messenger\MessageHandler. That message handler needs a transport to be injected.

Note that the service tag is on the transprot factories (not the transport). There is no limit regarding the number of services which can collect these factories or their ids (see TaggedHandlersPass). I also hope that one fine day Drupal will support !tagged_iterator. Then we can simply drop addTransportFactory() and instead inject the finished iterator as constructor argument.

Again it's tricky for me to know what to do.

• Several times I've described code that I believe would work.
• Several times the MR contains code that I believe doesn't meet the requirement, and I have explained the reasons as clearly as I can.
• I've never seen any reply to either part about why it would be wrong, we just keep apparently going round in circles.

> I think that is actually the crucial thing to understand.
You seem to believe that people don't understand your idea. Actually maybe we do understand - the discussion is about if it's the right solution😃.

> I understand that this would be the approach, but I don't agree that this is the correct thing to do.
+1 to pretty much everything that Berdir says. The first question is the requirement. I see two parts:

1. Collecting and creating transports. This is neutral, it contains no preferences, policy or configuration. There should be no need to override it.
2. Transport policy/configuration. Core provides is a simplistic default based on a single configured DSN. Contrib will override or create a completely different service.

The second question is design/implementation details of 1). Berdir suggests a factory factory which is fine in theory but actually I think Symfony doesn't really allow it: parseDsn(), which handles failover/roundrobin is private. The logic could vary between Symfony versions so it seems unwise to duplicate it. Therefore I suggest fromString(string $dsn) which is much easier to implement in the service, and also it's the easier for the service-user as it requires only one step (rather than one step to create the factory and one step use it to create the transport).

Pushed mailer_transport_decorator_example in the examples sandbox.

I prefer to work very focused on single, concrete steps one at a time. I'd like to explain what that means:

1. Concrete: Build production code, test code, example code, docs (e.g., issue summary) so as to reduce the risk that people fail to grasp important facts. Code is cheap, most of it should be thrown away at the end or extracted to other issues, docs, sandboxes, whatever.
2. Focused on single steps: Temporarily disregarding everything which isn't directly related to the task at hand.

I will continue to ignore parts of comments which do not have any direct influence on the current step I'm in. Please don't be offended by that. I won't forget issues brought up in this comments (they are recorded here as a matter of fact).

If there is a need for a service with a fromString() method, then that should be easy to satisfy.
One way would be to just expose the Transport facade under mailer.transport_factory (in order to construct that we'd need to reintroduce a factory for the facade. At least as long as we are lacking the tagged_iterator though). I'd move the current Drupal\mailer\Transport to mailer.config_transport_factory. In that case mailer.transport_factory would become part of mailer API.

One way would be to just expose the Transport facade under mailer.transport_factory (in order to construct that we'd need to reintroduce a factory for the facade. At least as long as we are lacking the tagged_iterator though).

@Berdir: I'm tempted to introduce a compiler pass which emulates tagged_iterator but just for the mailer.transport_factory. That compiler pass would collect services tagged with mailer.transport_factory and inject them as a constructor argument to Symfony\Component\Mailer\Transport. What do you think?

> I have an example in the working
Please can you clarify where is the example?

> Note that the service tag is on the transprot factories (not the transport). There is no limit regarding the number of services which can collect these factories or their ids (see TaggedHandlersPass). I also hope that one fine day Drupal will support !tagged_iterator. Then we can simply drop addTransportFactory() and instead inject the finished iterator as constructor argument.

I completely agree with this statement. However it seems unhelpful to force all other code to duplicate the collection logic. What is the disadvantage of the alternative that Berdir and I suggest? We can implement the collection code in Core as a service that anyone can re-use.

> Regarding the API: It is crucial in my point of view that there is a well known transport service in the container. This is not at least in order to support the messenger component. That one uses an event subscriber (on the messenger side) to dispatch an event from the queue directly to a transport S\C\Mailer\Messenger\MessageHandler. That message handler needs a transport to be injected.

OK great now we are starting a productive discussion of the reasons. I feel the reason that this issue is surprisingly difficult is that the Symfony classes don't really fit what we want in Drupal. What you describe is one possible solution, and I will outline 2 other possibilities below. However before I do that please can I ask my favourite question😃 - do we need to solve that problem as part of this issue?? Once we have a candidate design for the Mailer and Email classes then it would be easier to give the correct answer. I feel that what we need here is to add the transports to the DIC. Do you think it would work if this issue just creates a TransportFactory service? We could leave out all code relating to configuration for now, and perhaps get something useful here that is possible to commit??

Anyway here are my 3 ideas:
1) As @znerol says, define a single well-known transport service that implements Symfony\Component\Mailer\Transport\TransportInterface. As I mentioned in #44, there are some difficulties. We are quite likely to decorate Symfony Email in a Drupal Email. This interface gives no easy way to get back to the Drupal Email. I can see some possible ways around that, but they are a bit ugly. a) We could extend the Symfony email, add a variable that points to a Drupal email, then decorate the extended class. b) We could put the transport DSN inside a header.

2) Ignore the Symfony classes that are too restrictive and don't meet our needs. Instead we create our own versions.

3) Use the Symfony classes, but not as services. For example we can create a Symfony\Component\Mailer\Mailer on a throwaway basis per Email with the correct transport (this is what DSM does).

> I prefer to work very focused on single, concrete steps one at a time.

I completely agree with that. Perhaps the difficulty is that we don't have a clear definition of what the current step is??

Please can you clarify where is the example?

In the sandbox linked from #50 and also from the issue summary.

Made available the Symfony Transport facade in the container (under the same service id as in the Symfony Framework Bundle). Hence, there is now a supported way to call fromString() on a service which has references to all available transport factories.

Seeking feedback on the revised service layout.

(going to update the examples sandbox in a minute).

@Berdir and @AdamPS what are your thoughts about the mailer_dsn config? Would you rather prefer a key-value mapping instead of a URI like string? I mean, there is hopefully still some time to change that before 10.2 is shipping.

Wouldn't it be nice to deploy something like this in settings.php:

$config['system.mail']['mailer_dsn'] = [
  'scheme' => 'smtp',
  'host' => 'smtp.example.com',
  'port' => 25,
  'user' => 'user@some-domain.org'
  'password' => '^C>6P2EdL.b<?9jBXgOu8*A>'
];

Instead of that:

$config['system.mail']['mailer_dsn'] = 'smtp://user%40some-domain.org:%5EC%3E6P2EdL.b%3C%3F9jBXgOu8%2AA%3E@smtp.example.com:25'

The only thing we'd loose is out-of-the box support for roundrobin and failover transports in core. But with the decoration approach I've outlined above this would be easy to achieve in contrib / custom code.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Filed a separate issue for mailer_dsn config: ✨ Use structured DSN instead of URI in system.mail mailer_dsn Fixed

As suggested by @Berdir

what if we have the classes in Drupal\core, but services.yml in a module? the service names wouldn't even have to change then, we could stick with mailer.foo. and the classes can be internal/experimentl

Tried that now and I really like that. Left the test code in the mailer module as well.

I added an Architectural aspects section to the issue summary. I hope I captured the important arguments regarding Code location and the Entry point. @Berdir and @AdamPS would you mind checking on that?

I'm planning to add subsections for other subjects which were discussed here in order to help all participants to better understand where we still need to make decisions.

Yes that works better from my point of view thanks.

1) My main outstanding comment still not addressed.

You have specified service: mailer.transport as the main entry point. The interface is Symfony\Component\Mailer\Transport\TransportInterface which reference the Symfony Email class. This creates a problem for code (already existing in Contrib) that wishes a) to support multiple transports and b) uses a new Drupal email class that decorates the Symfony class. The code needs information from the Drupal class but only has access to the Symfony class.

2) Smaller comment. For "Out of scope" point 1, let's work out how Contrib can fill the gap. Currently DSM has code that calls Transport::getDefaultFactories() which is neat, because it allows the 3rd-party transports to work automatically if present, without errors if absent, and without code specific to any transport . The alternative of defining each in services.yml would perhaps need a module per transport. Once this issue lands, I expect DSM would override the mailer.transport_factory service.

• The service is defined as returning the final class \Symfony\Component\Mailer\Transport. This restricts the options for how to override it. It seems better to have an interface.
• Currently TransportFactoryCollection is final, which seems to be an unnecessary obstacle - Contrib would need to duplicate the code.

Let's focus on the entry point. TransportInterface defines a single method:

    public function send(RawMessage $message, Envelope $envelope = null): ?SentMessage;

It takes a $message of type RawMessage. This signature makes it trivial to send messages which have been serialized before (e.g., because they were queued for asynchronous delivery, cf. messenger component). The RawMessage (and all its subclasses including Email) are data objects. They can be extended when needed. But since there is by design no interface, they cannot be substituted (inheritance trumps composition in this case). From the point in time a message enters the transport subsystem via the send() method, it must be subclass of RawMessage.

Given that, the answers to #79.1 are:

a) The transport id can be stored on the message. The Symfony Transports class uses a custom header for that.
b) This question is related to a). It essentially boils down to this question: How should context be passed from a custom/contrib mail builder to its mail deliver layer through the send() method.

My recommendation is to keep it simple. Add a custom header and store a couple of key value pairs before handing off the message to mailer.transport. Then inspect the metadata in the custom/contrib TransportInterface implementation and act accordingly. If the metadata is missing or if the message isn't of the expected type, then the custom/contrib transport should just forward it to the default transport. That pattern is easy to implement using service decorators. It also works with multiple decorators (coming from different modules). If custom headers don't cut it, then one can subclass Email and use instanceof in the transport implementation.

I'd like to keep the discussion focused on the entry point, thus not going to respond to the rest of #79 at this time.

> From the point in time a message enters the transport subsystem via the send() method, it must be subclass of RawMessage.
I agree. However the current patch forces all emails to enter via a single "virtual"/"meta" transport. This transport then has to multiplex between the actual different underlying transports, and it has to do so based on the limited information in RawMessage. This seems like a clumsy and unnecessarily restricted approach compared to what DSM does as I will describe below.

> My recommendation is to keep it simple.
Yes but actually IMHO your recommendation is more complex and has more limitations than my suggestion.

> I'd like to keep the discussion focused on the entry point, thus not going to respond to the rest of #79 at this time.
IMHO the fact that you repeatedly ignore comments that you believe are not relevant is making this issue much more difficult. From my point of view, all your following comments and patches don't really make sense because they ignore points that are very much relevant from my perspective.

From my perspective point 2 is relevant to the entry point, because I have a different idea of the correct entry point. I feel that my suggestion makes sense in relation to the issue title - I am proposing a service that gives access to the transports, without any restrictions trying to force the caller to use them in a particular way.

1. The entry point is a service GlobalTransportFactory[Interface] with function fromString(string $dsn = NULL)
2. The implementation collects transport factories and creates an instance of symfony Transport class stored in a protected variable. Also I propose that all created transports are stored in a protected array variable keyed by DSN to allow re-use.
3. In case of NULL DSN, the GlobalTransportFactory calls the ConfiguredTransportInterface service which is like ConfiguredTransportFactoryInterface except it returns a string DSN instead of a transport.

In the @Mail plug-in we can write very simple code:

$dsn = $message['transport_dsn'] ?? NULL;
$transport = $this->transportFactory->fromString($dsn);
$transport->send($email);

In the Mailer library we can write very simple code:

$dsn = $email->getTransportDsn();
$transport = $this->transportFactory->fromString($dsn);
$transport->send($email);

Plus we don't need to create a meta/virtual transport at all. So all round it's very simple.

Both cases can eventually be enhanced to support a bus by creating a throw-away instance of the Symfony library Mailer class with the selected transport (or by copying the code of that class).

How does that sound??

This is based on the successful implementation in Contrib of DSM, and the code already works without needing anything from Core. If Core creates services that would actually make the DSM code more complex then potentially we'd keep the code we already have😃.

The current service structure and entry point respect the requirements of the Symfony messenger component. That one requires a TransportInterface in the container.

If core wants to be compatible with Symfony messenger, then the entry point / service structure I'm proposing here is the way to go.

That is essentially the decision we need to take now.

> The current service structure and entry point respect the requirements of the Symfony messenger component. That one requires a TransportInterface in the container.

Any chance you could provide a reference to the relevant code please?

The Symfony framework bundle registers an instance of Symfony\Component\Mailer\Messenger\MessageHandler and injects mailer.transports into its constructor.

Pinged people from Drupal Symfony messenger integration. They likely will know better than me.

Thanks for the links.

I suggest that we "ignore" the question of messenger in this issue. We would need to solve it as part of ✨ Create new mailer service based on symfony Active , and there are likely a range of options. We can adapt Drupal Mailer to fit Symfony, but less ideal for Drupal. Or we can make Drupal Mailer ideal for Drupal and then some adaption is needed for Symfony Messenger (MessageHandler is apparently a very simple class, so maybe there are options to ignore/replace it??). It seems difficult to know until we start to try the different options, creating the classes, services etc.

In any case the "entry point" for the Mailer service will presumably be something like Mailer::mail(). The mailer.transport service seems like a mostly internal thing that would only be used by the Mailer and Contrib modules that extend it, and only for the purpose of fitting with Symfony.

This issue (as I've said many times before😃) is apparently about adding transports to the DIC. Everything I've described in #81 seems to fit that idea very well. If we want to add a mailer.transport service later then there's nothing preventing that. The proposal of #81 would already be a significant step forwards
- We can adjust existing Contrib modules to use the new services.
- We can create a Contrib module that overrides the new services, providing a UI to edit transports (with full support for failover, 3rd-party, etc.options, etc.), taking code from DSM.
- We can use this same module with Core, DSM and DSM-L.

How does that sound???

The approach outlined in #81 assumes that transports are stateless. That isn't exactly true and under some corner cases could lead to surprising behavior. Attached are two scripts which I'll be going to use to illustrate a couple of things. The script test-global-transport-factory.php implements the approach from #81. The script test-transport-on-container.php implements the approach taken by Symfony Framework Bundle (and in the current MR). The scripts attempt to send 10 emails in a for-loop.

The following additional tools are recommended in order to reproduce these results:

• mailpit
• tcpdump
• ncat or some other flavor of nc/netcat with the --idle-timeout flag

Connection reuse

Preparation:

• Inspect the scripts. Make sure that the line define('TEST_DSN', 'smtp://127.0.0.1:1025') is uncommented and all other defines are commented out.
• Open a terminal and run mailpit (run without any flags mailpit defaults to 1025 for the SMTP port and 8025 for the web interface
• Open a terminal and run tcpdump. The given bpf filter will log every tcp packet with the SYN flag set which is directed at port 1025 on localhost. Hence, one line will be printed for each tcp connection attempt. People on macOS might need to adapt the interface name (lo0 instead of lo AFAIK):
  sudo tcpdump -q -i lo 'tcp[tcpflags] & (tcp-syn) != 0 and tcp dst port 1025'

Run the test scripts and observe the tcpdump output. Also check the mails in the mailpit inbox.

Result:

• When test-global-transport-factory.php is run, tcpdump prints 10 lines. The script opened one connection for each message.
• When test-transport-on-container.php is run, tcpdump prints 1 line. The transport only opens one connection and reuses it for all messages.

Failover behavior

Preparation:

• Inspect the scripts. Make sure that the line define('TEST_DSN', 'failover(smtp://127.0.0.1:1026 smtp://127.0.0.1:1025)'); is uncommented and all other defines are commented out.
• Open a terminal and run mailpit (run without any flags mailpit defaults to 1025 for the SMTP port and 8025 for the web interface
• Open a terminal and run tcpdump. The given bpf filter will log every tcp packet with the SYN flag set which is directed at port 1025 or 1026 on localhost. Hence, one line will be printed for each tcp connection attempt. People on macOS might need to adapt the interface name (lo0 instead of lo AFAIK):
  sudo tcpdump -q -i lo 'tcp[tcpflags] & (tcp-syn) != 0 and tcp dst portrange 1025-1026'
• Open a terminal and emulate a failing SMTP server on port 1026 (timing out after 5 seconds) using ncat:
  while true; do nc -i 5 -l 1026; done

Run the test scripts and observe the tcpdump output. Measure the time it takes to complete the whole script. Also check the mails in the mailpit inbox.

Result:

• When test-global-transport-factory.php is run, tcpdump prints 20 lines. The script tries to connect to port 1026 and then falls back to 1025 for each email. The script runs for roughly 50 seconds (10x5sec).
• When test-transport-on-container.php is run, tcpdump prints 2 lines. First it tries to connect to port 1026, then falls back to 1025 and sends all messages in a row. The script takes roughly 5 seconds to complete.

Conclusion

With this experiment, I'd like to point out that the approach #81 could lead to subtle scaling problems when people start to use the new mailer framework for non-trivial workloads.

Core doesn't send emails in a batch. For password resets, contact messages and update notifications it is irrelevant whether SMTP connections are reused and whether transports are keeping their state or not.

On the other hand, core doesn't need to support multiple transports in the first place. If contrib/custom likes to implement that, then they can already do so on top of the current MR (there is a working multitransport implementation in my examples sandbox → ).

I did an extensive research on Symfony messenger today. Starting with the following docs:

• Messenger: Sync & Queued Message Handling (top to bottom)
• The Messenger Component (top to bottom)
• Sending Emails with Mailer (Sending Messages Async chapter)

Then I went on to examine the contrib project Drupal Symfony Messenger → and its advanced fork Symfony Messenger + Drupal: Realtime Queues and Cron → .

In short, the Symfony messenger component is \Drupal::queue() on steroids. The component allows routing messages of a certain type to different message buses backed by a growing list of message transport types (Database, Redis, etc.). I wouldn't be surprised if this component is going to replace good old queue in some future Drupal release.

I went through the code and the issue queue of the aforementioned contrib modules and found two interesting issues (on the sm fork by @dpi):

• ✨ Symfony Mailer integration — core Postponed
• ✨ Symfony Mailer integration — contrib project Postponed which points to ✨ Add Symfony Messenger support for async messsages (emails as queues) Active in the DSM issue queue

Basically MR 69 is refactoring the DSM services structure in a way which is very similar to Symfony Framework Bundle and as a consequence also looks almost exactly the same as to the services structure proposed in this issue. Taking that MR as a starting point it was trivial to make core mailer in this issue work with the sm module. It is enough to add two additional service definitions:

  mailer.messenger.message_handler:
    class: Symfony\Component\Mailer\Messenger\MessageHandler
    arguments:
      - '@mailer.transport'
    tags:
      - { name: messenger.message_handler }
    public: false

  mailer.mailer:
    class: Symfony\Component\Mailer\Mailer
    arguments:
      - '@mailer.transport'
      - '@?messenger.default_bus'
      - '@event_dispatcher'
  Symfony\Component\Mailer\MailerInterface: '@mailer.mailer'

With those services, mails sent via \Drupal::service('mailer.mailer')->send() are automatically routed via Symfony messenger if the sm module is enabled and then reinjected into mailer transports via the MessageHandler.

With all those pieces in place it is much easier to follow the code flow by just stepping through all the components using a debugger.

Email async mailer docs point out that messages sent via the message bus need to be serializable. They especially mention that care should be taken when using doctrine entities in the context property of TemplatedEmail.

The messenger docs point out another detail which needs to be considered when Symfony messenger is used together with the mailer:

The only drawback is that '*' will also apply to the emails sent with the Symfony Mailer (which uses SendEmailMessage when Messenger is available). This could cause issues if your emails are not serializable (e.g. if they include file attachments as PHP resources/streams).

Since Drupal is making it easy to include files from different sources using stream wrappers, I do not think that people need to use resources/streams at all.

Btw. while going through the mailer docs again, I discovered that they introduced a way to add tags and metadata now. Transports and event subscribers may inspect that metadata and act accordingly (#80 is even simpler with that). Also some third-party transports will forward those tags and metadata to cloud messaging providers where they can be used for further processing as well. We can just translate current email ids to tags and be done:

$email->getHeaders()->add(new TagHeader('user_password_reset'));

All of that brings me to the following proposal for a path forward:

• The service mailer.mailer implementing MailerInterface should be added in this issue (using the original Symfony Mailer class) and it should be the main entry point for the mail delivery part of the new mailer module.
• Atop of the mail delivery layer, we can add utilities to simplify email building (in other issues).
• In order to reduce the risk of serialization problems in combination with Symfony messenger, I think we should render HTML messages before they are handed over to the mail delivery layer.
• Modules implementing hook_mail_alter now who wish to modify the content of messages probably should use the theming layer in the future (preprocess hooks etc).
• Modules implementing hook_mail_alter now who wish to modify headers or other non-body parts of the message should use event subscribers. They can use tag headers to target individual messages.

It's tough to keep up here. So far, it's mostly just been @znerol, @AdamPS and me being active here, with @znerol and @AdamPS having pretty different expectations of what this should do and how it should work. I'm being torn between them, seeing both advantages. I think we need some more feedback from the community at this point, specifically also framework maintainers.

@znerol did a great amount of research on how the transports API is expected to be used. That's not how I would design it, but it's also clear to me that there are advantages, for example around interoperability with other components like the messenger. This is nicely outlined in the issue summary.

I think one aspect that's not covered yet in the issue summary where there's still a disagreement is whether or not we should trigger the built-in events. Like @AdamPS, I have some concerns as well, due to things like translations, BC with the old mail API, context (theme, language, ...) and so on and given that it's still not clear whether or not and how we'll wrap Email objects to cover our use cases.

But thanks to the hybrid core+experimental module approach that I quite like, I'm OK with moving forward with this approach and see if we can build the systems that we need or decide we need in core on top of this. I'm not quite sure *how* we approach that given that this is now in an experimental module that we can't yet depend on, but we can figure that out in follow-up issues.

Still exploring the entry point. Materialized the research from #88 into this PR. The entry point to the mail deliver layer is now @Symfony\Component\Mailer\MailerInterface.

While looking for ideas on how to go about @Berdir questions regarding experimental modules, I looked around in sdc. That made me realize, that we can now rely on service aliases. That is especially cool for the optional dependencies of mailer.transport_factory.abstract and mailer.mailer:

  mailer.transport_factory.abstract:
    abstract: true
    class: Symfony\Component\Mailer\Transport\AbstractTransportFactory
    arguments:
      - '@Symfony\Contracts\EventDispatcher\EventDispatcherInterface'
      - '@?Symfony\Contracts\HttpClient\HttpClientInterface'
      - '@logger.channel.mail'
...
  mailer.mailer:
    class: Symfony\Component\Mailer\Mailer
    arguments:
      - '@Symfony\Component\Mailer\Transport\TransportInterface'
      - '@?Symfony\Component\Messenger\MessageBusInterface'
      - '@Symfony\Contracts\EventDispatcher\EventDispatcherInterface'
  Symfony\Component\Mailer\MailerInterface: '@mailer.mailer'

We do not need to standardize a service name here for neither the HttpClientInterface nor the MessageBusInterface. If any module introduces one of them, it is picked up automatically.

Updated the issue summary for #88.

Adding a variety of tidbits/opinions from discussions with @znerol on Slack from the past few weeks.

I'd like to see the transports class working: id `mailer.transport` class: Symfony\Component\Mailer\Transport\Transport.

Core should support creating transports from multiple DSN's into transports.

Mailer requires the concept of a default transport. By convention, this is the _first_ defined transport. Core could do something else, of course. I'd try to not invent new concepts whatever we do.

The default transport is autowired to Symfony\Component\Mailer\Transport\TransportInterface. I think its reasonable for core to always fall back to this transport.

Contrib/custom code should always have the option to send to an alternate transport. Either set in the intial Email object or added as email headers via X-Transport.

There should be some kind of factory in the container that produces Mailer objects, such that its arguments are autowired properly. The method I used for contrib Symfony mailer was a non shared service.

There should be some kind of factory in the container that produces Mailer objects, such that its arguments are autowired properly. The method I used for contrib Symfony mailer was a non shared service.

Thats the bit I do not understand. Why do you think mailer.mailer must be marked with shared: false? Upstream doesn't seem to dot that.

Actually I don't recommend it for core, its just contrib was already contructing anew each time.

Filed 📌 Add Psr\EventDispatcher\EventDispatcherInterface alias to core services Needs review , with that one we could use autowiring for mailer.transport_factory.abstract and mailer.mailer.

Thanks @znerol for putting so much work and energy into this issue. Unfortunately I have much less spare time and energy at this moment, and I find it tough even to keep up with all the developments here.

> The approach outlined in #81 assumes that transports are stateless.
Actually I don't think so. In my second numbered point (sorry it was brief and perhaps hard to understand) I suggested re-using transports for emails that specify the same transport DSN. This allows transports to have state and is good idea for performance (it cuts the cost of repeatedly creating the transport). It's true that currently DSM creates a transport per email however we can easily fix that.

NB if we autowire based on the Symfony Transports class (rather than using an Interface), we apparently prevent reuse because the fromString function creates a new transport each time.

In summary, it seems that

1. symfony presents a clear idea for how the mailer and transports should work
2. it does so in a way that makes it difficult for Drupal to be different: there are lots of final classes, private functions/variable and classes without interfaces
3. even where Symfony does create an interface, there are very good reasons why Drupal might want a different interface

Basically I agree, that it is pretty easy to introduce the Symfony interfaces and classes into Drupal by copying the Symfony Framework Bundle. I can see that this has some advantages, however you end up with something very different from DSM Contrib module. In Contrib, we have deeply integrated many Drupal mechanisms into the Mailer. To do that, we found it necessary to ignore many of the Symfony interfaces.

I believe that the most recent PR is likely to be fundamentally incompatible with DSM Contrib. It would potentially lead in a direction when Contrib used hooks to delete all the Core services. It could make migration from Contrib to Core difficult.

> with @znerol and @AdamPS having pretty different expectations of what this should do and how it should work.

Not necessarily😃. I am open to adopting some of the ideas of @znerol, it's just that I'm not convinced that it should be part of this issue. My point is that as soon as we put in place an entry point that it has huge consequences on the entire mail system design. The wrong interface can make it very difficult to integrate some Drupal key concepts.

IMHO the right approach is to create the entry point as part of ✨ Create new mailer service based on symfony Active . As part of deciding that interface we should have at minimum

• A working prototype implementation
• A feature comparison with Contrib DSM including a design that shows it would be possible for Contrib could add back the missing features (or if not, clear approval from the framework manager that these features don't matter)
• A working prototype migration path from Contrib DSM for the 23k sites currently using it

I feel that one potential problem at the moment is that the design for Core Mailer is perhaps being done without a deep understanding of what Contrib Mailer can do, the design chosen and the reasons why. I tried quite a lot of different things, and hit various problems, gradually leading to the current design. The discussion here includes some suggestions that I already tried, and found problems with.

I feel that Symfony is a toolbox. We can choose which tools to use, and don't have to use them all.

Clearly we do want to use the transports, and the email class as that's the whole purpose of this META issue. We don't have to expose them however - they could just be an implementation detail.

In terms of the rest, in Contrib DSM we didn't use much else

• We didn't expose any Symfony interfaces or the classes because they weren't quite what we wanted. It was easier to create new interfaces.
• We didn't use the Symfony events. Firstly they used the email class which we didn't use, secondly we had a different pipeline so needed new events (actually we used hooks, but it doesn't make much difference)
• We didn't use the Mailer service which anyway is just a few lines of code (we create one internally, but that's only because I didn't realise you can send directly to the transport)
• We didn't use the Messenger service. Currently we don't have an equivalent, maybe we should use it. But it seems to rely on other parts that we don't want - maybe we can adapt or decorate it??

With the current MR, I'm proposing an approach which is analogous to how the http-foundation component is being integrated. Drupal provides abstractions covering a wide range of use cases typical for application code (forms, entity displays, render arrays). All of those abstractions end up generating a Symfony Response representing headers and content sent to the user-agent.

Application code rarely needs to construct a Response directly. But since the lower layers do not actually have a reason to care about how a response is generated, Drupal doesn't prevent controllers to do exactly that.

Drupal also fires http-foundation events (e.g., RequestEvent). As a result it is possible to directly reuse Symfony RouterListener and other request subscribers even though route storage in Symfony is completely different than in Drupal.

The Symfony http-foundation component is being integrated into Drupal in a way which provides a convenient interface for application code and at the same time provides interoperability with existing Symfony and third-party components.

It is worth thinking of the Email class as a "cousin" of the Response class. Both represent the result of an action and both are responsible to convert it to a representation understood by a user-agent.

Once the mailer component integration is completed, it will rarely be necessary to construct Email instances directly. Instead, there will likely be abstractions in place (provided by core, contrib or custom code) covering a wide range of use cases typical for application code. But the lower layers (i.e., the mailer and the transport(s)) do not actually have a reason to care about how an e-mail is generated.

Abstractions are useful for the mail building part of the upcoming mailer implementation, just as abstractions are useful for the response building part of the existing http controller architecture.

It is important to understand that this MR targets the mail delivery part of the upcoming mailer implementation. This roughly compares to the lower layers of http-foundation. E.g., the http stack middlewares (page cache, session, etc.) and many of the response event subscribers.

I do not know of any Drupal issue which would be easier to resolve if the lower layers of http-foundation (including Response) would be completely hidden behind an abstraction. Even cache metadata bubbling has been solved in a way which is 100% compatible with the Symfony http-foundation component.

Consequently, I do not see any point in abstracting away access to the Email class or the MailerInterface (as suggested in #98). In fact, I am certain that the maintenance costs of such an abstraction would far outweigh any benefits.

Core developer resources are scarce. This MR proposes an approach which is unlikely to cause extensive maintenance overhead. At the same time, it ensures interoperability with additional Symfony and third-party components. All of this is achieved by following the successful integration pattern of the http-foundation component.

Tagging with Needs framework manager review in order to get a sign-off regarding the approach.

Quick status update:

• Drupal Symfony Mailer Lite → added support for Symfony Messenger in a way which is compatible with the approach proposed here (see ✨ Add Symfony Messenger support for async messsages (emails as queues) Needs review ).
• Core added 📌 Add Psr\EventDispatcher\EventDispatcherInterface alias to core services Needs review , so we can autowire: true for the mailer service now.

This issue still needs feedback from framework managers. Would be cool if we could make this happen in 10.3.

Squashed the early commits up to #3379794-37: Add symfony mailer transports to Dependency Injection Container → in order to minimize conflicts in future rebases.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Chase head and update 📌 [PP-1] Add a way to capture mails sent through the mailer.transport service during tests Postponed .

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Tracking 📌 Use autoconfigure more in core Active

Imagine this will need a change record or a few.

I popped into to see how this issue is progressing. I'm definitely not going to get sucked back into an argument. I'll just point out a few things and leave you to decide whether to pay any attention to them.

I find the scope of this issue quite confusing. The title is "Add symfony mailer transports to Dependency Injection Container" which suggests the lower parts of the email stack that shouldn't be used directly. In that case, it's basically fine.
- Forcing all emails through a single instance of TransportInterface is a bit awkward however it can work. It will require a de-multiplexing transport that checks for a special header that specifies the transport DSN. (Symfony already has one, but it assumes a fixed named set of transports and it doesn't allow dynamic setting of DSN e.g. in a hook.)
- I have some comments that would help reduce duplication in contrib. If they aren't accepted then contrib can simply ignore the core transport factory service and create a new one.

Then later on I read "As a developer I want my service to reference a mailer service in order to submit an e-mail message". That suggests that this issue creates a service and API (exposed directly from symfony) intended for use in contrib code that sends mail. In that case, I have some serious concerns as the interface seems totally unsuitable. AFAICS we would then lose some key things compared with the existing MailManagerInterface:
- module+key to help hooks identify mails (we could combine them into a single string)
- encoding key information into 'params' and then building the email from these (allows hooks to alter the params, or code can replace the building code entirely)
- building the email in a callback function to switch render context so avoiding pollution (and we can add switching of language and user)

Also we need to consider migration from the old mail system to the new. I expect that we will need a period of time when both old and new mails systems are supported. I feel that sites likely want to choose when to switch mail system independent of developers changing which API they use. This can be achieved by means of mapping code. However if modules are encouraged to write directly to the symfony services then it's less clear if that could work.

I propose that:

1) We should mark the mailer.mailer and mailer.transport services are @internal, and likely rename the first one.

2) Then in a later issue, core can create a new mailer service that contrib would actually use. This service sits above what's currently called mailer.mailer. Most of the bullets currently in Problem/Motivation would need to be moved to this later issue.

3) Finally contib code can create a further enhanced mailer service that sits above the core one. Currently contrib Drupal Symfony Mailer has many extra features that are popular with sites:
- deep integration into Drupal mechanisms, including theme/template/render, CSS libraries, configuration, plug-ins.
- generic "mailer policy UI" that exposes GUI access to many parts of the Email interface.

Specifically, the "later issue" is ✨ Create new mailer service based on symfony Active , and it gives abundant reasons that hopefully make it clear that we need a wrapper service above the symfony mailer one.

I've thought some more about multiple transports. I agree that core won't support the required transport configuration, however I do feel that we should offer the API to allow contrib to do so. Hence there should be a function on the email interface to set a transport DSN (which comes in the later issue), and this issue should have the required transport code to support it. I believe the required code is in fact simple, and if we don't do it then the core transport factory/config services would be mostly unused - because almost everyone would install the config module so that they can configure their default transport and it would replace the core services.

So in summary here is my suggestion:

1) Change the issue title to "Create the symfony mailer delivery layer". It has become much more powerful than just integrating transports.

2) Update the IS to remove all the existing "As a developer" bullets which instead go in the later issue. There are 2 things we can do:

• Developers of modules that already have written similar code to this issue (likely only DSM and DSM-L) can switch to use the core code.
• Developers can create a module with a UI to specify mailer transports.

3) Rename the mailer.mailer service to mailer.delivery and mark it as non-public.

4) Mark the mailer.transport service as non-public. Implement it with a new class named DemuxTransport, FanOutTransport or something similar which contains the following simple code.

• Check for a special header X-Transport-Dsn and then remove it, else use empty string (meaning use default transport).
• Call the transport manager to create a matching transport and store it for reuse in case of a future request with an identical DSN.
• Send the email using the new or existing transport.

5) Rename ConfiguredTransportFactoryInterface to TransportFactoryInterface with interface as follows.

• function createTransport(string $dsn = '');
• function setDefaultTransport(string $dsn);

6) Please do call Transport::getDefaultFactories() in the TransportFactoryAdapter. I hear and understand your IS "Note: Symfony mailer provides many transports...". However this simple addition will not cause any of the problems you describe as it simply makes use of something that Symfony already offers - and any issues raised relating to the supported list can be pointed upstream. This change gives automatic support for 14 3rd-party transports (in the version I happen to be using today) and that list will automatically increase. Having done that we can remove the 3 transport factory services, keeping only sendmail that we alter.

If we do those things then IMHO it would create a really useful transport delivery layer for both core and 2*contrib. I would then happily give my vote for RTBC.

Thank you for the feedback.

Re #114

• module+key to help hooks identify mails (we could combine them into a single string)

Symfony Mailer allows for tags and metadata attached to messages. These can be used to identify messages from within event subscribers. Some third party transports even pass them to external mail services, where they can be used further for grouping/routing/accounting etc.

• encoding key information into 'params' and then building the email from these (allows hooks to alter the params, or code can replace the building code entirely)
• building the email in a callback function to switch render context so avoiding pollution (and we can add switching of language and user)

There are two activities when it comes to send transactional mail from Drupal:

# Building mails
# Delivering mails

This issue is about the second activity. It is clear that the mail building part will need another round of exploration and experimentation to get to something simple and useful. But it is out of scope for this issue.

From the remaining comments I gather, that you are very concerned about the lower level part (i.e., the mail delivery) to be exposed in the container. Let me point out, that we do that a lot in core. For example, there is a database service giving developers raw query access to the database. But, it is much more safe and convenient to create entities and use entity queries. As a result, nobody uses the database directly for day-to-day tasks, even though it isn't explicitly marked as @internal. Hence, we shouldn't be concerned to expose mailer.mailer in the DIC.

Re #115 I'd rather keep things simple here. The mailer module is marked experimental. It is possible (and expected) that the API can change. If truly required, then the API surface and the inner workings can be made more sophisticated. But for the beginning, I prefer to follow closely what symfony mailer offers out of the box.

Thanks for the reply, and for all your hard work on this issue.

1)

I gather, that you are very concerned about the lower level part (i.e., the mail delivery) to be exposed in the container

Well I am responding to what you have written😃.

• The IS states that developers can write code in 7 specific ways. For me this implies that code would use the API from this issue directly.
• The example code seems to use the API from this issue directly.
• The example code seems to have the problems that I describe in #114.

If you replace all the above with a statement that this is a lower-level interface that would not normally be used directly then I agree that the problem would be solved.

2) I'm not sure you answered my point about whether mailer.mailer is definitely the right name for the delivery layer. Do you have a proposal for name of the layer above that is the actual mailer that people should use?

3)

But for the beginning, I prefer to follow closely what symfony mailer offers out of the box.

Well my doubts are mostly about TransportFactoryAdapter and ConfiguredTransportFactoryInterface which I guess aren't from symfony mailer, but our own adaption?? I feel that if we instead adapt in a slightly different way, then the result could be much more use to contrib. However it doesn't matter that much as presumably I could just override the mailer.transport service to a different class and ignore the other two services??

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Thank you for your feedback.

The IS states that developers can write code in 7 specific ways. For me this implies that code would use the API from this issue directly.

I revised the issue summary slightly to emphasize that this is bleeding-edge stuff. In order to spec out the scope, I did choose a common user story template. I can switch to another form of scope definition if the current one is not clear enough.

• The example code seems to use the API from this issue directly.
• The example code seems to have the problems that I describe in #114.

2) I'm not sure you answered my point about whether mailer.mailer is definitely the right name for the delivery layer. Do you have a proposal for name of the layer above that is the actual mailer that people should use?

There is some precedent in core for services without a service name. The only way to reference this services is the fully qualified interface name. Recent examples include the sdc ComponentLoader or the CsrfExceptionSubscriber. I'm going to try out whether this is possible for the mailer as well.

Well my doubts are mostly about TransportFactoryAdapter and ConfiguredTransportFactoryInterface which I guess aren't from symfony mailer, but our own adaption?? I feel that if we instead adapt in a slightly different way, then the result could be much more use to contrib.

See the comment on the GitLab MR. In summary, contrib and custom code wishing to customize the way transport(s) are configured is not supposed to reuse TransportFactoryAdapter. Instead such modules are supposed to supply their own implementation of ConfiguredTransportFactoryInterface.

Sorry, forgot to answer that one:

• The example code seems to use the API from this issue directly.
• The example code seems to have the problems that I describe in #114.

This issue and also many issues in the contrib projects which integrate symfony mailer show that it isn't too easy to understand the architecture and integration options of the upstream component. The example code validates the approach taken here. It isn't supposed to be the reference for the resulting product.

There is some precedent in core for services without a service name. The only way to reference this services is the fully qualified interface name.

Opened a second MR 9831 in order to explore the fqcn naming pattern. Also updated the sandbox module. That one should now work with both branches.

This comment is about policy and communication, which I feel are key for this complex issue.

1) Policy

I feel that there is a key policy decision here: is it OK for module code to chose to write directly to the delivery layer if it prefers??

The current mail API requires a particular structure with parameters and a callback, which gives key benefits for customisation and avoiding certain bugs. The current rendering code for generating HTML pages has key patterns including MarkupInterface, render array, themes, and access control. In both cases, we seem to have a happy consensus the developers don't try to bypass them.

The contrib DSM code effectively integrates these two on top using a new mailer the sites on top of the delivery layer. It provides various benefits including security. (If someone offers to fund me $500 for each security bug relating to email I find in a current stable contrib module I reckon I'd find quite a few😃.) If modules write directly to the delivery API then DSM can't really work for these modules - e.g. already the HTML is translated and rendered. So I am in favour that we also build a consensus here not to bypass this layer.

I found this comment in #116 📌 [PP-1] Add symfony mailer transports to DIC Postponed very helpful and it could usefully be added to the IS:

=======

There are two activities when it comes to send transactional mail from Drupal:

1. Building mails
2. Delivering mails

This issue is about the second activity. It is clear that the mail building part will need another round of exploration and experimentation to get to something simple and useful. But it is out of scope for this issue.

=========

To say that something is experimental means that it's the current preferred option however that might change. That's quite different from something that is contrary to what we actually want.

I feel that the current scope statement bullets 1,2,3,7 are actually part of building mails (#7 altering the building process). Therefore I would propose to put them as out of scope, as this issue doesn't yet provide the correct Drupal API for these cases. People could use it like that - but it's effectively the same as if they already use Symfony Mailer directly without needing this issue. Neither case is the Drupal preferred API. I agree you have proved that the delivery layer works for these cases and that's great.

On the other hand, bullets 4,5,6 are the proposed correct Drupal API as far as we know - but of course it's experimental.

====

2) Sandbox

It's great that you provided the Sandbox, and you link to it in the IS:

There is a Sandbox with examples demonstrating how custom / contrib is supposed to use this API.

Developers are likely to start copying parts of your code, so let's try to make this code as helpful as we can. Specifically I would be grateful if you agreed to add @todo markers in each place where module code uses the delivery service, and it should later be changed to use the building service.

I don't so much agree with the sentiment from #121 📌 [PP-1] Add symfony mailer transports to DIC Postponed

The example code validates the approach taken here. It isn't supposed to be the reference for the resulting product.

Secondly, my technical comments.

1) I'm not sure you answered point 6 from #115 📌 [PP-1] Add symfony mailer transports to DIC Postponed . Yes you have emphasised this is out of scope, however I feel I am raising a valid question about the reasoning given for that decision. I feel that Symfony has provided a useful solution for a genuine requirement. I don't understand your reluctance to use it.

Please do call Transport::getDefaultFactories() in the TransportFactoryAdapter. I hear and understand your IS "Note: Symfony mailer provides many transports...". However this simple addition will not cause any of the problems you describe as it simply makes use of something that Symfony already offers - and any issues raised relating to the supported list can be pointed upstream. This change gives automatic support for 14 3rd-party transports (in the version I happen to be using today) and that list will automatically increase. Having done that we can remove the 3 transport factory services, keeping only sendmail that we alter.

2)

There is some precedent in core for services without a service name.

Yes good idea. Perhaps we could even do it for all the services which Symfony seems to do already. The interface serves very well as the service name, and I don't see any reason for another identifier. Even for classes that don't auto-wire, $container->get(MyInterface::class) works well.

3)

See the comment on the GitLab MR. In summary, contrib and custom code wishing to customize the way transport(s) are configured is not supposed to reuse TransportFactoryAdapter. Instead such modules are supposed to supply their own implementation of ConfiguredTransportFactoryInterface.

I continued the discussion in the MR. Personally I prefer a simpler approach. I would be happy to see any changes here, but it's fine if not. The code in this MR doesn't cause me a problem as I am free to ignore it.

Thank you for the feedback.

Re #124

I feel that policy discussions merit their own issues. I did observe the practice in Drupal core to seek policy decisions in issues with the title prefix "[policy, no patch]". I like that approach because it gives non-code decisions the weight and visibility they deserve.

Regarding the issue summary. I removed the user stories and replaced them with a version of the statement from #116. Additionally I did reword the issue summary a little bit around the sandbox link.

I did update the list of services in order to reflect the new approach in !9831 and did hide the older MR.

Re #125

The Transport::getDefaultFactories() started out as a private method in the mailer component. It has been made public in order to satisfy the needs of projects which reuse the mailer component without a container (see symfony#35469. Drupal comes with a container. Prepopulating the list of transport factories by calling into Transport::getDefaultFactories() would lead to a weird situation where some transport factories would appear automagically under special circumstances (i.e., if their dependencies happen to be installed by composer), and others need to be registered explicitly. This inconsistency also would lead to a situation where some factories can be modified, removed or decorated (because they were explicitly registered in the container) and others not. In my opinion, this is a no-go DX wise.

znerol → changed the visibility of the branch 11.x to hidden.

znerol → changed the visibility of the branch 3379794-pp-1-add-symfony to hidden.

Removed the @logger.channel.mail argument in the abstract transport service definition in order to avoid 🐛 Core Symfony Mailer throws error on transport shutdown Active .

Great thanks @znerol those changes really helped.

I have created a new 2.x version of DSM contrib module that uses the service structure from this issue. So the current code is definitely usable. However Symfony mailer offers many different integration options, and I believe we could make some better choices for Drupal.

1) Transport.php has the code we need, however I would prefer to decorate it rather than expose it directly. We can create a class named something like UnifiedTransportFactory with a corresponding interface. The advantages are: a clearer name (it's a transport factory, not a transport); an interface file with comments; simplification of the service structure by removing one service; flexibility to override this service (Transport is a final class so it cannot be overridden).

2) Symfony mailer allows for the implementation of Symfony\Component\Mailer\Transport\TransportInterface to be either a single transport or a multi-transport. I feel that for Drupal we should always use a multi-transport - even if there is only one configured transport. Advantages: we only have one setup to test rather than two; it makes Core ready for multiple transports (similar to how MailManager was ready for multiple mail plugins) even though we don't provide a way to configure it; it simplifies the service structure removing one service and with a clearly named class that indicates the purpose; we can allow the X-Transport header to contain a DSN, so hook code can modify the transport.

I feel it would be great if some of the other 32 people following this issue could offer their views.

Removed the @logger.channel.mail argument in the abstract transport service definition in order to avoid...

Thank you, that was causing many weird errors for me too. Maybe it would help to put a comment in the service file to explain??

Regarding Transport::getDefaultFactories() , I don't see it as weird if the transport services automatically appear when the corresponding transport factory class is installed. This seems like exactly the desired behaviour.

I have two questions please.

1) It should be possible to automatically create a service for each of the default factories. Would this be any more attractive to you?

2) If we don't do this, can you describe what you would do instead? A contrib module could create all the service definitions in a services.yml file, but then they would exist even if the corresponding transport wasn't available. We could make one module for each transport, but that seems pretty inefficient.

Thanks for the suggestions. For this iteration I focused on the Symfony mailer Transport facade which rightfully has been the subject of quite some discussions in this issue.

Thus, I reworked TransportFactoryAdapter into TransportServiceFactory and removed its dependency on the Symfony mailer Transport facade. That unlocks a couple of interesting simplifications:

• We can use the AutowireIterator attribute to collect services tagged with mailer.transport_factory directly from the constructor.
• That renders TransportFactoryCollection superflous.
• In order to avoid the Symfony mailer Transport facade completely (no decoration necessary) it is enough to copy the simple fromDsnObject() method into a small TransportServiceFactoryTrait to make it reusable by transport service factory implementations.

Adapted the issue summary with the new service structure.

Updated the sandbox for new TransportServiceFactory.

Re #131.1:

The Symfony mailer Transport facade is now gone.

Re #131.2:

I think we have to accept that we disagree in that regard. I'd like to remind people here, that we seem to have reached consensus on that in Vienna last year 🌱 [META] Adopt the symfony mailer component Needs review . Core should just stick with one transport, but it should not get in the way of contrib / custom code if they replace it with something fancier.

Re #133:

I propose that we postpone adding those factories to a follow-up. An initial version doesn't require auto detection of available transports.

For the same reasons as in ✨ Add Alpha level Experimental Package Manager module Needs review , this issue likely neither needs a CR nor a release note snipped. Added a note about that to the issue summary.

I'm a bit late to the party but went looking for an issue like this after a recently being reminded of the poor DX of the current API.

I've tried to go through all 138 comments and it looks to me like they have all been addressed.

The new API in this issue feels like the right level of abstraction to me, and a core API plus experimental module seems like the right approach.

Looks like there is thorough test coverage too, and the IS is up to date.

We still need to get a framework manager to review, but otherwise I would RTBC it.

Thanks @znerol.

>> The Symfony mailer Transport facade is now gone.

Good, I'm glad that we are no longer using a 3rd-party final static class as an interface. The new code is simpler, nice work.

It does mean that this issue provides even less function as a support for Contrib - the "combined transport factory" with a fromString() function has now gone. This isn't necessarily a negative as a simple Core can be the right decision. DSM Contrib contains many features that sites already use and that this issue now specifically excludes. Therefore I believe I shall have to ignore almost all the code in this issue and create a separate implementation. This isn't necessarily anything we need to consider a problem as it is just a different implementation of the same interfaces. I am hopeful that we can still find some agreement in ✨ Create new mailer service based on symfony Active .

>> Core should just stick with one transport,

For clarification I am in agreement with this consensus that Core only support one transport (in terms of configuration). My suggestion was that we could nevertheless provide a transport layer implementation that allows multiple transports to exist. This would for example allow a hook to override the configured transport and instead use the NULL transport. I am aware that likely you still disagree, however I feel it's useful to clarify that my suggestion isn't necessarily against the agreed consensus.

I have two specific comments on the updated code.

1) There is now a function TransportServiceFactoryTrait::fromDsnObject(). However I believe the DSN object is only capable of representing a "simple" transport without high-availability or load balancing. For this reason, in my code I have used the fromString() function instead. Perhaps this trait should do the same?

2) The comment for TransportServiceFactoryInterface says this:

Contrib and custom code may choose to replace or decorate the transport service factory in order to provide a mailer transport instance which requires more complex setup.

In DSM contrib, I propose to override the definition for Symfony\Component\Mailer\Transport\TransportInterface directly as the implementation does not need a factory. Do you agree that this as a reasonable customisation option? If so, please could you alter the comment to include it as one of the options?

Re #141.1 The fromString() method is still available from the full-blown Symfony mailer Transport facade. It can be used by contrib and custom code to parse string DSNs. Search for MAILER_DSN in the examples sandbox for pointers on how to do that. Core stores the DSN in its structured form since ✨ Use structured DSN instead of URI in system.mail mailer_dsn Fixed . And 📌 Tighten config validation schema of system.mail mailer_dsn Fixed made it pass config validation. Also additional schemes can be brought in by custom and contrib modules for third-party transports. Hence, core does not have any need for a reimplementation of the fromString() (and consequently the parseDSN()) method. Its maintenance cost should be left with Symfony developers.

Re #141.2 Contrib and custom code certainly can do whatever fits the use case at hand. The recommended way to customize transport instantiation is to decorate the factory.

Chaising head 📌 [ignore] Convert everything everywhere all at once Active .