Webador / SlmMail

Send mail from Laminas or Mezzio using external mail services.
Other
106 stars 49 forks source link

Boom! Make SlmMail compatible with Zend\Mail\Transport #53

Closed Thinkscape closed 11 years ago

Thinkscape commented 11 years ago

This makes it possible to use it with stuff like SxMail and other modules relying on Zend\Mail\TransportInterface.

Frankly, I think this interface should be dropped in favor of Zend\Mail\TransportInterface, as they are identical.

bakura10 commented 11 years ago

We already have transport classes: https://github.com/juriansluiman/SlmMail/tree/master/src/SlmMail/Mail/Transport

SlmMail defines various factories for that.

juriansluiman commented 11 years ago

You can indeed load any transport classes which are using these services. They all implement Zend\Mail\Transport\TransportInterface.

This was also done because the services used to have different methods for send(), but they are now all normalized.

Thinkscape commented 11 years ago

Hmm... It's not technically "httptransport" in many cases.

I can live with that - only question is, why make it so complicated. What we could do is leave decorator and aliases for BC, but refactor "services" to be transports. That would make for much cleaner API.

Artur Bodera

On Sep 20, 2013, at 15:11, Jurian Sluiman notifications@github.com wrote:

You can indeed load any transport classeshttps://github.com/juriansluiman/SlmMail/blob/master/config/module.config.php#L44-L54which are using these services. They all implement Zend\Mail\Transport\TransportInterface.

This was also done because the services used to have different methods for send(), but they are now all normalized.

— Reply to this email directly or view it on GitHubhttps://github.com/juriansluiman/SlmMail/pull/53#issuecomment-24809046 .

bakura10 commented 11 years ago

I think it was done this way because services can access a lot more than just sending the mail. It can be a simple way to restrict what user can do by simply injecting a transport, that only have access to the "send" method.

Thinkscape commented 11 years ago

The way I understand @juriansluiman, is that it was an old limitation (from some early version).

Interfaces do not limit the number of methods you might have and classes can implement multiple interfaces. That means that it's perfectly valid for a class implementing TransportInterface to have any number of extra, provider-specific methods for performing various other tasks. Right now MailServiceInterfaces is just a plain copy of TransportInterface, so we just have code duplication with no benefit :-)

bakura10 commented 11 years ago

What about all those factories: https://github.com/juriansluiman/SlmMail/blob/master/config/module.config.php#L47-L54 ?

Should we change their factories so that the transport also returns services ?

Thinkscape commented 11 years ago

Easy. We leave TransportFactories, we create aliases which point ServiceFactories to TransportFactories (for BC), we refactor Services to become Transports and implement TransportInterface.

bakura10 commented 11 years ago

Sounds sane to me. Can you do that, as well as deprecating the HttpTransport class?

Thinkscape commented 11 years ago

Sure mate.

Thinkscape commented 11 years ago

Will create a fresh PR.

juriansluiman commented 11 years ago

This will be a (minor) bc break for people type hinting on the Http transport class.

We should clearly mark this for the next release. But yeah, because of the service interface, it's easier to directly address these services as transport. On 20 Sep 2013 16:42, "Michaël Gallego" notifications@github.com wrote:

Sounds sane to me. Can you do that, as well as deprecating the HttpTransport class?

— Reply to this email directly or view it on GitHubhttps://github.com/juriansluiman/SlmMail/pull/53#issuecomment-24815172 .

Thinkscape commented 11 years ago

@juriansluiman We can have HttpTransport extends TransportInterface - that would solve it :-)

Thinkscape commented 11 years ago

Hmm... wait.. no ... I see :-) Well, it'd still work because of aliases...