Further fixes & cleanup to username resolving

Signed-off-by: Pablo Herrera <[email protected]>

core/src/main/java/tc/oc/pgm/db/SQLDatastore.java

@@ -21,7 +21,6 @@
 import tc.oc.pgm.api.setting.Settings;
 import tc.oc.pgm.util.concurrent.ThreadSafeConnection;
 import tc.oc.pgm.util.named.NameStyle;
-import tc.oc.pgm.util.nms.NMSHacks;
 import tc.oc.pgm.util.skin.Skin;
 import tc.oc.pgm.util.text.TextParser;
 import tc.oc.pgm.util.usernames.UsernameResolver;
@@ -50,9 +49,6 @@ private class SQLUsername implements Username {
 
     SQLUsername(UUID id) {
       this.id = assertNotNull(id, "username id is null");
-      // It's better than no name at all until we resolve to something better
-      // Note: it appears like offline uuid -> name does nothing in sportpaper
-      if (Bukkit.isPrimaryThread()) name = NMSHacks.getPlayerName(id);
       UsernameResolvers.resolve(id).thenAccept(this::setName);
     }
 

core/src/main/java/tc/oc/pgm/db/SqlUsernameResolver.java

@@ -10,7 +10,7 @@
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
-import java.util.function.Function;
+import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import tc.oc.pgm.util.concurrent.ThreadSafeConnection;
@@ -25,15 +25,15 @@ public SqlUsernameResolver(SQLDatastore datastore) {
   }
 
   protected void process(UUID uuid, CompletableFuture<UsernameResponse> future) {
-    datastore.submitQuery(new SingleSelect(uuid, future));
+    datastore.submitQuery(new SingleSelect(uuid, future)).join();
   }
 
   @Override
   protected void process(List<UUID> uuids) {
     List<List<UUID>> partitions = Lists.partition(uuids, BATCH_SIZE);
     CompletableFuture<?>[] futures = new CompletableFuture[partitions.size()];
     for (int i = 0; i < partitions.size(); i++) {
-      futures[i] = datastore.submitQuery(new BatchSelect(partitions.get(i), this::getFuture));
+      futures[i] = datastore.submitQuery(new BatchSelect(partitions.get(i), this::complete));
     }
     CompletableFuture.allOf(futures).join();
   }
@@ -71,12 +71,11 @@ public void query(PreparedStatement statement) throws SQLException {
 
   private static class BatchSelect implements ThreadSafeConnection.Query {
     private final List<UUID> uuids;
-    private final Function<UUID, CompletableFuture<UsernameResponse>> futures;
+    private final BiConsumer<UUID, UsernameResponse> completion;
 
-    public BatchSelect(
-        List<UUID> uuids, Function<UUID, CompletableFuture<UsernameResponse>> futures) {
+    public BatchSelect(List<UUID> uuids, BiConsumer<UUID, UsernameResponse> completion) {
       this.uuids = uuids;
-      this.futures = futures;
+      this.completion = completion;
     }
 
     @Override
@@ -98,17 +97,16 @@ public void query(PreparedStatement statement) throws SQLException {
           UUID uuid = UUID.fromString(result.getString(1));
           leftover.remove(uuid);
 
-          futures
-              .apply(uuid)
-              .complete(
-                  UsernameResponse.of(
-                      result.getString(2),
-                      null,
-                      Instant.ofEpochMilli(result.getLong(3)),
-                      SqlUsernameResolver.class));
+          completion.accept(
+              uuid,
+              UsernameResponse.of(
+                  result.getString(2),
+                  null,
+                  Instant.ofEpochMilli(result.getLong(3)),
+                  SqlUsernameResolver.class));
         }
 
-        leftover.forEach(uuid -> futures.apply(uuid).complete(UsernameResponse.empty()));
+        leftover.forEach(uuid -> completion.accept(uuid, UsernameResponse.empty()));
       }
     }
   }

core/src/main/java/tc/oc/pgm/namedecorations/NameDecorationRegistryImpl.java

@@ -31,7 +31,6 @@
 import tc.oc.pgm.events.PlayerPartyChangeEvent;
 import tc.oc.pgm.util.named.NameDecorationProvider;
 import tc.oc.pgm.util.player.PlayerComponent;
-import tc.oc.pgm.util.player.PlayerRenderer;
 import tc.oc.pgm.util.text.TextFormatter;
 
 @SuppressWarnings("UnstableApiUsage")

core/src/main/java/tc/oc/pgm/stats/StatsMatchModule.java

@@ -74,7 +74,6 @@
 import tc.oc.pgm.tracker.info.ProjectileInfo;
 import tc.oc.pgm.util.named.NameStyle;
 import tc.oc.pgm.util.nms.NMSHacks;
-import tc.oc.pgm.util.player.PlayerComponent;
 import tc.oc.pgm.util.text.TextFormatter;
 import tc.oc.pgm.util.usernames.UsernameResolvers;
 import tc.oc.pgm.wool.MonumentWool;

core/src/main/java/tc/oc/pgm/util/player/PlayerComponent.java

@@ -4,7 +4,6 @@
 
 import java.util.UUID;
 import net.kyori.adventure.text.Component;
-import net.kyori.adventure.text.format.NamedTextColor;
 import net.kyori.adventure.text.format.Style;
 import net.kyori.adventure.text.format.TextColor;
 import net.kyori.adventure.text.format.TextDecoration;
@@ -24,7 +23,8 @@ public final class PlayerComponent implements RenderableComponent {
 
   public static final Component UNKNOWN =
       translatable("misc.unknown", PlayerRenderer.OFFLINE_COLOR, TextDecoration.ITALIC);
-  public static final Component CONSOLE = translatable("misc.console", PlayerRenderer.OFFLINE_COLOR);
+  public static final Component CONSOLE =
+      translatable("misc.console", PlayerRenderer.OFFLINE_COLOR);
   public static final PlayerComponent UNKNOWN_PLAYER =
       new PlayerComponent(null, new PlayerData(null, null, NameStyle.SIMPLE_COLOR), Style.empty());
 
@@ -69,7 +69,8 @@ public static Component player(
   public static PlayerComponent player(
       @Nullable MatchPlayerState player, @NotNull NameStyle style) {
     if (player == null) return UNKNOWN_PLAYER;
-    return new PlayerComponent(Bukkit.getPlayer(player.getId()), new PlayerData(player, style), Style.empty());
+    return new PlayerComponent(
+        Bukkit.getPlayer(player.getId()), new PlayerData(player, style), Style.empty());
   }
 
   public static PlayerComponent player(@Nullable MatchPlayer player, @NotNull NameStyle style) {

core/src/main/java/tc/oc/pgm/util/player/PlayerData.java

@@ -1,5 +1,7 @@
 package tc.oc.pgm.util.player;
 
+import static tc.oc.pgm.util.player.PlayerRenderer.OFFLINE_COLOR;
+
 import java.util.Objects;
 import java.util.UUID;
 import net.kyori.adventure.text.format.TextColor;
@@ -12,8 +14,6 @@
 import tc.oc.pgm.api.player.MatchPlayerState;
 import tc.oc.pgm.util.named.NameStyle;
 
-import static tc.oc.pgm.util.player.PlayerRenderer.OFFLINE_COLOR;
-
 class PlayerData {
   public final @Nullable UUID uuid;
 
@@ -44,8 +44,7 @@ public PlayerData(@NotNull MatchPlayer mp, @NotNull NameStyle style) {
 
     this.name = mp.getNameLegacy();
     this.nick = Integration.getNick(mp.getBukkit());
-    this.teamColor =
-        mp.getParty() == null ? OFFLINE_COLOR : mp.getParty().getTextColor();
+    this.teamColor = mp.getParty() == null ? OFFLINE_COLOR : mp.getParty().getTextColor();
     this.dead = mp.isDead();
     this.vanish = Integration.isVanished(mp.getBukkit());
     this.online = mp.getBukkit().isOnline();

util/src/main/java/tc/oc/pgm/util/usernames/AbstractBatchingUsernameResolver.java

@@ -9,13 +9,12 @@
 public abstract class AbstractBatchingUsernameResolver extends AbstractUsernameResolver
     implements UsernameResolver {
 
-  private final String prefix = "[" + getClass().getSimpleName() + "] ";
   protected List<UUID> currentBatch = null;
 
   @Override
   public synchronized CompletableFuture<UsernameResponse> resolve(UUID uuid) {
     CompletableFuture<UsernameResponse> response =
-        futureCache.computeIfAbsent(
+        futures.computeIfAbsent(
             uuid,
             key -> {
               if (currentBatch != null) currentBatch.add(uuid);
@@ -37,12 +36,12 @@ public synchronized CompletableFuture<Void> endBatch() {
     List<UUID> batch = currentBatch;
     currentBatch = null;
     if (batch != null && !batch.isEmpty()) {
-      Bukkit.getLogger().info(prefix + "Batch resolving " + batch.size() + " uuids");
+      Bukkit.getLogger().info(LOG_PREFIX + "Batch resolving " + batch.size() + " uuids");
 
       return CompletableFuture.runAsync(
           () -> {
             process(batch);
-            Bukkit.getLogger().info(prefix + "Done resolving " + batch.size() + " uuids");
+            Bukkit.getLogger().info(LOG_PREFIX + "Done resolving " + batch.size() + " uuids");
           },
           getExecutor());
     } else {

util/src/main/java/tc/oc/pgm/util/usernames/AbstractUsernameResolver.java

@@ -1,5 +1,6 @@
 package tc.oc.pgm.util.usernames;
 
+
 import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
@@ -9,7 +10,8 @@
 
 public abstract class AbstractUsernameResolver implements UsernameResolver {
 
-  protected final Map<UUID, CompletableFuture<UsernameResponse>> futureCache =
+  protected final String LOG_PREFIX = "[" + getClass().getSimpleName() + "] ";
+  protected final Map<UUID, CompletableFuture<UsernameResponse>> futures =
       new ConcurrentHashMap<>();
 
   protected Executor getExecutor() {
@@ -19,7 +21,7 @@ protected Executor getExecutor() {
   @Override
   public synchronized CompletableFuture<UsernameResponse> resolve(UUID uuid) {
     CompletableFuture<UsernameResponse> response =
-        futureCache.computeIfAbsent(uuid, this::createFuture);
+        futures.computeIfAbsent(uuid, this::createFuture);
     getExecutor().execute(() -> process(uuid, response));
     return response;
   }
@@ -29,16 +31,18 @@ public void startBatch() {}
 
   @Override
   public synchronized CompletableFuture<Void> endBatch() {
-    return CompletableFuture.allOf(futureCache.values().toArray(new CompletableFuture[0]));
+    return CompletableFuture.allOf(futures.values().toArray(new CompletableFuture[0]));
   }
 
   protected CompletableFuture<UsernameResponse> createFuture(UUID uuid) {
-    return new CompletableFuture<UsernameResponse>()
-        .whenComplete((o, t) -> futureCache.remove(uuid));
+    CompletableFuture<UsernameResponse> future = new CompletableFuture<>();
+    future.whenComplete((o, t) -> futures.remove(uuid));
+    return future;
   }
 
-  protected CompletableFuture<UsernameResponse> getFuture(UUID uuid) {
-    return futureCache.computeIfAbsent(uuid, this::createFuture);
+  protected void complete(UUID uuid, UsernameResponse response) {
+    CompletableFuture<UsernameResponse> future = futures.get(uuid);
+    if (future != null) future.complete(response);
   }
 
   protected abstract void process(UUID uuid, CompletableFuture<UsernameResponse> future);

util/src/main/java/tc/oc/pgm/util/usernames/ApiUsernameResolver.java

@@ -63,7 +63,14 @@ protected void process(List<UUID> uuids) {
     Instant now = Instant.now();
 
     int fails = 0;
+    boolean stopped = false;
     for (UUID id : uuids) {
+      // Even if there's an issue, we need to complete the futures.
+      if (stopped) {
+        complete(id, UsernameResponse.empty());
+        continue;
+      }
+
       String name = null;
       try {
         name = resolveSync(id);
@@ -72,17 +79,19 @@ protected void process(List<UUID> uuids) {
         errors.put(id, t);
         if (++fails > MAX_SEQUENTIAL_FAILURES
             || t instanceof UnknownHostException
-            || t instanceof NoRouteToHostException) break;
+            || t instanceof NoRouteToHostException) {
+          stopped = true;
+        }
       } finally {
-        getFuture(id).complete(UsernameResponse.of(name, now, ApiUsernameResolver.class));
+        complete(id, UsernameResponse.of(name, now, ApiUsernameResolver.class));
       }
     }
 
     if (!errors.isEmpty()) {
       Bukkit.getLogger()
           .log(
               Level.WARNING,
-              "Could not resolve " + errors.size() + " usernames",
+              LOG_PREFIX + "Could not resolve " + errors.size() + " usernames",
               errors.values().iterator().next());
     }
   }

util/src/main/java/tc/oc/pgm/util/usernames/BukkitUsernameResolver.java

@@ -9,6 +9,7 @@
 import org.bukkit.plugin.Plugin;
 import org.jetbrains.annotations.NotNull;
 import tc.oc.pgm.util.bukkit.BukkitUtils;
+import tc.oc.pgm.util.nms.NMSHacks;
 
 public final class BukkitUsernameResolver extends AbstractUsernameResolver {
   private static final SyncExecutor EXECUTOR = new SyncExecutor();
@@ -21,12 +22,25 @@ protected Executor getExecutor() {
 
   @Override
   protected void process(UUID uuid, CompletableFuture<UsernameResponse> future) {
-    OfflinePlayer player = Bukkit.getOfflinePlayer(uuid);
-    future.complete(
-        UsernameResponse.of(
-            player.getName(),
-            Instant.ofEpochMilli(player.getLastPlayed()),
-            BukkitUsernameResolver.class));
+    future.complete(resolveUser(uuid));
+  }
+
+  private UsernameResponse resolveUser(UUID uuid) {
+    OfflinePlayer pl = Bukkit.getOfflinePlayer(uuid);
+    if (pl.getName() != null) {
+      long lastPlayed = pl.getLastPlayed();
+      return UsernameResponse.of(
+          pl.getName(),
+          lastPlayed > 0 ? Instant.ofEpochMilli(lastPlayed) : Instant.now(),
+          BukkitUsernameResolver.class);
+    }
+
+    // Does vanilla user cache hold this player?
+    String name = NMSHacks.getPlayerName(uuid);
+    if (name != null) {
+      return UsernameResponse.of(name, Instant.now(), BukkitUsernameResolver.class);
+    }
+    return UsernameResponse.empty();
   }
 
   private static class SyncExecutor implements Executor {

util/src/main/java/tc/oc/pgm/util/usernames/ChainedUsernameResolver.java

@@ -19,9 +19,9 @@ public CompletableFuture<UsernameResolver.UsernameResponse> resolve(UUID uuid) {
     if (future.isDone() && future.getNow(UsernameResponse.empty()).isAcceptable()) {
       return future;
     }
+
     for (int i = 1; i < resolvers.length; i++) {
       UsernameResolver nextResolver = resolvers[i];
-
       future =
           future.thenCompose(
               first -> {