Open ggrossetie opened 9 years ago
Sounds fine to me.
There is also Amazon SES... any preference ?
I just had a look through the source of this and wanted to throw in some ideas for this enhancement. With some +1's on the proposed impl I'll be happy to contribute something, hopefully of some value :grinning: :
Preemptive question: I'm not too familiar with writing plugins, is it necessary to have everything in the same source file? It would probably be better to divide things up into packages, especially if we'd like to provide implementations for various providers.
Proposal:
SendGridClient
, MailJetClient
and so on available to be injected just like the SMTP MailerClient
is at the moment.WS
library to perform the HTTP calls, returning Future
's from each provider's send
methods.case class ProviderError(code: Int, message: Option[String])
case class ProviderSuccess(code: Int) // or just a case object ProviderSuccess depending on what the APIs return
type ProviderResponse = Future[Either[ProviderError, ProviderSuccess]]
Should give users a decent chance to troubleshoot potential errors, considering not only client/server side errors but also HTTP issues. Although maybe the type is too complex?
sendAll
method could be useful, which would send the same email to multiple recipients. Probably something that could be added later on.I'll stop here, I've probably written enough for now :stuck_out_tongue: Thanks for any feedback in advance :grinning:
Preemptive question: I'm not too familiar with writing plugins, is it necessary to have everything in the same source file? It would probably be better to divide things up into packages, especially if we'd like to provide implementations for various providers.
No it's not mandatory to have everything in the same source file. You can put providers implementations in another source file.
Have each provider as a separate module, i.e SendGridClient, MailJetClient and so on available to be injected just like the SMTP MailerClient is at the moment.
I think it could be interesting to "qualify" each provider to let the user choose one of them (like the MockMailer):
@Inject
@Named("sendgrid")
MailerClient mailer;
But I don't think we need a separated module for each provider. We could instead add new bindings on the MailerModule: https://github.com/playframework/play-mailer/blob/master/src/main/scala/play/api/libs/mailer/MailerPlugin.scala#L15
Use the async methods from the WS library to perform the HTTP calls, returning Future's from each provider's send methods.
Yes we will need to add an async send
method in the API but for now we can implement the sync send
method using Await.result()
.
Although maybe the type is too complex?
At least we need to return an id to identify the email sent. At the moment we just return the email id or a runtime exception if something went wrong.
A sendAll method could be useful, which would send the same email to multiple recipients. Probably something that could be added later on.
To iterate on a list of recipients ?
Something I'd like to get some advice/guidance on is whether or not to provide some meaningful abstraction over the multiple providers that will be (eventually) included.
I think it's too soon to abstract, let's just implement a few of them and then decide if an abstraction is needed.
I'll stop here, I've probably written enough for now :stuck_out_tongue: Thanks for any feedback in advance :grinning:
Thanks for the detailed proposal ! I think you can start hacking on this and open an early PR to get feedback. First I think it would better to use the current API:
extends MailerClient
send
methodThe we can work on the return type or add new methods.
Great, thanks @Mogztter for the quick feedback. I'll hack something up over the next few days and submit a PR.
@duketon Cool, let me know if you need help :smile:
May I add https://mandrillapp.com/api/docs/ to the list?
I've looked at the code briefly - I have code in Java that has delivered close to 1 billion emails over the past 2 decades (courier industry and people would scream bloody murder if they didn't get their notifications!)
I'm trying NOT to reinvent the wheel but instead see if a great set of wheels already exists elsewhere.
Is it possible to have MULTIPLE channels configured?
While I don't mind the default channel configured via the properties or config file I prefer to have the balance of channels in a database for "live" updates. I've also had rules for moving between channels -- auto fall back etc when ever a vendor has some downtime (and they routinely do have down time...)
I belive I could modify what you have here to accomplish this but I'm still whacking my head against too many things migrating from Java to Scala/Play to be able to do this anytime soon.
Just wanted to plant a seed if it were a viable option while your adding those vendor API endpoints.
May I add https://mandrillapp.com/api/docs/ to the list?
I started with this provider but didn't find time to complete this work
Is it possible to have MULTIPLE channels configured?
Yes with Play 2.4.0 you can inject multiple providers:
@Inject
@Named("sendgrid")
MailerClient sendGridClient;
@Inject
@Named("mandrill")
MailerClient mandrillClient;
The default implementation is Guice so I think you can also do programmatic injections (i.e. read a value from the database then dynamically inject the channel you want to use to send emails).
While I don't mind the default channel configured via the properties or config file I prefer to have the balance of channels in a database for "live" updates. I've also had rules for moving between channels -- auto fall back etc when ever a vendor has some downtime (and they routinely do have down time...) I believe I could modify what you have here to accomplish this but I'm still whacking my head against too many things migrating from Java to Scala/Play to be able to do this anytime soon. Just wanted to plant a seed if it were a viable option while your adding those vendor API endpoints
Thanks for your input, maybe we can implement something in the API to handle "fall back" when a provider is not available...
@duketon Do you still work on this ? do you need some help ?
@duketon Are you still keen working on this?
Has there been traction on this issue? ...
@Mogztter if the below can be done
@Inject @Named("sendgrid") MailerClient sendGridClient; @Inject @Named("mandrill") MailerClient mandrillClient;
.... how would the play.mailers look like in application.conf?
There are a lot of email service providers with Web API:
It could be nice to provide an abstraction/extension point to allow users to switch from one provider to another (with SMTP as the default provider). What do you think ?