From aa0be4012478f7af8ae5ef14113d1e394ee6fa21 Mon Sep 17 00:00:00 2001 From: Luck Date: Mon, 16 Oct 2017 18:55:17 +0100 Subject: [PATCH] Cleanup verbose handler & only send sponge OP command notification if the sender has permission --- .../luckperms/bukkit/LPBukkitPlugin.java | 4 +- .../luckperms/bungee/LPBungeePlugin.java | 4 +- .../common/treeview/PermissionVault.java | 7 +- .../common/verbose/VerboseHandler.java | 29 ++- .../common/verbose/VerboseListener.java | 170 ++++++++++-------- .../luckperms/sponge/LPSpongePlugin.java | 4 +- .../luckperms/sponge/SpongeListener.java | 2 +- .../service/persisted/PersistedSubject.java | 2 +- 8 files changed, 117 insertions(+), 105 deletions(-) diff --git a/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java b/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java index cbc29007..3e95aeb4 100644 --- a/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java +++ b/bukkit/src/main/java/me/lucko/luckperms/bukkit/LPBukkitPlugin.java @@ -327,8 +327,8 @@ public class LPBukkitPlugin extends JavaPlugin implements LuckPermsPlugin { started = false; defaultsProvider.close(); - permissionVault.setShutdown(true); - verboseHandler.setShutdown(true); + permissionVault.shutdown(); + verboseHandler.shutdown(); for (Player player : getServer().getOnlinePlayers()) { try { diff --git a/bungee/src/main/java/me/lucko/luckperms/bungee/LPBungeePlugin.java b/bungee/src/main/java/me/lucko/luckperms/bungee/LPBungeePlugin.java index 676f1d49..d71e0610 100644 --- a/bungee/src/main/java/me/lucko/luckperms/bungee/LPBungeePlugin.java +++ b/bungee/src/main/java/me/lucko/luckperms/bungee/LPBungeePlugin.java @@ -212,8 +212,8 @@ public class LPBungeePlugin extends Plugin implements LuckPermsPlugin { @Override public void onDisable() { - permissionVault.setShutdown(true); - verboseHandler.setShutdown(true); + permissionVault.shutdown(); + verboseHandler.shutdown(); getLog().info("Closing storage..."); storage.shutdown(); diff --git a/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java b/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java index 44f3a9e8..10add473 100644 --- a/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java +++ b/common/src/main/java/me/lucko/luckperms/common/treeview/PermissionVault.java @@ -27,7 +27,6 @@ package me.lucko.luckperms.common.treeview; import lombok.Getter; import lombok.NonNull; -import lombok.Setter; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableSet; @@ -55,7 +54,7 @@ public class PermissionVault implements Runnable { // a queue of permission strings to be processed by the tree private final Queue queue; - @Setter + // if the handler should shutdown private boolean shutdown = false; public PermissionVault(Executor executor) { @@ -114,4 +113,8 @@ public class PermissionVault implements Runnable { } } + public void shutdown() { + shutdown = true; + } + } diff --git a/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java b/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java index 6cfb1065..a5ed8114 100644 --- a/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java +++ b/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseHandler.java @@ -25,8 +25,6 @@ package me.lucko.luckperms.common.verbose; -import lombok.Setter; - import me.lucko.luckperms.api.Tristate; import me.lucko.luckperms.api.context.ContextSet; import me.lucko.luckperms.common.commands.sender.Sender; @@ -54,7 +52,6 @@ public class VerboseHandler implements Runnable { private boolean listening = false; // if the handler should shutdown - @Setter private boolean shutdown = false; public VerboseHandler(Executor executor, String pluginVersion) { @@ -98,8 +95,8 @@ public class VerboseHandler implements Runnable { * @param notify if the sender should be notified in chat on each check */ public void registerListener(Sender sender, String filter, boolean notify) { - listening = true; listeners.put(sender.getUuid(), new VerboseListener(pluginVersion, sender, filter, notify)); + listening = true; } /** @@ -112,33 +109,29 @@ public class VerboseHandler implements Runnable { // immediately flush, so the listener gets all current data flush(); - VerboseListener ret = listeners.remove(uuid); - - // stop listening if there are no listeners left - if (listeners.isEmpty()) { - listening = false; - } - - return ret; + return listeners.remove(uuid); } @Override public void run() { while (true) { + // remove listeners where the sender is no longer valid listeners.values().removeIf(l -> !l.getNotifiedSender().isValid()); - if (listeners.isEmpty()) { - listening = false; - } + // handle all checks in the queue flush(); + // break the loop if the handler has been shutdown if (shutdown) { return; } + // update listening state + listening = !listeners.isEmpty(); + try { - Thread.sleep(200); + Thread.sleep(100); } catch (InterruptedException ignored) {} } } @@ -153,4 +146,8 @@ public class VerboseHandler implements Runnable { } } } + + public void shutdown() { + shutdown = true; + } } diff --git a/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseListener.java b/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseListener.java index fc14cfa6..006594ef 100644 --- a/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseListener.java +++ b/common/src/main/java/me/lucko/luckperms/common/verbose/VerboseListener.java @@ -49,6 +49,7 @@ import java.util.Date; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import java.util.stream.Collectors; /** @@ -96,16 +97,23 @@ public class VerboseListener { * @param data the data to process */ public void acceptData(CheckData data) { + // increment handled counter counter.incrementAndGet(); + + // check if the data passes our filters if (!VerboseFilter.passesFilter(data, filter)) { return; } + + // increment the matched filter matchedCounter.incrementAndGet(); + // record the check, if we have space for it if (results.size() < DATA_TRUNCATION) { results.add(data); } + // handle notifications if (notify) { StringBuilder msgContent = new StringBuilder(); @@ -124,49 +132,25 @@ public class VerboseListener { .append(data.getResult().name().toLowerCase()); if (notifiedSender.isConsole()) { + // just send as a raw message Message.VERBOSE_LOG.send(notifiedSender, msgContent.toString()); } else { + + // form a hoverevent from the check trace TextComponent textComponent = TextUtils.fromLegacy(Message.VERBOSE_LOG.asString(notifiedSender.getPlatform().getLocaleManager(), msgContent.toString())); + + // build the text List hover = new ArrayList<>(); hover.add("&bOrigin: &2" + data.getCheckOrigin().name()); hover.add("&bContext: &r" + Util.contextSetToString(data.getCheckContext())); hover.add("&bTrace: &r"); - StackTraceElement[] checkTrace = data.getCheckTrace(); - - // how many lines have been printed - int count = 0; - // if we're printing elements yet - boolean printing = false; - - for (StackTraceElement e : checkTrace) { - // start printing when we escape LP internals code - boolean shouldStartPrinting = !printing && ( - (data.getCheckOrigin() == CheckOrigin.API || data.getCheckOrigin() == CheckOrigin.INTERNAL) || ( - !e.getClassName().startsWith("me.lucko.luckperms.") && - // all used within the checking impl somewhere - !e.getClassName().equals("java.util.concurrent.CompletableFuture") && - !e.getClassName().startsWith("com.github.benmanes.caffeine") && - !e.getClassName().equals("java.util.concurrent.ConcurrentHashMap") - ) - ); - - if (shouldStartPrinting) { - printing = true; - } - - if (!printing) continue; - if (count >= 15) break; - - hover.add("&7" + e.getClassName() + "." + e.getMethodName() + (e.getLineNumber() >= 0 ? ":" + e.getLineNumber() : "")); - count++; - } - - if (checkTrace.length > 15) { - int remain = checkTrace.length - 15; - hover.add("&f... and " + remain + " more"); + 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()), Constants.FORMAT_CHAR)); TextComponent msg = textComponent.toBuilder().applyDeep(comp -> comp.hoverEvent(e)).build(); notifiedSender.sendMessage(msg); @@ -183,6 +167,8 @@ public class VerboseListener { * @see PasteUtils#paste(String, List) */ public String uploadPasteData(boolean showTraces, boolean attachRaw) { + + // retrieve variables long now = System.currentTimeMillis(); String startDate = DATE_FORMAT.format(new Date(startTime)); String endDate = DATE_FORMAT.format(new Date(now)); @@ -196,6 +182,7 @@ public class VerboseListener { filter = "`" + filter + "`"; } + // start building the message output ImmutableList.Builder prettyOutput = ImmutableList.builder() .add("## Verbose Checking Output") .add("#### This file was automatically generated by [LuckPerms](https://github.com/lucko/LuckPerms) " + pluginVersion) @@ -212,98 +199,123 @@ public class VerboseListener { .add("| Include traces | " + showTraces + " |") .add(""); + // warn if data was truncated if (matchedCounter.get() > results.size()) { prettyOutput.add("**WARN:** Result set exceeded max size of " + DATA_TRUNCATION + ". The output below was truncated to " + DATA_TRUNCATION + " entries."); prettyOutput.add(""); } + // explain why some traces may be missing if (showTraces && results.size() > TRACE_DATA_TRUNCATION) { prettyOutput.add("**WARN:** Result set exceeded size of " + TRACE_DATA_TRUNCATION + ". The traced output below was truncated to " + TRACE_DATA_TRUNCATION + " entries. "); prettyOutput.add("Either refine the query using a more specific filter, or disable tracing by adding '--slim' to the end of the paste command."); prettyOutput.add(""); } + // print the format of the output prettyOutput.add("### Output") .add("Format: `` `` ``") .add("") .add("___") .add(""); + // build the csv output - will only be appended to if this is enabled. ImmutableList.Builder csvOutput = ImmutableList.builder() .add("User,Permission,Result"); + // how many instances have been printed so far AtomicInteger printedCount = new AtomicInteger(0); - results.forEach(c -> { + for (CheckData c : results) { if (!showTraces) { + + // if traces aren't being shown, just append using raw markdown prettyOutput.add("`" + c.getCheckTarget() + "` - " + c.getPermission() + " - " + getTristateSymbol(c.getResult()) + " "); + } else if (printedCount.incrementAndGet() > TRACE_DATA_TRUNCATION) { + + // if we've gone over the trace truncation, just append the raw info. + // we still have to use html, as the rest of this section is still using it. prettyOutput.add("
" + c.getCheckTarget() + " - " + c.getPermission() + " - " + getTristateSymbol(c.getResult())); + } else { + + // append the full output. prettyOutput.add("
" + c.getCheckTarget() + " - " + c.getPermission() + " - " + getTristateSymbol(c.getResult()) + "

"); + // append the spoiler text prettyOutput.add("
Origin: " + c.getCheckOrigin().name() + ""); prettyOutput.add("
Context: " + Util.stripColor(Util.contextSetToString(c.getCheckContext())) + ""); prettyOutput.add("
Trace:

");
 
-                // add trace
-                StackTraceElement[] checkTrace = c.getCheckTrace();
-
-                // how many lines have been printed
-                int count = 0;
-                // if we're printing elements yet
-                boolean printing = false;
-
-                for (StackTraceElement e : checkTrace) {
-                    // start printing when we escape LP internals code
-                    boolean shouldStartPrinting = !printing && (
-                            (c.getCheckOrigin() == CheckOrigin.API || c.getCheckOrigin() == CheckOrigin.INTERNAL) || (
-                                    !e.getClassName().startsWith("me.lucko.luckperms.") &&
-                                    // all used within the checking impl somewhere
-                                    !e.getClassName().equals("java.util.concurrent.CompletableFuture") &&
-                                    !e.getClassName().startsWith("com.github.benmanes.caffeine") &&
-                                    !e.getClassName().equals("java.util.concurrent.ConcurrentHashMap")
-                            )
-                    );
-
-                    if (shouldStartPrinting) {
-                        printing = true;
-                    }
-
-                    if (!printing) continue;
-                    if (count >= 30) break;
-
-                    prettyOutput.add(e.getClassName() + "." + e.getMethodName() + (e.getLineNumber() >= 0 ? ":" + e.getLineNumber() : ""));
-                    count++;
-                }
-
-                if (checkTrace.length > 30) {
-                    int remain = checkTrace.length - 30;
-                    prettyOutput.add("... and " + remain + " more");
+                int overflow = readStack(c, 30, e -> prettyOutput.add(e.getClassName() + "." + e.getMethodName() + (e.getLineNumber() >= 0 ? ":" + e.getLineNumber() : "")));
+                if (overflow != 0) {
+                    prettyOutput.add("... and " + overflow + " more");
                 }
 
                 prettyOutput.add("

"); } + // if we're including a raw csv output, append that too if (attachRaw) { csvOutput.add(escapeCommas(c.getCheckTarget()) + "," + escapeCommas(c.getPermission()) + "," + c.getResult().name().toLowerCase()); } - }); + } results.clear(); - List> content; + ImmutableList.Builder> content = ImmutableList.builder(); + content.add(Maps.immutableEntry("luckperms-verbose.md", prettyOutput.build().stream().collect(Collectors.joining("\n")))); + if (attachRaw) { - content = ImmutableList.of( - Maps.immutableEntry("luckperms-verbose.md", prettyOutput.build().stream().collect(Collectors.joining("\n"))), - Maps.immutableEntry("raw-data.csv", csvOutput.build().stream().collect(Collectors.joining("\n"))) - ); - } else { - content = ImmutableList.of( - Maps.immutableEntry("luckperms-verbose.md", prettyOutput.build().stream().collect(Collectors.joining("\n"))) - ); + content.add(Maps.immutableEntry("raw-data.csv", csvOutput.build().stream().collect(Collectors.joining("\n")))); } - return PasteUtils.paste("LuckPerms Verbose Checking Output", content); + return PasteUtils.paste("LuckPerms Verbose Checking Output", content.build()); + } + + /** + * Reads a stack trace from a {@link CheckData} instance. + * + * @param data the data to read from + * @param truncateLength the length when we should stop reading the stack + * @param consumer the element consumer + * @return how many elements were left unread, or 0 if everything was read + */ + private static int readStack(CheckData data, int truncateLength, Consumer consumer) { + StackTraceElement[] stack = data.getCheckTrace(); + + // how many lines have been printed + int count = 0; + // if we're printing elements yet + boolean printing = false; + + for (StackTraceElement e : stack) { + // start printing when we escape LP internals code + boolean shouldStartPrinting = !printing && ( + (data.getCheckOrigin() == CheckOrigin.API || data.getCheckOrigin() == CheckOrigin.INTERNAL) || ( + !e.getClassName().startsWith("me.lucko.luckperms.") && + // all used within the checking impl somewhere + !e.getClassName().equals("java.util.concurrent.CompletableFuture") && + !e.getClassName().startsWith("com.github.benmanes.caffeine") && + !e.getClassName().equals("java.util.concurrent.ConcurrentHashMap") + ) + ); + + if (shouldStartPrinting) { + printing = true; + } + + if (!printing) continue; + if (count >= truncateLength) break; + + consumer.accept(e); + count++; + } + + if (stack.length > truncateLength) { + return stack.length - truncateLength; + } + return 0; } private static String escapeCommas(String s) { diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/LPSpongePlugin.java b/sponge/src/main/java/me/lucko/luckperms/sponge/LPSpongePlugin.java index 3ec3c74a..c6a55548 100644 --- a/sponge/src/main/java/me/lucko/luckperms/sponge/LPSpongePlugin.java +++ b/sponge/src/main/java/me/lucko/luckperms/sponge/LPSpongePlugin.java @@ -296,8 +296,8 @@ public class LPSpongePlugin implements LuckPermsPlugin { @Listener public void onDisable(GameStoppingServerEvent event) { - permissionVault.setShutdown(true); - verboseHandler.setShutdown(true); + permissionVault.shutdown(); + verboseHandler.shutdown(); getLog().info("Closing storage..."); storage.shutdown(); diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java b/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java index d33d7b55..e10fae05 100644 --- a/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java +++ b/sponge/src/main/java/me/lucko/luckperms/sponge/SpongeListener.java @@ -196,7 +196,7 @@ public class SpongeListener { if (source == null) return; final String name = e.getCommand().toLowerCase(); - if (name.equals("op") || name.equals("deop")) { + if (((name.equals("op") || name.equals("minecraft:op")) && source.hasPermission("minecraft.command.op")) || ((name.equals("deop") || name.equals("minecraft:deop")) && source.hasPermission("minecraft.command.deop"))) { Message.OP_DISABLED_SPONGE.send(plugin.getSenderFactory().wrap(source)); } } diff --git a/sponge/src/main/java/me/lucko/luckperms/sponge/service/persisted/PersistedSubject.java b/sponge/src/main/java/me/lucko/luckperms/sponge/service/persisted/PersistedSubject.java index 5828bbc7..9cc7ae25 100644 --- a/sponge/src/main/java/me/lucko/luckperms/sponge/service/persisted/PersistedSubject.java +++ b/sponge/src/main/java/me/lucko/luckperms/sponge/service/persisted/PersistedSubject.java @@ -257,7 +257,7 @@ public class PersistedSubject implements LPSubject { @Override public Tristate getPermissionValue(@NonNull ImmutableContextSet contexts, @NonNull String node) { Tristate t = permissionLookupCache.get(PermissionLookup.of(node, contexts)); - service.getPlugin().getVerboseHandler().offerCheckData(CheckOrigin.INTERNAL, "local:" + getParentCollection().getIdentifier() + "/" + identifier, contexts, node, t); + service.getPlugin().getVerboseHandler().offerCheckData(CheckOrigin.INTERNAL, getParentCollection().getIdentifier() + "/" + identifier, contexts, node, t); return t; }