diff --git a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java index cf6c9bc2a..45d05ee32 100644 --- a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java @@ -66,7 +66,7 @@ public class ChangePasswordCommand extends ExecutableCommand { // Set the password final AuthMe plugin = wrapper.getAuthMe(); - plugin.getServer().getScheduler().runTaskAsynchronously(plugin, + wrapper.getServer().getScheduler().runTaskAsynchronously(plugin, new ChangePasswordTask(plugin, player, playerPass, playerPassVerify)); return true; } diff --git a/src/main/java/fr/xephi/authme/task/ChangePasswordTask.java b/src/main/java/fr/xephi/authme/task/ChangePasswordTask.java index 93d05f12f..05d93dfb5 100644 --- a/src/main/java/fr/xephi/authme/task/ChangePasswordTask.java +++ b/src/main/java/fr/xephi/authme/task/ChangePasswordTask.java @@ -36,11 +36,6 @@ public class ChangePasswordTask implements Runnable { this.newPassword = newPassword; } - /** - * Method run. - * - * @see java.lang.Runnable#run() - */ @Override public void run() { Messages m = plugin.getMessages(); diff --git a/src/test/java/fr/xephi/authme/ReflectionTestUtils.java b/src/test/java/fr/xephi/authme/ReflectionTestUtils.java index 6007f1a82..33579e805 100644 --- a/src/test/java/fr/xephi/authme/ReflectionTestUtils.java +++ b/src/test/java/fr/xephi/authme/ReflectionTestUtils.java @@ -1,6 +1,9 @@ package fr.xephi.authme; import java.lang.reflect.Field; +import java.lang.reflect.Method; + +import static java.lang.String.format; /** * Offers reflection functionality to set up tests. Use only when absolutely necessary. @@ -21,13 +24,53 @@ public final class ReflectionTestUtils { */ public static void setField(Class clazz, T instance, String fieldName, Object value) { try { - Field field = clazz.getDeclaredField(fieldName); - field.setAccessible(true); + Field field = getField(clazz, instance, fieldName); field.set(instance, value); - } catch (NoSuchFieldException | IllegalAccessException e) { + } catch (IllegalAccessException e) { throw new RuntimeException( - String.format("Could not set value to field '%s' for instance '%s' of class '%s'", + format("Could not set value to field '%s' for instance '%s' of class '%s'", fieldName, instance, clazz.getName()), e); } } + + private static Field getField(Class clazz, T instance, String fieldName) { + try { + Field field = clazz.getDeclaredField(fieldName); + field.setAccessible(true); + return field; + } catch (NoSuchFieldException e) { + throw new RuntimeException(format("Could not get field '%s' for instance '%s' of class '%s'", + fieldName, instance, clazz.getName()), e); + } + } + + + public static Object getFieldValue(Class clazz, T instance, String fieldName) { + Field field = getField(clazz, instance, fieldName); + try { + return field.get(instance); + } catch (IllegalAccessException e) { + throw new RuntimeException("Could not get value of field '" + fieldName + "'"); + } + } + + /** + * Return the method on the given class with the supplied parameter types. + * + * @param clazz The class to retrieve a method from + * @param methodName The name of the method + * @param parameterTypes The parameter types the method to retrieve has + * + * @return The method of the class, set to be accessible + */ + public static Method getMethod(Class clazz, String methodName, Class... parameterTypes) { + try { + Method method = clazz.getDeclaredMethod(methodName, parameterTypes); + method.setAccessible(true); + return method; + } catch (NoSuchMethodException e) { + throw new RuntimeException("Could not retrieve method '" + methodName + "' from class '" + + clazz.getName() + "'"); + } + } } diff --git a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java index 1cc820dd6..7d33a60ef 100644 --- a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java @@ -1,19 +1,29 @@ package fr.xephi.authme.command.executable.changepassword; +import fr.xephi.authme.AuthMe; +import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.command.CommandParts; import fr.xephi.authme.settings.MessageKey; import fr.xephi.authme.settings.Messages; import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.task.ChangePasswordTask; import fr.xephi.authme.util.WrapperMock; +import org.bukkit.Server; import org.bukkit.command.BlockCommandSender; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; +import org.bukkit.scheduler.BukkitScheduler; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; -import java.util.Arrays; +import java.util.Collections; +import static java.util.Arrays.asList; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.*; @@ -22,20 +32,21 @@ import static org.mockito.Mockito.*; */ public class ChangePasswordCommandTest { + private WrapperMock wrapperMock; private Messages messagesMock; private PlayerCache cacheMock; @Before public void setUpMocks() { - WrapperMock wrapper = WrapperMock.createInstance(); - messagesMock = wrapper.getMessages(); - cacheMock = wrapper.getPlayerCache(); + wrapperMock = WrapperMock.createInstance(); + messagesMock = wrapperMock.getMessages(); + cacheMock = wrapperMock.getPlayerCache(); // Only allow passwords with alphanumerical characters for the test Settings.getPassRegex = "[a-zA-Z0-9]+"; Settings.getPasswordMinLen = 2; Settings.passwordMaxLength = 50; - // TODO ljacqu 20151126: Verify the calls to getServer() (see commented code) + Settings.unsafePasswords = Collections.EMPTY_LIST; } @Test @@ -50,7 +61,7 @@ public class ChangePasswordCommandTest { // then verify(arguments, never()).get(anyInt()); - //verify(pluginMock, never()).getServer(); + assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false)); } @Test @@ -64,7 +75,7 @@ public class ChangePasswordCommandTest { // then verify(messagesMock).send(sender, MessageKey.NOT_LOGGED_IN); - //verify(pluginMock, never()).getServer(); + assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false)); } @Test @@ -78,7 +89,7 @@ public class ChangePasswordCommandTest { // then verify(messagesMock).send(sender, MessageKey.PASSWORD_MATCH_ERROR); - //verify(pluginMock, never()).getServer(); + assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false)); } @@ -93,7 +104,7 @@ public class ChangePasswordCommandTest { // then verify(messagesMock).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); - //verify(pluginMock, never()).getServer(); + assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false)); } @Test @@ -108,7 +119,7 @@ public class ChangePasswordCommandTest { // then verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); - //verify(pluginMock, never()).getServer(); + assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false)); } @Test @@ -123,7 +134,7 @@ public class ChangePasswordCommandTest { // then verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); - //verify(pluginMock, never()).getServer(); + assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false)); } @Test @@ -131,14 +142,34 @@ public class ChangePasswordCommandTest { // given CommandSender sender = initPlayerWithName("player", true); ChangePasswordCommand command = new ChangePasswordCommand(); - Settings.unsafePasswords = Arrays.asList("test", "abc123"); + Settings.unsafePasswords = asList("test", "abc123"); // when command.executeCommand(sender, new CommandParts(), new CommandParts("abc123")); // then verify(messagesMock).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); - //verify(pluginMock, never()).getServer(); + assertThat(wrapperMock.wasMockCalled(Server.class), equalTo(false)); + } + + @Test + public void shouldForwardTheDataForValidPassword() { + // given + CommandSender sender = initPlayerWithName("parker", true); + ChangePasswordCommand command = new ChangePasswordCommand(); + BukkitScheduler schedulerMock = mock(BukkitScheduler.class); + given(wrapperMock.getServer().getScheduler()).willReturn(schedulerMock); + + // when + command.executeCommand(sender, new CommandParts(), new CommandParts(asList("abc123", "abc123"))); + + // then + verify(messagesMock, never()).send(eq(sender), any(MessageKey.class)); + ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class); + verify(schedulerMock).runTaskAsynchronously(any(AuthMe.class), taskCaptor.capture()); + ChangePasswordTask task = taskCaptor.getValue(); + assertThat((String) ReflectionTestUtils.getFieldValue(ChangePasswordTask.class, task, "newPassword"), + equalTo("abc123")); } private Player initPlayerWithName(String name, boolean loggedIn) { diff --git a/src/test/java/fr/xephi/authme/util/UtilsTest.java b/src/test/java/fr/xephi/authme/util/UtilsTest.java index 3c79f63cf..95ba25246 100644 --- a/src/test/java/fr/xephi/authme/util/UtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/UtilsTest.java @@ -23,7 +23,6 @@ import static org.mockito.Mockito.*; */ public class UtilsTest { - private static WrapperMock wrapperMock; private static AuthMe authMeMock; private PermissionsManager permissionsManagerMock; @@ -34,7 +33,7 @@ public class UtilsTest { */ @BeforeClass public static void setUpMocks() { - wrapperMock = WrapperMock.createInstance(); + WrapperMock wrapperMock = WrapperMock.createInstance(); authMeMock = wrapperMock.getAuthMe(); } @@ -109,16 +108,11 @@ public class UtilsTest { } @Test - // Note ljacqu 20151122: This is a heavy test setup with reflections... If it causes trouble, skip it with @Ignore public void shouldRetrieveListOfOnlinePlayersFromReflectedMethod() { // given ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayersIsCollection", false); - try { - ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayers", - UtilsTest.class.getDeclaredMethod("onlinePlayersImpl")); - } catch (NoSuchMethodException e) { - throw new RuntimeException("Could not get method onlinePlayersImpl() in test class", e); - } + ReflectionTestUtils.setField(Utils.class, null, "getOnlinePlayers", + ReflectionTestUtils.getMethod(UtilsTest.class, "onlinePlayersImpl")); // when Collection players = Utils.getOnlinePlayers(); diff --git a/src/test/java/fr/xephi/authme/util/WrapperMock.java b/src/test/java/fr/xephi/authme/util/WrapperMock.java index efc5fa10f..f622c00bd 100644 --- a/src/test/java/fr/xephi/authme/util/WrapperMock.java +++ b/src/test/java/fr/xephi/authme/util/WrapperMock.java @@ -20,8 +20,6 @@ import java.util.logging.Logger; public class WrapperMock extends Wrapper { private Map, Object> mocks = new HashMap<>(); - private static WrapperMock singleton; - private File getDataFolderValue = new File("/"); private WrapperMock() { super(); @@ -33,9 +31,9 @@ public class WrapperMock extends Wrapper { * @return The created singleton */ public static WrapperMock createInstance() { - singleton = new WrapperMock(); - Wrapper.setSingleton(singleton); - return singleton; + WrapperMock instance = new WrapperMock(); + Wrapper.setSingleton(instance); + return instance; } @Override @@ -70,20 +68,19 @@ public class WrapperMock extends Wrapper { @Override public File getDataFolder() { - if (singleton.getDataFolderValue != null) { - return singleton.getDataFolderValue; - } - return getMock(File.class); + return new File("/"); } /** - * Set the data folder to be returned for test contexts. Defaults to File("/"); supply null to make WrapperMock - * return a mock of the File class as with the other fields. + * Return whether a mock of the given class type was created, i.e. verify whether a certain method was executed on + * the Wrapper to retrieve an entity. * - * @param file The data folder location to return + * @param mockClass The class of the mock to verify + * + * @return True if the mock has been created, false otherwise */ - public void setDataFolder(File file) { - this.getDataFolderValue = file; + public boolean wasMockCalled(Class mockClass) { + return mocks.get(mockClass) != null; } @SuppressWarnings("unchecked")