Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate with email service providers #10

Open
ggrossetie opened this issue Dec 3, 2014 · 12 comments
Open

Integrate with email service providers #10

ggrossetie opened this issue Dec 3, 2014 · 12 comments

Comments

@ggrossetie
Copy link
Member

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 ?

@jroper
Copy link
Member

jroper commented Dec 3, 2014

Sounds fine to me.

@ggrossetie
Copy link
Member Author

There is also Amazon SES... any preference ?

@duketon
Copy link

duketon commented May 18, 2015

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 😀 :

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:

  • 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.
  • Use the async methods from the WS library to perform the HTTP calls, returning Future's from each provider's send methods.
  • Define a custom class for responses. Something like
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?

  • A sendAll method could be useful, which would send the same email to multiple recipients. Probably something that could be added later on.
  • 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'd hazard to take a guess that most of them will share similar methods, at least one that "produces" and email and another that "sends" an email. Should they inherit from some abstract class?

I'll stop here, I've probably written enough for now 😛 Thanks for any feedback in advance 😀

@ggrossetie
Copy link
Member Author

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 😛 Thanks for any feedback in advance 😀

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:

  • Create a new class for your provider and extends MailerClient
  • override the send method
  • Implement the provider

The we can work on the return type or add new methods.

@duketon
Copy link

duketon commented May 18, 2015

Great, thanks @Mogztter for the quick feedback. I'll hack something up over the next few days and submit a PR.

@ggrossetie
Copy link
Member Author

@duketon Cool, let me know if you need help 😄

@techmag
Copy link
Contributor

techmag commented Jun 3, 2015

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.

@ggrossetie
Copy link
Member Author

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...

@ggrossetie
Copy link
Member Author

@duketon Do you still work on this ? do you need some help ?

@mkurz
Copy link
Member

mkurz commented Jan 14, 2016

@duketon Are you still keen working on this?

@sidthesloth
Copy link

Has there been traction on this issue? ...

@sidthesloth
Copy link

sidthesloth commented Mar 28, 2017

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants