Replace enum with map in converter command

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

@@ -2,6 +2,7 @@ package fr.xephi.authme.command.executable.authme;
 
 import ch.jalu.injector.Injector;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.command.CommandService;
 import fr.xephi.authme.command.ExecutableCommand;
@@ -19,12 +20,16 @@ import org.bukkit.command.CommandSender;
 
 import javax.inject.Inject;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Converter command: launches conversion based on its parameters.
  */
 public class ConverterCommand implements ExecutableCommand {
 
+    @VisibleForTesting
+    static final Map<String, Class<? extends Converter>> CONVERTERS = getConverters();
+
     @Inject
     private CommandService commandService;
 
@@ -40,14 +45,14 @@ public class ConverterCommand implements ExecutableCommand {
         String job = arguments.get(0);
 
         // Determine the job type
-        ConvertType jobType = ConvertType.fromName(job);
+        Class<? extends Converter> converterClass = CONVERTERS.get(job.toLowerCase());
-        if (jobType == null) {
+        if (converterClass == null) {
             commandService.send(sender, MessageKey.ERROR);
             return;
         }
 
         // Get the proper converter instance
-        final Converter converter = injector.newInstance(jobType.getConverterClass());
+        final Converter converter = injector.newInstance(converterClass);
 
         // Run the convert job
         bukkitService.runTaskAsynchronously(new Runnable() {
@@ -63,42 +68,24 @@ public class ConverterCommand implements ExecutableCommand {
         });
 
         // Show a status message
-        sender.sendMessage("[AuthMe] Successfully started " + jobType.getName());
+        sender.sendMessage("[AuthMe] Successfully started " + job);
     }
 
-    @VisibleForTesting
+    /**
-    enum ConvertType {
+     * Initializes a map with all available converters.
-        XAUTH("xauth", xAuthConverter.class),
+     *
-        CRAZYLOGIN("crazylogin", CrazyLoginConverter.class),
+     * @return map with all available converters
-        RAKAMAK("rakamak", RakamakConverter.class),
+     */
-        ROYALAUTH("royalauth", RoyalAuthConverter.class),
+    private static Map<String, Class<? extends Converter>> getConverters() {
-        VAUTH("vauth", vAuthConverter.class),
+        return ImmutableMap.<String, Class<? extends Converter>>builder()
-        SQLITE_TO_SQL("sqlitetosql", SqliteToSql.class),
+            .put("xauth", xAuthConverter.class)
-        MYSQL_TO_SQLITE("mysqltosqlite", MySqlToSqlite.class);
+            .put("crazylogin", CrazyLoginConverter.class)
-
+            .put("rakamak", RakamakConverter.class)
-        private final String name;
+            .put("royalauth", RoyalAuthConverter.class)
-        private final Class<? extends Converter> converterClass;
+            .put("vauth", vAuthConverter.class)
-
+            .put("sqlitetosql", SqliteToSql.class)
-        ConvertType(String name, Class<? extends Converter> converterClass) {
+            .put("mysqltosqlite", MySqlToSqlite.class)
-            this.name = name;
+            .build();
-            this.converterClass = converterClass;
     }
 
-        public static ConvertType fromName(String name) {
-            for (ConvertType type : ConvertType.values()) {
-                if (type.getName().equalsIgnoreCase(name)) {
-                    return type;
-                }
-            }
-            return null;
-        }
-
-        public String getName() {
-            return this.name;
-        }
-
-        public Class<? extends Converter> getConverterClass() {
-            return converterClass;
-        }
-    }
 }

src/test/java/fr/xephi/authme/command/executable/authme/ConverterCommandTest.java

@@ -3,25 +3,30 @@ package fr.xephi.authme.command.executable.authme;
 import ch.jalu.injector.Injector;
 import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.command.CommandService;
+import fr.xephi.authme.converter.Converter;
 import fr.xephi.authme.converter.RakamakConverter;
+import fr.xephi.authme.converter.vAuthConverter;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.util.BukkitService;
+import fr.xephi.authme.util.StringUtils;
 import org.bukkit.command.CommandSender;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 
-import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertThat;
 import static org.mockito.BDDMockito.given;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -45,6 +50,11 @@ public class ConverterCommandTest {
     @Mock
     private Injector injector;
 
+    @BeforeClass
+    public static void initLogger() {
+        TestHelper.setupLogger();
+    }
+
     @Test
     public void shouldHandleUnknownConversionType() {
         // given
@@ -61,44 +71,61 @@ public class ConverterCommandTest {
     }
 
     @Test
-    public void shouldHaveUniqueNameAndClassForEachType() {
+    public void shouldHaveUniqueClassForEachConverter() {
         // given
-        ConverterCommand.ConvertType[] types = ConverterCommand.ConvertType.values();
+        Set<Class<? extends Converter>> classes = new HashSet<>();
-        List<String> names = new ArrayList<>(types.length);
-        List<Class<?>> classes = new ArrayList<>(types.length);
 
         // when / then
-        for (ConverterCommand.ConvertType type : types) {
+        for (Map.Entry<String, Class<? extends Converter>> entry : ConverterCommand.CONVERTERS.entrySet()) {
-            assertThat("Name for '" + type + "' is not null",
+            assertThat("Name is not null or empty",
-                type.getName(), not(nullValue()));
+                StringUtils.isEmpty(entry.getKey()), equalTo(false));
-            assertThat("Class for '" + type + "' is not null",
+
-                type.getConverterClass(), not(nullValue()));
+            assertThat("Converter class is unique for each entry",
-            assertThat("Name '" + type.getName() + "' is unique",
+                classes.add(entry.getValue()), equalTo(true));
-                names, not(hasItem(type.getName())));
-            assertThat("Class '" + type.getConverterClass() + "' is unique",
-                classes, not(hasItem(type.getConverterClass())));
-            names.add(type.getName());
-            classes.add(type.getConverterClass());
         }
     }
 
     @Test
     public void shouldLaunchConverterForAllTypes() {
         // given
-        ConverterCommand.ConvertType type = ConverterCommand.ConvertType.RAKAMAK;
+        String converterName = "rakamak";
-        RakamakConverter converter = mock(RakamakConverter.class);
+        Class<? extends Converter> converterClass = ConverterCommand.CONVERTERS.get(converterName);
-        given(injector.newInstance(RakamakConverter.class)).willReturn(converter);
+        // Keep concrete class reference in mock: if this class is ever removed, we need to use another converterName
+        Converter converter = mock(RakamakConverter.class);
+        given(injector.newInstance(converterClass)).willReturn(converter);
         CommandSender sender = mock(CommandSender.class);
 
         // when
-        command.executeCommand(sender, Collections.singletonList(type.getName()));
+        command.executeCommand(sender, Collections.singletonList(converterName));
         TestHelper.runInnerRunnable(bukkitService);
 
         // then
         verify(converter).execute(sender);
         verifyNoMoreInteractions(converter);
-        verify(injector).newInstance(type.getConverterClass());
+        verify(injector).newInstance(converterClass);
         verifyNoMoreInteractions(injector);
     }
 
+    @Test
+    public void shouldCatchExceptionInConverterAndInformSender() {
+        // given
+        String converterName = "vauth";
+        Class<? extends Converter> converterClass = ConverterCommand.CONVERTERS.get(converterName);
+        Converter converter = mock(vAuthConverter.class);
+        doThrow(IllegalStateException.class).when(converter).execute(any(CommandSender.class));
+        given(injector.newInstance(converterClass)).willReturn(converter);
+        CommandSender sender = mock(CommandSender.class);
+
+        // when
+        command.executeCommand(sender, Collections.singletonList(converterName.toUpperCase()));
+        TestHelper.runInnerRunnable(bukkitService);
+
+        // then
+        verify(converter).execute(sender);
+        verifyNoMoreInteractions(converter);
+        verify(injector).newInstance(converterClass);
+        verifyNoMoreInteractions(injector);
+        verify(commandService).send(sender, MessageKey.ERROR);
+    }
+
 }