From f55bacc83d48c479338b0feaad59b1eab0a93998 Mon Sep 17 00:00:00 2001 From: Ben Woo <30431861+benwoo1110@users.noreply.github.com> Date: Wed, 11 Dec 2024 17:22:40 +0800 Subject: [PATCH 1/3] Improve error handling for string parser --- .../functions/DefaultStringParserProvider.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/mvplugins/multiverse/core/configuration/functions/DefaultStringParserProvider.java b/src/main/java/org/mvplugins/multiverse/core/configuration/functions/DefaultStringParserProvider.java index b300c3527..a7d19341a 100644 --- a/src/main/java/org/mvplugins/multiverse/core/configuration/functions/DefaultStringParserProvider.java +++ b/src/main/java/org/mvplugins/multiverse/core/configuration/functions/DefaultStringParserProvider.java @@ -2,10 +2,11 @@ import java.util.HashMap; import java.util.Map; -import java.util.Objects; import co.aikar.commands.ACFUtil; +import io.vavr.control.Option; import io.vavr.control.Try; +import org.mvplugins.multiverse.core.exceptions.MultiverseException; /** * Provides default string parsers for common types. @@ -50,19 +51,19 @@ public static NodeStringParser getDefaultStringParser(Class clazz) { private static final NodeStringParser INTEGER_STRING_PARSER = (input, type) -> Try.of( () -> ACFUtil.parseInt(input)) - .andThenTry(number -> Objects.requireNonNull(number, "Unable to convert '" + input + "' to number. (integer)")); + .flatMap(number -> Option.of(number).toTry(() -> new MultiverseException("Unable to convert '" + input + "' to number. (integer)"))); private static final NodeStringParser DOUBLE_STRING_PARSER = (input, type) -> Try.of( () -> ACFUtil.parseDouble(input)) - .andThenTry(number -> Objects.requireNonNull(number, "Unable to convert '" + input + "' to number. (double)")); + .flatMap(number -> Option.of(number).toTry(() -> new MultiverseException("Unable to convert '" + input + "' to number. (double)"))); private static final NodeStringParser FLOAT_STRING_PARSER = (input, type) -> Try.of( () -> ACFUtil.parseFloat(input)) - .andThenTry(number -> Objects.requireNonNull(number, "Unable to convert '" + input + "' to number. (float)")); + .flatMap(number -> Option.of(number).toTry(() -> new MultiverseException("Unable to convert '" + input + "' to number. (float)"))); private static final NodeStringParser LONG_STRING_PARSER = (input, type) -> Try.of( () -> ACFUtil.parseLong(input)) - .andThenTry(number -> Objects.requireNonNull(number, "Unable to convert '" + input + "' to number. (long)")); + .flatMap(number -> Option.of(number).toTry(() -> new MultiverseException("Unable to convert '" + input + "' to number. (long)"))); static { addDefaultStringParser(String.class, STRING_STRING_PARSER); From 80ad78802857aedea6f416c905105a8a8aacf638 Mon Sep 17 00:00:00 2001 From: Ben Woo <30431861+benwoo1110@users.noreply.github.com> Date: Wed, 11 Dec 2024 17:23:19 +0800 Subject: [PATCH 2/3] Abstract common action for command testing to a base class --- .../core/commands/BaseCommandTest.kt | 44 +++++++++++++++++++ .../core/commands/VersionCommandTest.kt | 30 +++++++------ 2 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 src/test/java/org/mvplugins/multiverse/core/commands/BaseCommandTest.kt diff --git a/src/test/java/org/mvplugins/multiverse/core/commands/BaseCommandTest.kt b/src/test/java/org/mvplugins/multiverse/core/commands/BaseCommandTest.kt new file mode 100644 index 000000000..e2e1409b7 --- /dev/null +++ b/src/test/java/org/mvplugins/multiverse/core/commands/BaseCommandTest.kt @@ -0,0 +1,44 @@ +package org.mvplugins.multiverse.core.commands + +import org.bukkit.ChatColor +import org.bukkit.command.ConsoleCommandSender +import org.bukkit.permissions.PermissionAttachment +import org.mockbukkit.mockbukkit.entity.PlayerMock +import org.mvplugins.multiverse.core.TestWithMockBukkit +import org.mvplugins.multiverse.core.commandtools.PluginLocales +import org.mvplugins.multiverse.core.utils.message.Message +import kotlin.test.BeforeTest +import kotlin.test.assertEquals +import kotlin.test.assertNotNull + +abstract class BaseCommandTest : TestWithMockBukkit() { + + protected lateinit var player: PlayerMock + protected lateinit var console: ConsoleCommandSender + + private lateinit var locales : PluginLocales + private lateinit var attachment : PermissionAttachment + + @BeforeTest + fun setUpCommand() { + locales = serviceLocator.getActiveService(PluginLocales::class.java).takeIf { it != null } ?: run { + throw IllegalStateException("PluginLocales is not available as a service") } + + console = server.consoleSender + player = server.addPlayer("benwoo1110"); + assertEquals(player, server.getPlayer("benwoo1110")) + attachment = player.addAttachment(multiverseCore) + assertNotNull(attachment) + } + + fun addPermission(permission: String) { + attachment.setPermission(permission, true) + } + + fun assertCommandOutput(message : Message) { + assertEquals( + ChatColor.stripColor(ChatColor.translateAlternateColorCodes('&',message.formatted(locales))), + ChatColor.stripColor(player.nextMessage()) + ) + } +} diff --git a/src/test/java/org/mvplugins/multiverse/core/commands/VersionCommandTest.kt b/src/test/java/org/mvplugins/multiverse/core/commands/VersionCommandTest.kt index e642e883b..e47644e1f 100644 --- a/src/test/java/org/mvplugins/multiverse/core/commands/VersionCommandTest.kt +++ b/src/test/java/org/mvplugins/multiverse/core/commands/VersionCommandTest.kt @@ -1,23 +1,16 @@ package org.mvplugins.multiverse.core.commands +import co.aikar.commands.MessageKeys import org.bukkit.Bukkit import org.bukkit.ChatColor -import org.mockbukkit.mockbukkit.entity.PlayerMock -import org.mvplugins.multiverse.core.TestWithMockBukkit -import kotlin.test.BeforeTest +import org.mvplugins.multiverse.core.utils.MVCorei18n +import org.mvplugins.multiverse.core.utils.message.Message +import org.mvplugins.multiverse.core.utils.message.MessageReplacement.replace import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertTrue -class VersionCommandTest : TestWithMockBukkit() { - - private lateinit var player: PlayerMock - - @BeforeTest - fun setUp() { - player = server.addPlayer("benwoo1110"); - assertEquals(player, server.getPlayer("benwoo1110")) - } +class VersionCommandTest : BaseCommandTest() { @Test fun `Run version command as console`() { @@ -29,8 +22,17 @@ class VersionCommandTest : TestWithMockBukkit() { @Test fun `Run version command as player`() { + addPermission("multiverse.core.version") + assertTrue(player.performCommand("mv version")) + assertCommandOutput(Message.of( + MVCorei18n.VERSION_MV, + "", + replace("{version}").with(multiverseCore.getDescription().getVersion()))) + } + + @Test + fun `Run version command as player - no permission`() { assertTrue(player.performCommand("mv version")) - val output = ChatColor.stripColor(player.nextMessage()) - assertEquals("I'm sorry, but you do not have permission to perform this command.", output) + assertCommandOutput(Message.of(MessageKeys.PERMISSION_DENIED, "")) } } From 0700d481d12502ae802397bd06ea3038f57cf713 Mon Sep 17 00:00:00 2001 From: Ben Woo <30431861+benwoo1110@users.noreply.github.com> Date: Wed, 11 Dec 2024 17:23:34 +0800 Subject: [PATCH 3/3] Add test for config command --- .../core/commands/ConfigCommandTest.kt | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 src/test/java/org/mvplugins/multiverse/core/commands/ConfigCommandTest.kt diff --git a/src/test/java/org/mvplugins/multiverse/core/commands/ConfigCommandTest.kt b/src/test/java/org/mvplugins/multiverse/core/commands/ConfigCommandTest.kt new file mode 100644 index 000000000..b5f10c1da --- /dev/null +++ b/src/test/java/org/mvplugins/multiverse/core/commands/ConfigCommandTest.kt @@ -0,0 +1,51 @@ +package org.mvplugins.multiverse.core.commands + +import co.aikar.commands.MessageKeys +import org.bukkit.Bukkit +import org.junit.jupiter.api.Test +import org.mvplugins.multiverse.core.config.MVCoreConfig +import org.mvplugins.multiverse.core.utils.MVCorei18n +import org.mvplugins.multiverse.core.utils.message.Message +import org.mvplugins.multiverse.core.utils.message.MessageReplacement.replace +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class ConfigCommandTest : BaseCommandTest() { + + @Test + fun `Modify config global-debug`() { + assertTrue(Bukkit.dispatchCommand(console, "mv config global-debug 2")) + val coreConfig = serviceLocator.getActiveService(MVCoreConfig::class.java).takeIf { it != null } ?: run { + throw IllegalStateException("MVCoreConfig is not available as a service") } + assertEquals(2, coreConfig.globalDebug) + } + + @Test + fun `Modify config global-debug player no permission`() { + assertTrue(Bukkit.dispatchCommand(player, "mv config global-debug 2")) + assertCommandOutput(Message.of(MessageKeys.PERMISSION_DENIED, "")) + + val coreConfig = serviceLocator.getActiveService(MVCoreConfig::class.java).takeIf { it != null } ?: run { + throw IllegalStateException("MVCoreConfig is not available as a service") } + assertEquals(0, coreConfig.globalDebug) + } + + @Test + fun `Modify non-existing config property`() { + addPermission("multiverse.core.config") + assertTrue(Bukkit.dispatchCommand(player, "mv config invalid-property test")) + player.nextMessage() // ignore the first line + assertCommandOutput(Message.of( + MVCorei18n.CONFIG_NODE_NOTFOUND, + "", + replace("{node}").with("invalid-property"))) + } + + @Test + fun `Modify config global-debug invalid type`() { + addPermission("multiverse.core.config") + assertTrue(Bukkit.dispatchCommand(player, "mv config global-debug what")) + player.nextMessage() // ignore the first line + assertCommandOutput(Message.of("Unable to convert 'what' to number. (integer)")) // todo: localize + } +}