From 589e589e4513da9a74925a358389e997039584b1 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 4 Sep 2016 13:59:23 +0200 Subject: [PATCH 1/3] #933 Add MySQL to SQLite converter - Create common parent for converting from one datasource type to another - Add MySQL to SQLite child - Create tests --- .../authme/command/CommandInitializer.java | 2 +- .../executable/authme/ConverterCommand.java | 7 +- .../AbstractDataSourceConverter.java | 90 +++++++++++++ .../authme/converter/ForceFlatToSqlite.java | 36 +---- .../xephi/authme/converter/MySqlToSqlite.java | 28 ++++ .../xephi/authme/converter/SqliteToSql.java | 35 ++--- .../xephi/authme/util/MigrationService.java | 2 +- .../AbstractDataSourceConverterTest.java | 126 ++++++++++++++++++ .../converter/ForceFlatToSqliteTest.java | 5 +- 9 files changed, 270 insertions(+), 61 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/converter/AbstractDataSourceConverter.java create mode 100644 src/main/java/fr/xephi/authme/converter/MySqlToSqlite.java create mode 100644 src/test/java/fr/xephi/authme/converter/AbstractDataSourceConverterTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index ce02b9a8d..0d5c6da07 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -282,7 +282,7 @@ public class CommandInitializer { .description("Converter command") .detailedDescription("Converter command for AuthMeReloaded.") .withArgument("job", "Conversion job: xauth / crazylogin / rakamak / " + - "royalauth / vauth / sqlitetosql", false) + "royalauth / vauth / sqliteToSql / mysqlToSqlite", false) .permission(AdminPermission.CONVERTER) .executableCommand(ConverterCommand.class) .build(); 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 a4a064869..83ccf8db5 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 @@ -7,6 +7,7 @@ import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.converter.Converter; import fr.xephi.authme.converter.CrazyLoginConverter; +import fr.xephi.authme.converter.MySqlToSqlite; import fr.xephi.authme.converter.RakamakConverter; import fr.xephi.authme.converter.RoyalAuthConverter; import fr.xephi.authme.converter.SqliteToSql; @@ -55,13 +56,14 @@ public class ConverterCommand implements ExecutableCommand { try { converter.execute(sender); } catch (Exception e) { + commandService.send(sender, MessageKey.ERROR); ConsoleLogger.logException("Error during conversion:", e); } } }); // Show a status message - sender.sendMessage("[AuthMe] Successfully converted from " + jobType.getName()); + sender.sendMessage("[AuthMe] Successfully started " + jobType.getName()); } @VisibleForTesting @@ -71,7 +73,8 @@ public class ConverterCommand implements ExecutableCommand { RAKAMAK("rakamak", RakamakConverter.class), ROYALAUTH("royalauth", RoyalAuthConverter.class), VAUTH("vauth", vAuthConverter.class), - SQLITETOSQL("sqlitetosql", SqliteToSql.class); + SQLITE_TO_SQL("sqlitetosql", SqliteToSql.class), + MYSQL_TO_SQLITE("mysqltosqlite", MySqlToSqlite.class); private final String name; private final Class converterClass; diff --git a/src/main/java/fr/xephi/authme/converter/AbstractDataSourceConverter.java b/src/main/java/fr/xephi/authme/converter/AbstractDataSourceConverter.java new file mode 100644 index 000000000..19be41c33 --- /dev/null +++ b/src/main/java/fr/xephi/authme/converter/AbstractDataSourceConverter.java @@ -0,0 +1,90 @@ +package fr.xephi.authme.converter; + +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.util.StringUtils; +import org.bukkit.command.CommandSender; +import org.bukkit.command.ConsoleCommandSender; + +import java.util.ArrayList; +import java.util.List; + +/** + * Converts from one AuthMe data source type to another. + * + * @param the source type to convert from + */ +public abstract class AbstractDataSourceConverter implements Converter { + + private DataSource destination; + private DataSourceType destinationType; + + /** + * Constructor. + * + * @param destination the data source to convert to + * @param destinationType the data source type of the destination. The given data source is checked that its + * type corresponds to this type before the conversion is started, enabling us to just pass + * the current data source and letting this class check that the types correspond. + */ + public AbstractDataSourceConverter(DataSource destination, DataSourceType destinationType) { + this.destination = destination; + this.destinationType = destinationType; + } + + // Implementation note: Because of ForceFlatToSqlite it is possible that the CommandSender is null, + // which is never the case when a converter is launched from the /authme converter command. + @Override + public void execute(CommandSender sender) { + if (!destinationType.equals(destination.getType())) { + if (sender != null) { + sender.sendMessage("Please configure your connection to " + + destinationType + " and re-run this command"); + } + return; + } + + S source; + try { + source = getSource(); + } catch (Exception e) { + logAndSendMessage(sender, "The data source to convert from could not be initialized"); + ConsoleLogger.logException("Could not initialize source:", e); + return; + } + + List skippedPlayers = new ArrayList<>(); + for (PlayerAuth auth : source.getAllAuths()) { + if (destination.isAuthAvailable(auth.getNickname())) { + skippedPlayers.add(auth.getNickname()); + } else { + destination.saveAuth(auth); + destination.updateQuitLoc(auth); + } + } + + if (!skippedPlayers.isEmpty()) { + logAndSendMessage(sender, "Skipped conversion for players which were already in " + + destinationType + ": " + StringUtils.join(", ", skippedPlayers)); + } + logAndSendMessage(sender, "Database successfully converted from " + source.getType() + + " to " + destinationType); + } + + /** + * @return the data source to convert from + * @throws Exception during initialization of source + */ + protected abstract S getSource() throws Exception; + + private static void logAndSendMessage(CommandSender sender, String message) { + ConsoleLogger.info(message); + // Make sure sender is not console user, which will see the message from ConsoleLogger already + if (sender != null && !(sender instanceof ConsoleCommandSender)) { + sender.sendMessage(message); + } + } + +} diff --git a/src/main/java/fr/xephi/authme/converter/ForceFlatToSqlite.java b/src/main/java/fr/xephi/authme/converter/ForceFlatToSqlite.java index 64f89b7e5..321ef2463 100644 --- a/src/main/java/fr/xephi/authme/converter/ForceFlatToSqlite.java +++ b/src/main/java/fr/xephi/authme/converter/ForceFlatToSqlite.java @@ -1,21 +1,14 @@ package fr.xephi.authme.converter; -import fr.xephi.authme.ConsoleLogger; -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 java.util.ArrayList; -import java.util.List; /** * Mandatory migration from the deprecated flat file datasource to SQLite. */ -public class ForceFlatToSqlite { +public class ForceFlatToSqlite extends AbstractDataSourceConverter { - private final DataSource source; - private final DataSource destination; + private final FlatFile source; /** * Constructor. @@ -24,29 +17,12 @@ public class ForceFlatToSqlite { * @param destination The datasource to copy the data to (sqlite) */ public ForceFlatToSqlite(FlatFile source, DataSource destination) { + super(destination, destination.getType()); this.source = source; - this.destination = destination; } - /** - * Perform the conversion. - */ - public void run() { - List skippedPlayers = new ArrayList<>(); - for (PlayerAuth auth : source.getAllAuths()) { - if (destination.isAuthAvailable(auth.getNickname())) { - skippedPlayers.add(auth.getNickname()); - } else { - destination.saveAuth(auth); - destination.updateQuitLoc(auth); - } - } - - if (!skippedPlayers.isEmpty()) { - ConsoleLogger.warning("Warning: skipped conversion for players which were already in SQLite: " - + StringUtils.join(", ", skippedPlayers)); - } - ConsoleLogger.info("Database successfully converted from " + source.getClass().getSimpleName() - + " to " + destination.getClass().getSimpleName()); + @Override + public FlatFile getSource() { + return source; } } diff --git a/src/main/java/fr/xephi/authme/converter/MySqlToSqlite.java b/src/main/java/fr/xephi/authme/converter/MySqlToSqlite.java new file mode 100644 index 000000000..76e58f960 --- /dev/null +++ b/src/main/java/fr/xephi/authme/converter/MySqlToSqlite.java @@ -0,0 +1,28 @@ +package fr.xephi.authme.converter; + +import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.datasource.DataSourceType; +import fr.xephi.authme.datasource.MySQL; +import fr.xephi.authme.settings.Settings; + +import javax.inject.Inject; +import java.sql.SQLException; + +/** + * Converts from MySQL to SQLite. + */ +public class MySqlToSqlite extends AbstractDataSourceConverter { + + private final Settings settings; + + @Inject + MySqlToSqlite(DataSource dataSource, Settings settings) { + super(dataSource, DataSourceType.SQLITE); + this.settings = settings; + } + + @Override + protected MySQL getSource() throws SQLException, ClassNotFoundException { + return new MySQL(settings); + } +} diff --git a/src/main/java/fr/xephi/authme/converter/SqliteToSql.java b/src/main/java/fr/xephi/authme/converter/SqliteToSql.java index dbb127f5e..fafa7e27d 100644 --- a/src/main/java/fr/xephi/authme/converter/SqliteToSql.java +++ b/src/main/java/fr/xephi/authme/converter/SqliteToSql.java @@ -1,45 +1,28 @@ package fr.xephi.authme.converter; -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.Settings; -import org.bukkit.command.CommandSender; import javax.inject.Inject; +import java.sql.SQLException; -public class SqliteToSql implements Converter { +/** + * Converts from SQLite to MySQL. + */ +public class SqliteToSql extends AbstractDataSourceConverter { private final Settings settings; - private final DataSource dataSource; - private final Messages messages; @Inject - SqliteToSql(Settings settings, DataSource dataSource, Messages messages) { + SqliteToSql(Settings settings, DataSource dataSource) { + super(dataSource, DataSourceType.MYSQL); this.settings = settings; - this.dataSource = dataSource; - this.messages = messages; } @Override - public void execute(CommandSender sender) { - 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()) { - dataSource.saveAuth(auth); - } - } catch (Exception e) { - messages.send(sender, MessageKey.ERROR); - ConsoleLogger.logException("Problem during SQLite to SQL conversion:", e); - } + protected SQLite getSource() throws SQLException, ClassNotFoundException { + return new SQLite(settings); } - } diff --git a/src/main/java/fr/xephi/authme/util/MigrationService.java b/src/main/java/fr/xephi/authme/util/MigrationService.java index cc0ef72a4..7b0a32d0a 100644 --- a/src/main/java/fr/xephi/authme/util/MigrationService.java +++ b/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; diff --git a/src/test/java/fr/xephi/authme/converter/AbstractDataSourceConverterTest.java b/src/test/java/fr/xephi/authme/converter/AbstractDataSourceConverterTest.java new file mode 100644 index 000000000..a14923f3a --- /dev/null +++ b/src/test/java/fr/xephi/authme/converter/AbstractDataSourceConverterTest.java @@ -0,0 +1,126 @@ +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.datasource.DataSourceType; +import org.bukkit.command.CommandSender; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.Mockito; + +import java.util.Arrays; +import java.util.List; + +import static org.hamcrest.Matchers.containsString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.argThat; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link AbstractDataSourceConverter}. + */ +public class AbstractDataSourceConverterTest { + + @BeforeClass + public static void initLogger() { + TestHelper.setupLogger(); + } + + @Test + public void shouldThrowForDestinationTypeMismatch() { + // given + DataSource destination = mock(DataSource.class); + given(destination.getType()).willReturn(DataSourceType.MYSQL); + DataSourceType destinationType = DataSourceType.SQLITE; + DataSource source = mock(DataSource.class); + Converter converter = new DataSourceConverterTestImpl<>(source, destination, destinationType); + CommandSender sender = mock(CommandSender.class); + + // when + converter.execute(sender); + + // then + verify(sender).sendMessage(argThat(containsString("Please configure your connection to SQLITE"))); + verify(destination, only()).getType(); + verifyZeroInteractions(source); + } + + @Test + public void shouldHandleSourceThrowingException() { + // given + DataSource source = mock(DataSource.class); + DataSource destination = mock(DataSource.class); + DataSourceType destinationType = DataSourceType.SQLITE; + given(destination.getType()).willReturn(destinationType); + DataSourceConverterTestImpl converter = + Mockito.spy(new DataSourceConverterTestImpl<>(source, destination, destinationType)); + doThrow(IllegalStateException.class).when(converter).getSource(); + CommandSender sender = mock(CommandSender.class); + + // when + converter.execute(sender); + + // then + verify(sender).sendMessage("The data source to convert from could not be initialized"); + verify(destination, only()).getType(); + verifyZeroInteractions(source); + } + + @Test + public void shouldConvertAndSkipExistingPlayers() { + // given + DataSource source = mock(DataSource.class); + DataSource destination = mock(DataSource.class); + DataSourceType destinationType = DataSourceType.MYSQL; + given(destination.getType()).willReturn(destinationType); + + List auths = + Arrays.asList(mockAuthWithName("Steven"), mockAuthWithName("bobby"), mockAuthWithName("Jack")); + given(source.getAllAuths()).willReturn(auths); + given(destination.isAuthAvailable(auths.get(0).getNickname())).willReturn(true); + + Converter converter = new DataSourceConverterTestImpl<>(source, destination, destinationType); + CommandSender sender = mock(CommandSender.class); + + // when + converter.execute(sender); + + // then + verify(destination).getType(); + verify(destination, times(3)).isAuthAvailable(anyString()); + verify(destination, times(2)).saveAuth(any(PlayerAuth.class)); + verify(destination, times(2)).updateQuitLoc(any(PlayerAuth.class)); + verifyNoMoreInteractions(destination); + verify(sender).sendMessage(argThat(containsString(auths.get(0).getNickname()))); + verify(sender).sendMessage(argThat(containsString("successfully converted"))); + } + + private static PlayerAuth mockAuthWithName(String name) { + PlayerAuth auth = mock(PlayerAuth.class); + given(auth.getNickname()).willReturn(name); + return auth; + } + + private static class DataSourceConverterTestImpl extends AbstractDataSourceConverter { + private final S source; + + DataSourceConverterTestImpl(S source, DataSource destination, DataSourceType destinationType) { + super(destination, destinationType); + this.source = source; + } + + @Override + protected S getSource() { + return source; + } + } +} diff --git a/src/test/java/fr/xephi/authme/converter/ForceFlatToSqliteTest.java b/src/test/java/fr/xephi/authme/converter/ForceFlatToSqliteTest.java index 903c58ff3..273d5530f 100644 --- a/src/test/java/fr/xephi/authme/converter/ForceFlatToSqliteTest.java +++ b/src/test/java/fr/xephi/authme/converter/ForceFlatToSqliteTest.java @@ -4,6 +4,7 @@ import com.google.common.io.Files; import fr.xephi.authme.TestHelper; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.datasource.DataSourceType; import fr.xephi.authme.datasource.FlatFile; import org.junit.Before; import org.junit.BeforeClass; @@ -20,6 +21,7 @@ import static fr.xephi.authme.AuthMeMatchers.hasAuthBasicData; import static fr.xephi.authme.AuthMeMatchers.hasAuthLocation; import static org.hamcrest.Matchers.hasItem; import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -51,10 +53,11 @@ public class ForceFlatToSqliteTest { public void shouldConvertToSqlite() { // given DataSource dataSource = mock(DataSource.class); + given(dataSource.getType()).willReturn(DataSourceType.MYSQL); ForceFlatToSqlite converter = new ForceFlatToSqlite(flatFile, dataSource); // when - converter.run(); + converter.execute(null); // then ArgumentCaptor authCaptor = ArgumentCaptor.forClass(PlayerAuth.class); From ee5ed139315a5fab683ce0f3e6be3f53a86103bc Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 4 Sep 2016 14:23:13 +0200 Subject: [PATCH 2/3] Replace enum with map in converter command --- .../executable/authme/ConverterCommand.java | 63 ++++++--------- .../authme/ConverterCommandTest.java | 77 +++++++++++++------ 2 files changed, 77 insertions(+), 63 deletions(-) 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); + } + } From 5930f705f2676971da042a05c74e794a0f765491 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 4 Sep 2016 14:46:28 +0200 Subject: [PATCH 3/3] Send more precise message when converter type does not exist --- .../authme/command/executable/authme/ConverterCommand.java | 2 +- .../command/executable/authme/ConverterCommandTest.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) 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 64cf5d196..a615f3ec9 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 @@ -47,7 +47,7 @@ public class ConverterCommand implements ExecutableCommand { // Determine the job type Class converterClass = CONVERTERS.get(job.toLowerCase()); if (converterClass == null) { - commandService.send(sender, MessageKey.ERROR); + sender.sendMessage("[AuthMe] Converter does not exist!"); return; } 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 a547929bf..d69e238d9 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 @@ -22,10 +22,12 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +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.any; +import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -64,7 +66,7 @@ public class ConverterCommandTest { command.executeCommand(sender, Collections.singletonList("invalid")); // then - verify(commandService).send(sender, MessageKey.ERROR); + verify(sender).sendMessage(argThat(containsString("Converter does not exist"))); verifyNoMoreInteractions(commandService); verifyZeroInteractions(injector); verifyZeroInteractions(bukkitService);