diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index b7474222e..5f4935169 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -78,7 +78,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Date; import java.util.List; -import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_ACCOUNT; @@ -118,9 +117,8 @@ public class AuthMe extends JavaPlugin { private AuthMeServiceInitializer initializer; /* - * Private instances (sessions, mail, and ProtocolLib) + * Private instances (mail and ProtocolLib) */ - private final ConcurrentHashMap sessions = new ConcurrentHashMap<>(); private SendMailSSL mail; private AuthMeInventoryPacketAdapter inventoryProtector; private AuthMeTabCompletePacketAdapter tabComplete; @@ -708,15 +706,6 @@ public class AuthMe extends JavaPlugin { return commandHandler.processCommand(sender, commandLabel, args); } - /** - * Get all current player sessions. - * - * @return A concurrent hashmap containing the sessions. - */ - public ConcurrentHashMap getSessions() { - return this.sessions; - } - /** * Get the mailing instance. * diff --git a/src/main/java/fr/xephi/authme/cache/SessionManager.java b/src/main/java/fr/xephi/authme/cache/SessionManager.java new file mode 100644 index 000000000..0c03dc0fc --- /dev/null +++ b/src/main/java/fr/xephi/authme/cache/SessionManager.java @@ -0,0 +1,75 @@ +package fr.xephi.authme.cache; + +import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.PluginSettings; +import org.bukkit.scheduler.BukkitTask; + +import javax.inject.Inject; +import java.util.concurrent.ConcurrentHashMap; + +public class SessionManager implements SettingsDependent { + + private final ConcurrentHashMap sessions = new ConcurrentHashMap<>(); + + private boolean enabled; + private int sessionTimeout; + + @Inject + SessionManager(NewSetting settings) { + loadSettings(settings); + } + + /** + * Check if a session for a player is currently being cached. + * + * @param name The name to check. + * @return True if a session is found. + */ + public boolean hasSession(String name) { + return sessions.containsKey(name); + } + + /** + * Add a player session to the cache. + * + * @param name The name of the player. + * @param task The task to run. + */ + public void addSession(String name, BukkitTask task) { + if (!enabled || sessionTimeout == 0) { + return; + } + + this.sessions.put(name, task); + } + + /** + * Cancels a player's session. After the task is cancelled, it will be removed from + * the cache. + * + * @param name The name of the player who's session to cancel. + */ + public void cancelSession(String name) { + BukkitTask task = sessions.get(name); + if (task != null) { + task.cancel(); + removeSession(name); + } + } + + /** + * Remove a player's session from the cache. + * + * @param name The name of the player. + */ + public void removeSession(String name) { + this.sessions.remove(name); + } + + @Override + public void loadSettings(NewSetting settings) { + this.enabled = settings.getProperty(PluginSettings.SESSIONS_ENABLED); + this.sessionTimeout = settings.getProperty(PluginSettings.SESSIONS_TIMEOUT); + } +} diff --git a/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java b/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java index ff67b4ec8..2d05d5ce6 100644 --- a/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java +++ b/src/main/java/fr/xephi/authme/hooks/BungeeCordMessage.java @@ -5,6 +5,7 @@ import com.google.common.io.ByteStreams; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.cache.SessionManager; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; @@ -29,6 +30,9 @@ public class BungeeCordMessage implements PluginMessageListener { @Inject private PlayerCache playerCache; + @Inject + private SessionManager sessionManager; + @Inject private AuthMe plugin; @@ -61,9 +65,8 @@ public class BungeeCordMessage implements PluginMessageListener { playerCache.updatePlayer(auth); dataSource.setLogged(name); //START 03062016 sgdc3: should fix #731 but we need to recode this mess - if (plugin.getSessions().containsKey(name)) { - plugin.getSessions().get(name).cancel(); - plugin.getSessions().remove(name); + if (sessionManager.hasSession(name)) { + sessionManager.cancelSession(name); } //END diff --git a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java index 0eedd8022..709a9daca 100644 --- a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java +++ b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java @@ -2,6 +2,7 @@ package fr.xephi.authme.process.join; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.cache.SessionManager; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.cache.limbo.LimboCache; @@ -55,6 +56,9 @@ public class AsynchronousJoin implements AsynchronousProcess { @Inject private LimboCache limboCache; + @Inject + private SessionManager sessionManager; + @Inject private PluginHooks pluginHooks; @@ -135,9 +139,8 @@ public class AsynchronousJoin implements AsynchronousProcess { // Session logic if (service.getProperty(PluginSettings.SESSIONS_ENABLED) && (playerCache.isAuthenticated(name) || database.isLogged(name))) { - if (plugin.getSessions().containsKey(name)) { - plugin.getSessions().get(name).cancel(); - plugin.getSessions().remove(name); + if (sessionManager.hasSession(name)) { + sessionManager.cancelSession(name); } PlayerAuth auth = database.getAuth(name); database.setUnlogged(name); diff --git a/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java b/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java index 6fb337f97..385145677 100644 --- a/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java +++ b/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java @@ -4,6 +4,7 @@ import com.google.common.io.ByteArrayDataOutput; import com.google.common.io.ByteStreams; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.cache.SessionManager; import fr.xephi.authme.events.LogoutEvent; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.process.ProcessService; @@ -36,6 +37,9 @@ public class ProcessSynchronousPlayerLogout implements SynchronousProcess { @Inject private LimboPlayerTaskManager limboPlayerTaskManager; + @Inject + private SessionManager sessionManager; + ProcessSynchronousPlayerLogout() { } @@ -58,9 +62,8 @@ public class ProcessSynchronousPlayerLogout implements SynchronousProcess { public void processSyncLogout(Player player) { final String name = player.getName().toLowerCase(); - if (plugin.getSessions().containsKey(name)) { - plugin.getSessions().get(name).cancel(); - plugin.getSessions().remove(name); + if (sessionManager.hasSession(name)) { + sessionManager.cancelSession(name); } if (service.getProperty(RestrictionSettings.PROTECT_INVENTORY_BEFORE_LOGIN)) { plugin.getInventoryProtector().sendBlankInventoryPacket(player); diff --git a/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java b/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java index 675a40ff1..a157287bf 100644 --- a/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java +++ b/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java @@ -1,6 +1,7 @@ package fr.xephi.authme.process.quit; import fr.xephi.authme.AuthMe; +import fr.xephi.authme.cache.SessionManager; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.cache.limbo.LimboCache; @@ -42,6 +43,9 @@ public class AsynchronousQuit implements AsynchronousProcess { @Inject private SyncProcessManager syncProcessManager; + @Inject + private SessionManager sessionManager; + AsynchronousQuit() { } @@ -82,23 +86,21 @@ public class AsynchronousQuit implements AsynchronousProcess { isOp = limbo.isOperator(); limboCache.deleteLimboPlayer(name); } - if (Settings.isSessionsEnabled && !isKick) { - if (Settings.getSessionTimeout != 0) { - if (plugin.isEnabled()) { - BukkitTask task = plugin.getServer().getScheduler().runTaskLaterAsynchronously(plugin, new Runnable() { + if (!isKick) { + if (plugin.isEnabled()) { + BukkitTask task = plugin.getServer().getScheduler().runTaskLaterAsynchronously(plugin, new Runnable() { - @Override - public void run() { - postLogout(name); - } + @Override + public void run() { + postLogout(name); + } - }, Settings.getSessionTimeout * TICKS_PER_MINUTE); + }, Settings.getSessionTimeout * TICKS_PER_MINUTE); - plugin.getSessions().put(name, task); - } else { - //plugin is disabled; we cannot schedule more tasks so run it directly here - postLogout(name); - } + sessionManager.addSession(name, task); + } else { + //plugin is disabled; we cannot schedule more tasks so run it directly here + postLogout(name); } } else { playerCache.removePlayer(name); @@ -117,6 +119,6 @@ public class AsynchronousQuit implements AsynchronousProcess { private void postLogout(String name) { PlayerCache.getInstance().removePlayer(name); database.setUnlogged(name); - plugin.getSessions().remove(name); + sessionManager.removeSession(name); } } diff --git a/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java b/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java new file mode 100644 index 000000000..6e03b3128 --- /dev/null +++ b/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java @@ -0,0 +1,98 @@ +package fr.xephi.authme.cache; + +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.PluginSettings; +import org.bukkit.scheduler.BukkitTask; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test for {@link SessionManager}. + */ +@RunWith(MockitoJUnitRunner.class) +public class SessionManagerTest { + + @Test + public void shouldHaveSession() { + // given + NewSetting settings = mockSettings(true, 10); + SessionManager manager = new SessionManager(settings); + String player = "playah"; + BukkitTask task = mock(BukkitTask.class); + + // when + manager.addSession(player, task); + + // then + assertThat(manager.hasSession(player), equalTo(true)); + } + + @Test + public void shouldNotHaveSession() { + // given + NewSetting settings = mockSettings(true, 10); + SessionManager manager = new SessionManager(settings); + String player = "playah"; + + // when/then + assertThat(manager.hasSession(player), equalTo(false)); + } + + @Test + public void shouldAddSession() { + // given + NewSetting settings = mockSettings(true, 10); + SessionManager manager = new SessionManager(settings); + String player = "playah"; + BukkitTask task = mock(BukkitTask.class); + + // when + manager.addSession(player, task); + + // then + assertThat(manager.hasSession(player), equalTo(true)); + } + + @Test + public void shouldNotAddSessionBecauseDisabled() { + // given + NewSetting settings = mockSettings(false, 10); + SessionManager manager = new SessionManager(settings); + String player = "playah"; + BukkitTask task = mock(BukkitTask.class); + + // when + manager.addSession(player, task); + + // then + assertThat(manager.hasSession(player), equalTo(false)); + } + + @Test + public void shouldNotAddSessionBecauseTimeoutIsZero() { + // given + NewSetting settings = mockSettings(true, 0); + SessionManager manager = new SessionManager(settings); + String player = "playah"; + BukkitTask task = mock(BukkitTask.class); + + // when + manager.addSession(player, task); + + // then + assertThat(manager.hasSession(player), equalTo(false)); + } + + private static NewSetting mockSettings(boolean isEnabled, int sessionTimeout) { + NewSetting settings = mock(NewSetting.class); + given(settings.getProperty(PluginSettings.SESSIONS_ENABLED)).willReturn(isEnabled); + given(settings.getProperty(PluginSettings.SESSIONS_TIMEOUT)).willReturn(sessionTimeout); + return settings; + } +}