Cleanup method handle utils & fix npe (#1284)

Signed-off-by: Pablo Herrera <[email protected]>
PGMDev · Dec 26, 2023 · 682f968 · 682f968
1 parent 3b3ae70
commit 682f968
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 65 deletions.
diff --git a/core/src/main/java/tc/oc/pgm/filters/FilterMatchModule.java b/core/src/main/java/tc/oc/pgm/filters/FilterMatchModule.java
@@ -457,24 +457,25 @@ else if (FlagStateChangeEvent.class.isAssignableFrom(event))
         } catch (Exception e) {
           match
               .getLogger()
-              .severe("Unable to get MethodHandle extracting Filterable or Player for " + event);
-          e.printStackTrace();
+              .log(Level.SEVERE, "No getter for Filterable or Player found in " + event, e);
           continue;
         }
         result =
             (l, e) -> {
               try {
                 final Object o = handle.invoke(e);
-                if (o instanceof Filterable) this.invalidate((Filterable<?>) o);
-                else if (o instanceof Player) this.invalidate(this.match.getPlayer((Player) o));
-                else
+                if (o instanceof Player) {
+                  MatchPlayer mp = this.match.getPlayer((Player) o);
+                  if (mp != null) invalidate(mp);
+                  else match.getLogger().warning("MatchPlayer not found for player " + o);
+                } else if (o instanceof Filterable) {
+                  this.invalidate((Filterable<?>) o);
+                } else {
                   throw new IllegalStateException(
                       "A cached MethodHandle returned a non-expected type. Was: " + o.getClass());
+                }
               } catch (Throwable t) {
-                match
-                    .getLogger()
-                    .severe("Unable to invoke cached MethodHandle extracting Filterable for " + e);
-                t.printStackTrace();
+                match.getLogger().log(Level.SEVERE, "Error extracting Filterable for " + e, t);
               }
             };
       }

diff --git a/core/src/main/java/tc/oc/pgm/util/MethodHandleUtils.java b/core/src/main/java/tc/oc/pgm/util/MethodHandleUtils.java
@@ -1,9 +1,11 @@
 package tc.oc.pgm.util;
 
+import com.google.common.collect.ImmutableList;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.LivingEntity;
@@ -16,73 +18,53 @@
 
 public final class MethodHandleUtils {
 
-  private static final MethodHandles.Lookup lookup = MethodHandles.lookup();
-  private static final Map<Class<? extends Event>, MethodHandle> cachedHandles = new HashMap<>();
+  private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
+  private static final Map<Class<? extends Event>, MethodHandle> CACHED_HANDLES = new HashMap<>();
 
-  private static final String[] filterableGetterNames =
-      new String[] {"getPlayer", "getParty", "getMatch"};
-  private static final MethodType[] filterableMethodTypes =
-      new MethodType[] {
-        MethodType.methodType(MatchPlayer.class),
-        MethodType.methodType(Party.class),
-        MethodType.methodType(Match.class)
-      };
+  private static final List<HandleFinder> FILTERABLE_GETTERS =
+      ImmutableList.of(
+          new HandleFinder(MatchPlayer.class, "getPlayer"),
+          new HandleFinder(Party.class, "getParty"),
+          new HandleFinder(Match.class, "getMatch"),
+          new HandleFinder(Player.class, "getPlayer"),
+          new HandleFinder(Player.class, "getActor"),
+          new HandleFinder(Entity.class, "getActor"),
+          new HandleFinder(LivingEntity.class, "getEntity"));
 
-  // #getActor is mostly for SportPaper (PlayerAction).
-  // #getPlayer covers events which does not provide Entity
-  // as the lower boundary
-  private static final String[] entityGetterNames =
-      new String[] {"getPlayer", "getActor", "getActor", "getEntity"};
-  private static final MethodType playerMethodType = MethodType.methodType(Player.class);
-  private static final MethodType[] entityMethodTypes =
-      new MethodType[] {
-        playerMethodType,
-        playerMethodType,
-        MethodType.methodType(Entity.class),
-        MethodType.methodType(LivingEntity.class)
-      };
+  public static MethodHandle getHandle(Class<? extends Event> event) throws NoSuchMethodException {
+    MethodHandle handle = CACHED_HANDLES.computeIfAbsent(event, MethodHandleUtils::findHandle);
 
-  public static MethodHandle getHandle(Class<? extends Event> event)
-      throws NoSuchMethodException, IllegalAccessException {
-    MethodHandle handle = cachedHandles.get(event);
-
-    if (handle == null) {
-      createExtractingMethodHandle(event);
+    if (handle == null)
+      throw new NoSuchMethodException(
+          "No method to extract a Filterable or Player found on " + event);
 
-      handle = cachedHandles.get(event);
-      if (handle == null)
-        throw new IllegalStateException(
-            "Newly created MethodHandle for " + event + "not found in the cache");
-    }
     return handle;
   }
 
-  private static void createExtractingMethodHandle(Class<? extends Event> event)
-      throws NoSuchMethodException, IllegalAccessException {
-    MethodHandle result = null;
-
-    for (int i = 0; result == null && i < filterableGetterNames.length; i++) {
-      result = findMethod(event, filterableGetterNames[i], filterableMethodTypes[i]);
-    }
-
-    for (int i = 0; result == null && i < entityGetterNames.length; i++) {
-      result = findMethod(event, entityGetterNames[i], entityMethodTypes[i]);
+  private static MethodHandle findHandle(Class<? extends Event> event) {
+    for (HandleFinder finder : FILTERABLE_GETTERS) {
+      MethodHandle handle = finder.find(event);
+      if (handle != null) return handle;
     }
+    return null;
+  }
 
-    if (result == null)
-      throw new NoSuchMethodException(
-          "No method to extract a Filterable or Entity(Player) found on " + event);
+  private static class HandleFinder {
+    private final MethodType type;
+    private final String name;
 
-    cachedHandles.put(event, result);
-  }
+    private HandleFinder(Class<?> returnType, String name) {
+      this.type = MethodType.methodType(returnType);
+      this.name = name;
+    }
 
-  private static @Nullable MethodHandle findMethod(Class<?> clazz, String name, MethodType type)
-      throws IllegalAccessException {
-    try {
-      return lookup.findVirtual(clazz, name, type);
-    } catch (NoSuchMethodException e) {
-      // No-Op
+    private @Nullable MethodHandle find(Class<?> clazz) {
+      try {
+        return LOOKUP.findVirtual(clazz, this.name, this.type);
+      } catch (NoSuchMethodException | IllegalAccessException e) {
+        // No-Op
+      }
+      return null;
     }
-    return null;
   }
 }