From 1c124ce74fedeaa9aae8485bee0cac679aa456aa Mon Sep 17 00:00:00 2001 From: Norbert Dejlich Date: Mon, 16 Dec 2024 21:08:27 +0100 Subject: [PATCH] GH-499 Fix argument range validation while parsing. (#499) --- buildSrc/src/main/kotlin/Versions.kt | 2 +- .../kotlin/litecommands-unit-test.gradle.kts | 12 ++--- litecommands-bukkit/build.gradle.kts | 4 +- .../bukkit/argument/BukkitTestSpec.java | 15 ++++++ .../bukkit/argument/LocationArgumentTest.java | 54 +++++++++++++++++++ .../argument/parser/ParserRegistryImpl.java | 17 +++++- .../rollczi/litecommands/unit/TestUtil.java | 4 ++ settings.gradle.kts | 2 +- 8 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/BukkitTestSpec.java create mode 100644 litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/LocationArgumentTest.java diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt index 926ffa919..4b86b49fa 100644 --- a/buildSrc/src/main/kotlin/Versions.kt +++ b/buildSrc/src/main/kotlin/Versions.kt @@ -6,7 +6,7 @@ object Versions { // Tests const val JUNIT_JUPITER = "5.11.3" const val ASSERTJ = "3.26.3" - const val MOCKITO = "5.14.2" + const val MOCKITO = "4.11.0" const val AWAITILITY = "4.2.2" // Bukkit diff --git a/buildSrc/src/main/kotlin/litecommands-unit-test.gradle.kts b/buildSrc/src/main/kotlin/litecommands-unit-test.gradle.kts index f1a1a9311..38485ff2b 100644 --- a/buildSrc/src/main/kotlin/litecommands-unit-test.gradle.kts +++ b/buildSrc/src/main/kotlin/litecommands-unit-test.gradle.kts @@ -11,12 +11,12 @@ dependencies { testImplementation(project(":litecommands-unit")) testImplementation(kotlin("stdlib-jdk8")) - testImplementation("org.mockito:mockito-core:5.14.2") - testImplementation("org.junit.jupiter:junit-jupiter-api:5.11.3") - testImplementation("org.junit.jupiter:junit-jupiter-params:5.11.3") - testImplementation("org.assertj:assertj-core:3.26.3") - testImplementation("org.awaitility:awaitility:4.2.2") - testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.11.3") + testImplementation("org.mockito:mockito-core:${Versions.MOCKITO}") + testImplementation("org.junit.jupiter:junit-jupiter-api:${Versions.JUNIT_JUPITER}") + testImplementation("org.junit.jupiter:junit-jupiter-params:${Versions.JUNIT_JUPITER}") + testImplementation("org.assertj:assertj-core:${Versions.ASSERTJ}") + testImplementation("org.awaitility:awaitility:${Versions.AWAITILITY}") + testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${Versions.JUNIT_JUPITER}") } tasks.getByName("test") { diff --git a/litecommands-bukkit/build.gradle.kts b/litecommands-bukkit/build.gradle.kts index 174c58881..2963ab7bd 100644 --- a/litecommands-bukkit/build.gradle.kts +++ b/litecommands-bukkit/build.gradle.kts @@ -1,6 +1,7 @@ plugins { `litecommands-java` `litecommands-java-8` + `litecommands-unit-test` `litecommands-repositories` `litecommands-publish` } @@ -11,10 +12,9 @@ dependencies { compileOnly("org.spigotmc:spigot-api:${Versions.SPIGOT_API}") compileOnly("org.spigotmc:spigot:${Versions.SPIGOT}") compileOnly("com.comphenix.protocol:ProtocolLib:${Versions.PROTOCOL_LIB}") + testImplementation("org.spigotmc:spigot-api:${Versions.SPIGOT_API}") } -val bukkitArtifact: String by extra - litecommandsPublish { artifactId = "litecommands-bukkit" } \ No newline at end of file diff --git a/litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/BukkitTestSpec.java b/litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/BukkitTestSpec.java new file mode 100644 index 000000000..64d8243f7 --- /dev/null +++ b/litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/BukkitTestSpec.java @@ -0,0 +1,15 @@ +package dev.rollczi.litecommands.bukkit.argument; + +import dev.rollczi.litecommands.invocation.Invocation; +import dev.rollczi.litecommands.unit.TestUtil; +import org.bukkit.command.CommandSender; +import org.mockito.Mockito; + +public class BukkitTestSpec { + + protected static Invocation invocation(String command, String... args) { + CommandSender sender = Mockito.mock(CommandSender.class); + return TestUtil.invocation(sender, command, args); + } + +} diff --git a/litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/LocationArgumentTest.java b/litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/LocationArgumentTest.java new file mode 100644 index 000000000..dbd672121 --- /dev/null +++ b/litecommands-bukkit/test/dev/rollczi/litecommands/bukkit/argument/LocationArgumentTest.java @@ -0,0 +1,54 @@ +package dev.rollczi.litecommands.bukkit.argument; + +import dev.rollczi.litecommands.argument.Argument; +import dev.rollczi.litecommands.argument.ArgumentKey; +import dev.rollczi.litecommands.argument.parser.ParserRegistryImpl; +import dev.rollczi.litecommands.argument.resolver.optional.OptionalArgumentResolver; +import dev.rollczi.litecommands.input.raw.RawInput; +import dev.rollczi.litecommands.invocation.Invocation; +import dev.rollczi.litecommands.message.MessageRegistry; +import dev.rollczi.litecommands.reflect.type.TypeRange; +import dev.rollczi.litecommands.reflect.type.TypeToken; +import dev.rollczi.litecommands.requirement.RequirementResult; +import java.util.Optional; +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; +import org.bukkit.Location; +import org.bukkit.command.CommandSender; +import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class LocationArgumentTest extends BukkitTestSpec { + + + public static final Argument> ARGUMENT = Argument.of("location", new TypeToken>() {}); + private ParserRegistryImpl parsers = new ParserRegistryImpl<>(); + + @BeforeEach + void before() { + parsers.registerParser(Location.class, ArgumentKey.DEFAULT, new LocationArgument(new MessageRegistry<>())); + parsers.registerParser(TypeRange.same(Optional.class), ArgumentKey.DEFAULT, new OptionalArgumentResolver<>()); + } + + @Test + void test() { + Invocation invocation = invocation("test", "pos1"); + RequirementResult> result = parsers.parse(invocation, ARGUMENT, RawInput.of("1", "2", "3")) + .await(); + + assertTrue(result.isSuccessful()); + assertThat(result.getSuccess()) + .hasValue(new Location(null, 1, 2, 3)); + } + + @Test + void testFail() { + Invocation invocation = invocation("test", "pos1"); + RequirementResult> result = parsers.parse(invocation, ARGUMENT, RawInput.of("pos1")) + .await(); + + assertTrue(result.isFailed()); + } + + +} diff --git a/litecommands-core/src/dev/rollczi/litecommands/argument/parser/ParserRegistryImpl.java b/litecommands-core/src/dev/rollczi/litecommands/argument/parser/ParserRegistryImpl.java index d45f6aa58..997a0e057 100644 --- a/litecommands-core/src/dev/rollczi/litecommands/argument/parser/ParserRegistryImpl.java +++ b/litecommands-core/src/dev/rollczi/litecommands/argument/parser/ParserRegistryImpl.java @@ -3,7 +3,9 @@ import dev.rollczi.litecommands.argument.Argument; import dev.rollczi.litecommands.argument.ArgumentKey; import dev.rollczi.litecommands.input.raw.RawInput; +import dev.rollczi.litecommands.invalidusage.InvalidUsage; import dev.rollczi.litecommands.invocation.Invocation; +import dev.rollczi.litecommands.range.Range; import dev.rollczi.litecommands.reflect.type.TypeRange; import dev.rollczi.litecommands.reflect.type.TypeIndex; import java.util.ArrayList; @@ -11,6 +13,7 @@ import java.util.List; import java.util.Map; +import java.util.Optional; import org.jetbrains.annotations.NotNull; public class ParserRegistryImpl implements ParserRegistry, ParserChainAccessor { @@ -94,8 +97,20 @@ public Parser getParserOrNull(Argument argument) { @Override public ParseResult parse(Invocation invocation, Argument argument, RawInput input) { Parser parser = getParser(argument); + Range range = parser.getRange(argument); - return parser.parse(invocation, argument, input); + if (range.isInRangeOrAbove(input.size())) { + return parser.parse(invocation, argument, input); + } + + if (!input.hasNext()) { + Optional> optional = argument.getDefaultValue(); + + return optional + .orElseGet(() -> ParseResult.failure(InvalidUsage.Cause.MISSING_ARGUMENT)); + } + + return ParseResult.failure(InvalidUsage.Cause.MISSING_PART_OF_ARGUMENT); } } diff --git a/litecommands-unit/src/dev/rollczi/litecommands/unit/TestUtil.java b/litecommands-unit/src/dev/rollczi/litecommands/unit/TestUtil.java index 141fb122f..2a22917ea 100644 --- a/litecommands-unit/src/dev/rollczi/litecommands/unit/TestUtil.java +++ b/litecommands-unit/src/dev/rollczi/litecommands/unit/TestUtil.java @@ -12,4 +12,8 @@ public static Invocation invocation(String command, String... args) return new Invocation<>(new TestSender(), new TestPlatformSender(), command, command, ParseableInput.raw(args)); } + public static Invocation invocation(SENDER sender, String command, String... args) { + return new Invocation<>(sender, new TestPlatformSender(), command, command, ParseableInput.raw(args)); + } + } diff --git a/settings.gradle.kts b/settings.gradle.kts index 747ef348b..378167344 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -32,7 +32,7 @@ include(":litecommands-jakarta", VERSION_11) // platforms include(":litecommands-velocity", VERSION_11, tests = false) include(":litecommands-bungee", tests = false) -include(":litecommands-bukkit", tests = false) +include(":litecommands-bukkit") include(":litecommands-minestom", VERSION_21) include("litecommands-jda", VERSION_11) include(":litecommands-sponge", VERSION_21, tests = false)