From f2d7fe908eed910ff19aa3eb9ca9a86685956e57 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 6 Aug 2016 23:20:30 +0200 Subject: [PATCH 1/2] #806 Create period cleanup task and implement cleanup for session records --- src/main/java/fr/xephi/authme/AuthMe.java | 7 +++++ .../fr/xephi/authme/cache/SessionManager.java | 16 +++++++++++- .../authme/initialization/HasCleanup.java | 14 ++++++++++ .../fr/xephi/authme/task/CleanupTask.java | 26 +++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 src/main/java/fr/xephi/authme/initialization/HasCleanup.java create mode 100644 src/main/java/fr/xephi/authme/task/CleanupTask.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index c1f999162..abdc39023 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -45,6 +45,7 @@ import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SettingsFieldRetriever; import fr.xephi.authme.settings.propertymap.PropertyMap; +import fr.xephi.authme.task.CleanupTask; import fr.xephi.authme.task.purge.PurgeService; import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.FileUtils; @@ -80,6 +81,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import static fr.xephi.authme.settings.properties.EmailSettings.RECALL_PLAYERS; +import static fr.xephi.authme.util.BukkitService.TICKS_PER_MINUTE; /** * The AuthMe main class. @@ -271,6 +273,11 @@ public class AuthMe extends JavaPlugin { // Purge on start if enabled PurgeService purgeService = injector.getSingleton(PurgeService.class); purgeService.runAutoPurge(); + + // Schedule clean up task + final int cleanupInterval = 5 * TICKS_PER_MINUTE; + CleanupTask cleanupTask = injector.getSingleton(CleanupTask.class); + cleanupTask.runTaskTimerAsynchronously(this, cleanupInterval, cleanupInterval); } protected void instantiateServices(Injector injector) { diff --git a/src/main/java/fr/xephi/authme/cache/SessionManager.java b/src/main/java/fr/xephi/authme/cache/SessionManager.java index fab1e1cfd..e01eddb08 100644 --- a/src/main/java/fr/xephi/authme/cache/SessionManager.java +++ b/src/main/java/fr/xephi/authme/cache/SessionManager.java @@ -1,10 +1,12 @@ package fr.xephi.authme.cache; +import fr.xephi.authme.initialization.HasCleanup; import fr.xephi.authme.initialization.SettingsDependent; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.PluginSettings; import javax.inject.Inject; +import java.util.Iterator; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -12,7 +14,7 @@ import java.util.concurrent.ConcurrentHashMap; * Manages sessions, allowing players to be automatically logged in if they join again * within a configurable amount of time. */ -public class SessionManager implements SettingsDependent { +public class SessionManager implements SettingsDependent, HasCleanup { private static final int MINUTE_IN_MILLIS = 60_000; // Player -> expiration of session in milliseconds @@ -74,4 +76,16 @@ public class SessionManager implements SettingsDependent { sessions.clear(); } } + + @Override + public void performCleanup() { + final long currentTime = System.currentTimeMillis(); + Iterator> iterator = sessions.entrySet().iterator(); + while (iterator.hasNext()) { + Map.Entry entry = iterator.next(); + if (entry.getValue() < currentTime) { + iterator.remove(); + } + } + } } diff --git a/src/main/java/fr/xephi/authme/initialization/HasCleanup.java b/src/main/java/fr/xephi/authme/initialization/HasCleanup.java new file mode 100644 index 000000000..db047d840 --- /dev/null +++ b/src/main/java/fr/xephi/authme/initialization/HasCleanup.java @@ -0,0 +1,14 @@ +package fr.xephi.authme.initialization; + +/** + * Common interface for types which have data that becomes outdated + * and that can be cleaned up periodically. + */ +public interface HasCleanup { + + /** + * Performs the cleanup action. + */ + void performCleanup(); + +} diff --git a/src/main/java/fr/xephi/authme/task/CleanupTask.java b/src/main/java/fr/xephi/authme/task/CleanupTask.java new file mode 100644 index 000000000..1a5bbdd65 --- /dev/null +++ b/src/main/java/fr/xephi/authme/task/CleanupTask.java @@ -0,0 +1,26 @@ +package fr.xephi.authme.task; + +import ch.jalu.injector.Injector; +import fr.xephi.authme.initialization.HasCleanup; +import org.bukkit.scheduler.BukkitRunnable; + +import javax.inject.Inject; + +/** + * Task run periodically to invoke the cleanup task on services. + */ +public class CleanupTask extends BukkitRunnable { + + @Inject + private Injector injector; + + CleanupTask() { + } + + @Override + public void run() { + for (HasCleanup service : injector.retrieveAllOfType(HasCleanup.class)) { + service.performCleanup(); + } + } +} From b4ea396d08f4cf9f77ad3fa5c6c831c1a9606f6d Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 7 Aug 2016 10:31:33 +0200 Subject: [PATCH 2/2] #806 Add tests and avoid sessions cleanup if they're disabled --- .../fr/xephi/authme/cache/SessionManager.java | 5 ++ .../authme/cache/SessionManagerTest.java | 39 ++++++++++++++++ .../fr/xephi/authme/task/CleanupTaskTest.java | 46 +++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 src/test/java/fr/xephi/authme/task/CleanupTaskTest.java diff --git a/src/main/java/fr/xephi/authme/cache/SessionManager.java b/src/main/java/fr/xephi/authme/cache/SessionManager.java index e01eddb08..95b6d8db5 100644 --- a/src/main/java/fr/xephi/authme/cache/SessionManager.java +++ b/src/main/java/fr/xephi/authme/cache/SessionManager.java @@ -1,5 +1,6 @@ package fr.xephi.authme.cache; +import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.initialization.HasCleanup; import fr.xephi.authme.initialization.SettingsDependent; import fr.xephi.authme.settings.Settings; @@ -74,11 +75,15 @@ public class SessionManager implements SettingsDependent, HasCleanup { // With this reload, the sessions feature has just been disabled, so clear all stored sessions if (oldEnabled && !enabled) { sessions.clear(); + ConsoleLogger.fine("Sessions disabled: cleared all sessions"); } } @Override public void performCleanup() { + if (!enabled) { + return; + } final long currentTime = System.currentTimeMillis(); Iterator> iterator = sessions.entrySet().iterator(); while (iterator.hasNext()) { diff --git a/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java b/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java index aa3ae5505..cfe8b2e89 100644 --- a/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java +++ b/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java @@ -9,7 +9,9 @@ import org.mockito.runners.MockitoJUnitRunner; import java.util.Map; +import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.anEmptyMap; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; @@ -122,6 +124,43 @@ public class SessionManagerTest { assertThat(getSessionsMap(manager), anEmptyMap()); } + @Test + public void shouldPerformCleanup() { + // given + Settings settings = mockSettings(true, 1); + SessionManager manager = new SessionManager(settings); + Map sessions = getSessionsMap(manager); + sessions.put("somebody", System.currentTimeMillis() - 123L); + sessions.put("someone", System.currentTimeMillis() + 4040L); + sessions.put("anyone", System.currentTimeMillis() - 1000L); + sessions.put("everyone", System.currentTimeMillis() + 60000L); + + // when + manager.performCleanup(); + + // then + assertThat(sessions, aMapWithSize(2)); + assertThat(sessions.keySet(), containsInAnyOrder("someone", "everyone")); + } + + @Test + public void shouldNotPerformCleanup() { + // given + Settings settings = mockSettings(false, 1); + SessionManager manager = new SessionManager(settings); + Map sessions = getSessionsMap(manager); + sessions.put("somebody", System.currentTimeMillis() - 123L); + sessions.put("someone", System.currentTimeMillis() + 4040L); + sessions.put("anyone", System.currentTimeMillis() - 1000L); + sessions.put("everyone", System.currentTimeMillis() + 60000L); + + // when + manager.performCleanup(); + + // then + assertThat(sessions, aMapWithSize(4)); // map not changed -> no cleanup performed + } + @SuppressWarnings("unchecked") private static Map getSessionsMap(SessionManager manager) { return (Map) ReflectionTestUtils diff --git a/src/test/java/fr/xephi/authme/task/CleanupTaskTest.java b/src/test/java/fr/xephi/authme/task/CleanupTaskTest.java new file mode 100644 index 000000000..bb3bf37d9 --- /dev/null +++ b/src/test/java/fr/xephi/authme/task/CleanupTaskTest.java @@ -0,0 +1,46 @@ +package fr.xephi.authme.task; + +import ch.jalu.injector.Injector; +import fr.xephi.authme.initialization.HasCleanup; +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.List; + +import static java.util.Arrays.asList; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link CleanupTask}. + */ +@RunWith(MockitoJUnitRunner.class) +public class CleanupTaskTest { + + @InjectMocks + private CleanupTask cleanupTask; + + @Mock + private Injector injector; + + @Test + public void shouldPerformCleanup() { + // given + List services = asList(mock(HasCleanup.class), mock(HasCleanup.class), mock(HasCleanup.class)); + given(injector.retrieveAllOfType(HasCleanup.class)).willReturn(services); + + // when + cleanupTask.run(); + + // then + verify(services.get(0)).performCleanup(); + verify(services.get(1)).performCleanup(); + verify(services.get(2)).performCleanup(); + verify(injector, only()).retrieveAllOfType(HasCleanup.class); + } +}