diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 83d71027..8c96894b 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -320,7 +320,6 @@ public class CommandInitializer { .description("Purge old data") .detailedDescription("Purge old AuthMeReloaded data longer than the specified number of days ago.") .withArgument("days", "Number of days", false) - .withArgument("all", "Add 'all' at the end to also purge players with lastlogin = 0", true) .permission(AdminPermission.PURGE) .executableCommand(PurgeCommand.class) .register(); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java index 4a3e86cf..58b235ed 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java @@ -41,24 +41,12 @@ public class PurgeCommand implements ExecutableCommand { return; } - // If second param is available, check that it is equal to "all" - boolean includeLastLoginZeroEntries = false; - if (arguments.size() >= 2) { - if ("all".equals(arguments.get(1))) { - includeLastLoginZeroEntries = true; - } else { - sender.sendMessage("Purge process aborted; use '/authme purge " + days + " all' " - + "to include users with lastlogin = 0"); - return; - } - } - // Create a calender instance to determine the date Calendar calendar = Calendar.getInstance(); calendar.add(Calendar.DATE, -days); long until = calendar.getTimeInMillis(); // Run the purge - purgeService.runPurge(sender, until, includeLastLoginZeroEntries); + purgeService.runPurge(sender, until); } } diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index b7eb6a18..7ef4908a 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -137,8 +137,8 @@ public class CacheDataSource implements DataSource { } @Override - public Set getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) { - return source.getRecordsToPurge(until, includeEntriesWithLastLoginZero); + public Set getRecordsToPurge(long until) { + return source.getRecordsToPurge(until); } @Override diff --git a/src/main/java/fr/xephi/authme/datasource/DataSource.java b/src/main/java/fr/xephi/authme/datasource/DataSource.java index 14faaf39..d491a492 100644 --- a/src/main/java/fr/xephi/authme/datasource/DataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/DataSource.java @@ -74,11 +74,9 @@ public interface DataSource extends Reloadable { * Get all records in the database whose last login was before the given time. * * @param until The minimum last login - * @param includeEntriesWithLastLoginZero Whether entries with lastlogin = 0 should be included or not, - * see issue #886 * @return The account names selected to purge */ - Set getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero); + Set getRecordsToPurge(long until); /** * Purge the given players from the database. diff --git a/src/main/java/fr/xephi/authme/datasource/FlatFile.java b/src/main/java/fr/xephi/authme/datasource/FlatFile.java index 298f6d19..e439f912 100644 --- a/src/main/java/fr/xephi/authme/datasource/FlatFile.java +++ b/src/main/java/fr/xephi/authme/datasource/FlatFile.java @@ -189,7 +189,7 @@ public class FlatFile implements DataSource { } @Override - public Set getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) { + public Set getRecordsToPurge(long until) { throw new UnsupportedOperationException("Flat file no longer supported"); } diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index c96f953b..45dc9ab5 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -407,12 +407,12 @@ public class MySQL implements DataSource { } @Override - public Set getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) { + public Set getRecordsToPurge(long until) { Set list = new HashSet<>(); - String select = "SELECT " + col.NAME + " FROM " + tableName + " WHERE " + col.LAST_LOGIN + " < ?"; - if (!includeEntriesWithLastLoginZero) { - select += " AND " + col.LAST_LOGIN + " <> 0"; - } + String select = "SELECT " + col.NAME + " FROM " + tableName + " WHERE GREATEST(" + + " COALESCE(" + col.LAST_LOGIN + ", 0)," + + " COALESCE(" + col.REGISTRATION_DATE + ", 0)" + + ") < ?;"; try (Connection con = getConnection(); PreparedStatement selectPst = con.prepareStatement(select)) { selectPst.setLong(1, until); diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index 17a427bd..aebf05e8 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -308,12 +308,12 @@ public class SQLite implements DataSource { } @Override - public Set getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) { + public Set getRecordsToPurge(long until) { Set list = new HashSet<>(); - String select = "SELECT " + col.NAME + " FROM " + tableName + " WHERE " + col.LAST_LOGIN + " < ?"; - if (!includeEntriesWithLastLoginZero) { - select += " AND " + col.LAST_LOGIN + " <> 0"; - } + String select = "SELECT " + col.NAME + " FROM " + tableName + " WHERE MAX(" + + " COALESCE(" + col.LAST_LOGIN + ", 0)," + + " COALESCE(" + col.REGISTRATION_DATE + ", 0)" + + ") < ?;"; try (PreparedStatement selectPst = con.prepareStatement(select)) { selectPst.setLong(1, until); try (ResultSet rs = selectPst.executeQuery()) { diff --git a/src/main/java/fr/xephi/authme/datasource/converter/RakamakConverter.java b/src/main/java/fr/xephi/authme/datasource/converter/RakamakConverter.java index 444a86bb..2c9e16b8 100644 --- a/src/main/java/fr/xephi/authme/datasource/converter/RakamakConverter.java +++ b/src/main/java/fr/xephi/authme/datasource/converter/RakamakConverter.java @@ -76,7 +76,7 @@ public class RakamakConverter implements Converter { for (Entry m : playerPassword.entrySet()) { String playerName = m.getKey(); HashedPassword psw = playerPassword.get(playerName); - String ip = useIp ? playerIp.get(playerName) : "127.0.0.1"; + String ip = playerIp.get(playerName); PlayerAuth auth = PlayerAuth.builder() .name(playerName) .realName(playerName) @@ -86,7 +86,7 @@ public class RakamakConverter implements Converter { database.saveAuth(auth); database.updateSession(auth); } - Utils.logAndSendMessage(sender, "Rakamak database has been imported correctly"); + Utils.logAndSendMessage(sender, "Rakamak database has been imported successfully"); } catch (IOException ex) { ConsoleLogger.logException("Can't open the rakamak database file! Does it exist?", ex); } diff --git a/src/main/java/fr/xephi/authme/task/purge/PurgeService.java b/src/main/java/fr/xephi/authme/task/purge/PurgeService.java index fe6d2fbf..c9ba7251 100644 --- a/src/main/java/fr/xephi/authme/task/purge/PurgeService.java +++ b/src/main/java/fr/xephi/authme/task/purge/PurgeService.java @@ -60,7 +60,7 @@ public class PurgeService { calendar.add(Calendar.DATE, -daysBeforePurge); long until = calendar.getTimeInMillis(); - runPurge(null, until, false); + runPurge(null, until); } /** @@ -69,11 +69,10 @@ public class PurgeService { * * @param sender Sender running the command * @param until The last login threshold in milliseconds - * @param includeEntriesWithLastLoginZero True to also purge players with lastlogin = 0, false otherwise */ - public void runPurge(CommandSender sender, long until, boolean includeEntriesWithLastLoginZero) { + public void runPurge(CommandSender sender, long until) { //todo: note this should may run async because it may executes a SQL-Query - Set toPurge = dataSource.getRecordsToPurge(until, includeEntriesWithLastLoginZero); + Set toPurge = dataSource.getRecordsToPurge(until); if (Utils.isCollectionEmpty(toPurge)) { logAndSendMessage(sender, "No players to purge"); return; diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java index 08a1f634..2400cb1c 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/PurgeCommandTest.java @@ -9,7 +9,6 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import java.util.Arrays; import java.util.Calendar; import java.util.Collections; @@ -74,7 +73,7 @@ public class PurgeCommandTest { // then ArgumentCaptor captor = ArgumentCaptor.forClass(Long.class); - verify(purgeService).runPurge(eq(sender), captor.capture(), eq(false)); + verify(purgeService).runPurge(eq(sender), captor.capture()); // Check the timestamp with a certain tolerance int toleranceMillis = 100; @@ -83,41 +82,6 @@ public class PurgeCommandTest { assertIsCloseTo(captor.getValue(), calendar.getTimeInMillis(), toleranceMillis); } - @Test - public void shouldProcessCommandWithAllParameter() { - // given - String interval = "32"; - CommandSender sender = mock(CommandSender.class); - - // when - command.executeCommand(sender, Arrays.asList(interval, "all")); - - // then - ArgumentCaptor captor = ArgumentCaptor.forClass(Long.class); - verify(purgeService).runPurge(eq(sender), captor.capture(), eq(true)); - - // Check the timestamp with a certain tolerance - int toleranceMillis = 100; - Calendar calendar = Calendar.getInstance(); - calendar.add(Calendar.DATE, -Integer.valueOf(interval)); - assertIsCloseTo(captor.getValue(), calendar.getTimeInMillis(), toleranceMillis); - } - - @Test - public void shouldRejectCommandWithInvalidSecondParameter() { - // given - String interval = "80"; - CommandSender sender = mock(CommandSender.class); - - // when - command.executeCommand(sender, Arrays.asList(interval, "bogus")); - - // then - verify(sender).sendMessage( - argThat(containsString("Purge process aborted; use '/authme purge " + interval + " all'"))); - verifyZeroInteractions(purgeService); - } - private static void assertIsCloseTo(long value1, long value2, long tolerance) { assertThat(Math.abs(value1 - value2), not(greaterThan(tolerance))); } diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java index 2f3c010d..b452161c 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java @@ -2,13 +2,13 @@ package fr.xephi.authme.datasource; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.security.crypts.HashedPassword; -import org.junit.Ignore; import org.junit.Test; import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Stream; import static fr.xephi.authme.AuthMeMatchers.equalToHash; import static fr.xephi.authme.AuthMeMatchers.hasAuthBasicData; @@ -332,24 +332,35 @@ public abstract class AbstractDataSourceIntegrationTest { } @Test - @Ignore // TODO #792: Fix purging logic public void shouldGetRecordsToPurge() { // given DataSource dataSource = getDataSource(); - PlayerAuth auth = PlayerAuth.builder().name("potato").lastLogin(0L).build(); - dataSource.saveAuth(auth); - dataSource.updateSession(auth); - // 1453242857 -> user, 1449136800 -> bobby, 0 -> potato + // 1453242857 -> user, 1449136800 -> bobby + + PlayerAuth potato = PlayerAuth.builder().name("potato") + .registrationDate(0L).lastLogin(1_455_000_000L).build(); + PlayerAuth tomato = PlayerAuth.builder().name("tomato") + .registrationDate(1_457_000_000L).lastLogin(null).build(); + PlayerAuth lettuce = PlayerAuth.builder().name("Lettuce") + .registrationDate(1_400_000_000L).lastLogin(1_453_000_000L).build(); + PlayerAuth onion = PlayerAuth.builder().name("onion") + .registrationDate(1_200_000_000L).lastLogin(1_300_000_000L).build(); + Stream.of(potato, tomato, lettuce, onion).forEach(auth -> { + dataSource.saveAuth(auth); + dataSource.updateSession(auth); + }); // when - Set records1 = dataSource.getRecordsToPurge(1450000000, true); - Set records2 = dataSource.getRecordsToPurge(1460000000, false); + Set records1 = dataSource.getRecordsToPurge(1_450_000_000); + Set records2 = dataSource.getRecordsToPurge(1_460_000_000); // then - assertThat(records1, containsInAnyOrder("bobby", "potato")); - assertThat(records2, containsInAnyOrder("bobby", "user")); + assertThat(records1, containsInAnyOrder("bobby", "onion")); + assertThat(records2, containsInAnyOrder("bobby", "onion", "user", "tomato", "potato", "lettuce")); // check that the entry was not deleted because of running this command assertThat(dataSource.isAuthAvailable("bobby"), equalTo(true)); + assertThat(dataSource.isAuthAvailable("tomato"), equalTo(true)); + assertThat(dataSource.isAuthAvailable("Lettuce"), equalTo(true)); } @Test diff --git a/src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java b/src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java index 4d3923e6..f27e01ed 100644 --- a/src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java +++ b/src/test/java/fr/xephi/authme/task/purge/PurgeServiceTest.java @@ -99,14 +99,14 @@ public class PurgeServiceTest { given(settings.getProperty(PurgeSettings.USE_AUTO_PURGE)).willReturn(true); given(settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER)).willReturn(60); Set playerNames = newHashSet("alpha", "bravo", "charlie", "delta"); - given(dataSource.getRecordsToPurge(anyLong(), eq(false))).willReturn(playerNames); + given(dataSource.getRecordsToPurge(anyLong())).willReturn(playerNames); // when purgeService.runAutoPurge(); // then ArgumentCaptor captor = ArgumentCaptor.forClass(Long.class); - verify(dataSource).getRecordsToPurge(captor.capture(), eq(false)); + verify(dataSource).getRecordsToPurge(captor.capture()); assertCorrectPurgeTimestamp(captor.getValue(), 60); assertThat(Boolean.TRUE, equalTo( ReflectionTestUtils.getFieldValue(PurgeService.class, purgeService, "isPurging"))); @@ -117,15 +117,14 @@ public class PurgeServiceTest { public void shouldRecognizeNoPlayersToPurge() { // given final long delay = 123012301L; - final boolean includeLastLoginZeroEntries = true; - given(dataSource.getRecordsToPurge(delay, includeLastLoginZeroEntries)).willReturn(Collections.emptySet()); + given(dataSource.getRecordsToPurge(delay)).willReturn(Collections.emptySet()); CommandSender sender = mock(CommandSender.class); // when - purgeService.runPurge(sender, delay, includeLastLoginZeroEntries); + purgeService.runPurge(sender, delay); // then - verify(dataSource).getRecordsToPurge(delay, includeLastLoginZeroEntries); + verify(dataSource).getRecordsToPurge(delay); verify(dataSource, never()).purgeRecords(anyCollection()); verify(sender).sendMessage("No players to purge"); verifyZeroInteractions(bukkitService, permissionsManager); @@ -135,18 +134,17 @@ public class PurgeServiceTest { public void shouldRunPurge() { // given final long delay = 1809714L; - final boolean includeLastLoginZeroEntries = false; Set playerNames = newHashSet("charlie", "delta", "echo", "foxtrot"); - given(dataSource.getRecordsToPurge(delay, includeLastLoginZeroEntries)).willReturn(playerNames); + given(dataSource.getRecordsToPurge(delay)).willReturn(playerNames); Player sender = mock(Player.class); UUID uuid = UUID.randomUUID(); given(sender.getUniqueId()).willReturn(uuid); // when - purgeService.runPurge(sender, delay, includeLastLoginZeroEntries); + purgeService.runPurge(sender, delay); // then - verify(dataSource).getRecordsToPurge(delay, includeLastLoginZeroEntries); + verify(dataSource).getRecordsToPurge(delay); verifyScheduledPurgeTask(uuid, playerNames); }