[BREAKING] Use event class instead of event for timings. Fixes BUKKIT-4664

TimedRegisteredListener uses a reference to the first event fired. This
causes a memory leak in the server for any references that event has. This
changes TimedRegisteredListener to only store a reference to the class of
the event.

This change is intentionally a breaking change, as it is an obscure part
of the API. A non-breaking change would require the leak to be maintained
or an immediate update for any plugins using the method, as it would be an
indirect break.

A unit test is also included to check behavior of shared superclass
functionality.

1 files changed, 56 insertions, 0 deletions

diff --git a/src/test/java/org/bukkit/plugin/TimedRegisteredListenerTest.java b/src/test/java/org/bukkit/plugin/TimedRegisteredListenerTest.java
new file mode 100644
index 00000000..b206b1f3
--- /dev/null
+++ b/src/test/java/org/bukkit/plugin/TimedRegisteredListenerTest.java
@@ -0,0 +1,56 @@
+package org.bukkit.plugin;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import org.bukkit.event.Event;
+import org.bukkit.event.EventException;
+import org.bukkit.event.EventPriority;
+import org.bukkit.event.Listener;
+import org.bukkit.event.block.BlockBreakEvent;
+import org.bukkit.event.player.PlayerEvent;
+import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.event.player.PlayerMoveEvent;
+import org.junit.Test;
+
+public class TimedRegisteredListenerTest {
+
+    @Test
+    public void testEventClass() throws EventException {
+        Listener listener = new Listener() {};
+        EventExecutor executor = new EventExecutor() {
+            public void execute(Listener listener, Event event) {}
+        };
+        TestPlugin plugin = new TestPlugin("Test");
+
+        PlayerInteractEvent interactEvent = new PlayerInteractEvent(null, null, null, null, null);
+        PlayerMoveEvent moveEvent = new PlayerMoveEvent(null, null, null);
+        BlockBreakEvent breakEvent = new BlockBreakEvent(null, null);
+
+        TimedRegisteredListener trl = new TimedRegisteredListener(listener, executor, EventPriority.NORMAL, plugin, false);
+
+        // Ensure that the correct event type is reported for a single event
+        trl.callEvent(interactEvent);
+        assertThat(trl.getEventClass(), is((Object) PlayerInteractEvent.class));
+        // Ensure that no superclass is used in lieu of the actual event, after two identical event types
+        trl.callEvent(interactEvent);
+        assertThat(trl.getEventClass(), is((Object) PlayerInteractEvent.class));
+        // Ensure that the closest superclass of the two events is chosen
+        trl.callEvent(moveEvent);
+        assertThat(trl.getEventClass(), is((Object) PlayerEvent.class));
+        // As above, so below
+        trl.callEvent(breakEvent);
+        assertThat(trl.getEventClass(), is((Object) Event.class));
+        // In the name of being thorough, check that it never travels down the hierarchy again.
+        trl.callEvent(breakEvent);
+        assertThat(trl.getEventClass(), is((Object) Event.class));
+
+        trl = new TimedRegisteredListener(listener, executor, EventPriority.NORMAL, plugin, false);
+
+        trl.callEvent(breakEvent);
+        assertThat(trl.getEventClass(), is((Object) BlockBreakEvent.class));
+        // Test moving up the class hierarchy by more than one class at a time
+        trl.callEvent(moveEvent);
+        assertThat(trl.getEventClass(), is((Object) Event.class));
+    }
+}