#655 Encapsulate mail service

- Change SendMailSSL to be injected into classes and created regardless of settings
- Various minor cleanups (remove accidentally committed test, add more precise logging statement)

src/main/java/fr/xephi/authme/AuthMe.java

@@ -25,7 +25,6 @@ import fr.xephi.authme.listener.AuthMePlayerListener;
 import fr.xephi.authme.listener.AuthMePlayerListener16;
 import fr.xephi.authme.listener.AuthMePlayerListener18;
 import fr.xephi.authme.listener.AuthMeServerListener;
-import fr.xephi.authme.mail.SendMailSSL;
 import fr.xephi.authme.output.ConsoleFilter;
 import fr.xephi.authme.output.Log4JFilter;
 import fr.xephi.authme.output.MessageKey;
@@ -79,8 +78,6 @@ import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_ACCOUNT;
-import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_PASSWORD;
 import static fr.xephi.authme.settings.properties.EmailSettings.RECALL_PLAYERS;
 
 /**
@@ -114,7 +111,6 @@ public class AuthMe extends JavaPlugin {
     private BukkitService bukkitService;
     private AuthMeServiceInitializer initializer;
     private GeoLiteAPI geoLiteApi;
-    private SendMailSSL mail;
 
     /**
      * Constructor.
@@ -248,9 +244,6 @@ public class AuthMe extends JavaPlugin {
         // Set console filter
         setupConsoleFilter();
 
-        // Set up the mail API
-        setupMailApi();
-
         // Do a backup on start
         // TODO: maybe create a backup manager?
         new PerformBackup(this, newSettings).doBackup(PerformBackup.BackupCause.START);
@@ -303,16 +296,6 @@ public class AuthMe extends JavaPlugin {
         initializer.get(API.class);
     }
 
-    /**
-     * Set up the mail API, if enabled.
-     */
-    private void setupMailApi() {
-        // Make sure the mail API is enabled
-        if (!newSettings.getProperty(MAIL_ACCOUNT).isEmpty() && !newSettings.getProperty(MAIL_PASSWORD).isEmpty()) {
-            this.mail = new SendMailSSL(this, newSettings);
-        }
-    }
-
     /**
      * Show the settings warnings, for various risky settings.
      */
@@ -673,15 +656,6 @@ public class AuthMe extends JavaPlugin {
         return commandHandler.processCommand(sender, commandLabel, args);
     }
 
-    /**
-     * Get the mailing instance.
-     *
-     * @return The send mail instance.
-     */
-    public SendMailSSL getMail() {
-        return this.mail;
-    }
-
     // -------------
     // Service getters (deprecated)
     // Use @Inject fields instead

src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java

@@ -75,7 +75,7 @@ public class ChangePasswordAdminCommand implements ExecutableCommand {
 
                 if (dataSource.updatePassword(auth)) {
                     commandService.send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
-                    ConsoleLogger.info(playerNameLowerCase + "'s password changed");
+                    ConsoleLogger.info(sender.getName() + " changed password of " + playerNameLowerCase);
                 } else {
                     commandService.send(sender, MessageKey.ERROR);
                 }

src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java

@@ -1,12 +1,12 @@
 package fr.xephi.authme.command.executable.email;
 
-import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.cache.auth.PlayerCache;
 import fr.xephi.authme.command.CommandService;
 import fr.xephi.authme.command.PlayerCommand;
 import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.mail.SendMailSSL;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.security.PasswordSecurity;
 import fr.xephi.authme.security.RandomString;
@@ -33,15 +33,14 @@ public class RecoverEmailCommand extends PlayerCommand {
     private PlayerCache playerCache;
 
     @Inject
-    // TODO #655: Remove injected AuthMe instance once Authme#mail is encapsulated
+    private SendMailSSL sendMailSsl;
-    private AuthMe plugin;
 
     @Override
     public void runCommand(Player player, List<String> arguments) {
         final String playerMail = arguments.get(0);
         final String playerName = player.getName();
 
-        if (plugin.getMail() == null) {
+        if (!sendMailSsl.hasAllInformation()) {
             ConsoleLogger.showError("Mail API is not set");
             commandService.send(player, MessageKey.ERROR);
             return;
@@ -76,7 +75,7 @@ public class RecoverEmailCommand extends PlayerCommand {
             }
             auth.setPassword(hashNew);
             dataSource.updatePassword(auth);
-            plugin.getMail().main(auth, thePass);
+            sendMailSsl.sendPasswordMail(auth, thePass);
             commandService.send(player, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE);
         } else {
             commandService.send(player, MessageKey.REGISTER_EMAIL_MESSAGE);

src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java

@@ -3,6 +3,7 @@ package fr.xephi.authme.command.executable.register;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.command.CommandService;
 import fr.xephi.authme.command.PlayerCommand;
+import fr.xephi.authme.mail.SendMailSSL;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.process.Management;
 import fr.xephi.authme.security.HashAlgorithm;
@@ -27,6 +28,9 @@ public class RegisterCommand extends PlayerCommand {
     @Inject
     private CommandService commandService;
 
+    @Inject
+    private SendMailSSL sendMailSsl;
+
     @Override
     public void runCommand(Player player, List<String> arguments) {
         if (commandService.getProperty(SecuritySettings.PASSWORD_HASH) == HashAlgorithm.TWO_FACTOR) {
@@ -63,11 +67,11 @@ public class RegisterCommand extends PlayerCommand {
     }
 
     private void handleEmailRegistration(Player player, List<String> arguments) {
-        if (commandService.getProperty(EmailSettings.MAIL_ACCOUNT).isEmpty()) {
+        if (!sendMailSsl.hasAllInformation()) {
-            player.sendMessage("Cannot register: no email address is set for the server. "
+            player.sendMessage("Cannot register: not all required settings are set for sending emails. "
                 + "Please contact an administrator");
-            ConsoleLogger.showError("Cannot register player '" + player.getName() + "': no email is set "
+            ConsoleLogger.showError("Cannot register player '" + player.getName() + "': no email or password is set "
-                + "to send emails from. Please add one in your config at " + EmailSettings.MAIL_ACCOUNT.getPath());
+                + "to send emails from. Please adjust your config at " + EmailSettings.MAIL_ACCOUNT.getPath());
             return;
         }
 

src/main/java/fr/xephi/authme/datasource/MySQL.java

@@ -54,12 +54,10 @@ public class MySQL implements DataSource {
             if (e instanceof IllegalArgumentException) {
                 ConsoleLogger.showError("Invalid database arguments! Please check your configuration!");
                 ConsoleLogger.showError("If this error persists, please report it to the developer!");
-                throw e;
             }
             if (e instanceof PoolInitializationException) {
                 ConsoleLogger.showError("Can't initialize database connection! Please check your configuration!");
                 ConsoleLogger.showError("If this error persists, please report it to the developer!");
-                throw new PoolInitializationException(e);
             }
             ConsoleLogger.showError("Can't use the Hikari Connection Pool! Please, report this error to the developer!");
             throw e;

src/main/java/fr/xephi/authme/mail/SendMailSSL.java

@@ -5,45 +5,72 @@ import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.properties.EmailSettings;
+import fr.xephi.authme.util.BukkitService;
 import fr.xephi.authme.util.StringUtils;
 import org.apache.commons.mail.EmailConstants;
 import org.apache.commons.mail.EmailException;
 import org.apache.commons.mail.HtmlEmail;
-import org.bukkit.Bukkit;
 
 import javax.activation.DataSource;
 import javax.activation.FileDataSource;
 import javax.imageio.ImageIO;
+import javax.inject.Inject;
 import javax.mail.Session;
 import java.io.File;
 import java.io.IOException;
 import java.security.Security;
 import java.util.Properties;
 
+import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_ACCOUNT;
+import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_PASSWORD;
+
 
 /**
  * @author Xephi59
  */
 public class SendMailSSL {
 
-    private final AuthMe plugin;
+    @Inject
-    private final NewSetting settings;
+    private AuthMe plugin;
+    @Inject
+    private NewSetting settings;
+    @Inject
+    private BukkitService bukkitService;
 
-    public SendMailSSL(AuthMe plugin, NewSetting settings) {
+    SendMailSSL() {
-        this.plugin = plugin;
-        this.settings = settings;
     }
 
-    public void main(final PlayerAuth auth, final String newPass) {
+    /**
-        final String mailText = replaceMailTags(settings.getEmailMessage(), plugin, auth, newPass);
+     * Returns whether all necessary settings are set for sending mails.
-        Bukkit.getScheduler().runTaskAsynchronously(plugin, new Runnable() {
+     *
+     * @return true if the necessary email settings are set, false otherwise
+     */
+    public boolean hasAllInformation() {
+        return !settings.getProperty(MAIL_ACCOUNT).isEmpty()
+            && !settings.getProperty(MAIL_PASSWORD).isEmpty();
+    }
+
+    /**
+     * Sends an email to the user with his new password.
+     *
+     * @param auth the player auth of the player
+     * @param newPass the new password
+     */
+    public void sendPasswordMail(final PlayerAuth auth, final String newPass) {
+        if (!hasAllInformation()) {
+            ConsoleLogger.showError("Cannot perform email registration: not all email settings are complete");
+            return;
+        }
+
+        final String mailText = replaceMailTags(settings.getEmailMessage(), auth, newPass);
+        bukkitService.runTaskAsynchronously(new Runnable() {
 
             @Override
             public void run() {
                 Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
                 HtmlEmail email;
                 try {
-                    email = initializeMail(auth, settings);
+                    email = initializeMail(auth.getEmail());
                 } catch (EmailException e) {
                     ConsoleLogger.logException("Failed to create email with the given settings:", e);
                     return;
@@ -54,7 +81,7 @@ public class SendMailSSL {
                 File file = null;
                 if (settings.getProperty(EmailSettings.PASSWORD_AS_IMAGE)) {
                     try {
-                        file = generateImage(auth, plugin, newPass);
+                        file = generateImage(auth.getNickname(), plugin, newPass);
                         content = embedImageIntoEmailContent(file, email, content);
                     } catch (IOException | EmailException e) {
                         ConsoleLogger.logException(
@@ -67,13 +94,12 @@ public class SendMailSSL {
                     file.delete();
                 }
             }
-
         });
     }
 
-    private static File generateImage(PlayerAuth auth, AuthMe plugin, String newPass) throws IOException {
+    private static File generateImage(String name, AuthMe plugin, String newPass) throws IOException {
         ImageGenerator gen = new ImageGenerator(newPass);
-        File file = new File(plugin.getDataFolder(), auth.getNickname() + "_new_pass.jpg");
+        File file = new File(plugin.getDataFolder(), name + "_new_pass.jpg");
         ImageIO.write(gen.generateImage(), "jpg", file);
         return file;
     }
@@ -85,8 +111,7 @@ public class SendMailSSL {
         return content.replace("<image />", "<img src=\"cid:" + tag + "\">");
     }
 
-    private static HtmlEmail initializeMail(PlayerAuth auth, NewSetting settings)
+    private HtmlEmail initializeMail(String emailAddress) throws EmailException {
-            throws EmailException {
         String senderMail = settings.getProperty(EmailSettings.MAIL_ACCOUNT);
         String senderName = StringUtils.isEmpty(settings.getProperty(EmailSettings.MAIL_SENDER_NAME))
             ? senderMail
@@ -98,12 +123,12 @@ public class SendMailSSL {
         email.setCharset(EmailConstants.UTF_8);
         email.setSmtpPort(port);
         email.setHostName(settings.getProperty(EmailSettings.SMTP_HOST));
-        email.addTo(auth.getEmail());
+        email.addTo(emailAddress);
         email.setFrom(senderMail, senderName);
         email.setSubject(settings.getProperty(EmailSettings.RECOVERY_MAIL_SUBJECT));
         email.setAuthentication(senderMail, mailPassword);
 
-        setPropertiesForPort(email, port, settings);
+        setPropertiesForPort(email, port);
         return email;
     }
 
@@ -124,15 +149,14 @@ public class SendMailSSL {
         }
     }
 
-    private static String replaceMailTags(String mailText, AuthMe plugin, PlayerAuth auth, String newPass) {
+    private String replaceMailTags(String mailText, PlayerAuth auth, String newPass) {
         return mailText
             .replace("<playername />", auth.getNickname())
             .replace("<servername />", plugin.getServer().getServerName())
             .replace("<generatedpass />", newPass);
     }
 
-    private static void setPropertiesForPort(HtmlEmail email, int port, NewSetting settings)
+    private void setPropertiesForPort(HtmlEmail email, int port) throws EmailException {
-            throws EmailException {
         switch (port) {
             case 587:
                 String oAuth2Token = settings.getProperty(EmailSettings.OAUTH2_TOKEN);

src/main/java/fr/xephi/authme/process/register/AsyncRegister.java

@@ -4,6 +4,7 @@ import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.cache.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.mail.SendMailSSL;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.permission.PermissionsManager;
 import fr.xephi.authme.process.AsynchronousProcess;
@@ -55,6 +56,9 @@ public class AsyncRegister implements AsynchronousProcess {
     @Inject
     private ValidationService validationService;
 
+    @Inject
+    private SendMailSSL sendMailSsl;
+
     AsyncRegister() { }
 
     private boolean preRegisterCheck(Player player, String password) {
@@ -137,7 +141,7 @@ public class AsyncRegister implements AsynchronousProcess {
         }
         database.updateEmail(auth);
         database.updateSession(auth);
-        plugin.getMail().main(auth, password);
+        sendMailSsl.sendPasswordMail(auth, password);
         syncProcessManager.processSyncEmailRegister(player);
     }
 

src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java

@@ -2,6 +2,7 @@ package fr.xephi.authme.command.executable.register;
 
 import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.command.CommandService;
+import fr.xephi.authme.mail.SendMailSSL;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.process.Management;
 import fr.xephi.authme.security.HashAlgorithm;
@@ -49,6 +50,8 @@ public class RegisterCommandTest {
     @Mock
     private Management management;
 
+    @Mock
+    private SendMailSSL sendMailSsl;
 
     @BeforeClass
     public static void setup() {
@@ -72,7 +75,7 @@ public class RegisterCommandTest {
 
         // then
         verify(sender).sendMessage(argThat(containsString("Player only!")));
-        verifyZeroInteractions(management);
+        verifyZeroInteractions(management, sendMailSsl);
     }
 
     @Test
@@ -86,6 +89,7 @@ public class RegisterCommandTest {
 
         // then
         verify(management).performRegister(player, "", "", true);
+        verifyZeroInteractions(sendMailSsl);
     }
 
     @Test
@@ -98,7 +102,7 @@ public class RegisterCommandTest {
 
         // then
         verify(commandService).send(player, MessageKey.USAGE_REGISTER);
-        verifyZeroInteractions(management);
+        verifyZeroInteractions(management, sendMailSsl);
     }
 
     @Test
@@ -112,7 +116,7 @@ public class RegisterCommandTest {
 
         // then
         verify(commandService).send(player, MessageKey.USAGE_REGISTER);
-        verifyZeroInteractions(management);
+        verifyZeroInteractions(management, sendMailSsl);
     }
 
     @Test
@@ -127,7 +131,7 @@ public class RegisterCommandTest {
 
         // then
         verify(commandService).send(player, MessageKey.USAGE_REGISTER);
-        verifyZeroInteractions(management);
+        verifyZeroInteractions(management, sendMailSsl);
     }
 
     @Test
@@ -135,14 +139,15 @@ public class RegisterCommandTest {
         // given
         given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true);
         given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(false);
-        given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("");
+        given(sendMailSsl.hasAllInformation()).willReturn(false);
         Player player = mock(Player.class);
 
         // when
         command.executeCommand(player, Collections.singletonList("myMail@example.tld"));
 
         // then
-        verify(player).sendMessage(argThat(containsString("no email address")));
+        verify(player).sendMessage(argThat(containsString("not all required settings are set for sending emails")));
+        verify(sendMailSsl).hasAllInformation();
         verifyZeroInteractions(management);
     }
 
@@ -155,6 +160,7 @@ public class RegisterCommandTest {
         given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true);
         given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true);
         given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com");
+        given(sendMailSsl.hasAllInformation()).willReturn(true);
         Player player = mock(Player.class);
 
         // when
@@ -175,6 +181,7 @@ public class RegisterCommandTest {
         given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true);
         given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true);
         given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com");
+        given(sendMailSsl.hasAllInformation()).willReturn(true);
         Player player = mock(Player.class);
 
         // when
@@ -182,6 +189,7 @@ public class RegisterCommandTest {
 
         // then
         verify(commandService).send(player, MessageKey.USAGE_REGISTER);
+        verify(sendMailSsl).hasAllInformation();
         verifyZeroInteractions(management);
     }
 
@@ -196,6 +204,7 @@ public class RegisterCommandTest {
         given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true);
         given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true);
         given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com");
+        given(sendMailSsl.hasAllInformation()).willReturn(true);
         Player player = mock(Player.class);
 
         // when
@@ -203,6 +212,7 @@ public class RegisterCommandTest {
 
         // then
         verify(commandService).validateEmail(playerMail);
+        verify(sendMailSsl).hasAllInformation();
         verify(management).performRegister(eq(player), argThat(stringWithLength(passLength)), eq(playerMail), eq(true));
     }
 
@@ -217,7 +227,7 @@ public class RegisterCommandTest {
 
         // then
         verify(commandService).send(player, MessageKey.PASSWORD_MATCH_ERROR);
-        verifyZeroInteractions(management);
+        verifyZeroInteractions(management, sendMailSsl);
     }
 
     @Test

src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java

@@ -120,11 +120,6 @@ public class SQLiteIntegrationTest extends AbstractDataSourceIntegrationTest {
         assertThat(sqLite.getAllAuths(), hasSize(1));
     }
 
-    @Test
-    public void shouldCreate2() throws SQLException {
-        con.createStatement().execute("SELECT 1 from authme");
-    }
-
     @Override
     protected DataSource getDataSource(String saltColumn) {
         when(settings.getProperty(DatabaseSettings.MYSQL_COL_SALT)).thenReturn(saltColumn);