{"action":"created","issue":{"url":"https://api.github.com/repos/playframework/play-mailer/issues/63","labels_url":"https://api.github.com/repos/playframework/play-mailer/issues/63/labels{/name}","comments_url":"https://api.github.com/repos/playframework/play-mailer/issues/63/comments","events_url":"https://api.github.com/repos/playframework/play-mailer/issues/63/events","html_url":"https://github.com/playframework/play-mailer/pull/63","id":101168929,"number":63,"title":"Immutable mailer instance","user":{"login":"Mogztter","id":333276,"avatar_url":"https://avatars.githubusercontent.com/u/333276?v=3","gravatar_id":"","url":"https://api.github.com/users/Mogztter","html_url":"https://github.com/Mogztter","followers_url":"https://api.github.com/users/Mogztter/followers","following_url":"https://api.github.com/users/Mogztter/following{/other_user}","gists_url":"https://api.github.com/users/Mogztter/gists{/gist_id}","starred_url":"https://api.github.com/users/Mogztter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Mogztter/subscriptions","organizations_url":"https://api.github.com/users/Mogztter/orgs","repos_url":"https://api.github.com/users/Mogztter/repos","events_url":"https://api.github.com/users/Mogztter/events{/privacy}","received_events_url":"https://api.github.com/users/Mogztter/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-08-15T11:53:35Z","updated_at":"2015-08-31T07:09:03Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/playframework/play-mailer/pulls/63","html_url":"https://github.com/playframework/play-mailer/pull/63","diff_url":"https://github.com/playframework/play-mailer/pull/63.diff","patch_url":"https://github.com/playframework/play-mailer/pull/63.patch"},"body":"This is a proposition to replace the `configure` method.\r\nCurrently the mailer instance is mutable, when it gets reconfigured, it gets reconfigured globally.\r\nThe goal is to have an immutable mailer instance. See: https://github.com/playframework/play-mailer/issues/51#issuecomment-121798831\r\n\r\n**Principle**\r\n * New module to provide a `SMTPConfiguration` from the `Configuration` object\r\n   * This module can be overridden with `play.modules.disabled` (not sure if this the right way to do it) to provide a custom SMTPConfiguration (see samples)\r\n * CommonsMailer injects a SMTPConfiguration\r\n\r\nNote: I think it would be better to rename `CommonsMailer` to `SMTPMailer` and `SMTPMailer` to `CommonsMailer`.\r\n\r\n**Pros**\r\n * Immutable mailer instance\r\n * If the mailer configuration is wrong, we get an error at the start of the server (i.e. when Guice initializes objects) rather than when the code is executed\r\n * It's easy to (manually) instantiate a mailer with a custom configuration `val mailer = new CommonsMailer(SMTPConfiguration(\"typesafe.org\", 1234))`\r\n\r\n**Cons**\r\n * It's less straight forward to configure the injected instance\r\n   * write custom module and provider + disable the default module and enable the custom module \r\n * \"Cryptic\" error message when the mailer configuration is wrong (because Guice is unable to create a `SMTPConfiguration`)\r\n\r\n ```\r\nProvisionException: Unable to provision, see the following errors:\r\n\r\n1) Error in custom provider, java.lang.RuntimeException: host needs to be set in order to use this plugin (or set play.mailer.mock to true in application.conf)\r\n  while locating play.api.libs.mailer.SMTPConfigurationProvider\r\n  while locating play.api.libs.mailer.SMTPConfiguration\r\n    for parameter 0 at play.api.libs.mailer.CommonsMailer.<init>(MailerPlugin.scala:86)\r\n  while locating play.api.libs.mailer.CommonsMailer\r\n  while locating play.api.libs.mailer.MailerClient\r\n    for parameter 0 at controllers.ApplicationJava.<init>(ApplicationJava.java:18)\r\n  while locating controllers.ApplicationJava\r\n    for parameter 1 at router.Routes.<init>(Routes.scala:32)\r\n  while locating router.Routes\r\n  while locating play.api.inject.RoutesProvider\r\n  while locating play.api.routing.Router\r\nCaused by: java.lang.RuntimeException: host needs to be set in order to use this plugin (or set play.mailer.mock to true in application.conf)\r\n```\r\n * Not binary compatible"},"comment":{"url":"https://api.github.com/repos/playframework/play-mailer/issues/comments/136284299","html_url":"https://github.com/playframework/play-mailer/pull/63#issuecomment-136284299","issue_url":"https://api.github.com/repos/playframework/play-mailer/issues/63","id":136284299,"user":{"login":"loicdescotte","id":638150,"avatar_url":"https://avatars.githubusercontent.com/u/638150?v=3","gravatar_id":"","url":"https://api.github.com/users/loicdescotte","html_url":"https://github.com/loicdescotte","followers_url":"https://api.github.com/users/loicdescotte/followers","following_url":"https://api.github.com/users/loicdescotte/following{/other_user}","gists_url":"https://api.github.com/users/loicdescotte/gists{/gist_id}","starred_url":"https://api.github.com/users/loicdescotte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/loicdescotte/subscriptions","organizations_url":"https://api.github.com/users/loicdescotte/orgs","repos_url":"https://api.github.com/users/loicdescotte/repos","events_url":"https://api.github.com/users/loicdescotte/events{/privacy}","received_events_url":"https://api.github.com/users/loicdescotte/received_events","type":"User","site_admin":false},"created_at":"2015-08-31T07:09:03Z","updated_at":"2015-08-31T07:09:03Z","body":"@Mogztter I've just read the code and it seems great :)  "}}