From 74085dad6f2cfac31226994c13bd8c68e3114d88 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:15:26 +0000 Subject: [PATCH 1/4] fix: complete offline pending return owners from a snapshot --- .../net/tfminecraft/recycler/Recycler.java | 1 + .../recycler/command/CommandManager.java | 24 ++++++++- .../recycler/LifecycleCommandsTest.java | 54 +++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/tfminecraft/recycler/Recycler.java b/src/main/java/net/tfminecraft/recycler/Recycler.java index 4038fe9..efe0813 100644 --- a/src/main/java/net/tfminecraft/recycler/Recycler.java +++ b/src/main/java/net/tfminecraft/recycler/Recycler.java @@ -49,6 +49,7 @@ public void onEnable() { } getServer().getPluginManager().registerEvents(recyclerManager, this); escrowManager.loadPending(); + commandManager.startCompletionRefresh(this); if (escrowManager.identity() instanceof net.tfminecraft.recycler.ownership.CharacterQueueBridge) net.tfminecraft.recycler.ownership.CharacterQueueBridge.start(this); getServer().getScheduler().runTaskTimer(this, () -> { diff --git a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java index 688bff8..89bd7c6 100644 --- a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java +++ b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.Locale; import java.util.UUID; +import java.util.TreeSet; import org.bukkit.Bukkit; import org.bukkit.OfflinePlayer; @@ -13,6 +14,7 @@ import org.bukkit.command.CommandSender; import org.bukkit.command.TabCompleter; import org.bukkit.entity.Player; +import org.bukkit.plugin.Plugin; import net.tfminecraft.recycler.Messages; import net.tfminecraft.recycler.Recycler; @@ -23,6 +25,23 @@ public final class CommandManager implements CommandExecutor, TabCompleter { private static final List ROOT_SUBCOMMANDS = List.of("reload", "escrow"); private static final List ESCROW_SUBCOMMANDS = List.of("list", "return"); + private List pendingOwnerNames = List.of(); + + public void startCompletionRefresh(Plugin plugin) { + refreshPendingOwnerNames(); + plugin.getServer().getScheduler().runTaskTimer(plugin, this::refreshPendingOwnerNames, 100L, 100L); + } + + private void refreshPendingOwnerNames() { + // Queue discovery and offline-name resolution may read disk; never do them while typing. + TreeSet names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + for (UUID owner : Recycler.plugin.getEscrowManager().listPendingPlayerIds()) { + String name = Bukkit.getOfflinePlayer(owner).getName(); + if (name != null) names.add(name); + } + pendingOwnerNames = List.copyOf(names); + } + @Override public boolean onCommand(CommandSender sender, Command command, String label, String[] args) { if (!sender.hasPermission("recycler.admin")) { @@ -104,11 +123,12 @@ public List onTabComplete(CommandSender sender, Command command, String return filterPrefix(ESCROW_SUBCOMMANDS, args[1]); } if (args.length == 3 && "escrow".equalsIgnoreCase(args[0]) && "return".equalsIgnoreCase(args[1])) { - List names = new ArrayList<>(); + TreeSet names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + names.addAll(pendingOwnerNames); for (Player player : Bukkit.getOnlinePlayers()) { names.add(player.getName()); } - return filterPrefix(names, args[2]); + return filterPrefix(new ArrayList<>(names), args[2]); } return Collections.emptyList(); } diff --git a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java index 2b58cb1..5806c75 100644 --- a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java +++ b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java @@ -90,6 +90,60 @@ void tabCompletionFiltersAtEveryDepth() { assertTrue(c.onTabComplete(sender, null, "r", args).isEmpty()); } + @Test + void returnCompletionIncludesRecognizedOfflineQueuesWithoutPerKeystrokeDiscovery() throws Exception { + var escrow = spy(new EscrowManager()); + when(plugin.getEscrowManager()).thenReturn(escrow); + var alice = server.addPlayer("Alice"); + var aliceRow = queued(escrow, token(alice).owner(), "pending"); + alice.disconnect(); + var bob = server.addPlayer("Bob"); + queued(escrow, token(bob).owner(), "escrow"); + var held = server.addPlayer("Herbie"); + queued(escrow, token(held).owner(), "held"); + held.disconnect(); + var finished = server.addPlayer("Finished"); + queued(escrow, token(finished).owner(), "delivered"); + finished.disconnect(); + UUID unknownId = UUID.randomUUID(); + queued(escrow, new net.tfminecraft.recycler.ownership.QueueIdentity.Owner(unknownId, UUID.randomUUID()), "pending"); + var sender = mock(CommandSender.class); + when(sender.hasPermission("recycler.admin")).thenReturn(true); + var commands = new CommandManager(); + try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS)) { + var unknown = mock(OfflinePlayer.class); + bukkit.when(() -> Bukkit.getOfflinePlayer(unknownId)).thenReturn(unknown); + commands.startCompletionRefresh(plugin); + clearInvocations(escrow); + bukkit.clearInvocations(); + assertEquals(List.of("Alice", "Bob", "Herbie"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", ""})); + assertEquals(List.of("Alice"), commands.onTabComplete(sender, null, "recycler", new String[]{"ESCROW", "RETURN", "aL"})); + assertEquals(List.of("Bob"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "b"})); + assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "Finished"}).isEmpty()); + assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "missing"}).isEmpty()); + verify(escrow, never()).listPendingPlayerIds(); + bukkit.verify(() -> Bukkit.getOfflinePlayer(any(UUID.class)), never()); + commands.onCommand(sender, null, "recycler", new String[]{"escrow", "return", "Alice"}); + verify(sender).sendMessage(contains("Player offline. Pending return file exists for §fAlice")); + when(sender.hasPermission("recycler.admin")).thenReturn(false); + assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", ""}).isEmpty()); + when(sender.hasPermission("recycler.admin")).thenReturn(true); + escrow.store().update(aliceRow, row -> row.addProperty("state", "delivered")); + var later = server.addPlayer("LaterOwner"); + queued(escrow, token(later).owner(), "pending"); + later.disconnect(); + server.getScheduler().performTicks(100); + assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "Alice"}).isEmpty()); + assertEquals(List.of("LaterOwner"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "la"})); + } + } + + private com.google.gson.JsonObject queued(EscrowManager escrow, net.tfminecraft.recycler.ownership.QueueIdentity.Owner owner, String state) throws IOException { + var payload = new com.google.gson.JsonObject(); + payload.addProperty("state", state); + return escrow.store().create(owner, UUID.randomUUID(), payload); + } + @Test void realPluginLifecycleCreatesResourcesRegistersManagersAndReloads() throws Exception { for (String name : List.of("TLibs", "MMOItems", "MythicLib", "ItemsAdder")) From ac22bb99d591fc86116e53c95848dc63988c5dea Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:21:57 +0000 Subject: [PATCH 2/4] fix: index pending return owners without polling --- .../net/tfminecraft/recycler/Recycler.java | 2 +- .../recycler/command/CommandManager.java | 20 ++-- .../recycler/manager/EscrowManager.java | 9 +- .../recycler/ownership/ReturnStore.java | 39 +++++- .../recycler/LifecycleCommandsTest.java | 83 ++++++++++++- .../ownership/ReturnStoreIndexTest.java | 113 ++++++++++++++++++ 6 files changed, 240 insertions(+), 26 deletions(-) create mode 100644 src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java diff --git a/src/main/java/net/tfminecraft/recycler/Recycler.java b/src/main/java/net/tfminecraft/recycler/Recycler.java index efe0813..1125293 100644 --- a/src/main/java/net/tfminecraft/recycler/Recycler.java +++ b/src/main/java/net/tfminecraft/recycler/Recycler.java @@ -49,7 +49,7 @@ public void onEnable() { } getServer().getPluginManager().registerEvents(recyclerManager, this); escrowManager.loadPending(); - commandManager.startCompletionRefresh(this); + commandManager.trackPendingOwners(); if (escrowManager.identity() instanceof net.tfminecraft.recycler.ownership.CharacterQueueBridge) net.tfminecraft.recycler.ownership.CharacterQueueBridge.start(this); getServer().getScheduler().runTaskTimer(this, () -> { diff --git a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java index 89bd7c6..a36b41a 100644 --- a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java +++ b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java @@ -14,7 +14,6 @@ import org.bukkit.command.CommandSender; import org.bukkit.command.TabCompleter; import org.bukkit.entity.Player; -import org.bukkit.plugin.Plugin; import net.tfminecraft.recycler.Messages; import net.tfminecraft.recycler.Recycler; @@ -27,17 +26,20 @@ public final class CommandManager implements CommandExecutor, TabCompleter { private List pendingOwnerNames = List.of(); - public void startCompletionRefresh(Plugin plugin) { - refreshPendingOwnerNames(); - plugin.getServer().getScheduler().runTaskTimer(plugin, this::refreshPendingOwnerNames, 100L, 100L); + public void trackPendingOwners() { + Recycler.plugin.getEscrowManager().store().onPendingOwnersChanged(this::refreshPendingOwnerNames); } - private void refreshPendingOwnerNames() { - // Queue discovery and offline-name resolution may read disk; never do them while typing. + private void refreshPendingOwnerNames(List owners) { + // Resolve profiles only when the authoritative queue index changes, never while typing. TreeSet names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); - for (UUID owner : Recycler.plugin.getEscrowManager().listPendingPlayerIds()) { - String name = Bukkit.getOfflinePlayer(owner).getName(); - if (name != null) names.add(name); + for (UUID owner : owners) { + try { + String name = Bukkit.getOfflinePlayer(owner).getName(); + if (name != null) names.add(name); + } catch (RuntimeException ex) { + Recycler.plugin.getLogger().warning("Recycler pending owner name unavailable: " + owner + " " + ex.getMessage()); + } } pendingOwnerNames = List.copyOf(names); } diff --git a/src/main/java/net/tfminecraft/recycler/manager/EscrowManager.java b/src/main/java/net/tfminecraft/recycler/manager/EscrowManager.java index 1ce1b4f..2861049 100644 --- a/src/main/java/net/tfminecraft/recycler/manager/EscrowManager.java +++ b/src/main/java/net/tfminecraft/recycler/manager/EscrowManager.java @@ -188,14 +188,7 @@ public boolean hasPending(UUID account) { public List listMemoryEscrowPlayerIds() { return List.of(); } public int countMemoryEscrow() { return 0; } public List listPendingPlayerIds() { - List accounts = new ArrayList<>(); - run(() -> { - for (Path path : store().allFiles()) { - try { JsonObject row = ReturnStore.read(path); if (!ReturnStore.terminal(row)) accounts.add(ReturnStore.owner(row).account()); } - catch (IOException | RuntimeException ex) { warning("Held queue: " + path); } - } - }); - return accounts.stream().distinct().toList(); + return store().pendingPlayerIds(); } public int countPendingFiles() { return describeQueues().size(); } private interface Work { void run() throws IOException; } diff --git a/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java b/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java index 1c95fd1..14158f2 100644 --- a/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java +++ b/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java @@ -9,6 +9,7 @@ import java.util.Base64; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.function.BooleanSupplier; import java.util.function.Consumer; @@ -20,6 +21,8 @@ public static final class Conflict extends IOException { } private static final List STATES = List.of("depositing", "escrow", "pending", "delivering", "held", "delivered", "consumed", "cancelled"); private final Path root; + private final Map pendingOwners = new java.util.LinkedHashMap<>(); + private Consumer> pendingOwnersChanged = accounts -> {}; private final java.util.Map>>> validated = new java.util.HashMap<>(); public ReturnStore(Path root) { this.root = root.toAbsolutePath().normalize(); } public Path root() { return root; } @@ -32,14 +35,35 @@ public static QueueIdentity.Owner owner(JsonObject row) { } public static String text(JsonObject row, String key) { return row.get(key).getAsString(); } + /** Recovery seeds this index; successful queue publications keep it current without discovery. */ + public List pendingPlayerIds() { return pendingOwners.values().stream().distinct().toList(); } + public void onPendingOwnersChanged(Consumer> listener) { + pendingOwnersChanged = listener; + listener.accept(pendingPlayerIds()); + } + private void index(Path path, JsonObject row) { + if (terminal(row)) forget(path); + else { + UUID account = owner(row).account(); + if (!account.equals(pendingOwners.put(path, account))) pendingOwnersChanged.accept(pendingPlayerIds()); + } + } + private void forget(Path path) { + if (pendingOwners.remove(path) != null) pendingOwnersChanged.accept(pendingPlayerIds()); + } + private JsonObject readRow(Path path) throws IOException { + try { + JsonObject row = read(path); validate(path, row); index(path, row); return row; + } catch (IOException ex) { forget(path); throw ex; } + } + public List rows(QueueIdentity.Owner owner) throws IOException { requireDirectory(root.resolve("character-returns")); Path folder = root.resolve("character-returns").resolve(owner.character().toString()); requireDirectory(folder); List rows = new ArrayList<>(); for (Path path : files(folder)) { - JsonObject row = read(path); - validate(path, row); + JsonObject row = readRow(path); if (!owner.equals(owner(row))) throw new Conflict("Return owner differs from its character folder: " + path); rows.add(row); } @@ -60,20 +84,23 @@ public JsonObject create(QueueIdentity.Owner owner, UUID id, JsonObject payload) row.addProperty("format", 1); row.addProperty("id", id.toString()); setOwner(row, owner); validate(file(owner, id), row); QueueFiles.write(file(owner, id), QueueJson.bytes(row), false); + index(file(owner, id), row); return row; } public JsonObject update(JsonObject expected, Consumer change) throws IOException { QueueIdentity.Owner owner = owner(expected); requireWritable(owner); Path file = file(owner, id(expected)); - JsonObject current = read(file); validate(file, current); + JsonObject current = readRow(file); if (!current.equals(expected)) throw new Conflict("Return changed before its effect: " + id(expected)); change.accept(current); validate(file, current); - write(file, current); return current; + write(file, current); index(file, current); return current; } public void recover() throws IOException { + pendingOwners.clear(); + pendingOwnersChanged.accept(pendingPlayerIds()); for (Path path : allFiles()) { try { - JsonObject row = read(path); validate(path, row); + JsonObject row = readRow(path); // No in-memory GUI survived a restart. Ambiguous inventory intents stay untouched. if ("escrow".equals(text(row, "state"))) update(row, copy -> copy.addProperty("state", "pending")); } catch (Conflict ex) { /* Retain unloadable documents for staff; other queues remain readable. */ } @@ -361,10 +388,12 @@ private void apply(Path file, JsonObject journal, JsonObject plan, BooleanSuppli if (!guard.getAsBoolean()) throw new IOException("Estate changed before return publication"); QueueFiles.write(target, Base64.getDecoder().decode(text(plan, "after")), false); } + index(target, readPayload(plan, "after")); plan.addProperty("phase", "published"); write(file, journal); if (remove) { if (!guard.getAsBoolean()) throw new IOException("Estate changed before source removal"); QueueFiles.delete(source); + forget(source); } plan.addProperty("phase", "moved"); write(file, journal); } diff --git a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java index 5806c75..d513bc0 100644 --- a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java +++ b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java @@ -8,6 +8,7 @@ import java.util.*; import net.tfminecraft.recycler.command.*; import net.tfminecraft.recycler.manager.*; +import net.tfminecraft.recycler.ownership.ReturnStore; import org.bukkit.*; import org.bukkit.command.*; import org.junit.jupiter.api.Test; @@ -110,19 +111,25 @@ void returnCompletionIncludesRecognizedOfflineQueuesWithoutPerKeystrokeDiscovery var sender = mock(CommandSender.class); when(sender.hasPermission("recycler.admin")).thenReturn(true); var commands = new CommandManager(); - try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS)) { + try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS); + var queues = mockStatic(ReturnStore.class, CALLS_REAL_METHODS)) { var unknown = mock(OfflinePlayer.class); bukkit.when(() -> Bukkit.getOfflinePlayer(unknownId)).thenReturn(unknown); - commands.startCompletionRefresh(plugin); + escrow.loadPending(); + commands.trackPendingOwners(); clearInvocations(escrow); bukkit.clearInvocations(); + queues.clearInvocations(); assertEquals(List.of("Alice", "Bob", "Herbie"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", ""})); assertEquals(List.of("Alice"), commands.onTabComplete(sender, null, "recycler", new String[]{"ESCROW", "RETURN", "aL"})); assertEquals(List.of("Bob"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "b"})); assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "Finished"}).isEmpty()); assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "missing"}).isEmpty()); + server.getScheduler().performTicks(300); + assertEquals(List.of("Alice", "Bob", "Herbie"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", ""})); verify(escrow, never()).listPendingPlayerIds(); bukkit.verify(() -> Bukkit.getOfflinePlayer(any(UUID.class)), never()); + queues.verify(() -> ReturnStore.read(any(Path.class)), never()); commands.onCommand(sender, null, "recycler", new String[]{"escrow", "return", "Alice"}); verify(sender).sendMessage(contains("Player offline. Pending return file exists for §fAlice")); when(sender.hasPermission("recycler.admin")).thenReturn(false); @@ -132,9 +139,74 @@ void returnCompletionIncludesRecognizedOfflineQueuesWithoutPerKeystrokeDiscovery var later = server.addPlayer("LaterOwner"); queued(escrow, token(later).owner(), "pending"); later.disconnect(); - server.getScheduler().performTicks(100); assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "Alice"}).isEmpty()); assertEquals(List.of("LaterOwner"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "la"})); + bukkit.clearInvocations(); + queues.clearInvocations(); + server.getScheduler().performTicks(300); + assertEquals(List.of("Bob", "Herbie", "LaterOwner"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", ""})); + queues.verify(() -> ReturnStore.read(any(Path.class)), never()); + bukkit.verify(() -> Bukkit.getOfflinePlayer(any(UUID.class)), never()); + var mixedCaseBob = mock(OfflinePlayer.class); + when(mixedCaseBob.getName()).thenReturn("bOb"); + bukkit.when(() -> Bukkit.getOfflinePlayer(bob.getUniqueId())).thenReturn(mixedCaseBob); + queued(escrow, token(bob).owner(), "held"); + assertEquals(List.of("bOb"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "B"})); + } + } + + @Test + void offlineCompletionFollowsEstateSourceRemovalAndRecipientTerminalization() throws Exception { + var escrow = new EscrowManager(); + when(plugin.getEscrowManager()).thenReturn(escrow); + var source = server.addPlayer("PreviousOwner"); + var from = token(source).owner(); + queued(escrow, from, "pending"); + source.disconnect(); + var recipient = server.addPlayer("RecipientOwner"); + var to = token(recipient).owner(); + recipient.disconnect(); + var commands = new CommandManager(); + commands.trackPendingOwners(); + var sender = mock(CommandSender.class); + when(sender.hasPermission("recycler.admin")).thenReturn(true); + String[] args = {"escrow", "return", ""}; + assertEquals(List.of("PreviousOwner"), commands.onTabComplete(sender, null, "recycler", args)); + UUID death = UUID.randomUUID(); + var frozen = net.tfminecraft.tlibs.character.estate.Recipient.FROZEN; + var decision = new net.tfminecraft.tlibs.character.estate.EstateDecision(death, + new net.tfminecraft.tlibs.character.CharacterRef(from.account(), from.character()), Optional.empty(), java.time.Instant.EPOCH, + Optional.empty(), Optional.empty(), Optional.empty(), frozen, + net.tfminecraft.tlibs.character.estate.Recipient.character(new net.tfminecraft.tlibs.character.CharacterRef(to.account(), to.character())), + frozen, net.tfminecraft.tlibs.character.estate.Recipient.SENDER, 1); + escrow.store().transfer(death, decision.toString(), from, to, () -> true); + assertEquals(List.of("RecipientOwner"), commands.onTabComplete(sender, null, "recycler", args)); + escrow.store().update(escrow.store().rows(to).getFirst(), row -> row.addProperty("state", "delivered")); + assertTrue(commands.onTabComplete(sender, null, "recycler", args).isEmpty()); + } + + @Test + void unavailableOfflineNameCannotInterruptDurableQueueChanges() throws Exception { + var escrow = new EscrowManager(); + when(plugin.getEscrowManager()).thenReturn(escrow); + var commands = new CommandManager(); + commands.trackPendingOwners(); + UUID unavailable = UUID.randomUUID(); + var owner = new net.tfminecraft.recycler.ownership.QueueIdentity.Owner(unavailable, UUID.randomUUID()); + try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS)) { + bukkit.when(() -> Bukkit.getOfflinePlayer(unavailable)).thenThrow(new IllegalStateException("profile unavailable")); + var row = queued(escrow, owner, "pending"); + assertEquals(List.of(unavailable), escrow.listPendingPlayerIds()); + assertEquals(row, ReturnStore.read(escrow.store().file(owner, ReturnStore.id(row)))); + var sender = mock(CommandSender.class); + when(sender.hasPermission("recycler.admin")).thenReturn(true); + assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", ""}).isEmpty()); + var known = mock(OfflinePlayer.class); + when(known.getName()).thenReturn("KnownOwner"); + bukkit.when(() -> Bukkit.getOfflinePlayer(unavailable)).thenReturn(known); + escrow.store().update(row, copy -> copy.addProperty("state", "consumed")); + queued(escrow, owner, "held"); + assertEquals(List.of("KnownOwner"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "kN"})); } } @@ -157,6 +229,11 @@ void realPluginLifecycleCreatesResourcesRegistersManagersAndReloads() throws Exc assertInstanceOf(CommandManager.class, loaded.getCommand("recycler").getExecutor()); assertTrue(loaded.reloadAll()); server.getScheduler().performTicks(2); + var store = spy(loaded.getEscrowManager().store()); + var field = EscrowManager.class.getDeclaredField("store"); + field.setAccessible(true); field.set(loaded.getEscrowManager(), store); + server.getScheduler().performTicks(300); + verify(store, never()).allFiles(); loaded.onEnable(); Files.writeString(loaded.getDataFolder().toPath().resolve("config.yml"), "bad: ["); assertFalse(loaded.reloadAll()); diff --git a/src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java b/src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java new file mode 100644 index 0000000..024cadc --- /dev/null +++ b/src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java @@ -0,0 +1,113 @@ +package net.tfminecraft.recycler.ownership; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import com.google.gson.JsonObject; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import net.tfminecraft.tlibs.character.CharacterRef; +import net.tfminecraft.tlibs.character.estate.EstateDecision; +import net.tfminecraft.tlibs.character.estate.Recipient; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class ReturnStoreIndexTest { + @TempDir Path root; + final QueueIdentity.Owner from = owner(), to = owner(); + + QueueIdentity.Owner owner() { return new QueueIdentity.Owner(UUID.randomUUID(), UUID.randomUUID()); } + JsonObject create(ReturnStore store, QueueIdentity.Owner owner, String state) throws IOException { + JsonObject row = new JsonObject(); row.addProperty("state", state); + return store.create(owner, UUID.randomUUID(), row); + } + + @Test void anAccountRemainsPendingUntilItsLastNonterminalCharacterRecordFinishes() throws Exception { + var store = new ReturnStore(root); + var first = create(store, from, "pending"); + var otherCharacter = new QueueIdentity.Owner(from.account(), UUID.randomUUID()); + var second = create(store, otherCharacter, "held"); + create(store, to, "delivered"); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + first = store.update(first, row -> row.addProperty("state", "delivering")); + store.update(first, row -> row.addProperty("state", "delivered")); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + store.update(second, row -> row.addProperty("state", "cancelled")); + assertTrue(store.pendingPlayerIds().isEmpty()); + assertEquals(1, store.rows(from).size()); + assertEquals(1, store.rows(otherCharacter).size()); + } + + @Test void recoverySeedsValidOwnersAndSnapshotReadsNeverDiscoverRetainedHistory() throws Exception { + var original = new ReturnStore(root); + create(original, from, "escrow"); + create(original, from, "delivering"); + create(original, to, "depositing"); + create(original, owner(), "consumed"); + var invalid = owner(); + var broken = create(original, invalid, "pending"); + Files.writeString(original.file(invalid, ReturnStore.id(broken)), "broken"); + var recovered = new ReturnStore(root); + recovered.recover(); + assertEquals(Set.of(from.account(), to.account()), Set.copyOf(recovered.pendingPlayerIds())); + assertEquals(Set.of("pending", "delivering"), recovered.rows(from).stream().map(row -> ReturnStore.text(row, "state")).collect(java.util.stream.Collectors.toSet())); + assertEquals("depositing", ReturnStore.text(recovered.rows(to).getFirst(), "state")); + try (var disk = mockStatic(ReturnStore.class, CALLS_REAL_METHODS)) { + for (int i = 0; i < 20; i++) assertEquals(Set.of(from.account(), to.account()), Set.copyOf(recovered.pendingPlayerIds())); + disk.verify(() -> ReturnStore.files(any(Path.class)), never()); + disk.verify(() -> ReturnStore.read(any(Path.class)), never()); + } + for (var row : recovered.rows(from)) recovered.update(row, copy -> copy.addProperty("state", "consumed")); + var last = recovered.rows(to).getFirst(); + Files.delete(recovered.file(to, ReturnStore.id(last))); + recovered.recover(); + assertTrue(recovered.pendingPlayerIds().isEmpty()); + } + + @Test void migrationAndEstateRemovalUpdateOwnersWithoutRediscoveringLegacyOrTerminalRecords() throws Exception { + Path legacy = root.resolve("pending_returns/" + from.account() + ".json"); + Files.createDirectories(legacy.getParent()); Files.writeString(legacy, "{\"type\":\"DIAMOND\"}"); + var identity = mock(QueueIdentity.class); + when(identity.heirsReady()).thenReturn(true); + when(identity.heir(from.account())).thenReturn(Optional.of(from)); + var store = new ReturnStore(root); + store.migrate(identity); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + var row = store.rows(from).getFirst(); + UUID death = UUID.randomUUID(); + var decision = new EstateDecision(death, new CharacterRef(from.account(), from.character()), Optional.empty(), Instant.EPOCH, + Optional.empty(), Optional.empty(), Optional.empty(), Recipient.FROZEN, + Recipient.character(new CharacterRef(to.account(), to.character())), Recipient.FROZEN, Recipient.SENDER, 1); + store.transfer(death, decision.toString(), from, to, () -> true); + assertEquals(List.of(to.account()), store.pendingPlayerIds()); + assertFalse(Files.exists(store.file(from, ReturnStore.id(row)))); + assertTrue(Files.exists(legacy)); + store.update(store.rows(to).getFirst(), copy -> copy.addProperty("state", "delivered")); + store.migrate(identity); + assertTrue(store.pendingPlayerIds().isEmpty()); + } + + @Test void failedWritesDoNotPublishOrTerminalizeAnIndexedOwner() throws Exception { + var store = new ReturnStore(root); + try (var disk = mockStatic(QueueFiles.class, CALLS_REAL_METHODS)) { + disk.when(() -> QueueFiles.write(any(), any(), anyBoolean())).thenThrow(new IOException("full disk")); + assertThrows(IOException.class, () -> create(store, from, "pending")); + assertTrue(store.pendingPlayerIds().isEmpty()); + } + var row = create(store, from, "pending"); + try (var disk = mockStatic(QueueFiles.class, CALLS_REAL_METHODS)) { + disk.when(() -> QueueFiles.write(any(), any(), eq(true))).thenThrow(new IOException("full disk")); + assertThrows(IOException.class, () -> store.update(row, copy -> copy.addProperty("state", "consumed"))); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + } + assertEquals(row, store.rows(from).getFirst()); + store.update(row, copy -> copy.addProperty("state", "consumed")); + assertTrue(store.pendingPlayerIds().isEmpty()); + } +} From 2dd26ad7c9e3ca2f420f7512823ef3bb98ad0e16 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:28:46 +0000 Subject: [PATCH 3/4] fix: refresh pending owner names on join --- .../net/tfminecraft/recycler/Recycler.java | 1 + .../recycler/command/CommandManager.java | 30 +++++-- .../recycler/LifecycleCommandsTest.java | 83 +++++++++++++++++++ 3 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/tfminecraft/recycler/Recycler.java b/src/main/java/net/tfminecraft/recycler/Recycler.java index 1125293..d35ddf5 100644 --- a/src/main/java/net/tfminecraft/recycler/Recycler.java +++ b/src/main/java/net/tfminecraft/recycler/Recycler.java @@ -50,6 +50,7 @@ public void onEnable() { getServer().getPluginManager().registerEvents(recyclerManager, this); escrowManager.loadPending(); commandManager.trackPendingOwners(); + getServer().getPluginManager().registerEvents(commandManager, this); if (escrowManager.identity() instanceof net.tfminecraft.recycler.ownership.CharacterQueueBridge) net.tfminecraft.recycler.ownership.CharacterQueueBridge.start(this); getServer().getScheduler().runTaskTimer(this, () -> { diff --git a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java index a36b41a..05cc46b 100644 --- a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java +++ b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java @@ -2,8 +2,11 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.TreeSet; @@ -14,34 +17,47 @@ import org.bukkit.command.CommandSender; import org.bukkit.command.TabCompleter; import org.bukkit.entity.Player; +import org.bukkit.event.EventHandler; +import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerJoinEvent; import net.tfminecraft.recycler.Messages; import net.tfminecraft.recycler.Recycler; import net.tfminecraft.recycler.manager.EscrowManager; -public final class CommandManager implements CommandExecutor, TabCompleter { +public final class CommandManager implements CommandExecutor, TabCompleter, Listener { private static final List ROOT_SUBCOMMANDS = List.of("reload", "escrow"); private static final List ESCROW_SUBCOMMANDS = List.of("list", "return"); - private List pendingOwnerNames = List.of(); + private Map pendingOwnerNames = new HashMap<>(); + private Set pendingOwners = Set.of(); public void trackPendingOwners() { Recycler.plugin.getEscrowManager().store().onPendingOwnersChanged(this::refreshPendingOwnerNames); } private void refreshPendingOwnerNames(List owners) { - // Resolve profiles only when the authoritative queue index changes, never while typing. - TreeSet names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + // Resolve profiles when the queue index changes; joins supply current names directly. + Map names = new HashMap<>(); for (UUID owner : owners) { try { String name = Bukkit.getOfflinePlayer(owner).getName(); - if (name != null) names.add(name); + if (name != null) names.put(owner, name); } catch (RuntimeException ex) { Recycler.plugin.getLogger().warning("Recycler pending owner name unavailable: " + owner + " " + ex.getMessage()); } } - pendingOwnerNames = List.copyOf(names); + pendingOwners = Set.copyOf(owners); + pendingOwnerNames = names; + } + + @EventHandler + public void onJoin(PlayerJoinEvent event) { + Player player = event.getPlayer(); + if (pendingOwners.contains(player.getUniqueId())) { + pendingOwnerNames.put(player.getUniqueId(), player.getName()); + } } @Override @@ -126,7 +142,7 @@ public List onTabComplete(CommandSender sender, Command command, String } if (args.length == 3 && "escrow".equalsIgnoreCase(args[0]) && "return".equalsIgnoreCase(args[1])) { TreeSet names = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); - names.addAll(pendingOwnerNames); + names.addAll(pendingOwnerNames.values()); for (Player player : Bukkit.getOnlinePlayers()) { names.add(player.getName()); } diff --git a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java index d513bc0..dd3409b 100644 --- a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java +++ b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java @@ -155,6 +155,87 @@ void returnCompletionIncludesRecognizedOfflineQueuesWithoutPerKeystrokeDiscovery } } + @Test + void pendingOwnerRenameOnJoinReplacesOldCompletionThroughLogoutWithoutQueueChanges() throws Exception { + for (String name : List.of("TLibs", "MMOItems", "MythicLib", "ItemsAdder")) + MockBukkit.createMockPlugin(name); + var loaded = MockBukkit.load(Recycler.class); + var commands = (CommandManager) loaded.getCommand("recycler").getExecutor(); + var escrow = loaded.getEscrowManager(); + var player = server.addPlayer("OldName"); + var owner = token(player).owner(); + for (int slot = 0; slot < 36; slot++) + player.getInventory().setItem(slot, new org.bukkit.inventory.ItemStack(Material.STONE, 64)); + escrow.putEscrow(player.getUniqueId(), item(Material.DIAMOND)); + escrow.pending(owner); + player.disconnect(); + var row = escrow.store().rows(owner).getFirst(); + Path file = escrow.store().file(owner, ReturnStore.id(row)); + byte[] before = Files.readAllBytes(file); + var store = spy(escrow.store()); + var field = EscrowManager.class.getDeclaredField("store"); + field.setAccessible(true); field.set(escrow, store); + var sender = mock(CommandSender.class); + when(sender.hasPermission("recycler.admin")).thenReturn(true); + String[] args = {"escrow", "return", ""}; + assertEquals(List.of("OldName"), commands.onTabComplete(sender, null, "recycler", args)); + player.setName("NewName"); + assertTrue(player.reconnect()); + var whileOnline = commands.onTabComplete(sender, null, "recycler", args); + player.disconnect(); + var afterLogout = commands.onTabComplete(sender, null, "recycler", args); + assertEquals("NewName", Bukkit.getOfflinePlayer(player.getUniqueId()).getName()); + commands.onCommand(sender, null, "recycler", new String[]{"escrow", "return", "NewName"}); + verify(sender).sendMessage(contains("Player offline. Pending return file exists for §fNewName")); + assertArrayEquals(before, Files.readAllBytes(file)); + verify(store, never()).allFiles(); + assertAll( + () -> assertEquals(List.of("NewName"), whileOnline), + () -> assertEquals(List.of("NewName"), afterLogout), + () -> assertEquals(List.of("NewName"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "nE"})), + () -> assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "oLd"}).isEmpty())); + clearInvocations(store); + try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS)) { + for (int i = 0; i < 10; i++) + assertEquals(List.of("NewName"), commands.onTabComplete(sender, null, "recycler", args)); + server.getScheduler().performTicks(300); + bukkit.verify(() -> Bukkit.getOfflinePlayer(any(UUID.class)), never()); + verify(store, never()).rows(any()); + verify(store, never()).allFiles(); + } + } + + @Test + void joiningAnUnnamedPendingOwnerCachesTheJoinedNameWithoutCachingOtherPlayers() throws Exception { + for (String name : List.of("TLibs", "MMOItems", "MythicLib", "ItemsAdder")) + MockBukkit.createMockPlugin(name); + var loaded = MockBukkit.load(Recycler.class); + var escrow = loaded.getEscrowManager(); + var commands = (CommandManager) loaded.getCommand("recycler").getExecutor(); + var sender = mock(CommandSender.class); + when(sender.hasPermission("recycler.admin")).thenReturn(true); + UUID account = UUID.randomUUID(); + var owner = new net.tfminecraft.recycler.ownership.QueueIdentity.Owner(account, UUID.randomUUID()); + String[] args = {"escrow", "return", ""}; + try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS)) { + var unnamed = mock(OfflinePlayer.class); + bukkit.when(() -> Bukkit.getOfflinePlayer(account)).thenReturn(unnamed); + queued(escrow, owner, "held"); + assertTrue(commands.onTabComplete(sender, null, "recycler", args).isEmpty()); + bukkit.clearInvocations(); + var joined = new org.mockbukkit.mockbukkit.entity.PlayerMock(server, "JoinedName", account); + server.addPlayer(joined); + assertEquals(List.of("JoinedName"), commands.onTabComplete(sender, null, "recycler", args)); + joined.disconnect(); + assertEquals(List.of("JoinedName"), commands.onTabComplete(sender, null, "recycler", args)); + var other = server.addPlayer("OtherOnline"); + assertEquals(List.of("JoinedName", "OtherOnline"), commands.onTabComplete(sender, null, "recycler", args)); + other.disconnect(); + assertEquals(List.of("JoinedName"), commands.onTabComplete(sender, null, "recycler", args)); + bukkit.verify(() -> Bukkit.getOfflinePlayer(any(UUID.class)), never()); + } + } + @Test void offlineCompletionFollowsEstateSourceRemovalAndRecipientTerminalization() throws Exception { var escrow = new EscrowManager(); @@ -227,6 +308,8 @@ void realPluginLifecycleCreatesResourcesRegistersManagersAndReloads() throws Exc assertNotNull(loaded.getRecyclerManager()); assertNotNull(loaded.getProviderChain()); assertInstanceOf(CommandManager.class, loaded.getCommand("recycler").getExecutor()); + assertTrue(Arrays.stream(org.bukkit.event.player.PlayerJoinEvent.getHandlerList().getRegisteredListeners()) + .anyMatch(listener -> listener.getPlugin() == loaded && listener.getListener() == loaded.getCommand("recycler").getExecutor())); assertTrue(loaded.reloadAll()); server.getScheduler().performTicks(2); var store = spy(loaded.getEscrowManager().store()); From c1daca528ad3ffc66534ffe8d39b4a62aca4b037 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:58:12 +0000 Subject: [PATCH 4/4] fix: retain pending owners across refreshes and transient reads --- .../recycler/command/CommandManager.java | 4 +- .../recycler/ownership/ReturnStore.java | 3 +- .../recycler/LifecycleCommandsTest.java | 53 ++++++++++++++++++- .../ownership/ReturnStoreIndexTest.java | 46 ++++++++++++++++ 4 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java index 05cc46b..f24c249 100644 --- a/src/main/java/net/tfminecraft/recycler/command/CommandManager.java +++ b/src/main/java/net/tfminecraft/recycler/command/CommandManager.java @@ -38,9 +38,11 @@ public void trackPendingOwners() { } private void refreshPendingOwnerNames(List owners) { - // Resolve profiles when the queue index changes; joins supply current names directly. + // Keep joined names while their owners remain pending; resolve only uncached profiles. Map names = new HashMap<>(); for (UUID owner : owners) { + String cached = pendingOwnerNames.get(owner); + if (cached != null) { names.put(owner, cached); continue; } try { String name = Bukkit.getOfflinePlayer(owner).getName(); if (name != null) names.put(owner, name); diff --git a/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java b/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java index 14158f2..d9d06f2 100644 --- a/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java +++ b/src/main/java/net/tfminecraft/recycler/ownership/ReturnStore.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; @@ -54,7 +55,7 @@ private void forget(Path path) { private JsonObject readRow(Path path) throws IOException { try { JsonObject row = read(path); validate(path, row); index(path, row); return row; - } catch (IOException ex) { forget(path); throw ex; } + } catch (Conflict | NoSuchFileException ex) { forget(path); throw ex; } } public List rows(QueueIdentity.Owner owner) throws IOException { diff --git a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java index dd3409b..a23b541 100644 --- a/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java +++ b/src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java @@ -151,7 +151,7 @@ void returnCompletionIncludesRecognizedOfflineQueuesWithoutPerKeystrokeDiscovery when(mixedCaseBob.getName()).thenReturn("bOb"); bukkit.when(() -> Bukkit.getOfflinePlayer(bob.getUniqueId())).thenReturn(mixedCaseBob); queued(escrow, token(bob).owner(), "held"); - assertEquals(List.of("bOb"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "B"})); + assertEquals(List.of("Bob"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "B"})); } } @@ -205,6 +205,44 @@ void pendingOwnerRenameOnJoinReplacesOldCompletionThroughLogoutWithoutQueueChang } } + @Test + void joinedRenameSurvivesUnrelatedQueueChangesWithAStaleOfflineProfile() throws Exception { + for (String name : List.of("TLibs", "MMOItems", "MythicLib", "ItemsAdder")) + MockBukkit.createMockPlugin(name); + var loaded = MockBukkit.load(Recycler.class); + var escrow = loaded.getEscrowManager(); + var commands = (CommandManager) loaded.getCommand("recycler").getExecutor(); + var player = server.addPlayer("OldName"); + var owner = token(player).owner(); + var row = queued(escrow, owner, "held"); + Path file = escrow.store().file(owner, ReturnStore.id(row)); + byte[] before = Files.readAllBytes(file); + player.disconnect(); + player.setName("NewName"); + assertTrue(player.reconnect()); + player.disconnect(); + var sender = mock(CommandSender.class); + when(sender.hasPermission("recycler.admin")).thenReturn(true); + String[] args = {"escrow", "return", ""}; + assertEquals(List.of("NewName"), commands.onTabComplete(sender, null, "recycler", args)); + try (var bukkit = mockStatic(Bukkit.class, CALLS_REAL_METHODS)) { + var stale = mock(OfflinePlayer.class); + when(stale.getName()).thenReturn("OldName"); + bukkit.when(() -> Bukkit.getOfflinePlayer(owner.account())).thenReturn(stale); + var unrelated = new net.tfminecraft.recycler.ownership.QueueIdentity.Owner(UUID.randomUUID(), UUID.randomUUID()); + bukkit.when(() -> Bukkit.getOfflinePlayer(unrelated.account())).thenReturn(mock(OfflinePlayer.class)); + var otherRow = queued(escrow, unrelated, "held"); + assertEquals(List.of("NewName"), commands.onTabComplete(sender, null, "recycler", args)); + escrow.store().update(otherRow, copy -> copy.addProperty("state", "delivered")); + assertEquals(List.of("NewName"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "nE"})); + assertTrue(commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "oLd"}).isEmpty()); + assertArrayEquals(before, Files.readAllBytes(file)); + bukkit.verify(() -> Bukkit.getOfflinePlayer(owner.account()), never()); + escrow.store().update(row, copy -> copy.addProperty("state", "delivered")); + assertTrue(commands.onTabComplete(sender, null, "recycler", args).isEmpty()); + } + } + @Test void joiningAnUnnamedPendingOwnerCachesTheJoinedNameWithoutCachingOtherPlayers() throws Exception { for (String name : List.of("TLibs", "MMOItems", "MythicLib", "ItemsAdder")) @@ -232,7 +270,18 @@ void joiningAnUnnamedPendingOwnerCachesTheJoinedNameWithoutCachingOtherPlayers() assertEquals(List.of("JoinedName", "OtherOnline"), commands.onTabComplete(sender, null, "recycler", args)); other.disconnect(); assertEquals(List.of("JoinedName"), commands.onTabComplete(sender, null, "recycler", args)); - bukkit.verify(() -> Bukkit.getOfflinePlayer(any(UUID.class)), never()); + var unrelated = new net.tfminecraft.recycler.ownership.QueueIdentity.Owner(UUID.randomUUID(), UUID.randomUUID()); + bukkit.when(() -> Bukkit.getOfflinePlayer(unrelated.account())).thenReturn(mock(OfflinePlayer.class)); + var unrelatedRow = queued(escrow, unrelated, "pending"); + assertEquals(List.of("JoinedName"), commands.onTabComplete(sender, null, "recycler", args)); + escrow.store().update(unrelatedRow, copy -> copy.addProperty("state", "consumed")); + assertEquals(List.of("JoinedName"), commands.onTabComplete(sender, null, "recycler", new String[]{"escrow", "return", "jO"})); + bukkit.verify(() -> Bukkit.getOfflinePlayer(account), never()); + escrow.store().update(escrow.store().rows(owner).getFirst(), copy -> copy.addProperty("state", "delivered")); + assertTrue(commands.onTabComplete(sender, null, "recycler", args).isEmpty()); + queued(escrow, owner, "held"); + assertTrue(commands.onTabComplete(sender, null, "recycler", args).isEmpty()); + bukkit.verify(() -> Bukkit.getOfflinePlayer(account)); } } diff --git a/src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java b/src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java index 024cadc..cbe73b8 100644 --- a/src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java +++ b/src/test/java/net/tfminecraft/recycler/ownership/ReturnStoreIndexTest.java @@ -6,8 +6,10 @@ import com.google.gson.JsonObject; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.time.Instant; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.Set; @@ -110,4 +112,48 @@ JsonObject create(ReturnStore store, QueueIdentity.Owner owner, String state) th store.update(row, copy -> copy.addProperty("state", "consumed")); assertTrue(store.pendingPlayerIds().isEmpty()); } + + @Test void transientReadFailuresRetainPendingOwnersUntilLaterValidationAndRecovery() throws Exception { + var store = new ReturnStore(root); + List> notifications = new ArrayList<>(); + store.onPendingOwnersChanged(notifications::add); + var row = create(store, from, "pending"); + Path file = store.file(from, ReturnStore.id(row)); + byte[] before = Files.readAllBytes(file); + try (var disk = mockStatic(ReturnStore.class, CALLS_REAL_METHODS)) { + disk.when(() -> ReturnStore.read(file)).thenThrow(new IOException("temporarily unavailable")); + assertThrows(IOException.class, () -> store.rows(from)); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + assertThrows(IOException.class, () -> store.update(row, copy -> copy.addProperty("state", "delivered"))); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + assertEquals(List.of(List.of(), List.of(from.account())), notifications); + } + assertArrayEquals(before, Files.readAllBytes(file)); + assertEquals(List.of(row), store.rows(from)); + assertEquals(List.of(List.of(), List.of(from.account())), notifications); + store.recover(); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + store.update(row, copy -> copy.addProperty("state", "delivered")); + assertTrue(store.pendingPlayerIds().isEmpty()); + } + + @Test void malformedInvalidAndDeletedRowsRemoveTheirIndexedOwner() throws Exception { + for (String defect : List.of("malformed", "invalid", "deleted")) { + var store = new ReturnStore(root.resolve(defect)); + var row = create(store, from, "pending"); + Path file = store.file(from, ReturnStore.id(row)); + if (defect.equals("malformed")) Files.writeString(file, "broken"); + else if (defect.equals("invalid")) { + var invalid = row.deepCopy(); invalid.addProperty("state", "unknown"); + Files.write(file, QueueJson.bytes(invalid)); + } else Files.delete(file); + assertEquals(List.of(from.account()), store.pendingPlayerIds()); + var failure = assertThrows(IOException.class, () -> store.update(row, copy -> fail("Unreadable rows must not be changed"))); + if (defect.equals("deleted")) assertInstanceOf(NoSuchFileException.class, failure); + else assertInstanceOf(ReturnStore.Conflict.class, failure); + assertTrue(store.pendingPlayerIds().isEmpty()); + store.recover(); + assertTrue(store.pendingPlayerIds().isEmpty()); + } + } }