diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ConverterCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ConverterCommand.java index 83ccf8db5..64cf5d196 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ConverterCommand.java +++ b/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> 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); - if (jobType == null) { + Class converterClass = CONVERTERS.get(job.toLowerCase()); + 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 { - XAUTH("xauth", xAuthConverter.class), - CRAZYLOGIN("crazylogin", CrazyLoginConverter.class), - RAKAMAK("rakamak", RakamakConverter.class), - ROYALAUTH("royalauth", RoyalAuthConverter.class), - VAUTH("vauth", vAuthConverter.class), - SQLITE_TO_SQL("sqlitetosql", SqliteToSql.class), - MYSQL_TO_SQLITE("mysqltosqlite", MySqlToSqlite.class); - - private final String name; - private final Class converterClass; - - ConvertType(String name, Class converterClass) { - this.name = name; - 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 getConverterClass() { - return converterClass; - } + /** + * Initializes a map with all available converters. + * + * @return map with all available converters + */ + private static Map> getConverters() { + return ImmutableMap.>builder() + .put("xauth", xAuthConverter.class) + .put("crazylogin", CrazyLoginConverter.class) + .put("rakamak", RakamakConverter.class) + .put("royalauth", RoyalAuthConverter.class) + .put("vauth", vAuthConverter.class) + .put("sqlitetosql", SqliteToSql.class) + .put("mysqltosqlite", MySqlToSqlite.class) + .build(); } + } diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/ConverterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/ConverterCommandTest.java index 2f4eaa944..a547929bf 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/ConverterCommandTest.java +++ b/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.not; -import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.equalTo; 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(); - List names = new ArrayList<>(types.length); - List> classes = new ArrayList<>(types.length); + Set> classes = new HashSet<>(); // when / then - for (ConverterCommand.ConvertType type : types) { - assertThat("Name for '" + type + "' is not null", - type.getName(), not(nullValue())); - assertThat("Class for '" + type + "' is not null", - type.getConverterClass(), not(nullValue())); - assertThat("Name '" + type.getName() + "' is unique", - names, not(hasItem(type.getName()))); - assertThat("Class '" + type.getConverterClass() + "' is unique", - classes, not(hasItem(type.getConverterClass()))); - names.add(type.getName()); - classes.add(type.getConverterClass()); + for (Map.Entry> entry : ConverterCommand.CONVERTERS.entrySet()) { + assertThat("Name is not null or empty", + StringUtils.isEmpty(entry.getKey()), equalTo(false)); + + assertThat("Converter class is unique for each entry", + classes.add(entry.getValue()), equalTo(true)); } } @Test public void shouldLaunchConverterForAllTypes() { // given - ConverterCommand.ConvertType type = ConverterCommand.ConvertType.RAKAMAK; - RakamakConverter converter = mock(RakamakConverter.class); - given(injector.newInstance(RakamakConverter.class)).willReturn(converter); + String converterName = "rakamak"; + Class converterClass = ConverterCommand.CONVERTERS.get(converterName); + // 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 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); + } + }