diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 70c8bfd6b..e9de7d941 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -48,6 +48,7 @@ import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.settings.OtherAccounts; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.Spawn; +import fr.xephi.authme.settings.custom.NewSetting; import fr.xephi.authme.util.GeoLiteAPI; import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; @@ -67,6 +68,7 @@ import org.bukkit.scheduler.BukkitTask; import org.mcstats.Metrics; import org.mcstats.Metrics.Graph; +import java.io.File; import java.io.IOException; import java.net.URL; import java.util.Calendar; @@ -97,6 +99,7 @@ public class AuthMe extends JavaPlugin { private CommandHandler commandHandler = null; private PermissionsManager permsMan = null; private Settings settings; + private NewSetting newSettings; private Messages messages; private JsonCache playerBackup; private ModuleManager moduleManager; @@ -213,6 +216,7 @@ public class AuthMe extends JavaPlugin { setEnabled(false); return; } + newSettings = createNewSetting(); // Set up messages & password security messages = Messages.getInstance(); @@ -233,7 +237,7 @@ public class AuthMe extends JavaPlugin { // Set up the permissions manager and command handler permsMan = initializePermissionsManager(); - commandHandler = initializeCommandHandler(permsMan, messages, passwordSecurity); + commandHandler = initializeCommandHandler(permsMan, messages, passwordSecurity, newSettings); // Set up the module manager setupModuleManager(); @@ -415,11 +419,12 @@ public class AuthMe extends JavaPlugin { } private CommandHandler initializeCommandHandler(PermissionsManager permissionsManager, Messages messages, - PasswordSecurity passwordSecurity) { + PasswordSecurity passwordSecurity, NewSetting settings) { HelpProvider helpProvider = new HelpProvider(permissionsManager); Set baseCommands = CommandInitializer.buildCommands(); CommandMapper mapper = new CommandMapper(baseCommands, messages, permissionsManager, helpProvider); - CommandService commandService = new CommandService(this, mapper, helpProvider, messages, passwordSecurity); + CommandService commandService = new CommandService( + this, mapper, helpProvider, messages, passwordSecurity, settings); return new CommandHandler(commandService); } @@ -441,19 +446,24 @@ public class AuthMe extends JavaPlugin { * @return True on success, false on failure. */ private boolean loadSettings() { - // TODO: new configuration style (more files) try { settings = new Settings(this); Settings.reload(); } catch (Exception e) { ConsoleLogger.writeStackTrace(e); - ConsoleLogger.showError("Can't load the configuration file... Something went wrong, to avoid security issues the server will shutdown!"); + ConsoleLogger.showError("Can't load the configuration file... Something went wrong. " + + "To avoid security issues the server will shut down!"); server.shutdown(); return true; } return false; } + private NewSetting createNewSetting() { + File configFile = new File(getDataFolder() + "config.yml"); + return new NewSetting(getConfig(), configFile); + } + /** * Set up the console filter. */ diff --git a/src/main/java/fr/xephi/authme/command/CommandService.java b/src/main/java/fr/xephi/authme/command/CommandService.java index 08a2790e8..8a986b0ee 100644 --- a/src/main/java/fr/xephi/authme/command/CommandService.java +++ b/src/main/java/fr/xephi/authme/command/CommandService.java @@ -2,6 +2,8 @@ package fr.xephi.authme.command; import java.util.List; +import fr.xephi.authme.settings.custom.NewSetting; +import fr.xephi.authme.settings.domain.Property; import org.bukkit.command.CommandSender; import fr.xephi.authme.AuthMe; @@ -24,6 +26,7 @@ public class CommandService { private final HelpProvider helpProvider; private final CommandMapper commandMapper; private final PasswordSecurity passwordSecurity; + private final NewSetting settings; /** * Constructor. @@ -35,12 +38,13 @@ public class CommandService { * @param passwordSecurity The Password Security instance */ public CommandService(AuthMe authMe, CommandMapper commandMapper, HelpProvider helpProvider, Messages messages, - PasswordSecurity passwordSecurity) { + PasswordSecurity passwordSecurity, NewSetting settings) { this.authMe = authMe; this.messages = messages; this.helpProvider = helpProvider; this.commandMapper = commandMapper; this.passwordSecurity = passwordSecurity; + this.settings = settings; } /** @@ -156,4 +160,15 @@ public class CommandService { return messages.retrieve(key); } + /** + * Retrieve the given property's value. + * + * @param property The property to retrieve + * @param The type of the property + * @return The property's value + */ + public T getProperty(Property property) { + return settings.getProperty(property); + } + } diff --git a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java index d21bac778..79f963f0b 100644 --- a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java @@ -6,7 +6,7 @@ import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.PlayerCommand; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.RandomString; -import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.custom.SecuritySettings; import fr.xephi.authme.util.Wrapper; import org.bukkit.entity.Player; @@ -28,20 +28,20 @@ public class CaptchaCommand extends PlayerCommand { return; } - if (!Settings.useCaptcha) { + if (!commandService.getProperty(SecuritySettings.USE_CAPTCHA)) { commandService.send(player, MessageKey.USAGE_LOGIN); return; } - if (!plugin.cap.containsKey(playerNameLowerCase)) { commandService.send(player, MessageKey.USAGE_LOGIN); return; } - if (Settings.useCaptcha && !captcha.equals(plugin.cap.get(playerNameLowerCase))) { + if (!captcha.equals(plugin.cap.get(playerNameLowerCase))) { plugin.cap.remove(playerNameLowerCase); - String randStr = RandomString.generate(Settings.captchaLength); + int captchaLength = commandService.getProperty(SecuritySettings.CAPTCHA_LENGTH); + String randStr = RandomString.generate(captchaLength); plugin.cap.put(playerNameLowerCase, randStr); commandService.send(player, MessageKey.CAPTCHA_WRONG_ERROR, plugin.cap.get(playerNameLowerCase)); return; diff --git a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java index 51bf6c106..8ba9f2cb7 100644 --- a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java @@ -43,6 +43,6 @@ public class RegisterCommand extends PlayerCommand { @Override public String getAlternativeCommand() { - return "authme register "; + return "/authme register "; } } diff --git a/src/main/java/fr/xephi/authme/settings/custom/NewSetting.java b/src/main/java/fr/xephi/authme/settings/custom/NewSetting.java index 00aa9220e..6f9abdc44 100644 --- a/src/main/java/fr/xephi/authme/settings/custom/NewSetting.java +++ b/src/main/java/fr/xephi/authme/settings/custom/NewSetting.java @@ -6,7 +6,7 @@ import fr.xephi.authme.settings.domain.Property; import fr.xephi.authme.settings.propertymap.PropertyMap; import fr.xephi.authme.util.CollectionUtils; import fr.xephi.authme.util.StringUtils; -import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.configuration.file.FileConfiguration; import java.io.File; import java.io.FileWriter; @@ -22,41 +22,43 @@ import java.util.Map; public class NewSetting { private File file; - private YamlConfiguration configuration; + private FileConfiguration configuration; /** * Constructor. * Loads the file as YAML and checks its integrity. * + * @param configuration The configuration to interact with * @param file The configuration file */ - public NewSetting(File file) { - this.configuration = YamlConfiguration.loadConfiguration(file); + public NewSetting(FileConfiguration configuration, File file) { + this.configuration = configuration; this.file = file; - PropertyMap propertyMap = SettingsFieldRetriever.getAllPropertyFields(); - if (!containsAllSettings(propertyMap)) { - save(propertyMap); - } + // TODO ljacqu 20160109: Ensure that save() works as desired (i.e. that it always produces valid YAML) + // and then uncomment the lines below. Once this is uncommented, the checks in the old Settings.java should + // be removed as we should check to rewrite the config.yml file only at one place + // -------- + // PropertyMap propertyMap = SettingsFieldRetriever.getAllPropertyFields(); + // if (!containsAllSettings(propertyMap)) { + // save(propertyMap); + // } } /** - * Simple constructor for testing purposes. Does not check for all properties and - * never saves to the file. + * Constructor for testing purposes, allowing more options. * - * @param yamlConfiguration The YamlConfiguration object to use + * @param configuration The FileConfiguration object to use * @param file The file to write to * @param propertyMap The property map whose properties should be verified for presence, or null to skip this */ @VisibleForTesting - NewSetting(YamlConfiguration yamlConfiguration, File file, PropertyMap propertyMap) { - this.configuration = yamlConfiguration; + NewSetting(FileConfiguration configuration, File file, PropertyMap propertyMap) { + this.configuration = configuration; this.file = file; - if (propertyMap != null) { - if (!containsAllSettings(propertyMap)) { - save(propertyMap); - } + if (propertyMap != null && !containsAllSettings(propertyMap)) { + save(propertyMap); } } @@ -67,7 +69,7 @@ public class NewSetting { * @param The property's type * @return The property's value */ - public T getOption(Property property) { + public T getProperty(Property property) { return property.getFromFile(configuration); } diff --git a/src/main/java/fr/xephi/authme/settings/domain/EnumPropertyType.java b/src/main/java/fr/xephi/authme/settings/domain/EnumPropertyType.java index 357067d8e..ed184bb7d 100644 --- a/src/main/java/fr/xephi/authme/settings/domain/EnumPropertyType.java +++ b/src/main/java/fr/xephi/authme/settings/domain/EnumPropertyType.java @@ -1,6 +1,6 @@ package fr.xephi.authme.settings.domain; -import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.configuration.file.FileConfiguration; import java.util.List; @@ -19,7 +19,7 @@ class EnumPropertyType> extends PropertyType { } @Override - public E getFromFile(Property property, YamlConfiguration configuration) { + public E getFromFile(Property property, FileConfiguration configuration) { String textValue = configuration.getString(property.getPath()); if (textValue == null) { return property.getDefaultValue(); @@ -34,7 +34,7 @@ class EnumPropertyType> extends PropertyType { } @Override - public boolean contains(Property property, YamlConfiguration configuration) { + public boolean contains(Property property, FileConfiguration configuration) { return super.contains(property, configuration) && mapToEnum(configuration.getString(property.getPath())) != null; } diff --git a/src/main/java/fr/xephi/authme/settings/domain/Property.java b/src/main/java/fr/xephi/authme/settings/domain/Property.java index c0fd2bfe0..6b15b5d3b 100644 --- a/src/main/java/fr/xephi/authme/settings/domain/Property.java +++ b/src/main/java/fr/xephi/authme/settings/domain/Property.java @@ -1,6 +1,6 @@ package fr.xephi.authme.settings.domain; -import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.configuration.file.FileConfiguration; import java.util.Arrays; import java.util.List; @@ -60,7 +60,7 @@ public class Property { * @param configuration The configuration to read the value from * @return The value, or default if not present */ - public T getFromFile(YamlConfiguration configuration) { + public T getFromFile(FileConfiguration configuration) { return type.getFromFile(this, configuration); } @@ -70,7 +70,7 @@ public class Property { * @param configuration The configuration to read the value from * @return The property value as YAML */ - public List formatValueAsYaml(YamlConfiguration configuration) { + public List formatValueAsYaml(FileConfiguration configuration) { return type.asYaml(this, configuration); } @@ -80,7 +80,7 @@ public class Property { * @param configuration The configuration file to verify * @return True if the property is present, false otherwise */ - public boolean isPresent(YamlConfiguration configuration) { + public boolean isPresent(FileConfiguration configuration) { return type.contains(this, configuration); } diff --git a/src/main/java/fr/xephi/authme/settings/domain/PropertyType.java b/src/main/java/fr/xephi/authme/settings/domain/PropertyType.java index 5338da7af..dc1975bba 100644 --- a/src/main/java/fr/xephi/authme/settings/domain/PropertyType.java +++ b/src/main/java/fr/xephi/authme/settings/domain/PropertyType.java @@ -1,6 +1,6 @@ package fr.xephi.authme.settings.domain; -import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.configuration.file.FileConfiguration; import java.util.ArrayList; import java.util.List; @@ -28,7 +28,7 @@ public abstract class PropertyType { * @param configuration The YAML configuration to read from * @return The read value, or the default value if absent */ - public abstract T getFromFile(Property property, YamlConfiguration configuration); + public abstract T getFromFile(Property property, FileConfiguration configuration); /** * Return the property's value (or its default) as YAML. @@ -37,7 +37,7 @@ public abstract class PropertyType { * @param configuration The YAML configuration to read from * @return The read value or its default in YAML format */ - public List asYaml(Property property, YamlConfiguration configuration) { + public List asYaml(Property property, FileConfiguration configuration) { return asYaml(getFromFile(property, configuration)); } @@ -48,7 +48,7 @@ public abstract class PropertyType { * @param configuration The configuration to verify * @return True if the property is present, false otherwise */ - public boolean contains(Property property, YamlConfiguration configuration) { + public boolean contains(Property property, FileConfiguration configuration) { return configuration.contains(property.getPath()); } @@ -66,7 +66,7 @@ public abstract class PropertyType { */ private static final class BooleanProperty extends PropertyType { @Override - public Boolean getFromFile(Property property, YamlConfiguration configuration) { + public Boolean getFromFile(Property property, FileConfiguration configuration) { return configuration.getBoolean(property.getPath(), property.getDefaultValue()); } @@ -81,7 +81,7 @@ public abstract class PropertyType { */ private static final class DoubleProperty extends PropertyType { @Override - public Double getFromFile(Property property, YamlConfiguration configuration) { + public Double getFromFile(Property property, FileConfiguration configuration) { return configuration.getDouble(property.getPath(), property.getDefaultValue()); } @@ -96,7 +96,7 @@ public abstract class PropertyType { */ private static final class IntegerProperty extends PropertyType { @Override - public Integer getFromFile(Property property, YamlConfiguration configuration) { + public Integer getFromFile(Property property, FileConfiguration configuration) { return configuration.getInt(property.getPath(), property.getDefaultValue()); } @@ -111,7 +111,7 @@ public abstract class PropertyType { */ private static final class StringProperty extends PropertyType { @Override - public String getFromFile(Property property, YamlConfiguration configuration) { + public String getFromFile(Property property, FileConfiguration configuration) { return configuration.getString(property.getPath(), property.getDefaultValue()); } @@ -131,7 +131,7 @@ public abstract class PropertyType { */ private static final class StringListProperty extends PropertyType> { @Override - public List getFromFile(Property> property, YamlConfiguration configuration) { + public List getFromFile(Property> property, FileConfiguration configuration) { if (!configuration.isList(property.getPath())) { return property.getDefaultValue(); } @@ -154,7 +154,7 @@ public abstract class PropertyType { } @Override - public boolean contains(Property> property, YamlConfiguration configuration) { + public boolean contains(Property> property, FileConfiguration configuration) { return configuration.contains(property.getPath()) && configuration.isList(property.getPath()); } } diff --git a/src/test/java/fr/xephi/authme/command/CommandServiceTest.java b/src/test/java/fr/xephi/authme/command/CommandServiceTest.java index 3b457291e..a08e18efd 100644 --- a/src/test/java/fr/xephi/authme/command/CommandServiceTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandServiceTest.java @@ -8,6 +8,9 @@ import fr.xephi.authme.output.Messages; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.Management; import fr.xephi.authme.security.PasswordSecurity; +import fr.xephi.authme.settings.custom.NewSetting; +import fr.xephi.authme.settings.custom.SecuritySettings; +import fr.xephi.authme.settings.domain.Property; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; import org.junit.Before; @@ -36,6 +39,7 @@ public class CommandServiceTest { private Messages messages; private PasswordSecurity passwordSecurity; private CommandService commandService; + private NewSetting settings; @Before public void setUpService() { @@ -44,7 +48,8 @@ public class CommandServiceTest { helpProvider = mock(HelpProvider.class); messages = mock(Messages.class); passwordSecurity = mock(PasswordSecurity.class); - commandService = new CommandService(authMe, commandMapper, helpProvider, messages, passwordSecurity); + settings = mock(NewSetting.class); + commandService = new CommandService(authMe, commandMapper, helpProvider, messages, passwordSecurity, settings); } @Test @@ -195,4 +200,18 @@ public class CommandServiceTest { assertThat(result, equalTo(givenMessages)); verify(messages).retrieve(key); } + + @Test + public void shouldRetrieveProperty() { + // given + Property property = SecuritySettings.CAPTCHA_LENGTH; + given(settings.getProperty(property)).willReturn(7); + + // when + int result = settings.getProperty(property); + + // then + assertThat(result, equalTo(7)); + verify(settings).getProperty(property); + } } diff --git a/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java index 60c72bd31..785b0b003 100644 --- a/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/captcha/CaptchaCommandTest.java @@ -5,7 +5,7 @@ import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; -import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.custom.SecuritySettings; import fr.xephi.authme.util.WrapperMock; import org.bukkit.command.BlockCommandSender; import org.bukkit.command.CommandSender; @@ -20,6 +20,7 @@ import java.util.Collections; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -35,8 +36,8 @@ public class CaptchaCommandTest { @Before public void setUpWrapperMock() { wrapperMock = WrapperMock.createInstance(); - Settings.useCaptcha = true; commandService = mock(CommandService.class); + given(commandService.getProperty(SecuritySettings.USE_CAPTCHA)).willReturn(true); } @Test @@ -46,7 +47,7 @@ public class CaptchaCommandTest { ExecutableCommand command = new CaptchaCommand(); // when - command.executeCommand(sender, new ArrayList(), mock(CommandService.class)); + command.executeCommand(sender, new ArrayList(), commandService); // then assertThat(wrapperMock.wasMockCalled(AuthMe.class), equalTo(false)); @@ -54,14 +55,14 @@ public class CaptchaCommandTest { } @Test - @Ignore public void shouldRejectIfCaptchaIsNotUsed() { // given Player player = mockPlayerWithName("testplayer"); ExecutableCommand command = new CaptchaCommand(); + given(commandService.getProperty(SecuritySettings.USE_CAPTCHA)).willReturn(false); // when - command.executeCommand(player, Collections.singletonList("1234"), mock(CommandService.class)); + command.executeCommand(player, Collections.singletonList("1234"), commandService); // then verify(commandService).send(player, MessageKey.USAGE_LOGIN); diff --git a/src/test/java/fr/xephi/authme/settings/custom/NewSettingIntegrationTest.java b/src/test/java/fr/xephi/authme/settings/custom/NewSettingIntegrationTest.java index dcb3c6833..73b80b020 100644 --- a/src/test/java/fr/xephi/authme/settings/custom/NewSettingIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/settings/custom/NewSettingIntegrationTest.java @@ -67,7 +67,7 @@ public class NewSettingIntegrationTest { .build(); for (Map.Entry, Object> entry : expectedValues.entrySet()) { assertThat("Property '" + entry.getKey().getPath() + "' has expected value", - settings.getOption(entry.getKey()), equalTo(entry.getValue())); + settings.getProperty(entry.getKey()), equalTo(entry.getValue())); } assertThat(file.exists(), equalTo(false)); } @@ -100,7 +100,7 @@ public class NewSettingIntegrationTest { .build(); for (Map.Entry, Object> entry : expectedValues.entrySet()) { assertThat("Property '" + entry.getKey().getPath() + "' has expected value", - settings.getOption(entry.getKey()), equalTo(entry.getValue())); + settings.getProperty(entry.getKey()), equalTo(entry.getValue())); } } diff --git a/src/test/java/fr/xephi/authme/settings/custom/NewSettingTest.java b/src/test/java/fr/xephi/authme/settings/custom/NewSettingTest.java index 080e1d0d0..64e213aed 100644 --- a/src/test/java/fr/xephi/authme/settings/custom/NewSettingTest.java +++ b/src/test/java/fr/xephi/authme/settings/custom/NewSettingTest.java @@ -41,10 +41,10 @@ public class NewSettingTest { // when / then NewSetting settings = new NewSetting(file, new File("conf.txt"), null); - assertThat(settings.getOption(TestConfiguration.VERSION_NUMBER), equalTo(20)); - assertThat(settings.getOption(TestConfiguration.SKIP_BORING_FEATURES), equalTo(true)); - assertThat(settings.getOption(TestConfiguration.RATIO_LIMIT), equalTo(4.25)); - assertThat(settings.getOption(TestConfiguration.SYSTEM_NAME), equalTo("myTestSys")); + assertThat(settings.getProperty(TestConfiguration.VERSION_NUMBER), equalTo(20)); + assertThat(settings.getProperty(TestConfiguration.SKIP_BORING_FEATURES), equalTo(true)); + assertThat(settings.getProperty(TestConfiguration.RATIO_LIMIT), equalTo(4.25)); + assertThat(settings.getProperty(TestConfiguration.SYSTEM_NAME), equalTo("myTestSys")); assertDefaultValue(TestConfiguration.DURATION_IN_SECONDS, settings); assertDefaultValue(TestConfiguration.DUST_LEVEL, settings); @@ -68,7 +68,7 @@ public class NewSettingTest { private static void assertDefaultValue(Property property, NewSetting setting) { assertThat(property.getPath() + " has default value", - setting.getOption(property).equals(property.getDefaultValue()), equalTo(true)); + setting.getProperty(property).equals(property.getDefaultValue()), equalTo(true)); } private static Answer withDefaultArgument() {