Code householding (PlayerListener / Settings)

- Use field on PlayerListener for storing nickname pattern -> repeatedly parsing pattern is expensive
- Remove unused legacy setting fields
- ForceFlatToSqlite cannot be run from converter command -> remove Converter interface to create more natural method signatures
This commit is contained in:
ljacqu 2016-05-28 22:00:50 +02:00
parent b48e080324
commit 25f5fdb45c
7 changed files with 37 additions and 34 deletions

View File

@ -5,7 +5,6 @@ import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.datasource.FlatFile; import fr.xephi.authme.datasource.FlatFile;
import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.StringUtils;
import org.bukkit.command.CommandSender;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
@ -13,7 +12,7 @@ import java.util.List;
/** /**
* Mandatory migration from the deprecated flat file datasource to SQLite. * Mandatory migration from the deprecated flat file datasource to SQLite.
*/ */
public class ForceFlatToSqlite implements Converter { public class ForceFlatToSqlite {
private final DataSource source; private final DataSource source;
private final DataSource destination; private final DataSource destination;
@ -32,9 +31,7 @@ public class ForceFlatToSqlite implements Converter {
/** /**
* Perform the conversion. * Perform the conversion.
*/ */
@Override public void run() {
// Note ljacqu 20160527: CommandSender is null here; it is only present because of the interface it implements
public void execute(CommandSender sender) {
List<String> skippedPlayers = new ArrayList<>(); List<String> skippedPlayers = new ArrayList<>();
for (PlayerAuth auth : source.getAllAuths()) { for (PlayerAuth auth : source.getAllAuths()) {
if (destination.isAuthAvailable(auth.getNickname())) { if (destination.isAuthAvailable(auth.getNickname())) {

View File

@ -11,6 +11,7 @@ import fr.xephi.authme.cache.auth.PlayerCache;
import fr.xephi.authme.cache.limbo.LimboCache; import fr.xephi.authme.cache.limbo.LimboCache;
import fr.xephi.authme.cache.limbo.LimboPlayer; import fr.xephi.authme.cache.limbo.LimboPlayer;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.initialization.Reloadable;
import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.output.Messages; import fr.xephi.authme.output.Messages;
import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PermissionsManager;
@ -53,8 +54,8 @@ import org.bukkit.event.player.PlayerQuitEvent;
import org.bukkit.event.player.PlayerRespawnEvent; import org.bukkit.event.player.PlayerRespawnEvent;
import org.bukkit.event.player.PlayerShearEntityEvent; import org.bukkit.event.player.PlayerShearEntityEvent;
import javax.annotation.PostConstruct;
import javax.inject.Inject; import javax.inject.Inject;
import java.util.Iterator; import java.util.Iterator;
import java.util.Set; import java.util.Set;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
@ -68,10 +69,9 @@ import static fr.xephi.authme.settings.properties.RestrictionSettings.ALLOW_UNAU
/** /**
* Listener class for player events. * Listener class for player events.
*/ */
public class AuthMePlayerListener implements Listener { public class AuthMePlayerListener implements Listener, Reloadable {
public static final ConcurrentHashMap<String, String> joinMessage = new ConcurrentHashMap<>(); public static final ConcurrentHashMap<String, String> joinMessage = new ConcurrentHashMap<>();
public static final ConcurrentHashMap<String, Boolean> causeByAuthMe = new ConcurrentHashMap<>();
@Inject @Inject
private AuthMe plugin; private AuthMe plugin;
@ -91,6 +91,10 @@ public class AuthMePlayerListener implements Listener {
private SpawnLoader spawnLoader; private SpawnLoader spawnLoader;
@Inject @Inject
private ValidationService validationService; private ValidationService validationService;
@Inject
private PermissionsManager permissionsManager;
private Pattern nicknamePattern;
private void sendLoginOrRegisterMessage(final Player player) { private void sendLoginOrRegisterMessage(final Player player) {
bukkitService.runTaskAsynchronously(new Runnable() { bukkitService.runTaskAsynchronously(new Runnable() {
@ -244,6 +248,7 @@ public class AuthMePlayerListener implements Listener {
}); });
} }
// Note ljacqu 20160528: AsyncPlayerPreLoginEvent is not fired by all servers in offline mode
@EventHandler(priority = EventPriority.HIGHEST) @EventHandler(priority = EventPriority.HIGHEST)
public void onPreLogin(AsyncPlayerPreLoginEvent event) { public void onPreLogin(AsyncPlayerPreLoginEvent event) {
PlayerAuth auth = dataSource.getAuth(event.getName()); PlayerAuth auth = dataSource.getAuth(event.getName());
@ -306,11 +311,8 @@ public class AuthMePlayerListener implements Listener {
return; return;
} }
// Get the permissions manager
PermissionsManager permsMan = plugin.getPermissionsManager();
if (event.getResult() == PlayerLoginEvent.Result.KICK_FULL) { if (event.getResult() == PlayerLoginEvent.Result.KICK_FULL) {
if (permsMan.hasPermission(player, PlayerStatePermission.IS_VIP)) { if (permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)) {
int playersOnline = bukkitService.getOnlinePlayers().size(); int playersOnline = bukkitService.getOnlinePlayers().size();
if (playersOnline > plugin.getServer().getMaxPlayers()) { if (playersOnline > plugin.getServer().getMaxPlayers()) {
event.allow(); event.allow();
@ -358,10 +360,9 @@ public class AuthMePlayerListener implements Listener {
return; return;
} }
String nickRegEx = settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS); if (name.equalsIgnoreCase("Player") || !nicknamePattern.matcher(player.getName()).matches()) {
Pattern nickPattern = Pattern.compile(nickRegEx); event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_CHARACTERS)
if (name.equalsIgnoreCase("Player") || !nickPattern.matcher(player.getName()).matches()) { .replace("REG_EX", nicknamePattern.pattern()));
event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_CHARACTERS).replace("REG_EX", nickRegEx));
event.setResult(PlayerLoginEvent.Result.KICK_OTHER); event.setResult(PlayerLoginEvent.Result.KICK_OTHER);
return; return;
} }
@ -388,7 +389,7 @@ public class AuthMePlayerListener implements Listener {
} }
if (antiBot.antibotKicked.contains(player.getName())) { if (antiBot.antibotKicked.contains(player.getName())) {
return; return;
} }
management.performQuit(player, false); management.performQuit(player, false);
@ -408,11 +409,9 @@ public class AuthMePlayerListener implements Listener {
return; return;
} }
if (antiBot.antibotKicked.contains(player.getName())) { if (!antiBot.antibotKicked.contains(player.getName())) {
return; plugin.getManagement().performQuit(player, true);
} }
plugin.getManagement().performQuit(player, true);
} }
@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST) @EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
@ -548,4 +547,17 @@ public class AuthMePlayerListener implements Listener {
event.setCancelled(true); event.setCancelled(true);
} }
} }
@PostConstruct
@Override
public void reload() {
String nickRegEx = settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS);
try {
nicknamePattern = Pattern.compile(nickRegEx);
} catch (Exception e) {
nicknamePattern = Pattern.compile(".*?");
ConsoleLogger.showError("Nickname pattern is not a valid regular expression! "
+ "Fallback to allowing all nicknames");
}
}
} }

View File

@ -11,7 +11,6 @@ import fr.xephi.authme.events.FirstSpawnTeleportEvent;
import fr.xephi.authme.events.ProtectInventoryEvent; import fr.xephi.authme.events.ProtectInventoryEvent;
import fr.xephi.authme.events.SpawnTeleportEvent; import fr.xephi.authme.events.SpawnTeleportEvent;
import fr.xephi.authme.hooks.PluginHooks; import fr.xephi.authme.hooks.PluginHooks;
import fr.xephi.authme.listener.AuthMePlayerListener;
import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.permission.PlayerStatePermission;
import fr.xephi.authme.process.AsynchronousProcess; import fr.xephi.authme.process.AsynchronousProcess;
@ -105,7 +104,6 @@ public class AsynchronousJoin implements AsynchronousProcess {
bukkitService.scheduleSyncDelayedTask(new Runnable() { bukkitService.scheduleSyncDelayedTask(new Runnable() {
@Override @Override
public void run() { public void run() {
AuthMePlayerListener.causeByAuthMe.putIfAbsent(name, true);
player.kickPlayer(service.retrieveSingleMessage(MessageKey.NOT_OWNER_ERROR)); player.kickPlayer(service.retrieveSingleMessage(MessageKey.NOT_OWNER_ERROR));
if (service.getProperty(RestrictionSettings.BAN_UNKNOWN_IP)) { if (service.getProperty(RestrictionSettings.BAN_UNKNOWN_IP)) {
plugin.getServer().banIP(ip); plugin.getServer().banIP(ip);
@ -124,7 +122,6 @@ public class AsynchronousJoin implements AsynchronousProcess {
bukkitService.scheduleSyncDelayedTask(new Runnable() { bukkitService.scheduleSyncDelayedTask(new Runnable() {
@Override @Override
public void run() { public void run() {
// TODO: Messages entry
player.kickPlayer(service.retrieveSingleMessage(MessageKey.SAME_IP_ONLINE)); player.kickPlayer(service.retrieveSingleMessage(MessageKey.SAME_IP_ONLINE));
} }
}); });

View File

@ -29,7 +29,6 @@ public final class Settings {
public static boolean protectInventoryBeforeLogInEnabled; public static boolean protectInventoryBeforeLogInEnabled;
public static boolean isStopEnabled; public static boolean isStopEnabled;
public static boolean reloadSupport; public static boolean reloadSupport;
public static boolean rakamakUseIp;
public static boolean removePassword; public static boolean removePassword;
public static boolean multiverse; public static boolean multiverse;
public static boolean bungee; public static boolean bungee;
@ -39,8 +38,6 @@ public final class Settings {
public static String getUnloggedinGroup; public static String getUnloggedinGroup;
public static String unRegisteredGroup; public static String unRegisteredGroup;
public static String getRegisteredGroup; public static String getRegisteredGroup;
public static String rakamakUsers;
public static String rakamakUsersIp;
public static String defaultWorld; public static String defaultWorld;
public static String crazyloginFileName; public static String crazyloginFileName;
public static int getSessionTimeout; public static int getSessionTimeout;
@ -83,10 +80,6 @@ public final class Settings {
protectInventoryBeforeLogInEnabled = load(RestrictionSettings.PROTECT_INVENTORY_BEFORE_LOGIN); protectInventoryBeforeLogInEnabled = load(RestrictionSettings.PROTECT_INVENTORY_BEFORE_LOGIN);
isStopEnabled = configFile.getBoolean("Security.SQLProblem.stopServer", true); isStopEnabled = configFile.getBoolean("Security.SQLProblem.stopServer", true);
reloadSupport = configFile.getBoolean("Security.ReloadCommand.useReloadCommandSupport", true); reloadSupport = configFile.getBoolean("Security.ReloadCommand.useReloadCommandSupport", true);
rakamakUsers = configFile.getString("Converter.Rakamak.fileName", "users.rak");
rakamakUsersIp = configFile.getString("Converter.Rakamak.ipFileName", "UsersIp.rak");
rakamakUseIp = configFile.getBoolean("Converter.Rakamak.useIp", false);
removePassword = configFile.getBoolean("Security.console.removePassword", true); removePassword = configFile.getBoolean("Security.console.removePassword", true);
maxLoginTry = configFile.getInt("Security.captcha.maxLoginTry", 5); maxLoginTry = configFile.getInt("Security.captcha.maxLoginTry", 5);
captchaLength = configFile.getInt("Security.captcha.captchaLength", 5); captchaLength = configFile.getInt("Security.captcha.captchaLength", 5);

View File

@ -69,7 +69,7 @@ public final class MigrationService {
try { try {
SQLite sqlite = new SQLite(settings); SQLite sqlite = new SQLite(settings);
ForceFlatToSqlite converter = new ForceFlatToSqlite(flatFile, sqlite); ForceFlatToSqlite converter = new ForceFlatToSqlite(flatFile, sqlite);
converter.execute(null); converter.run();
settings.setProperty(DatabaseSettings.BACKEND, DataSourceType.SQLITE); settings.setProperty(DatabaseSettings.BACKEND, DataSourceType.SQLITE);
settings.save(); settings.save();
return sqlite; return sqlite;

View File

@ -54,7 +54,7 @@ public class ForceFlatToSqliteTest {
ForceFlatToSqlite converter = new ForceFlatToSqlite(flatFile, dataSource); ForceFlatToSqlite converter = new ForceFlatToSqlite(flatFile, dataSource);
// when // when
converter.execute(null); converter.run();
// then // then
ArgumentCaptor<PlayerAuth> authCaptor = ArgumentCaptor.forClass(PlayerAuth.class); ArgumentCaptor<PlayerAuth> authCaptor = ArgumentCaptor.forClass(PlayerAuth.class);

View File

@ -106,7 +106,11 @@ public final class ListenerConsistencyTest {
// Exclude any methods with "$" in it: jacoco creates a "$jacocoInit" method we want to ignore, and // Exclude any methods with "$" in it: jacoco creates a "$jacocoInit" method we want to ignore, and
// methods like "access$000" are created by the compiler when a private member is being accessed by an inner // methods like "access$000" are created by the compiler when a private member is being accessed by an inner
// class, which is not of interest for us // class, which is not of interest for us
return !Modifier.isPrivate(method.getModifiers()) && !method.getName().contains("$"); if (Modifier.isPrivate(method.getModifiers()) || method.getName().contains("$")) {
return false;
}
// Skip reload() method (implementation of Reloadable interface)
return !"reload".equals(method.getName()) || method.getParameterTypes().length > 0;
} }
} }