fix: reloading, disabling & tests

Finally-A-Decent · Mar 10, 2025 · 3814483 · 3814483
1 parent c6387dc
commit 3814483
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 51 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -33,7 +33,9 @@ allprojects {
 
         implementation("com.github.puregero:multilib:1.2.4")
 
-        testImplementation("com.github.seeseemelk:MockBukkit-v1.19:3.1.0")
+        testImplementation("com.github.seeseemelk:MockBukkit-v1.20:3.93.2")
+        testImplementation("com.github.puregero:multilib:1.2.4")
+
         testImplementation("org.junit.jupiter:junit-jupiter-api:5.11.4")
         testImplementation("org.junit.jupiter:junit-jupiter-params:5.11.4")
         testImplementation("org.junit.jupiter:junit-jupiter-engine:5.11.4")

diff --git a/src/main/java/info/preva1l/hooker/Hooker.java b/src/main/java/info/preva1l/hooker/Hooker.java
@@ -1,11 +1,8 @@
 package info.preva1l.hooker;
 
 import com.github.puregero.multilib.MultiLib;
+import com.github.puregero.multilib.regionized.RegionizedTask;
 import info.preva1l.hooker.annotation.*;
-import org.bukkit.Bukkit;
-import org.bukkit.event.EventHandler;
-import org.bukkit.event.Listener;
-import org.bukkit.event.server.PluginDisableEvent;
 import org.bukkit.plugin.java.JavaPlugin;
 
 import java.io.File;
@@ -14,6 +11,8 @@
 import java.lang.reflect.Method;
 import java.net.URL;
 import java.util.*;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 
 /**
@@ -22,7 +21,7 @@
  * @author Preva1l
  */
 @SuppressWarnings("unused")
-public final class Hooker implements Listener {
+public final class Hooker {
     private static Hooker instance;
 
     private final RequirementRegistry requirementRegistry;
@@ -96,13 +95,20 @@ public static <T> Optional<T> getHook(Class<T> hookClass) {
 
     /**
      * Reloads any loaded hooks that are annotated with {@link Reloadable}
+     * </br>
+     * This method runs some tasks asynchronously (reloadable hooks marked as async),
+     * this is why we return a completable future.
+     *
+     * @return returns a completable future that completes when all hooks are reloaded
      */
-    public static void reload() {
+    public static CompletableFuture<Void> reload() {
         if (instance == null) throw new IllegalStateException("You cannot reload hooks when Hooker is not initialized!");
 
-        instance.owningPlugin.getLogger().info("Reloading hooks...");
-        int count = instance.reloadHooks();
-        instance.owningPlugin.getLogger().info("Reloaded " + count + " hooks!");
+        return CompletableFuture.runAsync(() -> {
+            instance.owningPlugin.getLogger().info("Reloading hooks...");
+            int count = instance.reloadHooks();
+            instance.owningPlugin.getLogger().info("Reloaded " + count + " hooks!");
+        });
     }
 
     /**
@@ -120,9 +126,8 @@ public static void load() {
      * Call this method at the top of {@link JavaPlugin#onEnable()}
      */
     public static void enable() {
-        if (instance == null) throw new IllegalStateException("You cannot reload hooks when Hooker is not initialized!");
+        if (instance == null) throw new IllegalStateException("You cannot load hooks when Hooker is not initialized!");
 
-        Bukkit.getPluginManager().registerEvents(instance, instance.owningPlugin);
         instance.owningPlugin.getLogger().info("Loading onEnable hooks...");
         int count = instance.loadHooks(instance.onEnableHooks);
         instance.owningPlugin.getLogger().info("Loaded " + count + " hooks!");
@@ -138,6 +143,14 @@ public static void enable() {
         );
     }
 
+    public static void disable() {
+        if (instance == null) return;
+
+        instance.owningPlugin.getLogger().info("Disabling hooks...");
+        int count = instance.disableHooks();
+        instance.owningPlugin.getLogger().info("Disabled " + count + " hooks!");
+    }
+
     /**
      * Register a custom requirement.
      *
@@ -153,18 +166,18 @@ public static void requirement(String requirement, Predicate<String> predicate)
         instance.requirementRegistry.register(requirement, predicate);
     }
 
-    @EventHandler
-    private void onDisable(PluginDisableEvent event) {
-        if (!event.getPlugin().equals(owningPlugin)) return;
-
-        disableHooks();
+    private int reloadHooks() {
+        int count = 0;
+        count += reloadHooks(onEnableHooks);
+        count += reloadHooks(lateHooks);
+        return count;
     }
 
-    private int reloadHooks() {
+    private int reloadHooks(List<Class<?>> hooks) {
         int count = 0;
-        for (Class<?> hookClass : onEnableHooks) {
+        for (Class<?> hookClass : hooks) {
             if (loadedHooks.containsKey(hookClass)) {
-                if (reloadLoadedHook(loadedHooks.get(hookClass))) count++;
+                if (reloadLoadedHook(loadedHooks.get(hookClass)).join()) count++;
                 continue;
             }
 
@@ -176,11 +189,15 @@ private int reloadHooks() {
         return count;
     }
 
-    private boolean reloadLoadedHook(Object hook) {
+    private CompletableFuture<Boolean> reloadLoadedHook(Object hook) {
+        CompletableFuture<Boolean> future = new CompletableFuture<>();
         Method tempMethod = null;
         Method tempMethod2 = null;
         Reloadable reloadable = hook.getClass().getAnnotation(Reloadable.class);
-        if (reloadable == null) return false;
+        if (reloadable == null) {
+            future.complete(false);
+            return future;
+        }
 
         for (Method method : hook.getClass().getDeclaredMethods()) {
             if (method.isAnnotationPresent(OnStart.class)) {
@@ -201,29 +218,42 @@ private boolean reloadLoadedHook(Object hook) {
         }
 
         if (reloadable.async()) {
-            MultiLib.getAsyncScheduler().runNow(owningPlugin, t -> {
-                try {
-                    if (stopMethod != null) {
-                        stopMethod.invoke(hook);
-                    }
-                    startMethod.invoke(hook);
-                } catch (IllegalAccessException | InvocationTargetException e) {
-                    throw new RuntimeException(e);
-                }
-            });
+            MultiLib.getAsyncScheduler().runNow(owningPlugin, reloadTask(hook, startMethod, stopMethod, future));
         } else {
-            MultiLib.getGlobalRegionScheduler().run(owningPlugin, t -> {
-                try {
-                    if (stopMethod != null) {
-                        stopMethod.invoke(hook);
-                    }
-                    startMethod.invoke(hook);
-                } catch (IllegalAccessException | InvocationTargetException e) {
-                    throw new RuntimeException(e);
-                }
-            });
+            MultiLib.getGlobalRegionScheduler().run(owningPlugin, reloadTask(hook, startMethod, stopMethod, future));
         }
-        return true;
+
+        future.thenAccept(result -> {
+            if (result) {
+                Hook hookAnnotation = hook.getClass().getAnnotation(Hook.class);
+                owningPlugin.getLogger().info("Reloaded hook: " + hookAnnotation.id());
+            }
+        });
+
+        return future;
+    }
+
+    private Consumer<RegionizedTask> reloadTask(
+            Object hook,
+            Method startMethod,
+            Method stopMethod,
+            CompletableFuture<Boolean> future
+    ) {
+        return t -> {
+            try {
+                if (stopMethod != null) {
+                    stopMethod.invoke(hook);
+                }
+                Object response = startMethod.invoke(hook);
+                if (response instanceof Boolean load) {
+                    future.complete(load);
+                } else {
+                    future.complete(true);
+                }
+            } catch (IllegalAccessException | InvocationTargetException e) {
+                throw new RuntimeException(e);
+            }
+        };
     }
 
     private int loadHooks(List<Class<?>> hooks) {
@@ -273,8 +303,9 @@ private boolean loadHook(Class<?> hookClass) {
         return true;
     }
 
-    private void disableHooks() {
-        for (Object hook : loadedHooks.values()) {
+    private int disableHooks() {
+        int count = 0;
+        for (Object hook : new ArrayList<>(loadedHooks.values())) {
             for (Method method : hook.getClass().getDeclaredMethods()) {
                 if (!method.isAnnotationPresent(OnStop.class)) continue;
                 try {
@@ -285,7 +316,11 @@ private void disableHooks() {
                 break;
             }
             loadedHooks.remove(hook.getClass());
+            Hook hookAnnotation = hook.getClass().getAnnotation(Hook.class);
+            owningPlugin.getLogger().info("Disabled hook: " + hookAnnotation.id());
+            count++;
         }
+        return count;
     }
 
     private void scanForHooks(ClassLoader loader) {

diff --git a/src/test/java/info/preva1l/hooker/HookerTests.java b/src/test/java/info/preva1l/hooker/HookerTests.java
@@ -1,25 +1,36 @@
 package info.preva1l.hooker;
 
 import be.seeseemelk.mockbukkit.MockBukkit;
+import be.seeseemelk.mockbukkit.ServerMock;
 import info.preva1l.hooker.example.MyPlugin;
+import info.preva1l.hooker.example.hooks.LateHook;
 import info.preva1l.hooker.example.hooks.OnEnableHook;
 import info.preva1l.hooker.example.hooks.OnLoadHook;
+import org.bukkit.event.server.PluginDisableEvent;
 import org.junit.jupiter.api.*;
 
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
 /**
  * Created on 9/03/2025
  *
  * @author Preva1l
  */
-@DisplayName("Hooker Tests (1.19)")
+@DisplayName("Hooker Tests")
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 public class HookerTests {
+    private static ServerMock serverMock;
+    private static MyPlugin plugin;
+
     @BeforeAll
     @DisplayName("Test Plugin Setup")
     public static void setUpPlugin() {
-        MockBukkit.mock();
+        serverMock = MockBukkit.mock();
         MockBukkit.createMockPlugin("PlaceholderAPI");
-        MockBukkit.load(MyPlugin.class);
+        plugin = MockBukkit.load(MyPlugin.class);
+        serverMock.getScheduler().waitAsyncTasksFinished();
     }
 
     @AfterAll
@@ -40,4 +51,34 @@ public void testLoadingOnLoad() {
     public void testLoadingOnEnable() {
         Assertions.assertTrue(Hooker.getHook(OnEnableHook.class).isPresent());
     }
+
+    @Order(3)
+    @Test
+    @DisplayName("Test Hook Reloading")
+    public void testReloadHooks() {
+        AtomicBoolean completed = new AtomicBoolean(false);
+
+        CompletableFuture.runAsync(() -> {
+            // we need to continue the ticking on a diff thread while we join the future
+            while (!completed.get()) {
+                serverMock.getScheduler().performOneTick();
+            }
+        });
+
+        Assertions.assertDoesNotThrow(() -> {
+            Hooker.reload().join();
+            completed.set(true);
+        });
+    }
+
+    @Order(4)
+    @Test
+    @DisplayName("Test Hooks Get Disabled")
+    public void testHooksGetDisabled() {
+        serverMock.getPluginManager().disablePlugin(plugin);
+
+        Assertions.assertFalse(Hooker.getHook(OnLoadHook.class).isPresent());
+        Assertions.assertFalse(Hooker.getHook(OnEnableHook.class).isPresent());
+        Assertions.assertFalse(Hooker.getHook(LateHook.class).isPresent());
+    }
 }
diff --git a/src/test/java/info/preva1l/hooker/example/MyPlugin.java b/src/test/java/info/preva1l/hooker/example/MyPlugin.java
@@ -16,7 +16,10 @@ public void onLoad() {
 
         Hooker.requirement(
                 "config",
-                value -> true
+                value -> switch (value) {
+                    case "test-hook" -> true;
+                    default -> false;
+                }
         );
 
         Hooker.load();
@@ -26,4 +29,9 @@ public void onLoad() {
     public void onEnable() {
         Hooker.enable();
     }
+
+    @Override
+    public void onDisable() {
+        Hooker.disable();
+    }
 }
diff --git a/src/test/java/info/preva1l/hooker/example/hooks/LateHook.java b/src/test/java/info/preva1l/hooker/example/hooks/LateHook.java
@@ -0,0 +1,22 @@
+package info.preva1l.hooker.example.hooks;
+
+import info.preva1l.hooker.HookOrder;
+import info.preva1l.hooker.annotation.Hook;
+import info.preva1l.hooker.annotation.OnStart;
+import info.preva1l.hooker.annotation.Reloadable;
+
+/**
+ * Created on 10/03/2025
+ *
+ * @author Preva1l
+ */
+@Hook(id = "lateHook", order = HookOrder.LATE)
+@Reloadable(async = true)
+public class LateHook {
+    @OnStart
+    public void onStart() {
+        System.out.println("lateHook is started!");
+    }
+
+    // @OnStop is optional!
+}
diff --git a/src/test/java/info/preva1l/hooker/example/hooks/OnEnableHook.java b/src/test/java/info/preva1l/hooker/example/hooks/OnEnableHook.java
@@ -9,7 +9,7 @@
  * @author Preva1l
  */
 @Hook(id = "onEnableHook")
-@Reloadable(async = true)
+@Reloadable
 @Require("PlaceholderAPI")
 public class OnEnableHook {
     @OnStart

diff --git a/src/test/java/info/preva1l/hooker/example/hooks/OnLoadHook.java b/src/test/java/info/preva1l/hooker/example/hooks/OnLoadHook.java
@@ -9,7 +9,6 @@
  * @author Preva1l
  */
 @Hook(id = "onLoadHook", order = HookOrder.LOAD)
-@Reloadable(async = true)
 @Require(type = "config", value = "test-hook")
 public class OnLoadHook {
     @OnStart