Minor fixes + code householding

- Fix SpawnCommandTest testing FirstSpawnCommand
- Fix javadoc errors
- Map TODO's to issue numbers where applicable
- Fix trivial TODO's
This commit is contained in:
ljacqu
2016-03-13 11:09:27 +01:00
parent 00125487f1
commit 5d12ec8b56
12 changed files with 46 additions and 32 deletions
@@ -25,7 +25,8 @@ public class DataManager {
/** /**
* Constructor for DataManager. * Constructor for DataManager.
* *
* @param plugin AuthMe * @param plugin The plugin instance
* @param pluginHooks Plugin hooks instance
*/ */
public DataManager(AuthMe plugin, PluginHooks pluginHooks) { public DataManager(AuthMe plugin, PluginHooks pluginHooks) {
this.plugin = plugin; this.plugin = plugin;
@@ -161,8 +162,13 @@ public class DataManager {
*/ */
public void purgeEssentials(List<String> cleared) { public void purgeEssentials(List<String> cleared) {
int i = 0; int i = 0;
// FIXME: essentials data folder may be null File essentialsDataFolder = pluginHooks.getEssentialsDataFolder();
final File userDataFolder = new File(pluginHooks.getEssentialsDataFolder(), "userdata"); if (essentialsDataFolder == null) {
ConsoleLogger.info("Cannot purge Essentials: plugin is not loaded");
return;
}
final File userDataFolder = new File(essentialsDataFolder, "userdata");
for (String name : cleared) { for (String name : cleared) {
try { try {
File playerFile = new File(userDataFolder, plugin.getServer().getOfflinePlayer(name).getUniqueId() + ".yml"); File playerFile = new File(userDataFolder, plugin.getServer().getOfflinePlayer(name).getUniqueId() + ".yml");
@@ -46,6 +46,8 @@ public class CommandService {
* @param permissionsManager The permissions manager * @param permissionsManager The permissions manager
* @param settings The settings manager * @param settings The settings manager
* @param ipAddressManager The IP address manager * @param ipAddressManager The IP address manager
* @param pluginHooks The plugin hooks instance
* @param spawnLoader The spawn loader
*/ */
public CommandService(AuthMe authMe, CommandMapper commandMapper, HelpProvider helpProvider, Messages messages, public CommandService(AuthMe authMe, CommandMapper commandMapper, HelpProvider helpProvider, Messages messages,
PasswordSecurity passwordSecurity, PermissionsManager permissionsManager, NewSetting settings, PasswordSecurity passwordSecurity, PermissionsManager permissionsManager, NewSetting settings,
@@ -40,7 +40,7 @@ public class ChangePasswordAdminCommand implements ExecutableCommand {
commandService.send(sender, MessageKey.INVALID_PASSWORD_LENGTH); commandService.send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
return; return;
} }
// TODO ljacqu 20160312: The UNSAFE_PASSWORDS should be all lowercase // TODO #602 20160312: The UNSAFE_PASSWORDS should be all lowercase
// -> introduce a lowercase String list property type // -> introduce a lowercase String list property type
if (commandService.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(playerPassLowerCase)) { if (commandService.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(playerPassLowerCase)) {
commandService.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); commandService.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR);
@@ -22,9 +22,9 @@ import java.util.Map.Entry;
*/ */
public class RakamakConverter implements Converter { public class RakamakConverter implements Converter {
public final AuthMe instance; private final AuthMe instance;
public final DataSource database; private final DataSource database;
public final CommandSender sender; private final CommandSender sender;
public RakamakConverter(AuthMe instance, CommandSender sender) { public RakamakConverter(AuthMe instance, CommandSender sender) {
this.instance = instance; this.instance = instance;
@@ -22,7 +22,8 @@ public class BungeeCordMessage implements PluginMessageListener {
/** /**
* Constructor for BungeeCordMessage. * Constructor for BungeeCordMessage.
* *
* @param plugin AuthMe * @param plugin The plugin instance
* @param ipAddressManager The IP address manager
*/ */
public BungeeCordMessage(AuthMe plugin, IpAddressManager ipAddressManager) { public BungeeCordMessage(AuthMe plugin, IpAddressManager ipAddressManager) {
this.plugin = plugin; this.plugin = plugin;
@@ -87,7 +87,7 @@ public class AuthMeEntityListener implements Listener {
} }
} }
// TODO: Need to check this, player can't throw snowball but the item is taken. // TODO #568: Need to check this, player can't throw snowball but the item is taken.
@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
public void onProjectileLaunch(ProjectileLaunchEvent event) { public void onProjectileLaunch(ProjectileLaunchEvent event) {
if (event.getEntity() == null) { if (event.getEntity() == null) {
@@ -103,7 +103,7 @@ public class AuthMeEntityListener implements Listener {
} }
player = (Player) shooter; player = (Player) shooter;
} else { } else {
// TODO ljacqu 20151220: Invoking getShooter() with null but method isn't static // TODO #568 20151220: Invoking getShooter() with null but method isn't static
try { try {
if (getShooter == null) { if (getShooter == null) {
getShooter = Projectile.class.getMethod("getShooter"); getShooter = Projectile.class.getMethod("getShooter");
@@ -80,11 +80,10 @@ public class AuthMePlayerListener implements Listener {
} }
event.setCancelled(true); event.setCancelled(true);
sendLoginRegisterMSG(player); sendLoginOrRegisterMessage(player);
} }
// TODO: new name private void sendLoginOrRegisterMessage(final Player player) {
private void sendLoginRegisterMSG(final Player player) {
plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() {
@Override @Override
public void run() { public void run() {
@@ -117,7 +116,7 @@ public class AuthMePlayerListener implements Listener {
return; return;
} }
event.setCancelled(true); event.setCancelled(true);
sendLoginRegisterMSG(event.getPlayer()); sendLoginOrRegisterMessage(event.getPlayer());
} }
@EventHandler(ignoreCancelled = true, priority = EventPriority.NORMAL) @EventHandler(ignoreCancelled = true, priority = EventPriority.NORMAL)
@@ -2,6 +2,7 @@ package fr.xephi.authme.process;
import fr.xephi.authme.AuthMe; import fr.xephi.authme.AuthMe;
import fr.xephi.authme.cache.IpAddressManager; import fr.xephi.authme.cache.IpAddressManager;
import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.hooks.PluginHooks; import fr.xephi.authme.hooks.PluginHooks;
import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.output.Messages; import fr.xephi.authme.output.Messages;
@@ -22,16 +23,19 @@ public class ProcessService {
private final NewSetting settings; private final NewSetting settings;
private final Messages messages; private final Messages messages;
private final AuthMe authMe; private final AuthMe authMe;
private final DataSource dataSource;
private final IpAddressManager ipAddressManager; private final IpAddressManager ipAddressManager;
private final PasswordSecurity passwordSecurity; private final PasswordSecurity passwordSecurity;
private final PluginHooks pluginHooks; private final PluginHooks pluginHooks;
private final SpawnLoader spawnLoader; private final SpawnLoader spawnLoader;
public ProcessService(NewSetting settings, Messages messages, AuthMe authMe, IpAddressManager ipAddressManager, public ProcessService(NewSetting settings, Messages messages, AuthMe authMe, DataSource dataSource,
PasswordSecurity passwordSecurity, PluginHooks pluginHooks, SpawnLoader spawnLoader) { IpAddressManager ipAddressManager, PasswordSecurity passwordSecurity, PluginHooks pluginHooks,
SpawnLoader spawnLoader) {
this.settings = settings; this.settings = settings;
this.messages = messages; this.messages = messages;
this.authMe = authMe; this.authMe = authMe;
this.dataSource = dataSource;
this.ipAddressManager = ipAddressManager; this.ipAddressManager = ipAddressManager;
this.passwordSecurity = passwordSecurity; this.passwordSecurity = passwordSecurity;
this.pluginHooks = pluginHooks; this.pluginHooks = pluginHooks;
@@ -98,4 +102,8 @@ public class ProcessService {
return spawnLoader; return spawnLoader;
} }
public DataSource getDataSource() {
return dataSource;
}
} }
@@ -34,12 +34,13 @@ public class AsynchronousUnregister implements Process {
private final ProcessService service; private final ProcessService service;
/** /**
* Constructor for AsynchronousUnregister. * Constructor.
* *
* @param player Player * @param player The player to perform the action for
* @param password String * @param password The password
* @param force boolean * @param force True to bypass password validation
* @param plugin AuthMe * @param plugin The plugin instance
* @param service The process service
*/ */
public AsynchronousUnregister(Player player, String password, boolean force, AuthMe plugin, public AsynchronousUnregister(Player player, String password, boolean force, AuthMe plugin,
ProcessService service) { ProcessService service) {
@@ -57,7 +58,7 @@ public class AsynchronousUnregister implements Process {
PlayerAuth cachedAuth = PlayerCache.getInstance().getAuth(name); PlayerAuth cachedAuth = PlayerCache.getInstance().getAuth(name);
if (force || plugin.getPasswordSecurity().comparePassword( if (force || plugin.getPasswordSecurity().comparePassword(
password, cachedAuth.getPassword(), player.getName())) { password, cachedAuth.getPassword(), player.getName())) {
if (!plugin.getDataSource().removeAuth(name)) { if (!service.getDataSource().removeAuth(name)) {
service.send(player, MessageKey.ERROR); service.send(player, MessageKey.ERROR);
return; return;
} }
@@ -106,8 +106,9 @@ public class BinTools {
throw new IllegalArgumentException("Input string may only contain hex digits, but found '" + c + "'"); throw new IllegalArgumentException("Input string may only contain hex digits, but found '" + c + "'");
} }
// TODO ljacqu 20151219: Move to a BinToolsTest class // Note ljacqu 20160313: This appears to be a test method that was present in the third-party source.
private static void testUtils(String[] args) { // We can keep it for troubleshooting in the future.
private static void testUtils() {
byte b[] = new byte[256]; byte b[] = new byte[256];
byte bb = 0; byte bb = 0;
for (int i = 0; i < 256; i++) { for (int i = 0; i < 256; i++) {
@@ -141,9 +141,8 @@ public class SpawnLoader {
World world = player.getWorld(); World world = player.getWorld();
Location spawnLoc = null; Location spawnLoc = null;
// TODO ljacqu 20160312: We should trim() the entries
for (String priority : spawnPriority) { for (String priority : spawnPriority) {
switch (priority.toLowerCase()) { switch (priority.toLowerCase().trim()) {
case "default": case "default":
if (world.getSpawnLocation() != null) { if (world.getSpawnLocation() != null) {
spawnLoc = world.getSpawnLocation(); spawnLoc = world.getSpawnLocation();
@@ -6,14 +6,13 @@ import fr.xephi.authme.settings.SpawnLoader;
import org.bukkit.Location; import org.bukkit.Location;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
import org.junit.Test; import org.junit.Test;
import org.mockito.ArgumentCaptor;
import java.util.Collections; import java.util.Collections;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
import static org.mockito.Matchers.argThat;
import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
@@ -51,15 +50,13 @@ public class SpawnCommandTest {
CommandService service = mock(CommandService.class); CommandService service = mock(CommandService.class);
given(service.getSpawnLoader()).willReturn(spawnLoader); given(service.getSpawnLoader()).willReturn(spawnLoader);
Player player = mock(Player.class); Player player = mock(Player.class);
ExecutableCommand command = new FirstSpawnCommand(); ExecutableCommand command = new SpawnCommand();
// when // when
command.executeCommand(player, Collections.EMPTY_LIST, service); command.executeCommand(player, Collections.EMPTY_LIST, service);
// then // then
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class); verify(player).sendMessage(argThat(containsString("Spawn has failed")));
verify(player).sendMessage(captor.capture());
assertThat(captor.getValue(), containsString("spawn has failed"));
verify(player, never()).teleport(any(Location.class)); verify(player, never()).teleport(any(Location.class));
} }
} }