From 682f9688f87ea36fb46aa7a5aef8b0d29afa41f2 Mon Sep 17 00:00:00 2001 From: Pablo Herrera Date: Tue, 26 Dec 2023 14:43:32 +0100 Subject: [PATCH] Cleanup method handle utils & fix npe (#1284) Signed-off-by: Pablo Herrera --- .../tc/oc/pgm/filters/FilterMatchModule.java | 19 ++-- .../tc/oc/pgm/util/MethodHandleUtils.java | 94 ++++++++----------- 2 files changed, 48 insertions(+), 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 index a8c6f2f71b..170f872881 100644 --- 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 index ef903c9031..4591b5bc94 100644 --- 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, MethodHandle> cachedHandles = new HashMap<>(); + private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); + private static final Map, 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 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 event) throws NoSuchMethodException { + MethodHandle handle = CACHED_HANDLES.computeIfAbsent(event, MethodHandleUtils::findHandle); - public static MethodHandle getHandle(Class 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 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 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; } }