#792 Include registration date into purging logic

- Take MAX(reg_date, login_date) as timestamp to compare against
- Remove the second "all" parameter to include entries with 0 registration date -> we expect registration date to always be set to the current date, so the parameter becomes obsolete
This commit is contained in:
ljacqu 2017-10-15 18:29:01 +02:00
parent 1df5308e56
commit ea58e20c3d
12 changed files with 50 additions and 93 deletions

View File

@ -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();

View File

@ -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);
}
}

View File

@ -137,8 +137,8 @@ public class CacheDataSource implements DataSource {
}
@Override
public Set<String> getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) {
return source.getRecordsToPurge(until, includeEntriesWithLastLoginZero);
public Set<String> getRecordsToPurge(long until) {
return source.getRecordsToPurge(until);
}
@Override

View File

@ -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 <a href="https://github.com/AuthMe/AuthMeReloaded/issues/886">issue #886</a>
* @return The account names selected to purge
*/
Set<String> getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero);
Set<String> getRecordsToPurge(long until);
/**
* Purge the given players from the database.

View File

@ -189,7 +189,7 @@ public class FlatFile implements DataSource {
}
@Override
public Set<String> getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) {
public Set<String> getRecordsToPurge(long until) {
throw new UnsupportedOperationException("Flat file no longer supported");
}

View File

@ -407,12 +407,12 @@ public class MySQL implements DataSource {
}
@Override
public Set<String> getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) {
public Set<String> getRecordsToPurge(long until) {
Set<String> 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);

View File

@ -308,12 +308,12 @@ public class SQLite implements DataSource {
}
@Override
public Set<String> getRecordsToPurge(long until, boolean includeEntriesWithLastLoginZero) {
public Set<String> getRecordsToPurge(long until) {
Set<String> 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()) {

View File

@ -76,7 +76,7 @@ public class RakamakConverter implements Converter {
for (Entry<String, HashedPassword> 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);
}

View File

@ -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<String> toPurge = dataSource.getRecordsToPurge(until, includeEntriesWithLastLoginZero);
Set<String> toPurge = dataSource.getRecordsToPurge(until);
if (Utils.isCollectionEmpty(toPurge)) {
logAndSendMessage(sender, "No players to purge");
return;

View File

@ -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<Long> 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<Long> 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)));
}

View File

@ -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<String> records1 = dataSource.getRecordsToPurge(1450000000, true);
Set<String> records2 = dataSource.getRecordsToPurge(1460000000, false);
Set<String> records1 = dataSource.getRecordsToPurge(1_450_000_000);
Set<String> 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

View File

@ -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<String> 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<Long> 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<String> 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);
}