Use more specific query to get logged in players without email

- Reduces the amount of data returned from the DB and the work required to build objects
This commit is contained in:
ljacqu 2017-04-18 22:30:54 +02:00
parent 2f7ebc0ecb
commit 04ca36fe53
10 changed files with 58 additions and 76 deletions

View File

@ -2,7 +2,6 @@ package fr.xephi.authme.command.executable.authme;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.command.ExecutableCommand;
import fr.xephi.authme.data.auth.PlayerAuth;
import fr.xephi.authme.data.auth.PlayerCache;
import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.message.MessageKey;
@ -65,16 +64,13 @@ public class ChangePasswordAdminCommand implements ExecutableCommand {
* @param sender the sender initiating the password change
*/
private void changePassword(String nameLowercase, String password, CommandSender sender) {
PlayerAuth auth = getAuth(nameLowercase);
if (auth == null) {
if (!isNameRegistered(nameLowercase)) {
commonService.send(sender, MessageKey.UNKNOWN_USER);
return;
}
HashedPassword hashedPassword = passwordSecurity.computeHash(password, nameLowercase);
auth.setPassword(hashedPassword);
if (dataSource.updatePassword(auth)) {
if (dataSource.updatePassword(nameLowercase, hashedPassword)) {
commonService.send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
ConsoleLogger.info(sender.getName() + " changed password of " + nameLowercase);
} else {
@ -82,12 +78,7 @@ public class ChangePasswordAdminCommand implements ExecutableCommand {
}
}
private PlayerAuth getAuth(String nameLowercase) {
if (playerCache.isAuthenticated(nameLowercase)) {
return playerCache.getAuth(nameLowercase);
} else if (dataSource.isAuthAvailable(nameLowercase)) {
return dataSource.getAuth(nameLowercase);
}
return null;
private boolean isNameRegistered(String nameLowercase) {
return playerCache.isAuthenticated(nameLowercase) || dataSource.isAuthAvailable(nameLowercase);
}
}

View File

@ -54,7 +54,6 @@ class DataStatistics implements DebugSection {
private void outputDatabaseStats(CommandSender sender) {
sender.sendMessage("Total players in DB: " + dataSource.getAccountsRegistered());
sender.sendMessage("Total marked as logged in in DB: " + dataSource.getLoggedPlayers().size());
if (dataSource instanceof CacheDataSource) {
CacheDataSource cacheDataSource = (CacheDataSource) this.dataSource;
sender.sendMessage("Cached PlayerAuth objects: " + cacheDataSource.getCachedAuths().size());

View File

@ -11,14 +11,15 @@ import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.data.auth.PlayerAuth;
import fr.xephi.authme.data.auth.PlayerCache;
import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.util.Utils;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
public class CacheDataSource implements DataSource {
@ -240,7 +241,10 @@ public class CacheDataSource implements DataSource {
}
@Override
public List<PlayerAuth> getLoggedPlayers() {
return new ArrayList<>(playerCache.getCache().values());
public List<String> getLoggedPlayersWithEmptyMail() {
return playerCache.getCache().values().stream()
.filter(auth -> Utils.isEmailEmpty(auth.getEmail()))
.map(PlayerAuth::getRealName)
.collect(Collectors.toList());
}
}

View File

@ -167,11 +167,11 @@ public interface DataSource extends Reloadable {
void purgeLogged();
/**
* Return all players which are logged in.
* Return all players which are logged in and whose email is not set.
*
* @return All logged in players
* @return logged in players with no email
*/
List<PlayerAuth> getLoggedPlayers();
List<String> getLoggedPlayersWithEmptyMail();
/**
* Return the number of registered accounts.

View File

@ -423,7 +423,7 @@ public class FlatFile implements DataSource {
}
@Override
public List<PlayerAuth> getLoggedPlayers() {
public List<String> getLoggedPlayersWithEmptyMail() {
throw new UnsupportedOperationException("Flat file no longer supported");
}

View File

@ -927,33 +927,20 @@ public class MySQL implements DataSource {
}
@Override
public List<PlayerAuth> getLoggedPlayers() {
List<PlayerAuth> auths = new ArrayList<>();
String sql = "SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;";
public List<String> getLoggedPlayersWithEmptyMail() {
List<String> players = new ArrayList<>();
String sql = "SELECT " + col.REAL_NAME + " FROM " + tableName + " WHERE " + col.IS_LOGGED + " = 1"
+ " AND (" + col.EMAIL + " = 'your@email.com' OR " + col.EMAIL + " IS NULL);";
try (Connection con = getConnection();
Statement st = con.createStatement();
ResultSet rs = st.executeQuery(sql)) {
while (rs.next()) {
PlayerAuth pAuth = buildAuthFromResultSet(rs);
if (hashAlgorithm == HashAlgorithm.XFBCRYPT) {
try (PreparedStatement pst = con.prepareStatement("SELECT data FROM xf_user_authenticate WHERE " + col.ID + "=?;")) {
int id = rs.getInt(col.ID);
pst.setInt(1, id);
ResultSet rs2 = pst.executeQuery();
if (rs2.next()) {
Blob blob = rs2.getBlob("data");
byte[] bytes = blob.getBytes(1, (int) blob.length());
pAuth.setPassword(new HashedPassword(XfBCrypt.getHashFromBlob(bytes)));
}
rs2.close();
}
}
auths.add(pAuth);
players.add(rs.getString(1));
}
} catch (SQLException ex) {
logSqlException(ex);
}
return auths;
return players;
}
private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException {

View File

@ -591,18 +591,18 @@ public class SQLite implements DataSource {
}
@Override
public List<PlayerAuth> getLoggedPlayers() {
List<PlayerAuth> auths = new ArrayList<>();
String sql = "SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;";
try (PreparedStatement pst = con.prepareStatement(sql); ResultSet rs = pst.executeQuery()) {
public List<String> getLoggedPlayersWithEmptyMail() {
List<String> players = new ArrayList<>();
String sql = "SELECT " + col.REAL_NAME + " FROM " + tableName + " WHERE " + col.IS_LOGGED + " = 1"
+ " AND (" + col.EMAIL + " = 'your@email.com' OR " + col.EMAIL + " IS NULL);";
try (Statement st = con.createStatement(); ResultSet rs = st.executeQuery(sql)) {
while (rs.next()) {
PlayerAuth auth = buildAuthFromResultSet(rs);
auths.add(auth);
players.add(rs.getString(1));
}
} catch (SQLException ex) {
logSqlException(ex);
}
return auths;
return players;
}
private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException {

View File

@ -3,7 +3,6 @@ package fr.xephi.authme.initialization;
import ch.jalu.injector.exceptions.InjectorReflectionException;
import fr.xephi.authme.AuthMe;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.data.auth.PlayerAuth;
import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.message.Messages;
@ -18,7 +17,6 @@ import fr.xephi.authme.settings.properties.DatabaseSettings;
import fr.xephi.authme.settings.properties.EmailSettings;
import fr.xephi.authme.settings.properties.PluginSettings;
import fr.xephi.authme.settings.properties.SecuritySettings;
import fr.xephi.authme.util.Utils;
import org.apache.logging.log4j.LogManager;
import org.bstats.Metrics;
import org.bukkit.Bukkit;
@ -110,13 +108,10 @@ public class OnStartupTasks {
bukkitService.runTaskTimerAsynchronously(new Runnable() {
@Override
public void run() {
for (PlayerAuth auth : dataSource.getLoggedPlayers()) {
String email = auth.getEmail();
if (Utils.isEmailEmpty(email)) {
Player player = bukkitService.getPlayerExact(auth.getRealName());
if (player != null) {
messages.send(player, MessageKey.ADD_EMAIL_MESSAGE);
}
for (String playerWithoutMail : dataSource.getLoggedPlayersWithEmptyMail()) {
Player player = bukkitService.getPlayerExact(playerWithoutMail);
if (player != null) {
messages.send(player, MessageKey.ADD_EMAIL_MESSAGE);
}
}
}

View File

@ -101,14 +101,11 @@ public class ChangePasswordAdminCommandTest {
CommandSender sender = mock(CommandSender.class);
String player = "my_user12";
String password = "passPass";
PlayerAuth auth = mock(PlayerAuth.class);
given(playerCache.isAuthenticated(player)).willReturn(true);
given(playerCache.getAuth(player)).willReturn(auth);
HashedPassword hashedPassword = mock(HashedPassword.class);
given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword);
given(dataSource.updatePassword(auth)).willReturn(true);
given(dataSource.updatePassword(player, hashedPassword)).willReturn(true);
given(validationService.validatePassword(password, player)).willReturn(new ValidationResult());
// when
@ -119,8 +116,7 @@ public class ChangePasswordAdminCommandTest {
verify(validationService).validatePassword(password, player);
verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
verify(passwordSecurity).computeHash(password, player);
verify(auth).setPassword(hashedPassword);
verify(dataSource).updatePassword(auth);
verify(dataSource).updatePassword(player, hashedPassword);
}
@Test
@ -129,15 +125,13 @@ public class ChangePasswordAdminCommandTest {
CommandSender sender = mock(CommandSender.class);
String player = "my_user12";
String password = "passPass";
PlayerAuth auth = mock(PlayerAuth.class);
given(playerCache.isAuthenticated(player)).willReturn(false);
given(dataSource.isAuthAvailable(player)).willReturn(true);
given(dataSource.getAuth(player)).willReturn(auth);
given(dataSource.updatePassword(auth)).willReturn(true);
given(validationService.validatePassword(password, player)).willReturn(new ValidationResult());
HashedPassword hashedPassword = mock(HashedPassword.class);
given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword);
given(dataSource.updatePassword(player, hashedPassword)).willReturn(true);
// when
command.executeCommand(sender, Arrays.asList(player, password));
@ -147,8 +141,7 @@ public class ChangePasswordAdminCommandTest {
verify(validationService).validatePassword(password, player);
verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
verify(passwordSecurity).computeHash(password, player);
verify(auth).setPassword(hashedPassword);
verify(dataSource).updatePassword(auth);
verify(dataSource).updatePassword(player, hashedPassword);
}
@Test
@ -157,14 +150,12 @@ public class ChangePasswordAdminCommandTest {
CommandSender sender = mock(CommandSender.class);
String player = "my_user12";
String password = "passPass";
PlayerAuth auth = mock(PlayerAuth.class);
given(playerCache.isAuthenticated(player)).willReturn(true);
given(playerCache.getAuth(player)).willReturn(auth);
given(validationService.validatePassword(password, player)).willReturn(new ValidationResult());
HashedPassword hashedPassword = mock(HashedPassword.class);
given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword);
given(dataSource.updatePassword(auth)).willReturn(false);
given(dataSource.updatePassword(player, hashedPassword)).willReturn(false);
// when
command.executeCommand(sender, Arrays.asList(player, password));
@ -174,8 +165,7 @@ public class ChangePasswordAdminCommandTest {
verify(validationService).validatePassword(password, player);
verify(service).send(sender, MessageKey.ERROR);
verify(passwordSecurity).computeHash(password, player);
verify(auth).setPassword(hashedPassword);
verify(dataSource).updatePassword(auth);
verify(dataSource).updatePassword(player, hashedPassword);
}
}

View File

@ -348,7 +348,8 @@ public abstract class AbstractDataSourceIntegrationTest {
public void shouldPerformOperationsOnIsLoggedColumnSuccessfully() {
DataSource dataSource = getDataSource();
// on startup no one should be marked as logged
assertThat(dataSource.getLoggedPlayers(), empty());
assertThat(dataSource.isLogged("user"), equalTo(false));
assertThat(dataSource.isLogged("bobby"), equalTo(false));
// Mark user as logged
dataSource.setLogged("user");
@ -361,15 +362,16 @@ public abstract class AbstractDataSourceIntegrationTest {
// Set bobby logged and unlog user
dataSource.setLogged("bobby");
dataSource.setUnlogged("user");
assertThat(dataSource.getLoggedPlayers(),
contains(hasAuthBasicData("bobby", "Bobby", "your@email.com", "123.45.67.89")));
assertThat(dataSource.isLogged("user"), equalTo(false));
assertThat(dataSource.isLogged("bobby"), equalTo(true));
// Set both as logged (even if Bobby already is logged)
dataSource.setLogged("user");
dataSource.setLogged("bobby");
dataSource.purgeLogged();
assertThat(dataSource.isLogged("user"), equalTo(false));
assertThat(dataSource.getLoggedPlayers(), empty());
assertThat(dataSource.isLogged("bobby"), equalTo(false));
}
@Test
@ -400,4 +402,18 @@ public abstract class AbstractDataSourceIntegrationTest {
assertThat(email1.getValue(), equalTo("user@example.org"));
assertThat(email2, is(DataSourceResult.unknownPlayer()));
}
@Test
public void shouldGetLoggedPlayersWithoutEmail() {
// given
DataSource dataSource = getDataSource();
dataSource.setLogged("bobby");
dataSource.setLogged("user");
// when
List<String> loggedPlayersWithEmptyMail = dataSource.getLoggedPlayersWithEmptyMail();
// then
assertThat(loggedPlayersWithEmptyMail, contains("Bobby"));
}
}