Improve handling of method discovery in AnnotatedEventManager (#2454)

discord-jda · Jun 3, 2023 · eada413 · eada413
1 parent 0c0b7b7
commit eada413
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 26 deletions.
diff --git a/src/main/java/net/dv8tion/jda/api/JDABuilder.java b/src/main/java/net/dv8tion/jda/api/JDABuilder.java
@@ -1817,7 +1817,7 @@ public JDA build()
         if (audioSendFactory != null)
             jda.setAudioSendFactory(audioSendFactory);
 
-        listeners.forEach(jda::addEventListener);
+        jda.addEventListener(listeners.toArray());
         jda.setStatus(JDA.Status.INITIALIZED);  //This is already set by JDA internally, but this is to make sure the listeners catch it.
 
         // Set the presence information before connecting to have the correct information ready when sending IDENTIFY

diff --git a/src/main/java/net/dv8tion/jda/api/hooks/AnnotatedEventManager.java b/src/main/java/net/dv8tion/jda/api/hooks/AnnotatedEventManager.java
@@ -18,6 +18,8 @@
 import net.dv8tion.jda.api.events.GenericEvent;
 import net.dv8tion.jda.internal.JDAImpl;
 import net.dv8tion.jda.internal.utils.ClassWalker;
+import net.dv8tion.jda.internal.utils.JDALogger;
+import org.slf4j.Logger;
 
 import javax.annotation.Nonnull;
 import java.lang.reflect.InvocationTargetException;
@@ -51,21 +53,36 @@
  */
 public class AnnotatedEventManager implements IEventManager
 {
+    private static final Logger LOGGER = JDALogger.getLog(AnnotatedEventManager.class);
     private final Set<Object> listeners = ConcurrentHashMap.newKeySet();
     private final Map<Class<?>, Map<Object, List<Method>>> methods = new ConcurrentHashMap<>();
 
     @Override
     public void register(@Nonnull Object listener)
     {
+        if (listener.getClass().isArray())
+        {
+            for (Object o : ((Object[]) listener))
+                register(o);
+            return;
+        }
+
         if (listeners.add(listener))
         {
-            updateMethods();
+            registerListenerMethods(listener);
         }
     }
 
     @Override
     public void unregister(@Nonnull Object listener)
     {
+        if (listener.getClass().isArray())
+        {
+            for (Object o : ((Object[]) listener))
+                unregister(o);
+            return;
+        }
+
         if (listeners.remove(listener))
         {
             updateMethods();
@@ -114,32 +131,34 @@ private void updateMethods()
         methods.clear();
         for (Object listener : listeners)
         {
-            boolean isClass = listener instanceof Class;
-            Class<?> c = isClass ? (Class) listener : listener.getClass();
-            Method[] allMethods = c.getDeclaredMethods();
-            for (Method m : allMethods)
-            {
-                if (!m.isAnnotationPresent(SubscribeEvent.class) || (isClass && !Modifier.isStatic(m.getModifiers())))
-                {
-                    continue;
-                }
-                Class<?>[] pType  = m.getParameterTypes();
-                if (pType.length == 1 && GenericEvent.class.isAssignableFrom(pType[0]))
-                {
-                    Class<?> eventClass = pType[0];
-                    if (!methods.containsKey(eventClass))
-                    {
-                        methods.put(eventClass, new ConcurrentHashMap<>());
-                    }
+            registerListenerMethods(listener);
+        }
+    }
 
-                    if (!methods.get(eventClass).containsKey(listener))
-                    {
-                        methods.get(eventClass).put(listener, new CopyOnWriteArrayList<>());
-                    }
+    private void registerListenerMethods(Object listener)
+    {
+        boolean isClass = listener instanceof Class;
+        Class<?> c = isClass ? (Class<?>) listener : listener.getClass();
+        Method[] allMethods = c.getDeclaredMethods();
+        for (Method m : allMethods)
+        {
+            if (!m.isAnnotationPresent(SubscribeEvent.class))
+                continue;
+            //Skip member methods if listener is a Class
+            if (isClass && !Modifier.isStatic(m.getModifiers()))
+                continue;
 
-                    methods.get(eventClass).get(listener).add(m);
-                }
+            final Class<?>[] parameterTypes = m.getParameterTypes();
+            if (parameterTypes.length != 1 || !GenericEvent.class.isAssignableFrom(parameterTypes[0]))
+            {
+                LOGGER.warn("Method '{}' annotated with @{} must have at most 1 parameter, which implements GenericEvent", m, SubscribeEvent.class.getSimpleName());
+                continue;
             }
+
+            Class<?> eventClass = parameterTypes[0];
+            methods.computeIfAbsent(eventClass, k -> new ConcurrentHashMap<>())
+                    .computeIfAbsent(listener, k -> new CopyOnWriteArrayList<>())
+                    .add(m);
         }
     }
 }
diff --git a/src/main/java/net/dv8tion/jda/api/sharding/DefaultShardManager.java b/src/main/java/net/dv8tion/jda/api/sharding/DefaultShardManager.java
@@ -539,7 +539,7 @@ protected JDAImpl buildInstance(final int shardId)
         if (this.sessionConfig.getAudioSendFactory() != null)
             jda.setAudioSendFactory(this.sessionConfig.getAudioSendFactory());
 
-        this.eventConfig.getListeners().forEach(jda::addEventListener);
+        jda.addEventListener(this.eventConfig.getListeners().toArray());
         this.eventConfig.getListenerProviders().forEach(provider -> jda.addEventListener(provider.apply(shardId)));
 
         // Set the presence information before connecting to have the correct information ready when sending IDENTIFY