refactor the way verbose filters are parsed - tokenize on first init as opposed to on each check

common/src/main/java/me/lucko/luckperms/common/commands/impl/misc/VerboseCommand.java

@@ -36,6 +36,7 @@ import me.lucko.luckperms.common.locale.LocaleManager;
 import me.lucko.luckperms.common.locale.Message;
 import me.lucko.luckperms.common.plugin.LuckPermsPlugin;
 import me.lucko.luckperms.common.utils.Predicates;
+import me.lucko.luckperms.common.verbose.InvalidFilterException;
 import me.lucko.luckperms.common.verbose.VerboseFilter;
 import me.lucko.luckperms.common.verbose.VerboseListener;
 
@@ -75,14 +76,18 @@ public class VerboseCommand extends SingleCommand {
 
             String filter = filters.isEmpty() ? "" : filters.stream().collect(Collectors.joining(" "));
 
-            if (!VerboseFilter.isValidFilter(filter)) {
+            VerboseFilter parsedFilter;
+            try {
+                parsedFilter = VerboseFilter.parse(filter);
+            } catch (InvalidFilterException e) {
+                e.printStackTrace();
                 Message.VERBOSE_INVALID_FILTER.send(sender, filter);
                 return CommandResult.FAILURE;
             }
 
             boolean notify = !mode.equals("record");
 
-            plugin.getVerboseHandler().registerListener(sender, filter, notify);
+            plugin.getVerboseHandler().registerListener(sender, parsedFilter, notify);
 
             if (notify) {
                 if (!filter.equals("")) {

common/src/main/java/me/lucko/luckperms/common/verbose/CheckOrigin.java

@@ -33,30 +33,21 @@ public enum CheckOrigin {
     /**
      * Indicates the check was caused by a 'hasPermission' check on the platform
      */
-    PLATFORM_PERMISSION_CHECK('C'),
+    PLATFORM_PERMISSION_CHECK,
 
     /**
      * Indicates the check was caused by a 'hasPermissionSet' type check on the platform
      */
-    PLATFORM_LOOKUP_CHECK('L'),
+    PLATFORM_LOOKUP_CHECK,
 
     /**
      * Indicates the check was caused by an API call
      */
-    API('A'),
+    API,
 
     /**
      * Indicates the check was caused by a LuckPerms internal
      */
-    INTERNAL('I');
+    INTERNAL
 
-    private final char code;
-
-    CheckOrigin(char code) {
-        this.code = code;
-    }
-
-    public char getCode() {
-        return this.code;
-    }
 }

common/src/main/java/me/lucko/luckperms/common/verbose/InvalidFilterException.java

@@ -0,0 +1,38 @@
+/*
+ * This file is part of LuckPerms, licensed under the MIT License.
+ *
+ *  Copyright (c) lucko (Luck) <luck@lucko.me>
+ *  Copyright (c) contributors
+ *
+ *  Permission is hereby granted, free of charge, to any person obtaining a copy
+ *  of this software and associated documentation files (the "Software"), to deal
+ *  in the Software without restriction, including without limitation the rights
+ *  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ *  copies of the Software, and to permit persons to whom the Software is
+ *  furnished to do so, subject to the following conditions:
+ *
+ *  The above copyright notice and this permission notice shall be included in all
+ *  copies or substantial portions of the Software.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ *  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ *  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ *  SOFTWARE.
+ */
+
+package me.lucko.luckperms.common.verbose;
+
+/**
+ * Exception thrown when attempting to compile a {@link VerboseFilter}
+ * using an invalid filter string.
+ */
+public class InvalidFilterException extends Exception {
+
+    public InvalidFilterException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+}

common/src/main/java/me/lucko/luckperms/common/verbose/VerboseFilter.java

@@ -25,141 +25,266 @@
 
 package me.lucko.luckperms.common.verbose;
 
+import com.google.common.collect.ImmutableList;
+
 import me.lucko.luckperms.common.utils.Scripting;
 
+import java.util.List;
 import java.util.StringTokenizer;
+import java.util.stream.Collectors;
 
 import javax.script.ScriptEngine;
+import javax.script.ScriptException;
 
 /**
- * Tests verbose filters
+ * Represents a verbose filter expression.
+ *
+ * <p>The filter is parsed when the instance is initialised - subsequent
+ * evaluations should be relatively fast.</p>
  */
 public final class VerboseFilter {
 
+    // the characters used in an expression which are part of the expression
+    // syntax - and not the filter itself.
+    private static final String DELIMITERS = " |&()!";
+
+    // the script engine to use when evaluating the expression
+    private final ScriptEngine engine;
+    // the parsed expression
+    private final List<Token> expression;
+
     /**
-     * Evaluates whether the passed check data passes the filter
+     * Compiles a {@link VerboseFilter} instance for the given filter string
+     *
+     * @param filter the filter
+     * @return a filter
+     * @throws InvalidFilterException if the filter is invalid
+     */
+    public static VerboseFilter parse(String filter) throws InvalidFilterException {
+        ScriptEngine engine = Scripting.getScriptEngine();
+        if (engine == null) {
+            throw new RuntimeException("Script engine not present");
+        }
+
+        return new VerboseFilter(engine, filter);
+    }
+
+    private VerboseFilter(ScriptEngine engine, String filter) throws InvalidFilterException {
+        this.engine = engine;
+
+        if (filter.isEmpty()) {
+            this.expression = ImmutableList.of();
+        } else {
+            try {
+                this.expression = generateExpression(engine, filter);
+            } catch (Exception e) {
+                throw new InvalidFilterException("Exception occurred whilst generating an expression for '" + filter + "'", e);
+            }
+        }
+    }
+
+    /**
+     * Parses a filter string into a list of 'tokens' forming the expression.
+     *
+     * Each token either represents part of the expressions syntax
+     * (logical and, logical or, brackets, or space) or a value.
+     *
+     * @param engine the script engine to test the expression with
+     * @param filter the filter string
+     * @return a parsed list of expressions
+     * @throws ScriptException if the engine throws an exception whilst evaluating the expression
+     */
+    private static List<Token> generateExpression(ScriptEngine engine, String filter) throws ScriptException {
+        // tokenize the filter using the filter characters as delimiters.
+        StringTokenizer tokenizer = new StringTokenizer(filter, DELIMITERS, true);
+
+        // use the tokenizer to parse the string to a list of 'tokens'.
+        ImmutableList.Builder<Token> expressionBuilder = ImmutableList.builder();
+        while (tokenizer.hasMoreTokens()) {
+            String token = tokenizer.nextToken();
+
+            if (isDelimiter(token)) {
+                // if the token is a delimiter, just append it to the expression as a constant
+                expressionBuilder.add(new ConstantToken(token));
+            } else {
+                // otherwise consider it to be a value
+                expressionBuilder.add(new VariableToken(token));
+            }
+        }
+
+        // build & test the expression
+        List<Token> expression = expressionBuilder.build();
+        testExpression(expression, engine);
+        return expression;
+    }
+
+    private static void testExpression(List<Token> expression, ScriptEngine engine) throws ScriptException {
+        // build a dummy version of the expression.
+        // all values are simply replaced by "true"
+        String dummyExpression = expression.stream().map(Token::forDummyExpression).collect(Collectors.joining());
+
+        // do a test run - if the engine returns a result without throwing an exception
+        // and the result is a boolean, we can consider the expression to be valid
+        String result = engine.eval(dummyExpression).toString();
+        if (!result.equals("true") && !result.equals("false")) {
+            throw new IllegalArgumentException("Expected true/false but got '" + result + "' instead.");
+        }
+    }
+
+    /**
+     * Evaluates whether the check data passes the filter
      *
      * @param data the check data
-     * @param filter the filter
      * @return if the check data passes the filter
      */
-    public static boolean passesFilter(CheckData data, String filter) {
+    public boolean evaluate(CheckData data) {
-        if (filter.equals("")) {
+        if (this.expression.isEmpty()) {
             return true;
         }
 
-        // get the script engine
+        // build an expression string for the passed check data.
-        ScriptEngine engine = Scripting.getScriptEngine();
+        String expressionString = this.expression.stream().map(token -> token.forExpression(data)).collect(Collectors.joining());
-        if (engine == null) {
-            return false;
-        }
-
-        // tokenize the filter
-        StringTokenizer tokenizer = new StringTokenizer(filter, " |&()!", true);
-
-        // build an expression which can be evaluated by the javascript engine
-        StringBuilder expressionBuilder = new StringBuilder();
-
-        // read the tokens
-        while (tokenizer.hasMoreTokens()) {
-            String token = tokenizer.nextToken();
-
-            // if the token is a delimiter, just append it to the expression
-            if (isDelim(token)) {
-                expressionBuilder.append(token);
-
-            } else {
-
-                // if the token is not a delimiter, it must be a string.
-                // we replace non-delimiters with a boolean depending on if the string matches the check data.
-                boolean value = data.getCheckTarget().equalsIgnoreCase(token) ||
-                        data.getPermission().toLowerCase().startsWith(token.toLowerCase()) ||
-                        data.getResult().name().equalsIgnoreCase(token);
-
-                expressionBuilder.append(value);
-            }
-        }
-
-        // build the expression
-        String expression = expressionBuilder.toString().replace("&", "&&").replace("|", "||");
 
         // evaluate the expression using the script engine
         try {
-            String result = engine.eval(expression).toString();
+            String result = this.engine.eval(expressionString).toString();
+
+            // validate return value
             if (!result.equals("true") && !result.equals("false")) {
-                throw new IllegalArgumentException(expression + " - " + result);
+                throw new IllegalArgumentException("Expected true/false but got '" + result + "' instead.");
             }
 
+            // return the result of the expression
             return Boolean.parseBoolean(result);
 
-        } catch (Throwable t) {
+        } catch (Throwable ex) {
-            t.printStackTrace();
+            // print the error & return false
+            ex.printStackTrace();
+            return false;
         }
+    }
 
-        return false;
+    public boolean isBlank() {
+        return this.expression.isEmpty();
+    }
+
+    @Override
+    public String toString() {
+        return this.expression.stream().map(Token::toString).collect(Collectors.joining());
     }
 
     /**
-     * Tests whether a filter is valid
+     * Returns true if the string is equal to one of the {@link #DELIMITERS}.
      *
-     * @param filter the filter to test
+     * @param string the string
-     * @return true if the filter is valid
+     * @return true if delimiter, false otherwise
      */
-    public static boolean isValidFilter(String filter) {
+    private static boolean isDelimiter(String string) {
-        if (filter.equals("")) {
+        switch (string.charAt(0)) {
-            return true;
+            case ' ':
-        }
+            case '|':
-
+            case '&':
-        // get the script engine
+            case '(':
-        ScriptEngine engine = Scripting.getScriptEngine();
+            case ')':
-        if (engine == null) {
+            case '!':
-            return false;
+                return true;
-        }
+            default:
-
+                return false;
-        // tokenize the filter
-        StringTokenizer tokenizer = new StringTokenizer(filter, " |&()!", true);
-
-        // build an expression which can be evaluated by the javascript engine
-        StringBuilder expressionBuilder = new StringBuilder();
-
-        // read the tokens
-        while (tokenizer.hasMoreTokens()) {
-            String token = tokenizer.nextToken();
-
-            // if the token is a delimiter, just append it to the expression
-            if (isDelim(token)) {
-                expressionBuilder.append(token);
-            } else {
-                expressionBuilder.append("true"); // dummy result
-            }
-        }
-
-        // build the expression
-        String expression = expressionBuilder.toString().replace("&", "&&").replace("|", "||");
-
-        // evaluate the expression using the script engine
-        try {
-            String result = engine.eval(expression).toString();
-            if (!result.equals("true") && !result.equals("false")) {
-                throw new IllegalArgumentException(expression + " - " + result);
-            }
-
-            return true;
-
-        } catch (Throwable t) {
-            return false;
         }
     }
 
-    private static boolean isDelim(String token) {
+    /**
-        return token.equals(" ") ||
+     * Represents a part of an expression
-                token.equals("|") ||
+     */
-                token.equals("&") ||
+    private interface Token {
-                token.equals("(") ||
+
-                token.equals(")") ||
+        /**
-                token.equals("!");
+         * Returns the value of this token when part of an evaluated expression
+         *
+         * @param data the data which an expression is being formed for
+         * @return the value to be used as part of the evaluated expression
+         */
+        String forExpression(CheckData data);
+
+        /**
+         * Returns a 'dummy' value for this token in order to build a test
+         * expression.
+         *
+         * @return the value to be used as part of the test expression
+         */
+        String forDummyExpression();
+
     }
 
-    private VerboseFilter() {}
+    /**
+     * Represents a constant part of the expression - tokens will only ever
+     * consist of the characters defined in the {@link #DELIMITERS} string.
+     */
+    private static final class ConstantToken implements Token {
+        private final String string;
+
+        private ConstantToken(String string) {
+            // replace single '&' and '|' character with double values
+            if (string.equals("&")) {
+                string = "&&";
+            } else if (string.equals("|")) {
+                string = "||";
+            }
+
+            this.string = string;
+        }
+
+        @Override
+        public String forExpression(CheckData data) {
+            return this.string;
+        }
+
+        @Override
+        public String forDummyExpression() {
+            return this.string;
+        }
+
+        @Override
+        public String toString() {
+            return this.string;
+        }
+    }
+
+    /**
+     * Represents a variable part of the token. When evaluated as an expression,
+     * this token will be replaced with a boolean 'true' or 'false' - depending
+     * on if the passed check data "matches" the value of this token.
+     *
+     * The check data will be deemed a "match" if:
+     * - the target of the check is equal to the value of the token
+     * - the permission being checked for starts with the value of the token
+     * - the result of the check is equal to the value of the token
+     */
+    private static final class VariableToken implements Token {
+        private final String value;
+
+        private VariableToken(String value) {
+            this.value = value;
+        }
+
+        @Override
+        public String forExpression(CheckData data) {
+            return Boolean.toString(
+                    data.getCheckTarget().equalsIgnoreCase(this.value) ||
+                    data.getPermission().toLowerCase().startsWith(this.value.toLowerCase()) ||
+                    data.getResult().name().equalsIgnoreCase(this.value)
+            );
+        }
+
+        @Override
+        public String forDummyExpression() {
+            return "true";
+        }
+
+        @Override
+        public String toString() {
+            return this.value;
+        }
+    }
 
 }

common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java

@@ -94,7 +94,7 @@ public class VerboseHandler implements Runnable {
      * @param filter the filter string
      * @param notify if the sender should be notified in chat on each check
      */
-    public void registerListener(Sender sender, String filter, boolean notify) {
+    public void registerListener(Sender sender, VerboseFilter filter, boolean notify) {
         this.listeners.put(sender.getUuid(), new VerboseListener(this.pluginVersion, sender, filter, notify));
         this.listening = true;
     }

common/src/main/java/me/lucko/luckperms/common/verbose/VerboseListener.java

@@ -57,36 +57,32 @@ public class VerboseListener {
 
     // how much data should we store before stopping.
     private static final int DATA_TRUNCATION = 10000;
-
     // how many traces should we add
     private static final int TRACE_DATA_TRUNCATION = 250;
+    // how many lines should we include in each stack trace send as a chat message
+    private static final int STACK_TRUNCATION_CHAT = 15;
+    // how many lines should we include in each stack trace in the web output
+    private static final int STACK_TRUNCATION_WEB = 30;
 
     // the time when the listener was first registered
     private final long startTime = System.currentTimeMillis();
-
     // the version of the plugin. (used when we paste data to gist)
     private final String pluginVersion;
-
     // the sender to notify each time the listener processes a check which passes the filter
     private final Sender notifiedSender;
-
+    // the filter
-    // the filter string
+    private VerboseFilter filter;
-    private final String filter;
-
     // if we should notify the sender
     private final boolean notify;
-
     // the number of checks we have processed
     private final AtomicInteger counter = new AtomicInteger(0);
-
     // the number of checks we have processed and accepted, based on the filter rules for this
     // listener
     private final AtomicInteger matchedCounter = new AtomicInteger(0);
-
     // the checks which passed the filter, up to a max size of #DATA_TRUNCATION
     private final List<CheckData> results = new ArrayList<>(DATA_TRUNCATION / 10);
 
-    public VerboseListener(String pluginVersion, Sender notifiedSender, String filter, boolean notify) {
+    public VerboseListener(String pluginVersion, Sender notifiedSender, VerboseFilter filter, boolean notify) {
         this.pluginVersion = pluginVersion;
         this.notifiedSender = notifiedSender;
         this.filter = filter;
@@ -102,8 +98,8 @@ public class VerboseListener {
         // increment handled counter
         this.counter.incrementAndGet();
 
-        // check if the data passes our filters
+        // check if the data passes our filter
-        if (!VerboseFilter.passesFilter(data, this.filter)) {
+        if (!this.filter.evaluate(data)) {
             return;
         }
 
@@ -117,49 +113,38 @@ public class VerboseListener {
 
         // handle notifications
         if (this.notify) {
-            StringBuilder msgContent = new StringBuilder();
+            sendNotification(data);
-
-            if (this.notifiedSender.isConsole()) {
-                msgContent.append("&8[&2")
-                        .append(data.getCheckOrigin().getCode())
-                        .append("&8] ");
-            }
-
-            msgContent.append("&a")
-                    .append(data.getCheckTarget())
-                    .append("&7 - &a")
-                    .append(data.getPermission())
-                    .append("&7 - ")
-                    .append(getTristateColor(data.getResult()))
-                    .append(data.getResult().name().toLowerCase());
-
-            if (this.notifiedSender.isConsole()) {
-                // just send as a raw message
-                Message.VERBOSE_LOG.send(this.notifiedSender, msgContent.toString());
-            } else {
-
-                // form a hoverevent from the check trace
-                TextComponent textComponent = TextUtils.fromLegacy(Message.VERBOSE_LOG.asString(this.notifiedSender.getPlatform().getLocaleManager(), msgContent.toString()));
-
-                // build the text
-                List<String> hover = new ArrayList<>();
-                hover.add("&bOrigin: &2" + data.getCheckOrigin().name());
-                hover.add("&bContext: &r" + CommandUtils.contextSetToString(data.getCheckContext()));
-                hover.add("&bTrace: &r");
-
-                int overflow = readStack(data, 15, e -> hover.add("&7" + e.getClassName() + "." + e.getMethodName() + (e.getLineNumber() >= 0 ? ":" + e.getLineNumber() : "")));
-                if (overflow != 0) {
-                    hover.add("&f... and " + overflow + " more");
-                }
-
-                // send the message
-                HoverEvent e = new HoverEvent(HoverEvent.Action.SHOW_TEXT, TextUtils.fromLegacy(TextUtils.joinNewline(hover.stream()), CommandManager.AMPERSAND_CHAR));
-                TextComponent msg = textComponent.toBuilder().applyDeep(comp -> comp.hoverEvent(e)).build();
-                this.notifiedSender.sendMessage(msg);
-            }
         }
     }
 
+    private void sendNotification(CheckData data) {
+        String msg = "&a" + data.getCheckTarget() + "&7 - &a" + data.getPermission() + "&7 - " + getTristateColor(data.getResult()) + data.getResult().name().toLowerCase();
+        if (this.notifiedSender.isConsole()) {
+            // just send as a raw message
+            Message.VERBOSE_LOG.send(this.notifiedSender, msg);
+            return;
+        }
+
+        // form a hoverevent from the check trace
+        TextComponent textComponent = TextUtils.fromLegacy(Message.VERBOSE_LOG.asString(this.notifiedSender.getPlatform().getLocaleManager(), msg));
+
+        // build the text
+        List<String> hover = new ArrayList<>();
+        hover.add("&bOrigin: &2" + data.getCheckOrigin().name());
+        hover.add("&bContext: &r" + CommandUtils.contextSetToString(data.getCheckContext()));
+        hover.add("&bTrace: &r");
+
+        int overflow = readStack(data, STACK_TRUNCATION_CHAT, e -> hover.add("&7" + e.getClassName() + "." + e.getMethodName() + (e.getLineNumber() >= 0 ? ":" + e.getLineNumber() : "")));
+        if (overflow != 0) {
+            hover.add("&f... and " + overflow + " more");
+        }
+
+        // send the message
+        HoverEvent hoverEvent = new HoverEvent(HoverEvent.Action.SHOW_TEXT, TextUtils.fromLegacy(TextUtils.joinNewline(hover.stream()), CommandManager.AMPERSAND_CHAR));
+        TextComponent text = textComponent.toBuilder().applyDeep(comp -> comp.hoverEvent(hoverEvent)).build();
+        this.notifiedSender.sendMessage(text);
+    }
+
     /**
      * Uploads the captured data in this listener to a paste and returns the url
      *
@@ -177,11 +162,11 @@ public class VerboseListener {
         long secondsTaken = (now - this.startTime) / 1000L;
         String duration = DateUtil.formatTimeShort(secondsTaken);
 
-        String filter = this.filter;
+        String filter;
-        if (filter == null || filter.equals("")){
+        if (this.filter.isBlank()){
             filter = "any";
         } else {
-            filter = "`" + filter + "`";
+            filter = "`" + this.filter.toString() + "`";
         }
 
         // start building the message output
@@ -250,7 +235,7 @@ public class VerboseListener {
                 prettyOutput.add("<br><b>Context:</b> <code>" + CommandUtils.stripColor(CommandUtils.contextSetToString(c.getCheckContext())) + "</code>");
                 prettyOutput.add("<br><b>Trace:</b><pre>");
 
-                int overflow = readStack(c, 30, e -> prettyOutput.add(e.getClassName() + "." + e.getMethodName() + (e.getLineNumber() >= 0 ? ":" + e.getLineNumber() : "")));
+                int overflow = readStack(c, STACK_TRUNCATION_WEB, e -> prettyOutput.add(e.getClassName() + "." + e.getMethodName() + (e.getLineNumber() >= 0 ? ":" + e.getLineNumber() : "")));
                 if (overflow != 0) {
                     prettyOutput.add("... and " + overflow + " more");
                 }

common/src/main/java/me/lucko/luckperms/common/webeditor/WebEditorUtils.java

@@ -114,7 +114,7 @@ public final class WebEditorUtils {
             try (OutputStream os = connection.getOutputStream()) {
                 StringWriter sw = new StringWriter();
                 new JsonWriter(sw).beginObject()
-                        .name("description").value("LuckPerms Web Permissions Editor Data")
+                        .name("description").value("LuckPerms Web Editor Data")
                         .name("public").value(false)
                         .name("files")
                         .beginObject().name(FILE_NAME)