Various minor changes

- AsynchronousLogin: call common permission methods through CommonService instead of PermissionsManager
- CommandManager: remove superfluous replacement of %p (handled by lazy tag replacer)
- Remove unused method in CommonService
- Create DebugSectionConsistencyTest
- SendMailSSL: Enable debug output if AuthMe log level is set to debug
- Add Utils#logAndSendMessage and replace existing, separate implementations
This commit is contained in:
ljacqu 2017-03-12 14:04:39 +01:00
parent 72acf28233
commit 10d8f00c92
13 changed files with 148 additions and 65 deletions

View File

@ -11,10 +11,10 @@ import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.service.CommonService;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.DatabaseSettings;
import fr.xephi.authme.util.Utils;
import org.bukkit.command.CommandSender;
import javax.inject.Inject;
import java.util.Collection;
import java.util.List;
/**
@ -44,8 +44,7 @@ public class ReloadCommand implements ExecutableCommand {
ConsoleLogger.setLoggingOptions(settings);
// We do not change database type for consistency issues, but we'll output a note in the logs
if (!settings.getProperty(DatabaseSettings.BACKEND).equals(dataSource.getType())) {
ConsoleLogger.info("Note: cannot change database type during /authme reload");
sender.sendMessage("Note: cannot change database type during /authme reload");
Utils.logAndSendMessage(sender, "Note: cannot change database type during /authme reload");
}
performReloadOnServices();
commonService.send(sender, MessageKey.CONFIG_RELOAD_SUCCESS);
@ -57,14 +56,10 @@ public class ReloadCommand implements ExecutableCommand {
}
private void performReloadOnServices() {
Collection<Reloadable> reloadables = injector.retrieveAllOfType(Reloadable.class);
for (Reloadable reloadable : reloadables) {
reloadable.reload();
}
injector.retrieveAllOfType(Reloadable.class)
.forEach(r -> r.reload());
Collection<SettingsDependent> settingsDependents = injector.retrieveAllOfType(SettingsDependent.class);
for (SettingsDependent dependent : settingsDependents) {
dependent.reload(settings);
}
injector.retrieveAllOfType(SettingsDependent.class)
.forEach(s -> s.reload(settings));
}
}

View File

@ -1,8 +1,10 @@
package fr.xephi.authme.mail;
import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.output.LogLevel;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.EmailSettings;
import fr.xephi.authme.settings.properties.PluginSettings;
import fr.xephi.authme.util.StringUtils;
import org.apache.commons.mail.EmailConstants;
import org.apache.commons.mail.EmailException;
@ -56,6 +58,9 @@ public class SendMailSSL {
email.setFrom(senderMail, senderName);
email.setSubject(settings.getProperty(EmailSettings.RECOVERY_MAIL_SUBJECT));
email.setAuthentication(settings.getProperty(EmailSettings.MAIL_ACCOUNT), mailPassword);
if (settings.getProperty(PluginSettings.LOG_LEVEL).includes(LogLevel.DEBUG)) {
email.setDebug(true);
}
setPropertiesForPort(email, port);
return email;

View File

@ -12,7 +12,6 @@ import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.events.AuthMeAsyncPreLoginEvent;
import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.permission.AdminPermission;
import fr.xephi.authme.permission.PermissionsManager;
import fr.xephi.authme.permission.PlayerPermission;
import fr.xephi.authme.permission.PlayerStatePermission;
import fr.xephi.authme.process.AsynchronousProcess;
@ -46,9 +45,6 @@ public class AsynchronousLogin implements AsynchronousProcess {
@Inject
private CommonService service;
@Inject
private PermissionsManager permissionsManager;
@Inject
private PlayerCache playerCache;
@ -300,10 +296,10 @@ public class AsynchronousLogin implements AsynchronousProcess {
for (Player onlinePlayer : bukkitService.getOnlinePlayers()) {
if (onlinePlayer.getName().equalsIgnoreCase(player.getName())
&& permissionsManager.hasPermission(onlinePlayer, PlayerPermission.SEE_OWN_ACCOUNTS)) {
&& service.hasPermission(onlinePlayer, PlayerPermission.SEE_OWN_ACCOUNTS)) {
service.send(onlinePlayer, MessageKey.ACCOUNTS_OWNED_SELF, Integer.toString(auths.size()));
onlinePlayer.sendMessage(message);
} else if (permissionsManager.hasPermission(onlinePlayer, AdminPermission.SEE_OTHER_ACCOUNTS)) {
} else if (service.hasPermission(onlinePlayer, AdminPermission.SEE_OTHER_ACCOUNTS)) {
service.send(onlinePlayer, MessageKey.ACCOUNTS_OWNED_OTHER,
player.getName(), Integer.toString(auths.size()));
onlinePlayer.sendMessage(message);
@ -323,7 +319,7 @@ public class AsynchronousLogin implements AsynchronousProcess {
boolean hasReachedMaxLoggedInPlayersForIp(Player player, String ip) {
// Do not perform the check if player has multiple accounts permission or if IP is localhost
if (service.getProperty(RestrictionSettings.MAX_LOGIN_PER_IP) <= 0
|| permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)
|| service.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)
|| "127.0.0.1".equalsIgnoreCase(ip)
|| "localhost".equalsIgnoreCase(ip)) {
return false;

View File

@ -65,16 +65,6 @@ public class CommonService {
messages.send(sender, key, replacements);
}
/**
* Retrieves a message.
*
* @param key the key of the message
* @return the message, split by line
*/
public String[] retrieveMessage(MessageKey key) {
return messages.retrieve(key);
}
/**
* Retrieves a message in one piece.
*
@ -102,6 +92,7 @@ public class CommonService {
* @param player the player to process
* @param group the group to add the player to
*/
// TODO ljacqu 20170304: Move this out of CommonService
public void setGroup(Player player, AuthGroupType group) {
authGroupHandler.setGroup(player, group);
}

View File

@ -74,7 +74,7 @@ public class CommandManager implements Reloadable {
private void executeCommands(Player player, List<Command> commands) {
for (Command command : commands) {
final String execution = command.getCommand().replace("%p", player.getName());
final String execution = command.getCommand();
if (Executor.CONSOLE.equals(command.getExecutor())) {
bukkitService.dispatchConsoleCommand(execution);
} else {

View File

@ -9,14 +9,13 @@ import fr.xephi.authme.settings.properties.PurgeSettings;
import fr.xephi.authme.util.Utils;
import org.bukkit.OfflinePlayer;
import org.bukkit.command.CommandSender;
import org.bukkit.command.ConsoleCommandSender;
import javax.inject.Inject;
import java.util.Calendar;
import java.util.Collection;
import java.util.Set;
// TODO: move into services. -sgdc3
import static fr.xephi.authme.util.Utils.logAndSendMessage;
/**
* Initiates purge tasks.
@ -119,12 +118,4 @@ public class PurgeService {
void executePurge(Collection<OfflinePlayer> players, Collection<String> names) {
purgeExecutor.executePurge(players, names);
}
private static void logAndSendMessage(CommandSender sender, String message) {
ConsoleLogger.info(message);
// Make sure sender is not console user, which will see the message from ConsoleLogger already
if (sender != null && !(sender instanceof ConsoleCommandSender)) {
sender.sendMessage(message);
}
}
}

View File

@ -1,6 +1,8 @@
package fr.xephi.authme.util;
import fr.xephi.authme.ConsoleLogger;
import org.bukkit.command.CommandSender;
import org.bukkit.command.ConsoleCommandSender;
import java.util.Collection;
import java.util.regex.Pattern;
@ -51,6 +53,22 @@ public final class Utils {
}
}
/**
* Sends a message to the given sender (null safe), and logs the message to the console.
* This method is aware that the command sender might be the console sender and avoids
* displaying the message twice in this case.
*
* @param sender the sender to inform
* @param message the message to log and send
*/
public static void logAndSendMessage(CommandSender sender, String message) {
ConsoleLogger.info(message);
// Make sure sender is not console user, which will see the message from ConsoleLogger already
if (sender != null && !(sender instanceof ConsoleCommandSender)) {
sender.sendMessage(message);
}
}
/**
* Null-safe way to check whether a collection is empty or not.
*

View File

@ -0,0 +1,51 @@
package fr.xephi.authme.command.executable.authme.debug;
import fr.xephi.authme.ClassCollector;
import org.junit.BeforeClass;
import org.junit.Test;
import java.lang.reflect.Modifier;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
/**
* Consistency tests for {@link DebugSection} implementors.
*/
public class DebugSectionConsistencyTest {
private static List<Class<?>> debugClasses;
@BeforeClass
public static void collectClasses() {
debugClasses = new ClassCollector("src/main/java", "fr/xephi/authme/command/executable/authme/debug")
.collectClasses();
}
@Test
public void shouldAllBePackagePrivate() {
for (Class<?> clazz : debugClasses) {
if (clazz != DebugCommand.class) {
assertThat(clazz + " should be package-private",
Modifier.isPublic(clazz.getModifiers()), equalTo(false));
}
}
}
@Test
public void shouldHaveDifferentSubcommandName() throws IllegalAccessException, InstantiationException {
Set<String> names = new HashSet<>();
for (Class<?> clazz : debugClasses) {
if (DebugSection.class.isAssignableFrom(clazz) && !clazz.isInterface()) {
DebugSection debugSection = (DebugSection) clazz.newInstance();
if (!names.add(debugSection.getName())) {
fail("Encountered name '" + debugSection.getName() + "' a second time in " + clazz);
}
}
}
}
}

View File

@ -4,8 +4,10 @@ import ch.jalu.injector.testing.BeforeInjecting;
import ch.jalu.injector.testing.DelayedInjectionRunner;
import ch.jalu.injector.testing.InjectDelayed;
import fr.xephi.authme.TestHelper;
import fr.xephi.authme.output.LogLevel;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.EmailSettings;
import fr.xephi.authme.settings.properties.PluginSettings;
import org.apache.commons.mail.EmailException;
import org.apache.commons.mail.HtmlEmail;
import org.junit.BeforeClass;
@ -49,6 +51,7 @@ public class SendMailSSLTest {
public void initFields() throws IOException {
given(settings.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("mail@example.org");
given(settings.getProperty(EmailSettings.MAIL_PASSWORD)).willReturn("pass1234");
given(settings.getProperty(PluginSettings.LOG_LEVEL)).willReturn(LogLevel.INFO);
}
@Test
@ -67,6 +70,7 @@ public class SendMailSSLTest {
given(settings.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn(senderAccount);
String senderName = "Server administration";
given(settings.getProperty(EmailSettings.MAIL_SENDER_NAME)).willReturn(senderName);
given(settings.getProperty(PluginSettings.LOG_LEVEL)).willReturn(LogLevel.DEBUG);
// when
HtmlEmail email = sendMailSSL.initializeMail("recipient@example.com");

View File

@ -6,10 +6,9 @@ import fr.xephi.authme.data.auth.PlayerCache;
import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.events.AuthMeAsyncPreLoginEvent;
import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.permission.PermissionsManager;
import fr.xephi.authme.permission.PlayerStatePermission;
import fr.xephi.authme.service.CommonService;
import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.service.CommonService;
import fr.xephi.authme.settings.properties.DatabaseSettings;
import fr.xephi.authme.settings.properties.HooksSettings;
import fr.xephi.authme.settings.properties.PluginSettings;
@ -61,8 +60,6 @@ public class AsynchronousLoginTest {
private LimboPlayerTaskManager limboPlayerTaskManager;
@Mock
private BukkitService bukkitService;
@Mock
private PermissionsManager permissionsManager;
@BeforeClass
public static void initLogger() {
@ -182,7 +179,7 @@ public class AsynchronousLoginTest {
// given
Player player = mockPlayer("Carl");
given(commonService.getProperty(RestrictionSettings.MAX_LOGIN_PER_IP)).willReturn(2);
given(permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(false);
given(commonService.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(false);
mockOnlinePlayersInBukkitService();
// when
@ -190,7 +187,7 @@ public class AsynchronousLoginTest {
// then
assertThat(result, equalTo(false));
verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verify(bukkitService).getOnlinePlayers();
}
@ -213,14 +210,14 @@ public class AsynchronousLoginTest {
// given
Player player = mockPlayer("Frank");
given(commonService.getProperty(RestrictionSettings.MAX_LOGIN_PER_IP)).willReturn(1);
given(permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(true);
given(commonService.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(true);
// when
boolean result = asynchronousLogin.hasReachedMaxLoggedInPlayersForIp(player, "127.0.0.4");
// then
assertThat(result, equalTo(false));
verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verifyZeroInteractions(bukkitService);
}
@ -229,7 +226,7 @@ public class AsynchronousLoginTest {
// given
Player player = mockPlayer("Ian");
given(commonService.getProperty(RestrictionSettings.MAX_LOGIN_PER_IP)).willReturn(2);
given(permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(false);
given(commonService.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(false);
mockOnlinePlayersInBukkitService();
// when
@ -237,7 +234,7 @@ public class AsynchronousLoginTest {
// then
assertThat(result, equalTo(true));
verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verify(bukkitService).getOnlinePlayers();
}

View File

@ -84,21 +84,6 @@ public class CommonServiceTest {
verify(messages).send(sender, key, replacements);
}
@Test
public void shouldRetrieveMessage() {
// given
MessageKey key = MessageKey.ACCOUNT_NOT_ACTIVATED;
String[] lines = new String[]{"First message line", "second line"};
given(messages.retrieve(key)).willReturn(lines);
// when
String[] result = commonService.retrieveMessage(key);
// then
assertThat(result, equalTo(lines));
verify(messages).retrieve(key);
}
@Test
public void shouldRetrieveSingleMessage() {
// given

View File

@ -58,7 +58,7 @@ public class PluginHookServiceTest {
assertThat(pluginHookService.isEssentialsAvailable(), equalTo(true));
}
// Note ljacqu 20160312: Cannot test with Multiverse or CombatTagPlus because their classes are declared final
// Note ljacqu 20160312: Cannot test with CombatTagPlus because its class is declared final
@Test
public void shouldHookIntoEssentialsAtInitialization() {

View File

@ -1,13 +1,20 @@
package fr.xephi.authme.util;
import fr.xephi.authme.TestHelper;
import org.bukkit.command.CommandSender;
import org.bukkit.command.ConsoleCommandSender;
import org.bukkit.entity.Player;
import org.junit.BeforeClass;
import org.junit.Test;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
/**
* Test for {@link Utils}.
@ -49,6 +56,49 @@ public class UtilsTest {
TestHelper.validateHasOnlyPrivateEmptyConstructor(Utils.class);
}
@Test
public void shouldLogAndSendMessage() {
// given
Logger logger = TestHelper.setupLogger();
Player player = mock(Player.class);
String message = "Finished adding foo to the bar";
// when
Utils.logAndSendMessage(player, message);
// then
verify(logger).info(message);
verify(player).sendMessage(message);
}
@Test
public void shouldHandleNullAsCommandSender() {
// given
Logger logger = TestHelper.setupLogger();
String message = "Test test, test.";
// when
Utils.logAndSendMessage(null, message);
// then
verify(logger).info(message);
}
@Test
public void shouldNotSendToCommandSenderTwice() {
// given
Logger logger = TestHelper.setupLogger();
CommandSender sender = mock(ConsoleCommandSender.class);
String message = "Test test, test.";
// when
Utils.logAndSendMessage(sender, message);
// then
verify(logger).info(message);
verifyZeroInteractions(sender);
}
@Test
public void shouldCheckIfClassIsLoaded() {
// given / when / then