From d8136a4bfe2cd0ae6ebd37887e08ba93997d19f6 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:00:54 +0000 Subject: [PATCH 1/2] fix: complete retained trade IDs and filter command keywords --- .../bartershops/IntentCommand.java | 20 ++++++- .../tfminecraft/bartershops/ShopTrades.java | 35 ++++++++++- .../bartershops/IntentCommandTest.java | 60 +++++++++++++++++++ 3 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/tfminecraft/bartershops/IntentCommand.java b/src/main/java/net/tfminecraft/bartershops/IntentCommand.java index 299aa3c..ab0eb16 100644 --- a/src/main/java/net/tfminecraft/bartershops/IntentCommand.java +++ b/src/main/java/net/tfminecraft/bartershops/IntentCommand.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.nio.file.Path; import java.util.List; +import java.util.Locale; import java.util.Optional; import java.util.UUID; import java.util.logging.Logger; @@ -135,9 +136,22 @@ private static String describe(ShopTrades.Request request) { @Override public List onTabComplete(CommandSender sender, Command command, String label, String[] args) { if (!sender.hasPermission(ShopEvents.ADMIN_PERMISSION)) return List.of(); - if (args.length == 1) return List.of("intent"); - if (args.length == 3) return List.of("clear"); - if (args.length == 4 && args[2].equalsIgnoreCase("clear")) return List.of("--confirm"); + if (args.length == 1) return matching(List.of("intent"), args[0]); + if (args.length < 2 || !args[0].equalsIgnoreCase("intent")) return List.of(); + if (args.length == 2) return matching(trades.retainedShopIds(), args[1]); + try { + UUID.fromString(args[1]); + } catch (IllegalArgumentException ex) { + return List.of(); + } + if (args.length == 3) return matching(List.of("clear"), args[2]); + if (args.length == 4 && args[2].equalsIgnoreCase("clear")) + return matching(List.of("--confirm"), args[3]); return List.of(); } + + private static List matching(List options, String prefix) { + String typed = prefix.toLowerCase(Locale.ROOT); + return options.stream().filter(option -> option.toLowerCase(Locale.ROOT).startsWith(typed)).toList(); + } } diff --git a/src/main/java/net/tfminecraft/bartershops/ShopTrades.java b/src/main/java/net/tfminecraft/bartershops/ShopTrades.java index 7470d91..4309b40 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopTrades.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopTrades.java @@ -4,6 +4,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; +import java.util.HashSet; +import java.util.Set; import java.util.List; import java.util.Map; import java.util.Optional; @@ -28,10 +30,32 @@ record Intent(String operationId, Request request, String state) {} private final Path directory; private final Logger logger; private final Map active = new HashMap<>(); + private final Set retained = new HashSet<>(); ShopTrades(Path directory, Logger logger) { this.directory = directory; this.logger = logger; + if (Files.exists(directory)) { + try (var files = Files.list(directory)) { + files.filter(path -> path.getFileName().toString().endsWith(".json")).forEach(path -> { + String name = path.getFileName().toString(); + try { + retained.add(UUID.fromString(name.substring(0, name.length() - 5)).toString()); + } catch (IllegalArgumentException ignored) { + // Unrelated files cannot identify a shop. + } + }); + } catch (IOException ex) { + logger.warning("Could not index retained shop trades: " + ex); + } + } + } + + /** Startup disk index plus trades retained by this process; no completion-time I/O. */ + List retainedShopIds() { + Set ids = new HashSet<>(retained); + ids.addAll(active.keySet()); + return ids.stream().sorted().toList(); } /** Whether a trade at this shop is unfinished, in this process or from an earlier one. */ @@ -45,6 +69,7 @@ Optional inspect(String shopId) throws IOException { if (intent != null) return Optional.of(intent); Path file = intentFile(shopId); if (!Files.exists(file)) return Optional.empty(); + retained.add(shopId); try { intent = JSON.fromJson(Files.readString(file), Intent.class); } catch (JsonParseException ex) { @@ -59,6 +84,7 @@ Optional inspect(String shopId) throws IOException { void clear(String shopId) throws IOException { DurableFiles.delete(intentFile(shopId)); active.remove(shopId); + retained.remove(shopId); } private Path intentFile(String shopId) { @@ -74,7 +100,10 @@ Outcome trade(Request request, Runnable deliver) { } Intent intent = active.get(request.shopId()); // Files from an earlier process may describe an already delivered item. Never replay them. - if (intent == null && Files.exists(file)) return Outcome.REVIEW; + if (intent == null && Files.exists(file)) { + retained.add(request.shopId()); + return Outcome.REVIEW; + } if (intent != null && (!intent.request().equals(request) || !"prepared".equals(intent.state()))) return Outcome.REVIEW; try { @@ -82,6 +111,7 @@ Outcome trade(Request request, Runnable deliver) { intent = new Intent("shop:" + request.shopId() + ":" + UUID.randomUUID(), request, "prepared"); DurableFiles.write(file, JSON.toJson(intent)); active.put(request.shopId(), intent); + retained.add(request.shopId()); } Result result = Denars.status(intent.operationId()).orElse(null); if (result == null) { @@ -97,6 +127,7 @@ Outcome trade(Request request, Runnable deliver) { if (result == Result.REFUSED) { DurableFiles.delete(file); active.remove(request.shopId()); + retained.remove(request.shopId()); return Outcome.REFUSED; } if (result == Result.UNAVAILABLE) return Outcome.WAITING; @@ -113,8 +144,10 @@ Outcome trade(Request request, Runnable deliver) { deliver.run(); DurableFiles.delete(file); active.remove(request.shopId()); + retained.remove(request.shopId()); return Outcome.APPLIED; } catch (IOException | RuntimeException ex) { + if (Files.exists(file)) retained.add(request.shopId()); logger.severe("Shop trade requires review; retained intent " + file + ": " + ex); return Outcome.REVIEW; } diff --git a/src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java b/src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java index 7d13ff2..1a6af21 100644 --- a/src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java +++ b/src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java @@ -239,6 +239,66 @@ void tabCompletesTheCommandWords() { assertEquals(List.of(), command.onTabComplete(staff, bukkit, "bartershops", new String[] {"intent", shopId, "x", ""})); } + @Test + void completionKeywordsRespectPrefixPathUuidAndPermission() { + Command bukkit = mock(Command.class); + assertEquals(List.of("intent"), command.onTabComplete(staff, bukkit, "bartershops", new String[]{"In"})); + assertEquals(List.of("clear"), command.onTabComplete(staff, bukkit, "bartershops", new String[]{"INTENT", shopId, "Cl"})); + assertEquals(List.of("--confirm"), command.onTabComplete(staff, bukkit, "bartershops", new String[]{"intent", shopId, "CLEAR", "--C"})); + for (String[] args : List.of(new String[0], new String[]{"x"}, new String[]{"wrong", shopId, ""}, + new String[]{"intent", "not-a-uuid", ""}, new String[]{"intent", shopId, "z"}, + new String[]{"wrong", shopId, "clear", ""}, new String[]{"intent", "bad", "clear", ""}, + new String[]{"intent", shopId, "clear", "nope"}, new String[]{"intent", shopId, "clear", "--confirm", ""})) + assertEquals(List.of(), command.onTabComplete(staff, bukkit, "bartershops", args)); + when(staff.hasPermission(ShopEvents.ADMIN_PERMISSION)).thenReturn(false); + assertEquals(List.of(), command.onTabComplete(staff, bukkit, "bartershops", new String[]{"intent", ""})); + } + + @Test + void completionIndexesOnlyRetainedFilesAtStartupAndTracksClearing() throws Exception { + writeIntent("delivering"); + String brokenId = UUID.randomUUID().toString(); + Files.writeString(intentFile().resolveSibling(brokenId + ".json"), "{broken"); + Files.writeString(intentFile().resolveSibling("not-a-shop.json"), "{}"); + Files.writeString(intentFile().resolveSibling("notes.txt"), "{}"); + trades = new ShopTrades(directory.resolve("trades"), Logger.getAnonymousLogger()); + command = new IntentCommand(trades, audit(), Logger.getAnonymousLogger()); + var bukkit = mock(Command.class); + assertEquals(java.util.Set.of(shopId, brokenId), new java.util.HashSet<>( + command.onTabComplete(staff, bukkit, "bartershops", new String[]{"intent", ""}))); + assertEquals(List.of(shopId), command.onTabComplete(staff, bukkit, "bartershops", + new String[]{"INTENT", shopId.substring(0, 16).toUpperCase(java.util.Locale.ROOT)})); + assertEquals(List.of(), command.onTabComplete(staff, bukkit, "bartershops", new String[]{"intent", "not"})); + // Moving the directory proves repeated completion reads the index, not the filesystem. + Path moved = directory.resolve("moved"); + Files.move(directory.resolve("trades"), moved); + assertEquals(2, command.onTabComplete(staff, bukkit, "bartershops", new String[]{"intent", ""}).size()); + Files.move(moved, directory.resolve("trades")); + trades.clear(shopId); + assertEquals(List.of(brokenId), command.onTabComplete(staff, bukkit, "bartershops", new String[]{"intent", ""})); + } + + @Test + void completionTracksNewWaitingAndCompletedTrades() { + intentInMemory(Result.UNAVAILABLE); + assertEquals(List.of(shopId), trades.retainedShopIds()); + denars.when(() -> Denars.status(anyString())).thenReturn(Optional.of(Result.APPLIED)); + assertEquals(ShopTrades.Outcome.APPLIED, trades.trade(request(), () -> {})); + assertTrue(trades.retainedShopIds().isEmpty()); + intentInMemory(Result.UNAVAILABLE); + denars.when(() -> Denars.status(anyString())).thenReturn(Optional.of(Result.REFUSED)); + assertEquals(ShopTrades.Outcome.REFUSED, trades.trade(request(), () -> fail("delivery"))); + assertTrue(trades.retainedShopIds().isEmpty()); + } + + @Test + void unreadableIndexDirectoryLogsAndLeavesAnEmptySnapshot() throws Exception { + Path file = directory.resolve("not-a-directory"); Files.writeString(file, "x"); + Logger logger = mock(Logger.class); + assertTrue(new ShopTrades(file, logger).retainedShopIds().isEmpty()); + verify(logger).warning(contains("Could not index retained shop trades")); + } + private void run(String... args) { assertTrue(command.onCommand(staff, mock(Command.class), "bartershops", args)); } From ded8d8f6fc0f955f64b5b2c18321bbacaa7ee980 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:55:25 +0000 Subject: [PATCH 2/2] fix: validate retained trade filenames and tolerate scan failures --- .../tfminecraft/bartershops/ShopTrades.java | 6 ++-- .../bartershops/ShopTradesTest.java | 32 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/tfminecraft/bartershops/ShopTrades.java b/src/main/java/net/tfminecraft/bartershops/ShopTrades.java index 4309b40..04a4669 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopTrades.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopTrades.java @@ -1,6 +1,7 @@ package net.tfminecraft.bartershops; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; @@ -40,12 +41,13 @@ record Intent(String operationId, Request request, String state) {} files.filter(path -> path.getFileName().toString().endsWith(".json")).forEach(path -> { String name = path.getFileName().toString(); try { - retained.add(UUID.fromString(name.substring(0, name.length() - 5)).toString()); + String shopId = UUID.fromString(name.substring(0, name.length() - 5)).toString(); + if (path.equals(intentFile(shopId))) retained.add(shopId); } catch (IllegalArgumentException ignored) { // Unrelated files cannot identify a shop. } }); - } catch (IOException ex) { + } catch (IOException | UncheckedIOException ex) { logger.warning("Could not index retained shop trades: " + ex); } } diff --git a/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java b/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java index fc49059..6593c94 100644 --- a/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java +++ b/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java @@ -206,4 +206,36 @@ void unreadableOrIncompleteRecordsCannotBeInspected() throws Exception { assertThrows(IOException.class, () -> trades.inspect(shopId)); assertTrue(trades.hasIntent(shopId)); } + @Test + void startupIndexOffersOnlyIntentFilenamesThatInspectionResolves() throws Exception { + Files.writeString(intent(), new com.google.gson.Gson().toJson( + new ShopTrades.Intent("retained-operation", request(), "prepared"))); + String shortened = "1-1-1-1-1"; + String uppercase = "ABCDEFAB-ABCD-4ABC-8ABC-ABCDEFABCDEF"; + Files.writeString(directory.resolve(shortened + ".json"), "{}"); + Files.writeString(directory.resolve(uppercase + ".json"), "{}"); + ShopTrades trades = new ShopTrades(directory, logger); + assertEquals(List.of(shopId), trades.retainedShopIds()); + assertEquals("prepared", trades.inspect(shopId).orElseThrow().state()); + assertTrue(trades.inspect(UUID.fromString(shortened).toString()).isEmpty()); + assertTrue(trades.inspect(UUID.fromString(uppercase).toString()).isEmpty()); + } + + @Test + void traversalFailureKeepsAlreadyIndexedTradesAndLogsWithoutPreventingStartup() throws Exception { + Files.writeString(intent(), "{}"); + var closed = new java.util.concurrent.atomic.AtomicBoolean(); + var paths = java.util.stream.Stream.concat(java.util.stream.Stream.of(intent()), + java.util.stream.Stream.generate(() -> { + throw new java.io.UncheckedIOException(new IOException("Directory access lost")); + })).onClose(() -> closed.set(true)); + try (var files = mockStatic(Files.class, CALLS_REAL_METHODS)) { + files.when(() -> Files.list(directory)).thenReturn(paths); + ShopTrades trades = assertDoesNotThrow(() -> new ShopTrades(directory, logger)); + assertEquals(List.of(shopId), trades.retainedShopIds()); + verify(logger).warning(contains("Could not index retained shop trades")); + assertTrue(closed.get()); + } + } + }