Revise converter architecture + add integration test for CrazyLogin converter

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

@@ -10,6 +10,7 @@ import fr.xephi.authme.converter.RoyalAuthConverter;
 import fr.xephi.authme.converter.SqliteToSql;
 import fr.xephi.authme.converter.vAuthConverter;
 import fr.xephi.authme.converter.xAuthConverter;
+import fr.xephi.authme.initialization.AuthMeServiceInitializer;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.util.BukkitService;
 import org.bukkit.command.CommandSender;
@@ -17,6 +18,9 @@ import org.bukkit.command.CommandSender;
 import javax.inject.Inject;
 import java.util.List;
 
+/**
+ * Converter command: launches conversion based on its parameters.
+ */
 public class ConverterCommand implements ExecutableCommand {
 
     @Inject
@@ -25,8 +29,11 @@ public class ConverterCommand implements ExecutableCommand {
     @Inject
     private BukkitService bukkitService;
 
+    @Inject
+    private AuthMeServiceInitializer initializer;
+
     @Override
-    public void executeCommand(CommandSender sender, List<String> arguments, CommandService commandService) {
+    public void executeCommand(final CommandSender sender, List<String> arguments, CommandService commandService) {
         // Get the conversion job
         String job = arguments.get(0);
 
@@ -38,49 +45,34 @@ public class ConverterCommand implements ExecutableCommand {
         }
 
         // Get the proper converter instance
-        Converter converter = null;
+        final Converter converter = initializer.newInstance(jobType.getConverterClass());
-        switch (jobType) {
-            case XAUTH:
-                converter = new xAuthConverter(authMe, sender);
-                break;
-            case CRAZYLOGIN:
-                converter = new CrazyLoginConverter(authMe, sender);
-                break;
-            case RAKAMAK:
-                converter = new RakamakConverter(authMe, sender);
-                break;
-            case ROYALAUTH:
-                converter = new RoyalAuthConverter(authMe);
-                break;
-            case VAUTH:
-                converter = new vAuthConverter(authMe, sender);
-                break;
-            case SQLITETOSQL:
-                converter = new SqliteToSql(authMe, sender, commandService.getSettings());
-                break;
-            default:
-                break;
-        }
 
         // Run the convert job
-        bukkitService.runTaskAsynchronously(converter);
+        bukkitService.runTaskAsynchronously(new Runnable() {
+            @Override
+            public void run() {
+                converter.execute(sender);
+            }
+        });
 
         // Show a status message
         sender.sendMessage("[AuthMe] Successfully converted from " + jobType.getName());
     }
 
-    public enum ConvertType {
+    private enum ConvertType {
-        XAUTH("xauth"),
+        XAUTH("xauth", xAuthConverter.class),
-        CRAZYLOGIN("crazylogin"),
+        CRAZYLOGIN("crazylogin", CrazyLoginConverter.class),
-        RAKAMAK("rakamak"),
+        RAKAMAK("rakamak", RakamakConverter.class),
-        ROYALAUTH("royalauth"),
+        ROYALAUTH("royalauth", RoyalAuthConverter.class),
-        VAUTH("vauth"),
+        VAUTH("vauth", vAuthConverter.class),
-        SQLITETOSQL("sqlitetosql");
+        SQLITETOSQL("sqlitetosql", SqliteToSql.class);
 
-        final String name;
+        private final String name;
+        private final Class<? extends Converter> converterClass;
 
-        ConvertType(String name) {
+        ConvertType(String name, Class<? extends Converter> converterClass) {
             this.name = name;
+            this.converterClass = converterClass;
         }
 
         public static ConvertType fromName(String name) {
@@ -92,8 +84,12 @@ public class ConverterCommand implements ExecutableCommand {
             return null;
         }
 
-        String getName() {
+        public String getName() {
             return this.name;
         }
+
+        public Class<? extends Converter> getConverterClass() {
+            return converterClass;
+        }
     }
 }

src/main/java/fr/xephi/authme/converter/Converter.java

@@ -1,7 +1,16 @@
 package fr.xephi.authme.converter;
 
+import org.bukkit.command.CommandSender;
+
 /**
- * Common supertype for AuthMe converters.
+ * Interface for AuthMe converters.
  */
-public interface Converter extends Runnable {
+public interface Converter {
+
+    /**
+     * Execute the conversion.
+     *
+     * @param sender the sender who initialized the conversion
+     */
+    void execute(CommandSender sender);
 }

src/main/java/fr/xephi/authme/converter/CrazyLoginConverter.java

@@ -1,47 +1,46 @@
 package fr.xephi.authme.converter;
 
-import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.datasource.DataSource;
-import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.initialization.DataFolder;
+import fr.xephi.authme.settings.NewSetting;
+import fr.xephi.authme.settings.properties.ConverterSettings;
 import org.bukkit.command.CommandSender;
 
+import javax.inject.Inject;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
 
 /**
- * @author Xephi59
+ * Converter for CrazyLogin to AuthMe.
  */
 public class CrazyLoginConverter implements Converter {
 
     private final DataSource database;
-    private final CommandSender sender;
+    private final NewSetting settings;
+    private final File dataFolder;
 
-    /**
+    @Inject
-     * Constructor for CrazyLoginConverter.
+    CrazyLoginConverter(@DataFolder File dataFolder, DataSource dataSource, NewSetting settings) {
-     *
+        this.dataFolder = dataFolder;
-     * @param instance AuthMe
+        this.database = dataSource;
-     * @param sender   CommandSender
+        this.settings = settings;
-     */
-    public CrazyLoginConverter(AuthMe instance, CommandSender sender) {
-        this.database = instance.getDataSource();
-        this.sender = sender;
     }
 
     @Override
-    public void run() {
+    public void execute(CommandSender sender) {
-        String fileName = Settings.crazyloginFileName;
+        String fileName = settings.getProperty(ConverterSettings.CRAZYLOGIN_FILE_NAME);
-        try {
+        File source = new File(dataFolder, fileName);
-            File source = new File(AuthMe.getInstance().getDataFolder() + File.separator + fileName);
+        if (!source.exists()) {
-            if (!source.exists()) {
+            sender.sendMessage("CrazyLogin file not found, please put " + fileName + " in AuthMe folder!");
-                sender.sendMessage("Error while trying to import data, please put " + fileName + " in AuthMe folder!");
+            return;
-                return;
+        }
-            }
+
-            String line;
+        String line;
-            BufferedReader users = new BufferedReader(new FileReader(source));
+        try (BufferedReader users = new BufferedReader(new FileReader(source))) {
             while ((line = users.readLine()) != null) {
                 if (line.contains("|")) {
                     String[] args = line.split("\\|");
@@ -49,22 +48,21 @@ public class CrazyLoginConverter implements Converter {
                         continue;
                     }
                     String playerName = args[0];
-                    String psw = args[1];
+                    String password = args[1];
-                    if (psw != null) {
+                    if (password != null) {
                         PlayerAuth auth = PlayerAuth.builder()
                             .name(playerName.toLowerCase())
                             .realName(playerName)
-                            .password(psw, null)
+                            .password(password, null)
                             .build();
                         database.saveAuth(auth);
                     }
                 }
             }
-            users.close();
             ConsoleLogger.info("CrazyLogin database has been imported correctly");
         } catch (IOException ex) {
-            ConsoleLogger.showError(ex.getMessage());
             ConsoleLogger.showError("Can't open the crazylogin database file! Does it exist?");
+            ConsoleLogger.logException("Encountered", ex);
         }
     }
 

src/main/java/fr/xephi/authme/converter/ForceFlatToSqlite.java

@@ -5,6 +5,7 @@ import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.datasource.FlatFile;
 import fr.xephi.authme.util.StringUtils;
+import org.bukkit.command.CommandSender;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -32,7 +33,8 @@ public class ForceFlatToSqlite implements Converter {
      * Perform the conversion.
      */
     @Override
-    public void run() {
+    // Note ljacqu 20160527: CommandSender is null here; it is only present because of the interface it implements
+    public void execute(CommandSender sender) {
         List<String> skippedPlayers = new ArrayList<>();
         for (PlayerAuth auth : source.getAllAuths()) {
             if (destination.isAuthAvailable(auth.getNickname())) {

src/main/java/fr/xephi/authme/converter/RakamakConverter.java

@@ -4,11 +4,14 @@ import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.initialization.DataFolder;
 import fr.xephi.authme.security.PasswordSecurity;
 import fr.xephi.authme.security.crypts.HashedPassword;
-import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.settings.NewSetting;
+import fr.xephi.authme.settings.properties.ConverterSettings;
 import org.bukkit.command.CommandSender;
 
+import javax.inject.Inject;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
@@ -23,22 +26,23 @@ public class RakamakConverter implements Converter {
 
     private final AuthMe instance;
     private final DataSource database;
-    private final CommandSender sender;
+    private final NewSetting settings;
     private final File pluginFolder;
 
-    public RakamakConverter(AuthMe instance, CommandSender sender) {
+    @Inject
+    RakamakConverter(@DataFolder File dataFolder, AuthMe instance, DataSource dataSource, NewSetting settings) {
         this.instance = instance;
-        this.database = instance.getDataSource();
+        this.database = dataSource;
-        this.sender = sender;
+        this.settings = settings;
-        pluginFolder = instance.getDataFolder();
+        this.pluginFolder = dataFolder;
     }
 
     @Override
     // TODO ljacqu 20151229: Restructure this into smaller portions
-    public void run() {
+    public void execute(CommandSender sender) {
-        boolean useIP = Settings.rakamakUseIp;
+        boolean useIP = settings.getProperty(ConverterSettings.RAKAMAK_USE_IP);
-        String fileName = Settings.rakamakUsers;
+        String fileName = settings.getProperty(ConverterSettings.RAKAMAK_FILE_NAME);
-        String ipFileName = Settings.rakamakUsersIp;
+        String ipFileName = settings.getProperty(ConverterSettings.RAKAMAK_IP_FILE_NAME);
         File source = new File(pluginFolder, fileName);
         File ipfiles = new File(pluginFolder, ipFileName);
         HashMap<String, String> playerIP = new HashMap<>();

src/main/java/fr/xephi/authme/converter/RoyalAuthConverter.java

@@ -5,9 +5,11 @@ import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.datasource.DataSource;
 import org.bukkit.OfflinePlayer;
+import org.bukkit.command.CommandSender;
 import org.bukkit.configuration.file.FileConfiguration;
 import org.bukkit.configuration.file.YamlConfiguration;
 
+import javax.inject.Inject;
 import java.io.File;
 
 import static fr.xephi.authme.util.StringUtils.makePath;
@@ -19,13 +21,14 @@ public class RoyalAuthConverter implements Converter {
     private final AuthMe plugin;
     private final DataSource dataSource;
 
-    public RoyalAuthConverter(AuthMe plugin) {
+    @Inject
+    RoyalAuthConverter(AuthMe plugin, DataSource dataSource) {
         this.plugin = plugin;
-        this.dataSource = plugin.getDataSource();
+        this.dataSource = dataSource;
     }
 
     @Override
-    public void run() {
+    public void execute(CommandSender sender) {
         for (OfflinePlayer player : plugin.getServer().getOfflinePlayers()) {
             try {
                 String name = player.getName().toLowerCase();

src/main/java/fr/xephi/authme/converter/SqliteToSql.java

@@ -1,40 +1,43 @@
 package fr.xephi.authme.converter;
 
-import fr.xephi.authme.settings.NewSetting;
-import org.bukkit.command.CommandSender;
-
-import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
+import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.datasource.DataSourceType;
 import fr.xephi.authme.datasource.SQLite;
 import fr.xephi.authme.output.MessageKey;
+import fr.xephi.authme.output.Messages;
+import fr.xephi.authme.settings.NewSetting;
+import org.bukkit.command.CommandSender;
+
+import javax.inject.Inject;
 
 public class SqliteToSql implements Converter {
 
-    private final AuthMe plugin;
-    private final CommandSender sender;
     private final NewSetting settings;
+    private final DataSource dataSource;
+    private final Messages messages;
 
-    public SqliteToSql(AuthMe plugin, CommandSender sender, NewSetting settings) {
+    @Inject
-        this.plugin = plugin;
+    SqliteToSql(NewSetting settings, DataSource dataSource, Messages messages) {
-        this.sender = sender;
         this.settings = settings;
+        this.dataSource = dataSource;
+        this.messages = messages;
     }
 
     @Override
-    public void run() {
+    public void execute(CommandSender sender) {
-        if (plugin.getDataSource().getType() != DataSourceType.MYSQL) {
+        if (dataSource.getType() != DataSourceType.MYSQL) {
             sender.sendMessage("Please configure your mySQL connection and re-run this command");
             return;
         }
         try {
             SQLite data = new SQLite(settings);
             for (PlayerAuth auth : data.getAllAuths()) {
-                plugin.getDataSource().saveAuth(auth);
+                dataSource.saveAuth(auth);
             }
         } catch (Exception e) {
-            plugin.getMessages().send(sender, MessageKey.ERROR);
+            messages.send(sender, MessageKey.ERROR);
             ConsoleLogger.logException("Problem during SQLite to SQL conversion:", e);
         }
     }

src/main/java/fr/xephi/authme/converter/vAuthConverter.java

@@ -4,18 +4,19 @@ import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.ConsoleLogger;
 import org.bukkit.command.CommandSender;
 
+import javax.inject.Inject;
+
 public class vAuthConverter implements Converter {
 
     private final AuthMe plugin;
-    private final CommandSender sender;
 
-    public vAuthConverter(AuthMe plugin, CommandSender sender) {
+    @Inject
+    vAuthConverter(AuthMe plugin) {
         this.plugin = plugin;
-        this.sender = sender;
     }
 
     @Override
-    public void run() {
+    public void execute(CommandSender sender) {
         try {
             new vAuthFileReader(plugin).convert();
         } catch (Exception e) {

src/main/java/fr/xephi/authme/converter/xAuthConverter.java

@@ -3,18 +3,19 @@ package fr.xephi.authme.converter;
 import fr.xephi.authme.AuthMe;
 import org.bukkit.command.CommandSender;
 
+import javax.inject.Inject;
+
 public class xAuthConverter implements Converter {
 
     private final AuthMe plugin;
-    private final CommandSender sender;
 
-    public xAuthConverter(AuthMe plugin, CommandSender sender) {
+    @Inject
+    xAuthConverter(AuthMe plugin) {
         this.plugin = plugin;
-        this.sender = sender;
     }
 
     @Override
-    public void run() {
+    public void execute(CommandSender sender) {
         try {
             Class.forName("de.luricos.bukkit.xAuth.xAuth");
             xAuthToFlat converter = new xAuthToFlat(plugin, sender);

src/main/java/fr/xephi/authme/util/MigrationService.java

@@ -69,7 +69,7 @@ public final class MigrationService {
             try {
                 SQLite sqlite = new SQLite(settings);
                 ForceFlatToSqlite converter = new ForceFlatToSqlite(flatFile, sqlite);
-                converter.run();
+                converter.execute(null);
                 settings.setProperty(DatabaseSettings.BACKEND, DataSourceType.SQLITE);
                 settings.save();
                 return sqlite;

src/test/java/fr/xephi/authme/converter/CrazyLoginConverterTest.java

@@ -0,0 +1,95 @@
+package fr.xephi.authme.converter;
+
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.cache.auth.PlayerAuth;
+import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.settings.NewSetting;
+import fr.xephi.authme.settings.properties.ConverterSettings;
+import org.bukkit.command.CommandSender;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.io.File;
+import java.util.List;
+
+import static fr.xephi.authme.AuthMeMatchers.equalToHash;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Matchers.argThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Test for {@link CrazyLoginConverter}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class CrazyLoginConverterTest {
+
+    private CrazyLoginConverter crazyLoginConverter;
+
+    @Mock
+    private DataSource dataSource;
+
+    @Mock
+    private NewSetting settings;
+
+    private File dataFolder = TestHelper.getJarFile("/converter/");
+
+    @BeforeClass
+    public static void initializeLogger() {
+        TestHelper.setupLogger();
+    }
+
+    @Before
+    public void instantiateConverter() {
+        crazyLoginConverter = new CrazyLoginConverter(dataFolder, dataSource, settings);
+    }
+
+    @Test
+    public void shouldImportUsers() {
+        // given
+        given(settings.getProperty(ConverterSettings.CRAZYLOGIN_FILE_NAME)).willReturn("crazylogin.db");
+        CommandSender sender = mock(CommandSender.class);
+
+        // when
+        crazyLoginConverter.execute(sender);
+
+        // then
+        ArgumentCaptor<PlayerAuth> authCaptor = ArgumentCaptor.forClass(PlayerAuth.class);
+        verify(dataSource, times(2)).saveAuth(authCaptor.capture());
+        List<PlayerAuth> savedAuths = authCaptor.getAllValues();
+        assertNameAndRealName(savedAuths.get(0), "qotato", "qotaTo");
+        assertThat(savedAuths.get(0).getPassword(), equalToHash("8267663ab198a96437b9f455429a2c1b6c943111613c217bf2703c14d08a309d34e510ddb5549507b1500759dbcf9d4a99bc765ff37b32bd31adbb1e92e74ac5"));
+        assertNameAndRealName(savedAuths.get(1), "bobby", "Bobby");
+        assertThat(savedAuths.get(1).getPassword(), equalToHash("ad50dbc841e6321210530801f5219a5ffb4c7c41f11878d465374a4b8db2965c50f69b6098918a58e4adea312e3633c7724b15e24a217009e6fa2b3c299d55f2"));
+    }
+
+    @Test
+    public void shouldStopForNonExistentFile() {
+        // given
+        given(settings.getProperty(ConverterSettings.CRAZYLOGIN_FILE_NAME)).willReturn("invalid-file");
+        CommandSender sender = mock(CommandSender.class);
+
+        // when
+        crazyLoginConverter.execute(sender);
+
+        // then
+        verifyZeroInteractions(dataSource);
+        verify(sender).sendMessage(argThat(containsString("file not found")));
+    }
+
+    private static void assertNameAndRealName(PlayerAuth auth, String name, String realName) {
+        assertThat(auth.getNickname(), equalTo(name));
+        assertThat(auth.getRealName(), equalTo(realName));
+    }
+
+}

src/test/java/fr/xephi/authme/converter/ForceFlatToSqliteTest.java

@@ -54,7 +54,7 @@ public class ForceFlatToSqliteTest {
         ForceFlatToSqlite converter = new ForceFlatToSqlite(flatFile, dataSource);
 
         // when
-        converter.run();
+        converter.execute(null);
 
         // then
         ArgumentCaptor<PlayerAuth> authCaptor = ArgumentCaptor.forClass(PlayerAuth.class);

src/test/resources/converter/crazylogin.db

@@ -0,0 +1,3 @@
+name|password|ips|lastAction|loginFails|passwordExpired
+qotaTo|8267663ab198a96437b9f455429a2c1b6c943111613c217bf2703c14d08a309d34e510ddb5549507b1500759dbcf9d4a99bc765ff37b32bd31adbb1e92e74ac5|127.0.0.1|2016-05-27 21:45:57|0|false
+Bobby|ad50dbc841e6321210530801f5219a5ffb4c7c41f11878d465374a4b8db2965c50f69b6098918a58e4adea312e3633c7724b15e24a217009e6fa2b3c299d55f2|127.0.0.1|2016-05-27 21:45:40|0|false