From 6e6d34905ac39bd2ed9548729041febadd42da2f Mon Sep 17 00:00:00 2001 From: yoyosource Date: Thu, 1 Sep 2022 17:34:49 +0200 Subject: [PATCH 1/4] Fix AbstractSWCommand register --- src/de/steamwar/command/AbstractSWCommand.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/de/steamwar/command/AbstractSWCommand.java b/src/de/steamwar/command/AbstractSWCommand.java index e30ac7b..c23fd91 100644 --- a/src/de/steamwar/command/AbstractSWCommand.java +++ b/src/de/steamwar/command/AbstractSWCommand.java @@ -144,16 +144,20 @@ public abstract class AbstractSWCommand { for (Method method : methods) { add(Register.class, method, i -> i > 0, true, null, (anno, parameters) -> { if (!anno.help()) return; + boolean error = false; if (parameters.length != 2) { commandSystemWarning(() -> "The method '" + method.toString() + "' is lacking parameters or has too many"); + error = true; } if (!parameters[parameters.length - 1].isVarArgs()) { commandSystemWarning(() -> "The method '" + method.toString() + "' is lacking the varArgs parameters as last Argument"); + error = true; } if (parameters[parameters.length - 1].getType().getComponentType() != String.class) { commandSystemWarning(() -> "The method '" + method.toString() + "' is lacking the varArgs parameters of type '" + String.class.getTypeName() + "' as last Argument"); - return; + error = true; } + if (error) return; commandHelpList.add(new SubCommand<>(this, method, anno.value(), new HashMap<>(), new HashMap<>(), true, null, anno.noTabComplete())); }); From c4a8ae4c9f5bf54a6f76360136e7cb4be5e42e9b Mon Sep 17 00:00:00 2001 From: yoyosource Date: Sun, 4 Sep 2022 21:25:23 +0200 Subject: [PATCH 2/4] Update some stuff --- src/de/steamwar/command/CommandPart.java | 1 + src/de/steamwar/command/SubCommand.java | 7 +++++-- testsrc/de/steamwar/command/SimpleCommand.java | 6 ++++++ testsrc/de/steamwar/command/SimpleCommandTest.java | 10 ++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/de/steamwar/command/CommandPart.java b/src/de/steamwar/command/CommandPart.java index 80f0048..e3c7663 100644 --- a/src/de/steamwar/command/CommandPart.java +++ b/src/de/steamwar/command/CommandPart.java @@ -23,6 +23,7 @@ import lombok.AllArgsConstructor; import lombok.Setter; import java.lang.reflect.Array; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; diff --git a/src/de/steamwar/command/SubCommand.java b/src/de/steamwar/command/SubCommand.java index 9a497e5..71a4945 100644 --- a/src/de/steamwar/command/SubCommand.java +++ b/src/de/steamwar/command/SubCommand.java @@ -46,6 +46,11 @@ public class SubCommand { SubCommand(AbstractSWCommand abstractSWCommand, Method method, String[] subCommand, Map> localTypeMapper, Map> localValidator, boolean help, String[] description, boolean noTabComplete) { this.abstractSWCommand = abstractSWCommand; this.method = method; + try { + this.method.setAccessible(true); + } catch (SecurityException e) { + throw new SecurityException(e.getMessage(), e); + } this.subCommand = subCommand; this.description = description; this.noTabComplete = noTabComplete; @@ -78,7 +83,6 @@ public class SubCommand { return false; } } - method.setAccessible(true); method.invoke(abstractSWCommand, senderFunction.apply(sender)); } else { List objects = new ArrayList<>(); @@ -91,7 +95,6 @@ public class SubCommand { } } objects.add(0, senderFunction.apply(sender)); - method.setAccessible(true); method.invoke(abstractSWCommand, objects.toArray()); } } catch (CommandNoHelpException e) { diff --git a/testsrc/de/steamwar/command/SimpleCommand.java b/testsrc/de/steamwar/command/SimpleCommand.java index 16d565f..89cd016 100644 --- a/testsrc/de/steamwar/command/SimpleCommand.java +++ b/testsrc/de/steamwar/command/SimpleCommand.java @@ -19,6 +19,7 @@ package de.steamwar.command; +import de.steamwar.command.AbstractSWCommand.Register; import de.steamwar.command.dto.ExecutionIdentifier; import de.steamwar.command.dto.TestSWCommand; @@ -28,6 +29,11 @@ public class SimpleCommand extends TestSWCommand { super("simple"); } + @Register(help = true) + public void test(String s, String... varargs) { + throw new ExecutionIdentifier("RunSimple with Varargs"); + } + @Register public void simple(String s) { throw new ExecutionIdentifier("RunSimple with noArgs"); diff --git a/testsrc/de/steamwar/command/SimpleCommandTest.java b/testsrc/de/steamwar/command/SimpleCommandTest.java index dcb286c..aebd904 100644 --- a/testsrc/de/steamwar/command/SimpleCommandTest.java +++ b/testsrc/de/steamwar/command/SimpleCommandTest.java @@ -39,6 +39,16 @@ public class SimpleCommandTest { } } + @Test + public void testVarArgs() { + SimpleCommand cmd = new SimpleCommand(); + try { + cmd.execute("test", "", new String[] {"a", "b", "c"}); + } catch (Exception e) { + assertCMDFramework(e, ExecutionIdentifier.class, "RunSimple with Varargs"); + } + } + @Test public void testSimpleParsingNoResult() { SimpleCommand cmd = new SimpleCommand(); From 9df92595b2f344585bd3e1639cedc761680b7761 Mon Sep 17 00:00:00 2001 From: YoyoNow Date: Tue, 6 Sep 2022 10:39:45 +0200 Subject: [PATCH 3/4] =?UTF-8?q?=E2=80=9Etestsrc/de/steamwar/command/Simple?= =?UTF-8?q?Command.java=E2=80=9C=20=C3=A4ndern?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- testsrc/de/steamwar/command/SimpleCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsrc/de/steamwar/command/SimpleCommand.java b/testsrc/de/steamwar/command/SimpleCommand.java index 89cd016..95aa43d 100644 --- a/testsrc/de/steamwar/command/SimpleCommand.java +++ b/testsrc/de/steamwar/command/SimpleCommand.java @@ -29,7 +29,7 @@ public class SimpleCommand extends TestSWCommand { super("simple"); } - @Register(help = true) + @Register(value = "a", help = true) public void test(String s, String... varargs) { throw new ExecutionIdentifier("RunSimple with Varargs"); } From 9439cc81a56f964ec00037c8257a13838508682e Mon Sep 17 00:00:00 2001 From: yoyosource Date: Fri, 9 Sep 2022 20:48:52 +0200 Subject: [PATCH 4/4] Add BetterException handling for exceptions in tabComplete and validate since there now exception should be thrown there --- .../command/CommandFrameworkException.java | 76 ++++++++++++++----- src/de/steamwar/command/CommandPart.java | 44 ++++++----- src/de/steamwar/command/SWCommandUtils.java | 4 +- src/de/steamwar/command/SubCommand.java | 2 +- .../command/BetterExceptionCommand.java | 57 ++++++++++++++ .../command/BetterExceptionCommandTest.java | 53 +++++++++++++ 6 files changed, 196 insertions(+), 40 deletions(-) create mode 100644 testsrc/de/steamwar/command/BetterExceptionCommand.java create mode 100644 testsrc/de/steamwar/command/BetterExceptionCommandTest.java diff --git a/src/de/steamwar/command/CommandFrameworkException.java b/src/de/steamwar/command/CommandFrameworkException.java index 74db6d3..53ee025 100644 --- a/src/de/steamwar/command/CommandFrameworkException.java +++ b/src/de/steamwar/command/CommandFrameworkException.java @@ -21,39 +21,79 @@ package de.steamwar.command; import java.io.PrintStream; import java.io.PrintWriter; +import java.lang.reflect.Executable; import java.lang.reflect.InvocationTargetException; +import java.util.Arrays; +import java.util.function.Function; +import java.util.stream.Stream; public class CommandFrameworkException extends RuntimeException { - private InvocationTargetException invocationTargetException; - private String alias; - private String[] args; + private Function causeMessage; + private Throwable cause; + private Function stackTraceExtractor; + private String extraStackTraces; private String message; + static CommandFrameworkException commandGetExceptions(String type, Class clazzType, Executable executable, int index) { + return new CommandFrameworkException(throwable -> { + return CommandFrameworkException.class.getTypeName() + ": Error while getting " + type + " for " + clazzType.getTypeName() + " with parameter index " + index; + }, null, throwable -> Stream.empty(), executable.getDeclaringClass().getTypeName() + "." + executable.getName() + "(Unknown Source)"); + } + + static CommandFrameworkException commandPartExceptions(String type, Throwable cause, String current, Class clazzType, Executable executable, int index) { + return new CommandFrameworkException(e -> { + return CommandFrameworkException.class.getTypeName() + ": Error while " + type + " (" + current + ") to type " + clazzType.getTypeName() + " with parameter index " + index; + }, cause, exception -> { + StackTraceElement[] stackTraceElements = exception.getStackTrace(); + int last = 0; + for (int i = 0; i < stackTraceElements.length; i++) { + if (stackTraceElements[i].getClassName().equals(CommandPart.class.getTypeName())) { + last = i; + break; + } + } + return Arrays.stream(stackTraceElements).limit(last - 1); + }, executable.getDeclaringClass().getTypeName() + "." + executable.getName() + "(Unknown Source)"); + } + CommandFrameworkException(InvocationTargetException invocationTargetException, String alias, String[] args) { - super(invocationTargetException); - this.invocationTargetException = invocationTargetException; - this.alias = alias; - this.args = args; + this(e -> { + StringBuilder st = new StringBuilder(); + st.append(e.getCause().getClass().getTypeName()); + if (e.getCause().getMessage() != null) { + st.append(": ").append(e.getCause().getMessage()); + } + if (alias != null && !alias.isEmpty()) { + st.append("\n").append("Performed command: " + alias + " " + String.join(" ", args)); + } + return st.toString(); + }, invocationTargetException, e -> { + StackTraceElement[] stackTraceElements = e.getCause().getStackTrace(); + return Arrays.stream(stackTraceElements).limit(stackTraceElements.length - e.getStackTrace().length); + }, null); + } + + private CommandFrameworkException(Function causeMessage, T cause, Function> stackTraceExtractor, String extraStackTraces) { + super(causeMessage.apply(cause), cause); + this.causeMessage = causeMessage; + this.cause = cause; + this.stackTraceExtractor = stackTraceExtractor; + this.extraStackTraces = extraStackTraces; } public synchronized String getBuildStackTrace() { if (message != null) { return message; } - StackTraceElement[] stackTraceElements = invocationTargetException.getCause().getStackTrace(); StringBuilder st = new StringBuilder(); - st.append(invocationTargetException.getCause().getClass().getTypeName()); - if (invocationTargetException.getCause().getMessage() != null) { - st.append(": ").append(invocationTargetException.getCause().getMessage()); - } - st.append("\n"); - if (alias != null && !alias.isEmpty()) { - st.append("Performed command: ").append(alias).append(" ").append(String.join(" ", args)).append("\n"); - } - for (int i = 0; i < stackTraceElements.length - invocationTargetException.getStackTrace().length; i++) { - st.append("\tat ").append(stackTraceElements[i].toString()).append("\n"); + st.append(causeMessage.apply(cause)).append("\n"); + ((Stream) stackTraceExtractor.apply(cause)).forEach(stackTraceElement -> { + st.append("\tat ").append(stackTraceElement.toString()).append("\n"); + }); + if (extraStackTraces != null) { + st.append("\tat ").append(extraStackTraces).append("\n"); } message = st.toString(); return message; diff --git a/src/de/steamwar/command/CommandPart.java b/src/de/steamwar/command/CommandPart.java index e3c7663..9f21ec8 100644 --- a/src/de/steamwar/command/CommandPart.java +++ b/src/de/steamwar/command/CommandPart.java @@ -23,7 +23,7 @@ import lombok.AllArgsConstructor; import lombok.Setter; import java.lang.reflect.Array; -import java.util.ArrayList; +import java.lang.reflect.Parameter; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -56,34 +56,31 @@ class CommandPart { @Setter private boolean allowNullValues = false; - public CommandPart(AbstractSWCommand command, AbstractTypeMapper typeMapper, AbstractValidator validator, Class varArgType, String optional) { + private Parameter parameter; + private int parameterIndex; + + public CommandPart(AbstractSWCommand command, AbstractTypeMapper typeMapper, AbstractValidator validator, Class varArgType, String optional, Parameter parameter, int parameterIndex) { this.command = command; this.typeMapper = typeMapper; this.validator = validator; this.varArgType = varArgType; this.optional = optional; + this.parameter = parameter; + this.parameterIndex = parameterIndex; validatePart(); } public void setNext(CommandPart next) { if (varArgType != null) { - throw new IllegalArgumentException("There can't be a next part if this is a vararg part!"); + throw new IllegalArgumentException("There can't be a next part if this is a vararg part! In method " + parameter.getDeclaringExecutable() + " with parameter " + parameterIndex); } this.next = next; } private void validatePart() { if (optional != null && varArgType != null) { - throw new IllegalArgumentException("A vararg part can't have an optional part!"); - } - - if (optional != null) { - try { - typeMapper.map(null, EMPTY_ARRAY, optional); - } catch (Exception e) { - throw new IllegalArgumentException("The optional part is not valid!"); - } + throw new IllegalArgumentException("A vararg part can't have an optional part! In method " + parameter.getDeclaringExecutable() + " with parameter " + parameterIndex); } } @@ -114,7 +111,7 @@ class CommandPart { if (!ignoreAsArgument) { if (!onlyUseIfNoneIsGiven) { current.add(typeMapper.map(sender, EMPTY_ARRAY, optional)); - } else if(startIndex >= args.length) { + } else if (startIndex >= args.length) { current.add(typeMapper.map(sender, EMPTY_ARRAY, optional)); } else { throw new CommandParseException(); @@ -171,14 +168,23 @@ class CommandPart { private Collection tabCompletes(T sender, String[] args, int startIndex) { return TabCompletionCache.tabComplete(sender, typeMapper, command, () -> { - return typeMapper.tabCompletes(sender, Arrays.copyOf(args, startIndex), args[startIndex]); + try { + return typeMapper.tabCompletes(sender, Arrays.copyOf(args, startIndex), args[startIndex]); + } catch (Throwable e) { + throw CommandFrameworkException.commandPartExceptions("tabcompleting", e, args[startIndex], (varArgType != null ? varArgType : parameter.getType()), parameter.getDeclaringExecutable(), parameterIndex); + } }); } private CheckArgumentResult checkArgument(Consumer errors, T sender, String[] args, int index) { + Object value; try { - Object value = typeMapper.map(sender, Arrays.copyOf(args, index), args[index]); - if (validator != null && errors != null) { + value = typeMapper.map(sender, Arrays.copyOf(args, index), args[index]); + } catch (Exception e) { + return new CheckArgumentResult(false, null); + } + if (validator != null && errors != null) { + try { if (!validator.validate(sender, value, (s, objects) -> { errors.accept(() -> { command.sendMessage(sender, s, objects); @@ -186,10 +192,10 @@ class CommandPart { })) { return new CheckArgumentResult(false, null); } + } catch (Throwable e) { + throw CommandFrameworkException.commandPartExceptions("validating", e, args[index], (varArgType != null ? varArgType : parameter.getType()), parameter.getDeclaringExecutable(), parameterIndex); } - return new CheckArgumentResult(allowNullValues || value != null, value); - } catch (Exception e) { - return new CheckArgumentResult(false, null); } + return new CheckArgumentResult(allowNullValues || value != null, value); } } diff --git a/src/de/steamwar/command/SWCommandUtils.java b/src/de/steamwar/command/SWCommandUtils.java index f6aa8f3..32c46c4 100644 --- a/src/de/steamwar/command/SWCommandUtils.java +++ b/src/de/steamwar/command/SWCommandUtils.java @@ -78,7 +78,7 @@ public class SWCommandUtils { CommandPart first = null; CommandPart current = null; for (String s : subCommand) { - CommandPart commandPart = new CommandPart(command, createMapper(s), null, null, null); + CommandPart commandPart = new CommandPart(command, createMapper(s), null, null, null, null, -1); commandPart.setIgnoreAsArgument(true); if (current != null) { current.setNext(commandPart); @@ -96,7 +96,7 @@ public class SWCommandUtils { AbstractSWCommand.OptionalValue optionalValue = parameter.getAnnotation(AbstractSWCommand.OptionalValue.class); AbstractSWCommand.AllowNull allowNull = parameter.getAnnotation(AbstractSWCommand.AllowNull.class); - CommandPart commandPart = new CommandPart<>(command, typeMapper, validator, varArgType, optionalValue != null ? optionalValue.value() : null); + CommandPart commandPart = new CommandPart<>(command, typeMapper, validator, varArgType, optionalValue != null ? optionalValue.value() : null, parameter, i); commandPart.setOnlyUseIfNoneIsGiven(optionalValue != null && optionalValue.onlyUINIG()); commandPart.setAllowNullValues(allowNull != null); if (current != null) { diff --git a/src/de/steamwar/command/SubCommand.java b/src/de/steamwar/command/SubCommand.java index 71a4945..9eb6e4f 100644 --- a/src/de/steamwar/command/SubCommand.java +++ b/src/de/steamwar/command/SubCommand.java @@ -97,7 +97,7 @@ public class SubCommand { objects.add(0, senderFunction.apply(sender)); method.invoke(abstractSWCommand, objects.toArray()); } - } catch (CommandNoHelpException e) { + } catch (CommandNoHelpException | CommandFrameworkException e) { throw e; } catch (CommandParseException e) { return false; diff --git a/testsrc/de/steamwar/command/BetterExceptionCommand.java b/testsrc/de/steamwar/command/BetterExceptionCommand.java new file mode 100644 index 0000000..cd38179 --- /dev/null +++ b/testsrc/de/steamwar/command/BetterExceptionCommand.java @@ -0,0 +1,57 @@ +/* + * This file is a part of the SteamWar software. + * + * Copyright (C) 2022 SteamWar.de-Serverteam + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package de.steamwar.command; + +import de.steamwar.command.dto.TestSWCommand; +import de.steamwar.command.dto.TestTypeMapper; + +import java.util.Collection; + +public class BetterExceptionCommand extends TestSWCommand { + + public BetterExceptionCommand() { + super("betterexception"); + } + + @Register + public void exceptionOnTabComplete(String s, @Mapper("exception") @Validator("exception") String s1) { + } + + @Mapper("exception") + @Validator("exception") + public TestTypeMapper tabCompleteException() { + return new TestTypeMapper() { + @Override + public String map(String sender, String[] previousArguments, String s) { + return null; + } + + @Override + public boolean validate(String sender, String value, MessageSender messageSender) { + throw new SecurityException(); + } + + @Override + public Collection tabCompletes(String sender, String[] previousArguments, String s) { + throw new SecurityException(); + } + }; + } +} diff --git a/testsrc/de/steamwar/command/BetterExceptionCommandTest.java b/testsrc/de/steamwar/command/BetterExceptionCommandTest.java new file mode 100644 index 0000000..170ad60 --- /dev/null +++ b/testsrc/de/steamwar/command/BetterExceptionCommandTest.java @@ -0,0 +1,53 @@ +/* + * This file is a part of the SteamWar software. + * + * Copyright (C) 2022 SteamWar.de-Serverteam + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package de.steamwar.command; + +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; + +public class BetterExceptionCommandTest { + + @Test + public void testTabCompleteException() { + BetterExceptionCommand cmd = new BetterExceptionCommand(); + try { + cmd.tabComplete("test", "", new String[]{""}); + assert false; + } catch (Exception e) { + assertThat(e, is(instanceOf(CommandFrameworkException.class))); + assertThat(e.getMessage(), is("de.steamwar.command.CommandFrameworkException: Error while tabcompleting () to type java.lang.String with parameter index 1")); + } + } + + @Test + public void testValidationException() { + BetterExceptionCommand cmd = new BetterExceptionCommand(); + try { + cmd.execute("test", "", new String[]{""}); + assert false; + } catch (Exception e) { + assertThat(e, is(instanceOf(CommandFrameworkException.class))); + assertThat(e.getMessage(), is("de.steamwar.command.CommandFrameworkException: Error while validating () to type java.lang.String with parameter index 1")); + } + } +}