From 0ac62d6b63362dd3507987045618a4b224addc5d Mon Sep 17 00:00:00 2001 From: Andre601 <11576465+Andre601@users.noreply.github.com> Date: Wed, 18 May 2022 21:11:09 +0200 Subject: [PATCH 1/2] Log missing required plugin for expansion + some logger improvements --- .../clip/placeholderapi/PlaceholderAPI.java | 23 +++-- .../placeholderapi/PlaceholderAPIPlugin.java | 5 +- .../manager/LocalExpansionManager.java | 86 +++++++++++-------- .../java/me/clip/placeholderapi/util/Msg.java | 28 +++++- 4 files changed, 92 insertions(+), 50 deletions(-) diff --git a/src/main/java/me/clip/placeholderapi/PlaceholderAPI.java b/src/main/java/me/clip/placeholderapi/PlaceholderAPI.java index b149834..f9a83e7 100644 --- a/src/main/java/me/clip/placeholderapi/PlaceholderAPI.java +++ b/src/main/java/me/clip/placeholderapi/PlaceholderAPI.java @@ -332,10 +332,9 @@ public final class PlaceholderAPI { @Deprecated @ApiStatus.ScheduledForRemoval(inVersion = "2.13.0") public static boolean registerPlaceholderHook(Plugin plugin, PlaceholderHook placeholderHook) { - PlaceholderAPIPlugin.getInstance().getLogger().warning(plugin.getName() - + " is attempting to register placeholders via a PlaceholderHook class which is no longer supported!" - + " Please reach out to " + plugin.getDescription().getAuthors().toString() - + " and let them know that they need to update ASAP!"); + Msg.warn("Nag author(s) %s of plugin %s about their usage of the deprecated PlaceholderHook" + + " class! This class will be removed in v2.13.0!", plugin.getDescription().getAuthors(), + plugin.getName()); return false; } @@ -351,8 +350,8 @@ public final class PlaceholderAPI { @ApiStatus.ScheduledForRemoval(inVersion = "2.13.0") public static boolean registerPlaceholderHook(String identifier, PlaceholderHook placeholderHook) { - PlaceholderAPIPlugin.getInstance().getLogger().warning(identifier - + " is attempting to register placeholders via a PlaceholderHook class which is no longer supported!"); + Msg.warn("%s is attempting to register placeholders via deprecated PlaceholderHook class." + + " This class is no longer supported and will be removed in v2.13.0!", identifier); return false; } @@ -366,10 +365,9 @@ public final class PlaceholderAPI { @Deprecated @ApiStatus.ScheduledForRemoval(inVersion = "2.13.0") public static boolean unregisterPlaceholderHook(Plugin plugin) { - PlaceholderAPIPlugin.getInstance().getLogger().warning(plugin.getName() - + " is attempting to unregister placeholders via the PlaceholderAPI class which is no longer supported!" - + " Please reach out to " + plugin.getDescription().getAuthors().toString() - + " and let them know that they need to update ASAP!"); + Msg.warn("Nag author(s) %s of plugin %s about their usage of the PlaceholderAPI class." + + " This way of unregistering placeholders is no longer supported and will be removed" + + " in v2.13.0!", plugin.getDescription().getAuthors(), plugin.getName()); return false; } @@ -383,8 +381,9 @@ public final class PlaceholderAPI { @Deprecated @ApiStatus.ScheduledForRemoval(inVersion = "2.13.0") public static boolean unregisterPlaceholderHook(String identifier) { - PlaceholderAPIPlugin.getInstance().getLogger().warning(identifier - + " is attempting to unregister placeholders through the PlaceholderAPI class which is no longer supported!"); + Msg.warn("%s is attempting to unregister placeholders via PlaceholderAPI class." + + " This way of unregistering placeholders is no longer supported and will be removed" + + " in v2.13.0!", identifier); return false; } diff --git a/src/main/java/me/clip/placeholderapi/PlaceholderAPIPlugin.java b/src/main/java/me/clip/placeholderapi/PlaceholderAPIPlugin.java index 78a50be..74b8d15 100644 --- a/src/main/java/me/clip/placeholderapi/PlaceholderAPIPlugin.java +++ b/src/main/java/me/clip/placeholderapi/PlaceholderAPIPlugin.java @@ -23,7 +23,6 @@ package me.clip.placeholderapi; import java.text.SimpleDateFormat; import java.util.HashMap; import java.util.Map; -import java.util.logging.Level; import me.clip.placeholderapi.commands.PlaceholderCommandRouter; import me.clip.placeholderapi.configuration.PlaceholderAPIConfig; import me.clip.placeholderapi.expansion.PlaceholderExpansion; @@ -32,6 +31,7 @@ import me.clip.placeholderapi.expansion.manager.CloudExpansionManager; import me.clip.placeholderapi.expansion.manager.LocalExpansionManager; import me.clip.placeholderapi.listeners.ServerLoadEventListener; import me.clip.placeholderapi.updatechecker.UpdateChecker; +import me.clip.placeholderapi.util.Msg; import net.kyori.adventure.platform.bukkit.BukkitAudiences; import org.bstats.bukkit.Metrics; import org.bstats.charts.AdvancedPie; @@ -121,7 +121,8 @@ public final class PlaceholderAPIPlugin extends JavaPlugin { try { return new SimpleDateFormat(getInstance().getPlaceholderAPIConfig().dateFormat()); } catch (final IllegalArgumentException ex) { - getInstance().getLogger().log(Level.WARNING, "configured date format is invalid", ex); + Msg.warn("Configured Date format ('%s') is invalid! Defaulting to 'MM/dd/yy HH:mm:ss'", + ex, getInstance().getPlaceholderAPIConfig().dateFormat()); return new SimpleDateFormat("MM/dd/yy HH:mm:ss"); } } diff --git a/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java b/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java index 8b55e26..bdc4cca 100644 --- a/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java +++ b/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java @@ -22,11 +22,32 @@ package me.clip.placeholderapi.expansion.manager; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; +import java.io.File; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantLock; +import java.util.stream.Collectors; import me.clip.placeholderapi.PlaceholderAPIPlugin; import me.clip.placeholderapi.events.ExpansionRegisterEvent; import me.clip.placeholderapi.events.ExpansionUnregisterEvent; import me.clip.placeholderapi.events.ExpansionsLoadedEvent; -import me.clip.placeholderapi.expansion.*; +import me.clip.placeholderapi.expansion.Cacheable; +import me.clip.placeholderapi.expansion.Cleanable; +import me.clip.placeholderapi.expansion.Configurable; +import me.clip.placeholderapi.expansion.PlaceholderExpansion; +import me.clip.placeholderapi.expansion.Taskable; +import me.clip.placeholderapi.expansion.VersionSpecific; import me.clip.placeholderapi.expansion.cloud.CloudExpansion; import me.clip.placeholderapi.util.FileUtil; import me.clip.placeholderapi.util.Futures; @@ -45,16 +66,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.Unmodifiable; -import java.io.File; -import java.lang.reflect.Modifier; -import java.util.*; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.ReentrantLock; -import java.util.logging.Level; -import java.util.stream.Collectors; - public final class LocalExpansionManager implements Listener { @NotNull @@ -81,7 +92,7 @@ public final class LocalExpansionManager implements Listener { this.folder = new File(plugin.getDataFolder(), EXPANSIONS_FOLDER_NAME); if (!this.folder.exists() && !folder.mkdirs()) { - plugin.getLogger().log(Level.WARNING, "failed to create expansions folder!"); + Msg.warn("Failed to create expansions folder!"); } } @@ -168,8 +179,17 @@ public final class LocalExpansionManager implements Listener { Objects.requireNonNull(expansion.getAuthor(), "The expansion author is null!"); Objects.requireNonNull(expansion.getIdentifier(), "The expansion identifier is null!"); Objects.requireNonNull(expansion.getVersion(), "The expansion version is null!"); - + if (!expansion.register()) { + if (expansion.getRequiredPlugin() != null && !expansion.getRequiredPlugin().isEmpty()) { + if (!Bukkit.getPluginManager().isPluginEnabled(expansion.getRequiredPlugin())) { + Msg.warn("Cannot load expansion %s due to a missing plugin: %s", + expansion.getIdentifier(), expansion.getRequiredPlugin()); + return Optional.empty(); + } + } + + Msg.warn("Cannot load expansion %s due to an unknown issue.", expansion.getIdentifier()); return Optional.empty(); } @@ -182,10 +202,8 @@ public final class LocalExpansionManager implements Listener { } else { reason = " - One of its properties is null which is not allowed!"; } - - plugin.getLogger().severe("Failed to load expansion class " + clazz.getSimpleName() + - reason); - plugin.getLogger().log(Level.SEVERE, "", ex); + + Msg.severe("Failed to load expansion class %s%s", ex, clazz.getSimpleName(), reason); } return Optional.empty(); @@ -234,8 +252,8 @@ public final class LocalExpansionManager implements Listener { if (expansion instanceof VersionSpecific) { VersionSpecific nms = (VersionSpecific) expansion; if (!nms.isCompatibleWith(PlaceholderAPIPlugin.getServerVersion())) { - plugin.getLogger().warning("Your server version is not compatible with expansion " + - expansion.getIdentifier() + " " + expansion.getVersion()); + Msg.warn("Your server version is incompatible with expansion %s %s", + expansion.getIdentifier(), expansion.getVersion()); return false; } } @@ -262,9 +280,9 @@ public final class LocalExpansionManager implements Listener { if (expansion instanceof Listener) { Bukkit.getPluginManager().registerEvents(((Listener) expansion), plugin); } - - plugin.getLogger().info("Successfully registered expansion: " + expansion.getIdentifier() + - " [" + expansion.getVersion() + "]"); + + Msg.info("Successfully registered expansion: %s [%s]", expansion.getIdentifier(), + expansion.getVersion()); if (expansion instanceof Taskable) { ((Taskable) expansion).start(); @@ -315,13 +333,12 @@ public final class LocalExpansionManager implements Listener { return true; } - private void registerAll(@NotNull final CommandSender sender) { - plugin.getLogger().info("Placeholder expansion registration initializing..."); + Msg.info("Placeholder expansion registratuib initializing..."); Futures.onMainThread(plugin, findExpansionsOnDisk(), (classes, exception) -> { if (exception != null) { - plugin.getLogger().log(Level.SEVERE, "failed to load class files of expansions", exception); + Msg.severe("Failed to load class files of expansion.", exception); return; } @@ -388,8 +405,8 @@ public final class LocalExpansionManager implements Listener { final Class expansionClass = FileUtil.findClass(file, PlaceholderExpansion.class); if (expansionClass == null) { - plugin.getLogger().severe("Failed to load Expansion: " + file.getName() + ", as it does not have" + - " a class which extends PlaceholderExpansion."); + Msg.severe("Failed to load Expansion %s, as it does not have a class which" + + " extends PlaceholderExpansion", file.getName()); return null; } @@ -397,16 +414,15 @@ public final class LocalExpansionManager implements Listener { .map(method -> new MethodSignature(method.getName(), method.getParameterTypes())) .collect(Collectors.toSet()); if (!expansionMethods.containsAll(ABSTRACT_EXPANSION_METHODS)) { - plugin.getLogger().severe("Failed to load Expansion: " + file.getName() + ", as it does not have the" + - " required methods declared for a PlaceholderExpansion."); + Msg.severe("Failed to load Expansion %s, as it does not have the required" + + " methods declared for a PlaceholderExpansion.", file.getName()); return null; } return expansionClass; } catch (final VerifyError ex) { - plugin.getLogger().severe("Failed to load Expansion class " + file.getName() + - " (Is a dependency missing?)"); - plugin.getLogger().severe("Cause: " + ex.getClass().getSimpleName() + " " + ex.getMessage()); + Msg.severe("Failed to load Expansion class %s (Is a dependency missing?", file.getName()); + Msg.severe("Cause: %s %s", ex.getClass().getSimpleName(), ex.getMessage()); return null; } catch (final Exception ex) { throw new CompletionException(ex); @@ -424,9 +440,8 @@ public final class LocalExpansionManager implements Listener { if (ex.getCause() instanceof LinkageError) { throw ((LinkageError) ex.getCause()); } - - plugin.getLogger().warning("There was an issue with loading an expansion."); + Msg.warn("There was an issue with loading an Expansion."); return null; } } @@ -456,7 +471,8 @@ public final class LocalExpansionManager implements Listener { } expansion.unregister(); - plugin.getLogger().info("Unregistered placeholder expansion: " + expansion.getName()); + Msg.info("Unregistered placeholder Expansion %s", expansion.getIdentifier()); + Msg.info("Reason: Required plugin %s was disabled.", name); } } diff --git a/src/main/java/me/clip/placeholderapi/util/Msg.java b/src/main/java/me/clip/placeholderapi/util/Msg.java index 4e67f00..98b9e09 100644 --- a/src/main/java/me/clip/placeholderapi/util/Msg.java +++ b/src/main/java/me/clip/placeholderapi/util/Msg.java @@ -21,14 +21,40 @@ package me.clip.placeholderapi.util; import java.util.Arrays; +import java.util.logging.Level; import java.util.stream.Collectors; +import me.clip.placeholderapi.PlaceholderAPIPlugin; import org.bukkit.Bukkit; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; import org.jetbrains.annotations.NotNull; public final class Msg { - + + public static void log(Level level, String msg, Object... args) { + PlaceholderAPIPlugin.getInstance().getLogger().log(level, String.format(msg, args)); + } + + public static void info(String msg, Object... args) { + log(Level.INFO, msg, args); + } + + public static void warn(String msg, Object... args) { + log(Level.WARNING, msg, args); + } + + public static void warn(String msg, Throwable throwable, Object... args){ + PlaceholderAPIPlugin.getInstance().getLogger().log(Level.WARNING, String.format(msg, args), throwable); + } + + public static void severe(String msg, Object... args) { + log(Level.SEVERE, msg, args); + } + + public static void severe(String msg, Throwable throwable, Object... args) { + PlaceholderAPIPlugin.getInstance().getLogger().log(Level.SEVERE, String.format(msg, args), throwable); + } + public static void msg(@NotNull final CommandSender sender, @NotNull final String... messages) { if (messages.length == 0) { return; From 651e14a797bacfc1397455386355b8df4cfe0e37 Mon Sep 17 00:00:00 2001 From: Andre601 <11576465+Andre601@users.noreply.github.com> Date: Thu, 19 May 2022 02:46:40 +0200 Subject: [PATCH 2/2] Make plugin check before register to avoid possible exceptions --- .../expansion/manager/LocalExpansionManager.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java b/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java index bdc4cca..b5aefc7 100644 --- a/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java +++ b/src/main/java/me/clip/placeholderapi/expansion/manager/LocalExpansionManager.java @@ -180,15 +180,15 @@ public final class LocalExpansionManager implements Listener { Objects.requireNonNull(expansion.getIdentifier(), "The expansion identifier is null!"); Objects.requireNonNull(expansion.getVersion(), "The expansion version is null!"); - if (!expansion.register()) { - if (expansion.getRequiredPlugin() != null && !expansion.getRequiredPlugin().isEmpty()) { - if (!Bukkit.getPluginManager().isPluginEnabled(expansion.getRequiredPlugin())) { - Msg.warn("Cannot load expansion %s due to a missing plugin: %s", - expansion.getIdentifier(), expansion.getRequiredPlugin()); - return Optional.empty(); - } + if (expansion.getRequiredPlugin() != null && !expansion.getRequiredPlugin().isEmpty()) { + if (!Bukkit.getPluginManager().isPluginEnabled(expansion.getRequiredPlugin())) { + Msg.warn("Cannot load expansion %s due to a missing Plugin: %s", expansion.getIdentifier(), + expansion.getRequiredPlugin()); + return Optional.empty(); } - + } + + if (!expansion.register()) { Msg.warn("Cannot load expansion %s due to an unknown issue.", expansion.getIdentifier()); return Optional.empty(); }