Created transports can not be altered

This is harder than I thought. Passing in the event subscriber to the transports does not help me with my use case. That only allows to alter the message and envelope, the transport is passed in only as a class string.

\Symfony\Component\Mailer\Transport::fromDsns() goes through factories, but EsmtpTransportFactory() doesn't have a way to set that either *and* is final. The Transport class is final *and* uses private methods.

Why does symfony/mailer make it _so_ hard to customize things. Why even support configurable authenticators when it's impossible to set them without copy pasting 200 lines of code?

I'm frustrated and need to stop for today.

This is partially very similar to the lengthy discussion in the core issue to add transport discovery ( 📌 [PP-1] Add symfony mailer transports to DIC Postponed ).

Right now, my only idea is that we instantiate the Transport object ourselves, for now with the default factories, with the option to extend it later, and then call fromString() on each, and introduce a new method or maybe optional interface with a method so that the config plugin can customize the created transport or alternatively an event where we pass the transports through and then we create the Transports() object from that afterwards.

To get what you want I think a custom factory, you could try to wrangle into something extending/wrapping the final \Symfony\Component\Mailer\Transport\Smtp\EsmtpTransportFactory. Or more likely: implement your own factory producing \Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport objects, passing authenticators as the constructor arg or calling setAuthenticators after construct.

To achieve this we need a way for symfony mailer to pick up transport factories, so we're going to need similar infrastructure to https://github.com/symfony/symfony/blob/add24fa38e0438f1188211cce7adf418... rather than calling \Symfony\Component\Mailer\Transport::fromDsns statically, which only avails the factories in getDefaultFactories.

Why does symfony/mailer make it _so_ hard to customize things

If in doubt, usually a lot of container juggling! 😅

Some notes on factory/transport side

https://symfony.com/doc/current/mailer.html#overriding-default-smtp-auth...

https://symfony.com/doc/current/mailer.html#custom-transport-factories

Why does symfony/mailer make it _so_ hard to customize things. Why even support configurable authenticators when it's impossible to set them without copy pasting 200 lines of code?

Unfortunately the Transport class exposes two separate and very different API entry points. In my opinion the dividing line is public static methods vs non-static ones. The former (static methods, i.e. fromDsn(), fromDsns() and getDefaultFactories()) simplifies transport construction for very small projects lacking a DI container. The latter provides all the flexibility (including custom factories) which come in handy at some point in larger code bases.

Let's put it that way: In projects with a DI container, any code path going through any of the public static methods of the Transport class is taking shortcuts and as a result is missing important extension points.

That should be discussed over in 📌 [PP-1] Add symfony mailer transports to DIC Postponed though.

dan.munn → made their first commit to this issue’s fork.

I've just raised what I'd class as MVP at least to be able to leverage the mailer.transport_factory tag by allowing the TransportsFactory to hold factories (seeded with the default)

Thanks for the feedback, that's a lot more activity than I expected in a relatively small contrib module (that's quickly changing now I guess with the swiftmailer situation).

Yes, what I can do to avoid code duplication is wrap EsmtpTransportFactory()) in my own and then, with that patch, register it. I guess it makes sense to use my own made up scheme for it, so I can control which configured transports are configured like that. Will try this.

@znerol: Yeah, you're right, unlike core, it is easier to gradually improve in contrib, and it's getting there, until recently, it just initialized the transport inline in the mail plugin. I just with the naming was better in those classes, still makes no sense to me that fromDsns() is just the static/quickstart version of fromStrings() with custom initialization.

In regards to the merge request, if we go with the default instead of explicitly defining them all as services, maybe we could initialize the defaults in a way that puts them at the end, so you have a chance to override the defaults if you want? And I think it should also use a service tag namespaced to this module, to prevent conflicts with core once that lands or other modules? Might be a bit unexpected otherwise although not sure if anything could really break.

I figure given they are defaults that we can rely on them to be available without having to register them - I have updated the MR so that we merge them to the end of the array when retrieving the transport factories though as otherwise it inadvertently gives them priority.

The tag name is deliberate though, as that way we're reacting to the service tag thats in the symfony mailer documentation. In theory it means if someone is doing work using this module, that we're not incompatible with how symfony_mailer or potentially even the linked conversation for core.

Fortunately the compiler pass on the container should deal with many different service collectors using the same tag; Symfony is able to register each service using the mailer.transport_factory service tag against each collector (collecting this tag ofc). So we wouldn't be in competition AFAIK.

Transport::getDefaultFactories() is another shortcut which will fall on our feet sooner or later. I humbly recommend to avoid that as well.

I still think we should namespace the tag and prefer drupal coding standards of using module namespaces over the official symfony standard.

If both core and this module use the same tag, then once the core issue gets committed, we end up with the factories registered twice here, and if we switch to registering the defaults explicitly as well in the module, then both core and this collector will have everything twice, which could negatively affect customizations that either module is doing.

This is IMHO an intermediate step until core provides this functionality for us, then it makes sense to refactor this anyway. Or once the core mailer module is no longer experimental I guess.

I completely agree that once this comes into core that the whole concept of maybe even the majority of the wiring this module is doing becomes obseleted, in the mean time though I guess what I'm thinking about is effectively keeping things as close-to a per-documentation implementation of a symfony transport factory registration.

We're not taking it over, we're just another factory that can listen out for other registrations of the transport factory - which is the sole reason I was going with the `mailer.transport_factory` tag - it keeps us aligned to what Symfony mailer is telling you to do in the documentation vs this being firewalled off.

Transport::getDefaultFactories() is another shortcut which will fall on our feet sooner or later. I humbly recommend to avoid that as well.

I totally appreciate the point, but in the same thought, it was not about trying to replicate the fully fledged implementation, just a means to allow a base line with some extensibility to it. Key thing is that its not on the deprecation path just yet, and given Symfony 7.1 still maintains it (and we're using ^6.4 in 10.x, and given Symfony's current release still contains this API, 7.1 (which is released in May) still has it, so deprecation would (if it is indeed deprecated) be 8.0 which by their own definition is due November 2025. So wondering what the worry is around falling back on this?

Key thing is that its not on the deprecation path just yet,

The reason why I'm suggesting to avoid Transport::getDefaultFactories() has nothing to do with deprecation. At least I am not aware of any plans to deprecated it.

On the other hand, there is a good reason for this patch here to actually use Transport::getDefaultFactories(): Backward compatibility. Sticking with Transport::getDefaultFactories() avoids breaking mail delivery on existing sites which are currently using a third-party transport and relying on the fact that the static part of Transport API is magically registering factories.

I'm checking in to see what the status of the MR is. Does it fix the problem for everyone on this thread? I know there's still a question about the namespace for the tag, but other than that, is this functional? I don't have an advanced use case that I need this for, so it's hard for me to test.

I just want to see if we can merge this, because it seems like it's also related to or blocks the resolution of ✨ Symfony mailer transports are unable to to dispatch events Needs review and ✨ Module now silently falls back to null transport if there is no valid transport Needs review .

Yes, this is working well for us. I now have a transport plugin with my own unique DNS prefix/name, and a corresponding transport plugin that instantiates an Smtp factory and then adds the authentication thing.

Name of the tag is your call I guess, I still think that the problem outlined in #13 which would result in multiple extra factories being registered could be confusing or even break something once the core issue is committed. Up to you.

BC is a good argument per #15 to keep the current call, the implementation as it is now still allows to entirely replace a given default factory with a service. And the whole thing is temporary, once core provides the factories then I guess this module can do a major new release, require 10.3 or whatever and build on top of that.

Once this lands, or we decide on changes happy to update as needed - same goes for the connected issue :) - and it does work for me (I'm also using this patch to inject alternative transports)

After playing around with this a bit, I don't see much benefit to using the mailer namespace for the service collector tag. It may make it slightly easier for someone implementing a custom transport factory, but that person is already writing custom code, so if they have to register with our module-level tag as well, it seems easy enough. If I weigh that against the possibility of breaking all sites using this module when core does something on that namespace, it doesn't seem worth the risk.

I changed the namespace in the services.yml file. I think that's all I need to do, but can someone confirm?

Otherwise, I think this is good to merge.

Yep thats all that was needed, LGTM then

I've merged this change. Thanks everyone!

Automatically closed - issue fixed for 2 weeks with no activity.