From c766b5c25936da3dc7d15b07be5ddc7564e8b6d4 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 12 Mar 2017 13:51:03 +0100 Subject: [PATCH] #1036 Add restoration options for Limbo allowFlight / fly speed / walk speed - Introduce options to define how allow flight, fly & walk speed should be restored from LimboPlayer - Create consistency tests for line length in SectionComments methods and to ensure that all SettingsHolder classes are part of the returned ConfigurationData --- .checkstyle.xml | 2 +- docs/config.md | 45 +++++--- .../data/limbo/AllowFlightRestoreType.java | 43 +++++++ .../xephi/authme/data/limbo/LimboService.java | 19 ++- .../data/limbo/WalkFlySpeedRestoreType.java | 81 +++++++++++++ .../properties/AuthMeSettingsRetriever.java | 8 +- .../settings/properties/LimboSettings.java | 57 +++++++++ .../fr/xephi/authme/ReflectionTestUtils.java | 5 +- .../limbo/AllowFlightRestoreTypeTest.java | 72 ++++++++++++ .../authme/data/limbo/LimboServiceTest.java | 33 +++--- .../limbo/WalkFlySpeedRestoreTypeTest.java | 108 ++++++++++++++++++ .../settings/SettingsConsistencyTest.java | 66 +++++++++++ .../SettingsClassConsistencyTest.java | 24 ++++ 13 files changed, 518 insertions(+), 45 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java create mode 100644 src/main/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreType.java create mode 100644 src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java create mode 100644 src/test/java/fr/xephi/authme/data/limbo/AllowFlightRestoreTypeTest.java create mode 100644 src/test/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreTypeTest.java diff --git a/.checkstyle.xml b/.checkstyle.xml index d8f9076d1..4ff5d8dfb 100644 --- a/.checkstyle.xml +++ b/.checkstyle.xml @@ -145,7 +145,7 @@ - + diff --git a/docs/config.md b/docs/config.md index ff2d54fb4..3e750b2d8 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1,5 +1,5 @@ - + ## AuthMe Configuration The first time you run AuthMe it will create a config.yml file in the plugins/AuthMe folder, @@ -73,17 +73,6 @@ ExternalBoardOptions: phpbbActivatedGroupId: 2 # Wordpress prefix defined during WordPress installation wordpressTablePrefix: 'wp_' -Converter: - Rakamak: - # Rakamak file name - fileName: 'users.rak' - # Rakamak use IP? - useIP: false - # Rakamak IP file name - ipFileName: 'UsersIp.rak' - CrazyLogin: - # CrazyLogin database file name - fileName: 'accounts.db' settings: sessions: # Do you want to enable the session feature? @@ -323,6 +312,8 @@ Email: mailSMTP: 'smtp.gmail.com' # Email SMTP server port mailPort: 465 + # Only affects port 25: enable TLS/STARTTLS? + useTls: true # Email account which sends the mails mailAccount: '' # Email account password @@ -444,6 +435,23 @@ Security: # Seconds a user has to wait for before a password recovery mail may be sent again # This prevents an attacker from abusing AuthMe's email feature. cooldown: 60 +# Before a user logs in, various properties are temporarily removed from the player, +# such as OP status, ability to fly, and walk/fly speed. +# Once the user is logged in, we add back the properties we previously saved. +# In this section, you may define how the properties should be restored. +limbo: + # Whether the player is allowed to fly: RESTORE, ENABLE, DISABLE. + # RESTORE sets back the old property from the player. + restoreAllowFlight: 'RESTORE' + # Restore fly speed: RESTORE, DEFAULT, MAX_RESTORE, RESTORE_NO_ZERO. + # RESTORE: restore the speed the player had; + # DEFAULT: always set to default speed; + # MAX_RESTORE: take the maximum of the player's current speed and the previous one + # RESTORE_NO_ZERO: Like 'restore' but sets speed to default if the player's speed was 0 + restoreFlySpeed: 'MAX_RESTORE' + # Restore walk speed: RESTORE, DEFAULT, MAX_RESTORE, RESTORE_NO_ZERO. + # See above for a description of the values. + restoreWalkSpeed: 'MAX_RESTORE' BackupSystem: # Enable or disable automatic backup ActivateBackup: false @@ -453,6 +461,17 @@ BackupSystem: OnServerStop: true # Windows only mysql installation Path MysqlWindowsPath: 'C:\Program Files\MySQL\MySQL Server 5.1\' +Converter: + Rakamak: + # Rakamak file name + fileName: 'users.rak' + # Rakamak use IP? + useIP: false + # Rakamak IP file name + ipFileName: 'UsersIp.rak' + CrazyLogin: + # CrazyLogin database file name + fileName: 'accounts.db' ``` To change settings on a running server, save your changes to config.yml and use @@ -460,4 +479,4 @@ To change settings on a running server, save your changes to config.yml and use --- -This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Sat Feb 25 21:59:18 CET 2017 +This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Sun Mar 12 13:39:50 CET 2017 diff --git a/src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java b/src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java new file mode 100644 index 000000000..f085afbb4 --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java @@ -0,0 +1,43 @@ +package fr.xephi.authme.data.limbo; + +import org.bukkit.entity.Player; + +import java.util.function.Function; + +/** + * Possible types to restore the "allow flight" property + * from LimboPlayer to Bukkit Player. + */ +public enum AllowFlightRestoreType { + + /** Set value from LimboPlayer to Player. */ + RESTORE(LimboPlayer::isCanFly), + + /** Always set flight enabled to true. */ + ENABLE(l -> true), + + /** Always set flight enabled to false. */ + DISABLE(l -> false); + + private final Function valueGetter; + + /** + * Constructor. + * + * @param valueGetter function with which the value to set on the player can be retrieved + */ + AllowFlightRestoreType(Function valueGetter) { + this.valueGetter = valueGetter; + } + + /** + * Restores the "allow flight" property from the LimboPlayer to the Player. + * This method behaves differently for each restoration type. + * + * @param player the player to modify + * @param limbo the limbo player to read from + */ + public void restoreAllowFlight(Player player, LimboPlayer limbo) { + player.setAllowFlight(valueGetter.apply(limbo)); + } +} diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboService.java b/src/main/java/fr/xephi/authme/data/limbo/LimboService.java index 775337a83..b0c93d7d1 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboService.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboService.java @@ -13,6 +13,10 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import static fr.xephi.authme.settings.properties.LimboSettings.RESTORE_ALLOW_FLIGHT; +import static fr.xephi.authme.settings.properties.LimboSettings.RESTORE_FLY_SPEED; +import static fr.xephi.authme.settings.properties.LimboSettings.RESTORE_WALK_SPEED; + /** * Service for managing players that are in "limbo," a temporary state players are * put in which have joined but not yet logged in yet. @@ -53,18 +57,9 @@ public class LimboService { ConsoleLogger.debug("No LimboPlayer found for `{0}` - cannot restore", lowerName); } else { player.setOp(limbo.isOperator()); - player.setAllowFlight(limbo.isCanFly()); - float walkSpeed = limbo.getWalkSpeed(); - float flySpeed = limbo.getFlySpeed(); - // Reset the speed value if it was 0 - if (walkSpeed < 0.01f) { - walkSpeed = LimboPlayer.DEFAULT_WALK_SPEED; - } - if (flySpeed < 0.01f) { - flySpeed = LimboPlayer.DEFAULT_FLY_SPEED; - } - player.setWalkSpeed(walkSpeed); - player.setFlySpeed(flySpeed); + settings.getProperty(RESTORE_ALLOW_FLIGHT).restoreAllowFlight(player, limbo); + settings.getProperty(RESTORE_FLY_SPEED).restoreFlySpeed(player, limbo); + settings.getProperty(RESTORE_WALK_SPEED).restoreWalkSpeed(player, limbo); limbo.clearTasks(); ConsoleLogger.debug("Restored LimboPlayer stats for `{0}`", lowerName); } diff --git a/src/main/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreType.java b/src/main/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreType.java new file mode 100644 index 000000000..960cdd435 --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreType.java @@ -0,0 +1,81 @@ +package fr.xephi.authme.data.limbo; + +import org.bukkit.entity.Player; + +/** + * Possible types to restore the walk and fly speed from LimboPlayer + * back to Bukkit Player. + */ +public enum WalkFlySpeedRestoreType { + + /** Restores from LimboPlayer to Player. */ + RESTORE { + @Override + public void restoreFlySpeed(Player player, LimboPlayer limbo) { + player.setFlySpeed(limbo.getFlySpeed()); + } + + @Override + public void restoreWalkSpeed(Player player, LimboPlayer limbo) { + player.setWalkSpeed(limbo.getWalkSpeed()); + } + }, + + /** Restores from LimboPlayer, using the default speed if the speed on LimboPlayer is 0. */ + RESTORE_NO_ZERO { + @Override + public void restoreFlySpeed(Player player, LimboPlayer limbo) { + float limboFlySpeed = limbo.getFlySpeed(); + player.setFlySpeed(limboFlySpeed > 0.01f ? limboFlySpeed : LimboPlayer.DEFAULT_FLY_SPEED); + } + + @Override + public void restoreWalkSpeed(Player player, LimboPlayer limbo) { + float limboWalkSpeed = limbo.getWalkSpeed(); + player.setWalkSpeed(limboWalkSpeed > 0.01f ? limboWalkSpeed : LimboPlayer.DEFAULT_WALK_SPEED); + } + }, + + /** Uses the max speed of Player (current speed) and the LimboPlayer. */ + MAX_RESTORE { + @Override + public void restoreFlySpeed(Player player, LimboPlayer limbo) { + player.setFlySpeed(Math.max(player.getFlySpeed(), limbo.getFlySpeed())); + } + + @Override + public void restoreWalkSpeed(Player player, LimboPlayer limbo) { + player.setWalkSpeed(Math.max(player.getWalkSpeed(), limbo.getWalkSpeed())); + } + }, + + /** Always sets the default speed to the player. */ + DEFAULT { + @Override + public void restoreFlySpeed(Player player, LimboPlayer limbo) { + player.setFlySpeed(LimboPlayer.DEFAULT_FLY_SPEED); + } + + @Override + public void restoreWalkSpeed(Player player, LimboPlayer limbo) { + player.setWalkSpeed(LimboPlayer.DEFAULT_WALK_SPEED); + } + }; + + /** + * Restores the fly speed from Limbo to Player according to the restoration type. + * + * @param player the player to modify + * @param limbo the limbo player to read from + */ + public abstract void restoreFlySpeed(Player player, LimboPlayer limbo); + + /** + * Restores the walk speed from Limbo to Player according to the restoration type. + * + * @param player the player to modify + * @param limbo the limbo player to read from + */ + public abstract void restoreWalkSpeed(Player player, LimboPlayer limbo); + +} diff --git a/src/main/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetriever.java b/src/main/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetriever.java index 8cfb29dc4..2fbe7ea24 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetriever.java +++ b/src/main/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetriever.java @@ -20,9 +20,9 @@ public final class AuthMeSettingsRetriever { */ public static ConfigurationData buildConfigurationData() { return ConfigurationDataBuilder.collectData( - DatabaseSettings.class, ConverterSettings.class, PluginSettings.class, - RestrictionSettings.class, EmailSettings.class, HooksSettings.class, - ProtectionSettings.class, PurgeSettings.class, SecuritySettings.class, - RegistrationSettings.class, BackupSettings.class); + DatabaseSettings.class, PluginSettings.class, RestrictionSettings.class, + EmailSettings.class, HooksSettings.class, ProtectionSettings.class, + PurgeSettings.class, SecuritySettings.class, RegistrationSettings.class, + LimboSettings.class, BackupSettings.class, ConverterSettings.class); } } diff --git a/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java b/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java new file mode 100644 index 000000000..f861c4990 --- /dev/null +++ b/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java @@ -0,0 +1,57 @@ +package fr.xephi.authme.settings.properties; + +import ch.jalu.configme.Comment; +import ch.jalu.configme.SectionComments; +import ch.jalu.configme.SettingsHolder; +import ch.jalu.configme.properties.Property; +import com.google.common.collect.ImmutableMap; +import fr.xephi.authme.data.limbo.AllowFlightRestoreType; +import fr.xephi.authme.data.limbo.WalkFlySpeedRestoreType; + +import java.util.Map; + +import static ch.jalu.configme.properties.PropertyInitializer.newProperty; + +/** + * Settings for the LimboPlayer feature. + */ +public final class LimboSettings implements SettingsHolder { + + @Comment({ + "Whether the player is allowed to fly: RESTORE, ENABLE, DISABLE.", + "RESTORE sets back the old property from the player." + }) + public static final Property RESTORE_ALLOW_FLIGHT = + newProperty(AllowFlightRestoreType.class, "limbo.restoreAllowFlight", AllowFlightRestoreType.RESTORE); + + @Comment({ + "Restore fly speed: RESTORE, DEFAULT, MAX_RESTORE, RESTORE_NO_ZERO.", + "RESTORE: restore the speed the player had;", + "DEFAULT: always set to default speed;", + "MAX_RESTORE: take the maximum of the player's current speed and the previous one", + "RESTORE_NO_ZERO: Like 'restore' but sets speed to default if the player's speed was 0" + }) + public static final Property RESTORE_FLY_SPEED = + newProperty(WalkFlySpeedRestoreType.class, "limbo.restoreFlySpeed", WalkFlySpeedRestoreType.MAX_RESTORE); + + @Comment({ + "Restore walk speed: RESTORE, DEFAULT, MAX_RESTORE, RESTORE_NO_ZERO.", + "See above for a description of the values." + }) + public static final Property RESTORE_WALK_SPEED = + newProperty(WalkFlySpeedRestoreType.class, "limbo.restoreWalkSpeed", WalkFlySpeedRestoreType.MAX_RESTORE); + + private LimboSettings() { + } + + @SectionComments + public static Map createSectionComments() { + String[] limboExplanation = { + "Before a user logs in, various properties are temporarily removed from the player,", + "such as OP status, ability to fly, and walk/fly speed.", + "Once the user is logged in, we add back the properties we previously saved.", + "In this section, you may define how the properties should be restored." + }; + return ImmutableMap.of("limbo", limboExplanation); + } +} diff --git a/src/test/java/fr/xephi/authme/ReflectionTestUtils.java b/src/test/java/fr/xephi/authme/ReflectionTestUtils.java index fdd716bd5..bc0876669 100644 --- a/src/test/java/fr/xephi/authme/ReflectionTestUtils.java +++ b/src/test/java/fr/xephi/authme/ReflectionTestUtils.java @@ -75,10 +75,11 @@ public final class ReflectionTestUtils { } } - public static Object invokeMethod(Method method, Object instance, Object... parameters) { + @SuppressWarnings("unchecked") + public static V invokeMethod(Method method, Object instance, Object... parameters) { method.setAccessible(true); try { - return method.invoke(instance, parameters); + return (V) method.invoke(instance, parameters); } catch (InvocationTargetException | IllegalAccessException e) { throw new UnsupportedOperationException("Could not invoke method '" + method + "'", e); } diff --git a/src/test/java/fr/xephi/authme/data/limbo/AllowFlightRestoreTypeTest.java b/src/test/java/fr/xephi/authme/data/limbo/AllowFlightRestoreTypeTest.java new file mode 100644 index 000000000..8c22cc756 --- /dev/null +++ b/src/test/java/fr/xephi/authme/data/limbo/AllowFlightRestoreTypeTest.java @@ -0,0 +1,72 @@ +package fr.xephi.authme.data.limbo; + +import org.bukkit.entity.Player; +import org.junit.Test; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link AllowFlightRestoreType}. + */ +public class AllowFlightRestoreTypeTest { + + @Test + public void shouldRestoreValue() { + // given + LimboPlayer limboWithFly = newLimboWithAllowFlight(true); + LimboPlayer limboWithoutFly = newLimboWithAllowFlight(false); + Player player1 = mock(Player.class); + Player player2 = mock(Player.class); + + // when + AllowFlightRestoreType.RESTORE.restoreAllowFlight(player1, limboWithFly); + AllowFlightRestoreType.RESTORE.restoreAllowFlight(player2, limboWithoutFly); + + // then + verify(player1).setAllowFlight(true); + verify(player2).setAllowFlight(false); + } + + @Test + public void shouldEnableFlight() { + // given + LimboPlayer limboWithFly = newLimboWithAllowFlight(true); + LimboPlayer limboWithoutFly = newLimboWithAllowFlight(false); + Player player1 = mock(Player.class); + Player player2 = mock(Player.class); + + // when + AllowFlightRestoreType.ENABLE.restoreAllowFlight(player1, limboWithFly); + AllowFlightRestoreType.ENABLE.restoreAllowFlight(player2, limboWithoutFly); + + // then + verify(player1).setAllowFlight(true); + verify(player2).setAllowFlight(true); + } + + + @Test + public void shouldDisableFlight() { + // given + LimboPlayer limboWithFly = newLimboWithAllowFlight(true); + LimboPlayer limboWithoutFly = newLimboWithAllowFlight(false); + Player player1 = mock(Player.class); + Player player2 = mock(Player.class); + + // when + AllowFlightRestoreType.DISABLE.restoreAllowFlight(player1, limboWithFly); + AllowFlightRestoreType.DISABLE.restoreAllowFlight(player2, limboWithoutFly); + + // then + verify(player1).setAllowFlight(false); + verify(player2).setAllowFlight(false); + } + + private static LimboPlayer newLimboWithAllowFlight(boolean allowFlight) { + LimboPlayer limbo = mock(LimboPlayer.class); + given(limbo.isCanFly()).willReturn(allowFlight); + return limbo; + } +} diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java b/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java index 69d3c3704..4abe481ce 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java @@ -5,6 +5,7 @@ import fr.xephi.authme.TestHelper; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.SpawnLoader; +import fr.xephi.authme.settings.properties.LimboSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.Location; import org.bukkit.entity.Player; @@ -19,8 +20,6 @@ import org.mockito.junit.MockitoJUnitRunner; import java.util.Map; -import static org.hamcrest.Matchers.anEmptyMap; -import static org.hamcrest.Matchers.both; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -85,7 +84,8 @@ public class LimboServiceTest { verify(player).setFlySpeed(0.0f); verify(player).setWalkSpeed(0.0f); - LimboPlayer limbo = getLimboMap().get("bobby"); + assertThat(limboService.hasLimboPlayer("Bobby"), equalTo(true)); + LimboPlayer limbo = limboService.getLimboPlayer("Bobby"); assertThat(limbo, not(nullValue())); assertThat(limbo.isOperator(), equalTo(true)); assertThat(limbo.getWalkSpeed(), equalTo(0.3f)); @@ -114,7 +114,7 @@ public class LimboServiceTest { verify(player).setFlySpeed(0.0f); verify(player).setWalkSpeed(0.0f); - LimboPlayer limbo = getLimboMap().get("charles"); + LimboPlayer limbo = limboService.getLimboPlayer("charles"); assertThat(limbo, not(nullValue())); assertThat(limbo.isOperator(), equalTo(false)); assertThat(limbo.getWalkSpeed(), equalTo(0.1f)); @@ -127,24 +127,31 @@ public class LimboServiceTest { @Test public void shouldClearTasksOnAlreadyExistingLimbo() { // given - LimboPlayer limbo = mock(LimboPlayer.class); - getLimboMap().put("carlos", limbo); + LimboPlayer existingLimbo = mock(LimboPlayer.class); + getLimboMap().put("carlos", existingLimbo); Player player = newPlayer("Carlos"); // when limboService.createLimboPlayer(player, false); // then - verify(limbo).clearTasks(); - assertThat(getLimboMap().get("carlos"), both(not(sameInstance(limbo))).and(not(nullValue()))); + verify(existingLimbo).clearTasks(); + LimboPlayer newLimbo = limboService.getLimboPlayer("Carlos"); + assertThat(newLimbo, not(nullValue())); + assertThat(newLimbo, not(sameInstance(existingLimbo))); } @Test public void shouldRestoreData() { // given - Player player = newPlayer("John", true, 0.4f, false, 0.2f); - LimboPlayer limbo = Mockito.spy(convertToLimboPlayer(player, null, "")); + LimboPlayer limbo = Mockito.spy(convertToLimboPlayer( + newPlayer("John", true, 0.4f, false, 0.0f), null, "")); getLimboMap().put("john", limbo); + Player player = newPlayer("John", false, 0.2f, false, 0.7f); + + given(settings.getProperty(LimboSettings.RESTORE_ALLOW_FLIGHT)).willReturn(AllowFlightRestoreType.ENABLE); + given(settings.getProperty(LimboSettings.RESTORE_WALK_SPEED)).willReturn(WalkFlySpeedRestoreType.RESTORE); + given(settings.getProperty(LimboSettings.RESTORE_FLY_SPEED)).willReturn(WalkFlySpeedRestoreType.RESTORE_NO_ZERO); // when limboService.restoreData(player); @@ -152,10 +159,10 @@ public class LimboServiceTest { // then verify(player).setOp(true); verify(player).setWalkSpeed(0.4f); - verify(player).setAllowFlight(false); - verify(player).setFlySpeed(0.2f); + verify(player).setAllowFlight(true); + verify(player).setFlySpeed(LimboPlayer.DEFAULT_FLY_SPEED); verify(limbo).clearTasks(); - assertThat(getLimboMap(), anEmptyMap()); + assertThat(limboService.hasLimboPlayer("John"), equalTo(false)); } @Test diff --git a/src/test/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreTypeTest.java b/src/test/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreTypeTest.java new file mode 100644 index 000000000..f8aa2719c --- /dev/null +++ b/src/test/java/fr/xephi/authme/data/limbo/WalkFlySpeedRestoreTypeTest.java @@ -0,0 +1,108 @@ +package fr.xephi.authme.data.limbo; + +import org.bukkit.entity.Player; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import static fr.xephi.authme.data.limbo.LimboPlayer.DEFAULT_FLY_SPEED; +import static fr.xephi.authme.data.limbo.LimboPlayer.DEFAULT_WALK_SPEED; +import static fr.xephi.authme.data.limbo.WalkFlySpeedRestoreType.DEFAULT; +import static fr.xephi.authme.data.limbo.WalkFlySpeedRestoreType.MAX_RESTORE; +import static fr.xephi.authme.data.limbo.WalkFlySpeedRestoreType.RESTORE; +import static fr.xephi.authme.data.limbo.WalkFlySpeedRestoreType.RESTORE_NO_ZERO; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link WalkFlySpeedRestoreType}. + */ +@RunWith(Parameterized.class) +public class WalkFlySpeedRestoreTypeTest { + + private final TestParameters parameters; + + public WalkFlySpeedRestoreTypeTest(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void shouldRestoreToExpectedValue() { + // given + LimboPlayer limbo = mock(LimboPlayer.class); + given(limbo.getWalkSpeed()).willReturn(parameters.givenLimboWalkSpeed); + given(limbo.getFlySpeed()).willReturn(parameters.givenLimboFlySpeed); + + Player player = mock(Player.class); + given(player.getWalkSpeed()).willReturn(parameters.givenPlayerWalkSpeed); + given(player.getFlySpeed()).willReturn(parameters.givenPlayerFlySpeed); + + // when + parameters.testedType.restoreWalkSpeed(player, limbo); + parameters.testedType.restoreFlySpeed(player, limbo); + + // then + verify(player).setWalkSpeed(parameters.expectedWalkSpeed); + verify(player).setFlySpeed(parameters.expectedFlySpeed); + } + + @Parameterized.Parameters(name = "{0}") + public static List buildParams() { + List parameters = Arrays.asList( + create(RESTORE).withLimbo(0.1f, 0.4f).withPlayer(0.3f, 0.9f).expect(0.1f, 0.4f), + create(RESTORE).withLimbo(0.9f, 0.2f).withPlayer(0.3f, 0.0f).expect(0.9f, 0.2f), + create(MAX_RESTORE).withLimbo(0.3f, 0.8f).withPlayer(0.5f, 0.2f).expect(0.5f, 0.8f), + create(MAX_RESTORE).withLimbo(0.4f, 0.2f).withPlayer(0.1f, 0.4f).expect(0.4f, 0.4f), + create(RESTORE_NO_ZERO).withLimbo(0.1f, 0.2f).withPlayer(0.5f, 0.1f).expect(0.1f, 0.2f), + create(RESTORE_NO_ZERO).withLimbo(0.0f, 0.005f).withPlayer(0.4f, 0.8f).expect(DEFAULT_WALK_SPEED, DEFAULT_FLY_SPEED), + create(DEFAULT).withLimbo(0.1f, 0.7f).withPlayer(0.4f, 0.0f).expect(DEFAULT_WALK_SPEED, DEFAULT_FLY_SPEED) + ); + + // Convert List to List + return parameters.stream().map(p -> new Object[]{p}).collect(Collectors.toList()); + } + + private static TestParameters create(WalkFlySpeedRestoreType testedType) { + TestParameters params = new TestParameters(); + params.testedType = testedType; + return params; + } + + private static final class TestParameters { + private WalkFlySpeedRestoreType testedType; + private float givenLimboWalkSpeed; + private float givenLimboFlySpeed; + private float givenPlayerWalkSpeed; + private float givenPlayerFlySpeed; + private float expectedWalkSpeed; + private float expectedFlySpeed; + + TestParameters withLimbo(float walkSpeed, float flySpeed) { + this.givenLimboWalkSpeed = walkSpeed; + this.givenLimboFlySpeed = flySpeed; + return this; + } + + TestParameters withPlayer(float walkSpeed, float flySpeed) { + this.givenPlayerWalkSpeed = walkSpeed; + this.givenPlayerFlySpeed = flySpeed; + return this; + } + + TestParameters expect(float walkSpeed, float flySpeed) { + this.expectedWalkSpeed = walkSpeed; + this.expectedFlySpeed = flySpeed; + return this; + } + + @Override + public String toString() { + return testedType + " {" + expectedWalkSpeed + ", " + expectedFlySpeed + "}"; + } + } +} diff --git a/src/test/java/fr/xephi/authme/settings/SettingsConsistencyTest.java b/src/test/java/fr/xephi/authme/settings/SettingsConsistencyTest.java index 88724a847..7d6c53be6 100644 --- a/src/test/java/fr/xephi/authme/settings/SettingsConsistencyTest.java +++ b/src/test/java/fr/xephi/authme/settings/SettingsConsistencyTest.java @@ -1,15 +1,25 @@ package fr.xephi.authme.settings; +import ch.jalu.configme.SectionComments; +import ch.jalu.configme.SettingsHolder; import ch.jalu.configme.configurationdata.ConfigurationData; import ch.jalu.configme.properties.Property; +import fr.xephi.authme.ClassCollector; +import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.settings.properties.AuthMeSettingsRetriever; import org.junit.BeforeClass; import org.junit.Test; +import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; +import static com.google.common.base.Preconditions.checkArgument; import static org.junit.Assert.fail; /** @@ -66,4 +76,60 @@ public class SettingsConsistencyTest { } } + @Test + public void shouldNotHaveVeryLongSectionCommentLines() { + // given + List sectionCommentMethods = getSectionCommentMethods(); + Set badMethods = new HashSet<>(); + + // when + for (Method method : sectionCommentMethods) { + boolean hasTooLongLine = getSectionComments(method).stream() + .anyMatch(line -> line.length() > MAX_COMMENT_LENGTH); + if (hasTooLongLine) { + badMethods.add(method); + } + } + + // then + if (!badMethods.isEmpty()) { + String methodList = badMethods.stream() + .map(m -> m.getName() + " in " + m.getDeclaringClass().getSimpleName()) + .collect(Collectors.joining("\n- ")); + fail("Found SectionComments methods with too long comments:\n- " + methodList); + } + } + + /** + * Gets all {@link SectionComments} methods from {@link SettingsHolder} implementations. + */ + @SuppressWarnings("unchecked") + private List getSectionCommentMethods() { + // Find all SettingsHolder classes + List> settingsClasses = + new ClassCollector("src/main/java", "fr/xephi/authme/settings/properties/") + .collectClasses(SettingsHolder.class); + checkArgument(!settingsClasses.isEmpty(), "Could not find any SettingsHolder classes"); + + // Find all @SectionComments methods in these classes + return settingsClasses.stream() + .map(Class::getDeclaredMethods) + .flatMap(Arrays::stream) + .filter(method -> method.isAnnotationPresent(SectionComments.class)) + .collect(Collectors.toList()); + } + + /** + * Returns all comments returned from the given SectionComments method, flattened into one list. + * + * @param sectionCommentsMethod the method whose comments should be retrieved + * @return flattened list of all comments provided by the method + */ + private static List getSectionComments(Method sectionCommentsMethod) { + // @SectionComments methods are static + Map comments = ReflectionTestUtils.invokeMethod(sectionCommentsMethod, null); + return comments.values().stream() + .flatMap(Arrays::stream) + .collect(Collectors.toList()); + } } diff --git a/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java b/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java index a88dbfb9a..d0d741359 100644 --- a/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java +++ b/src/test/java/fr/xephi/authme/settings/properties/SettingsClassConsistencyTest.java @@ -1,6 +1,7 @@ package fr.xephi.authme.settings.properties; import ch.jalu.configme.SettingsHolder; +import ch.jalu.configme.configurationdata.ConfigurationData; import ch.jalu.configme.properties.Property; import fr.xephi.authme.ClassCollector; import fr.xephi.authme.ReflectionTestUtils; @@ -10,11 +11,13 @@ import org.junit.Test; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; @@ -74,6 +77,27 @@ public class SettingsClassConsistencyTest { } } + /** + * Checks that {@link AuthMeSettingsRetriever} returns a ConfigurationData with all + * available SettingsHolder classes. + */ + @Test + public void shouldHaveAllClassesInConfigurationData() { + // given + long totalProperties = classes.stream() + .map(Class::getDeclaredFields) + .flatMap(Arrays::stream) + .filter(field -> Property.class.isAssignableFrom(field.getType())) + .count(); + + // when + ConfigurationData configData = AuthMeSettingsRetriever.buildConfigurationData(); + + // then + assertThat("ConfigurationData should have " + totalProperties + " properties (as found manually)", + configData.getProperties(), hasSize((int) totalProperties)); + } + @Test public void shouldHaveHiddenEmptyConstructorOnly() { for (Class clazz : classes) {