From d88c031efd669bf4b2e755425b273b67cf322926 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:05:30 +0000 Subject: [PATCH 1/5] feat: pause a shop while its owner is dead Both trade directions refuse with "This shop's owner has died." when the owning character is not alive, and stay closed while the owner cannot be read. A revived owner's shop trades again. Pins TLibs 2.6.0. Co-Authored-By: Claude Opus 5.5 --- pom.xml | 2 +- .../tfminecraft/bartershops/ShopEvents.java | 9 ++++ .../bartershops/ShopEventsTest.java | 48 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 1dff392..197c3cc 100644 --- a/pom.xml +++ b/pom.xml @@ -14,7 +14,7 @@ full 0.3.0 4.0.0 - 2.4.0 + 2.6.0 diff --git a/src/main/java/net/tfminecraft/bartershops/ShopEvents.java b/src/main/java/net/tfminecraft/bartershops/ShopEvents.java index 361e5ac..b4f9a5d 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopEvents.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopEvents.java @@ -42,7 +42,9 @@ import com.destroystokyo.paper.event.block.BlockDestroyEvent; +import net.tfminecraft.tlibs.character.CharacterInfo; import net.tfminecraft.tlibs.character.CharacterRef; +import net.tfminecraft.tlibs.character.CharacterStatus; import net.tfminecraft.tlibs.character.Characters; import net.tfminecraft.bartershops.sf.ShopEmbargo; @@ -259,6 +261,13 @@ public void useShop(PlayerInteractEvent e) { p.sendMessage("§cThis shop is closed until its owner and your active character are available."); return; } + // A dead owner's shop waits for the estate to give it on; both directions stay closed. + Optional ownerStatus = Characters.info(owner.get().characterId()).map(CharacterInfo::status); + if (ownerStatus.isEmpty() || ownerStatus.get() != CharacterStatus.ALIVE) { + refused(p, ownerStatus.isPresent() ? "§cThis shop's owner has died." + : "§cThis shop is closed until its owner and your active character are available."); + return; + } if (!ShopEmbargo.ready()) { refused(p, "§cShops are waiting for faction and tax data. Try again shortly."); return; diff --git a/src/test/java/net/tfminecraft/bartershops/ShopEventsTest.java b/src/test/java/net/tfminecraft/bartershops/ShopEventsTest.java index 4d7f363..61ded10 100644 --- a/src/test/java/net/tfminecraft/bartershops/ShopEventsTest.java +++ b/src/test/java/net/tfminecraft/bartershops/ShopEventsTest.java @@ -69,8 +69,10 @@ import net.tfminecraft.bartershops.sf.ShopEmbargo; import net.tfminecraft.denareconomy.api.Denars; import net.tfminecraft.denareconomy.api.Result; +import net.tfminecraft.tlibs.character.CharacterInfo; import net.tfminecraft.tlibs.character.CharacterRef; import net.tfminecraft.tlibs.character.CharacterSession; +import net.tfminecraft.tlibs.character.CharacterStatus; import net.tfminecraft.tlibs.character.Characters; import java.nio.file.Path; import java.nio.file.Files; @@ -322,6 +324,7 @@ void setUpListener() { characters.when(Characters::available).thenReturn(true); characters.when(Characters::legacyHeirsReady).thenReturn(true); characters.when(() -> Characters.currentSession(any(Player.class))).thenReturn(Optional.of(new CharacterSession(CUSTOMER_REF, 1))); + ownerStatus(CharacterStatus.ALIVE); denar = mockStatic(Denars.class); denar.when(() -> Denars.pouch(any())).thenAnswer(call -> "char:" + call.getArgument(0).characterId() + ":pouch"); denar.when(() -> Denars.bank(any())).thenAnswer(call -> "char:" + call.getArgument(0).characterId() + ":bank"); @@ -1278,6 +1281,51 @@ void anotherCharacterOnTheSameAccountCannotBreakTheShop() { verify(events.db, never()).deleteFile(any()); } + @ParameterizedTest + @CsvSource({"buy,DEAD", "sell,DEAD", "buy,MISSING", "sell,MISSING"}) + void deadOwnersShopIsPausedInBothDirections(String type, CharacterStatus status) { + ShopSign shop = tradingShop(type); + ownerStatus(status); + + events.useShop(click(block)); + + assertRefused("§cThis shop's owner has died."); + denar.verifyNoInteractions(); + embargo.verify(() -> ShopEmbargo.blocked(any(), any()), never()); + verify(shop.getStorageLoc(), never()).getBlock(); + } + + @Test + void shopWhoseOwnerCannotBeReadStaysClosed() { + tradingShop("buy"); + characters.when(() -> Characters.info(OWNER_CHARACTER)).thenReturn(Optional.empty()); + + events.useShop(click(block)); + + assertRefused("§cThis shop is closed until its owner and your active character are available."); + denar.verifyNoInteractions(); + } + + @Test + void revivedOwnersShopTradesAgain() { + Inventory storage = storage(tradingShop("buy")); + stock(storage, new Stack(Material.DIRT, 8)); + received(inventory); + ownerStatus(CharacterStatus.DEAD); + events.useShop(click(block)); + denar.verifyNoInteractions(); + + ownerStatus(CharacterStatus.ALIVE); + events.useShop(click(block)); + + verifyTrade("buy"); + } + + private void ownerStatus(CharacterStatus status) { + characters.when(() -> Characters.info(any(UUID.class))).thenAnswer(call -> Optional.of(new CharacterInfo( + new CharacterRef(OWNER, call.getArgument(0)), "Owner", status, null, Set.of(), 0, null))); + } + @Test void unmigratedShopCannotTradeOrBeBrokenExceptByStaff() { ShopSign shop = tradingShop("buy"); From eed2e89f04ca8c3f72c1ccb07ed22cb364f70548 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:05:59 +0000 Subject: [PATCH 2/5] feat: add /bartershops intent for unfinished trades Staff with bartershops.admin inspect the trade record that keeps a shop closed and clear it. A prepared trade whose posting is absent or refused clears at once while the ledger is ready; a prepared trade with an applied posting, a delivering or conflict record, or an unreadable one is shown and clears only with --confirm after staff put things right. Nothing clears while the ledger is not ready. Clearing removes the file and this process's copy. Every use is written to logs/staff-audit.log. Co-Authored-By: Claude Opus 5.5 --- .../bartershops/IntentCommand.java | 143 ++++++++++ .../tfminecraft/bartershops/ShopAudit.java | 25 ++ .../net/tfminecraft/bartershops/ShopMain.java | 8 + .../tfminecraft/bartershops/ShopTrades.java | 35 ++- src/main/resources/plugin.yml | 8 +- .../bartershops/IntentCommandTest.java | 269 ++++++++++++++++++ .../tfminecraft/bartershops/ShopMainTest.java | 5 + .../bartershops/ShopTradesTest.java | 34 +++ 8 files changed, 525 insertions(+), 2 deletions(-) create mode 100644 src/main/java/net/tfminecraft/bartershops/IntentCommand.java create mode 100644 src/main/java/net/tfminecraft/bartershops/ShopAudit.java create mode 100644 src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java diff --git a/src/main/java/net/tfminecraft/bartershops/IntentCommand.java b/src/main/java/net/tfminecraft/bartershops/IntentCommand.java new file mode 100644 index 0000000..299aa3c --- /dev/null +++ b/src/main/java/net/tfminecraft/bartershops/IntentCommand.java @@ -0,0 +1,143 @@ +package net.tfminecraft.bartershops; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.logging.Logger; + +import org.bukkit.command.Command; +import org.bukkit.command.CommandExecutor; +import org.bukkit.command.CommandSender; +import org.bukkit.command.TabCompleter; + +import net.tfminecraft.denareconomy.api.Denars; +import net.tfminecraft.denareconomy.api.Result; + +/** + * {@code /bartershops intent [clear [--confirm]]}: staff inspect and clear the unfinished + * trade that keeps a shop closed. Only a prepared trade that moved no money clears without + * {@code --confirm}; every other state may already have moved money or items, so staff settle it by + * hand first. Nothing clears while the ledger is not ready. + */ +final class IntentCommand implements CommandExecutor, TabCompleter { + static final String USAGE = "§eUsage: /bartershops intent [clear [--confirm]]"; + private static final String UNREADABLE = "unreadable"; + + enum Clear { YES, CONFIRM, LEDGER } + + private final ShopTrades trades; + private final Path auditFile; + private final Logger logger; + + IntentCommand(ShopTrades trades, Path auditFile, Logger logger) { + this.trades = trades; + this.auditFile = auditFile; + this.logger = logger; + } + + /** Whether a trade may be cleared: the table in the BarterShops guide. */ + static Clear clearable(String state, Optional posting, boolean ledgerReady) { + if (!ledgerReady) return Clear.LEDGER; + boolean noMoney = posting.isEmpty() || posting.get() == Result.REFUSED; + return "prepared".equals(state) && noMoney ? Clear.YES : Clear.CONFIRM; + } + + @Override + public boolean onCommand(CommandSender sender, Command command, String label, String[] args) { + if (!sender.hasPermission(ShopEvents.ADMIN_PERMISSION)) { + sender.sendMessage("§cYou do not have permission to use this command."); + return true; + } + boolean clear = args.length >= 3 && args[2].equalsIgnoreCase("clear"); + boolean confirm = clear && args.length == 4 && args[3].equalsIgnoreCase("--confirm"); + if (args.length < 2 || !args[0].equalsIgnoreCase("intent") || (args.length >= 3 && !clear) + || (args.length == 4 && !confirm) || args.length > 4) { + sender.sendMessage(USAGE); + return true; + } + String shopId; + try { + shopId = UUID.fromString(args[1]).toString(); + } catch (IllegalArgumentException ex) { + sender.sendMessage("§cA shop ID is a UUID, the name of its file in Data/."); + return true; + } + ShopTrades.Intent intent; + try { + Optional found = trades.inspect(shopId); + if (found.isEmpty()) { + sender.sendMessage("§aShop " + shopId + " has no unfinished trade."); + return true; + } + intent = found.get(); + } catch (IOException ex) { + intent = new ShopTrades.Intent(null, null, UNREADABLE); + sender.sendMessage("§cThe trade record of shop " + shopId + " cannot be read: " + ex.getMessage()); + } + boolean ledgerReady = Denars.ready(); + Optional posting = ledgerReady && intent.operationId() != null + ? Denars.status(intent.operationId()) : Optional.empty(); + String postingText = !ledgerReady ? "unknown (ledger not ready)" + : intent.operationId() == null ? "unknown" : posting.map(Result::name).orElse("absent"); + String summary = "shop " + shopId + " state " + intent.state() + ", posting " + postingText + + (intent.operationId() == null ? "" : ", operation " + intent.operationId()) + + (intent.request() == null ? "" : ", " + describe(intent.request())); + Clear verdict = clearable(intent.state(), posting, ledgerReady); + if (!clear) { + show(sender, shopId, intent, postingText, verdict); + ShopAudit.record(auditFile, logger, sender.getName(), "viewed the trade at " + summary); + return true; + } + if (verdict == Clear.LEDGER) { + sender.sendMessage("§cThe ledger is not ready, so nothing was cleared. Try again once DenarEconomy is up."); + return true; + } + if (verdict == Clear.CONFIRM && !confirm) { + show(sender, shopId, intent, postingText, verdict); + return true; + } + try { + trades.clear(shopId); + } catch (IOException ex) { + sender.sendMessage("§cCould not remove the trade record: " + ex.getMessage()); + return true; + } + ShopAudit.record(auditFile, logger, sender.getName(), + "cleared the trade at " + summary + (confirm ? " (confirmed)" : "")); + sender.sendMessage("§aCleared the trade at shop " + shopId + "; it trades again."); + sender.sendMessage("§7If its owner is dead, run /estate retry bartershops."); + return true; + } + + private static void show(CommandSender sender, String shopId, ShopTrades.Intent intent, String posting, Clear verdict) { + sender.sendMessage("§eShop " + shopId + ": unfinished trade, state " + intent.state() + ", posting " + posting); + if (intent.operationId() != null) sender.sendMessage("§7Operation " + intent.operationId()); + if (intent.request() != null) sender.sendMessage("§7" + describe(intent.request())); + String command = "/bartershops intent " + shopId + " clear"; + sender.sendMessage(switch (verdict) { + case YES -> "§aNo money moved. " + command + " removes it."; + case LEDGER -> "§cThe ledger is not ready; nothing can be cleared yet."; + case CONFIRM -> "prepared".equals(intent.state()) + ? "§cThe payment applied but the items did not move. Hand over the items, then " + command + " --confirm." + : "§cThe items may already have moved. Check the chest and the customer, put things right, then " + + command + " --confirm."; + }); + } + + private static String describe(ShopTrades.Request request) { + return request.direction() + " of " + request.quantity() + " for " + request.cents() + " cents, customer " + + (request.customer() == null ? "?" : request.customer().characterId()) + ", owner " + + (request.owner() == null ? "?" : request.owner().characterId()); + } + + @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"); + return List.of(); + } +} diff --git a/src/main/java/net/tfminecraft/bartershops/ShopAudit.java b/src/main/java/net/tfminecraft/bartershops/ShopAudit.java new file mode 100644 index 0000000..51819b0 --- /dev/null +++ b/src/main/java/net/tfminecraft/bartershops/ShopAudit.java @@ -0,0 +1,25 @@ +package net.tfminecraft.bartershops; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.time.Instant; +import java.util.logging.Logger; + +/** Staff shop commands go to the server log and to {@code plugins/BarterShops/logs/staff-audit.log}. */ +final class ShopAudit { + private ShopAudit() {} + + static void record(Path file, Logger log, String actor, String action) { + log.info("[Shop audit] " + actor + " " + action); + try { + Files.createDirectories(file.toAbsolutePath().getParent()); + Files.writeString(file, Instant.now() + " " + actor + " " + action + System.lineSeparator(), + StandardCharsets.UTF_8, StandardOpenOption.CREATE, StandardOpenOption.APPEND); + } catch (IOException ex) { + log.warning("Could not write the shop audit log: " + ex.getMessage()); + } + } +} diff --git a/src/main/java/net/tfminecraft/bartershops/ShopMain.java b/src/main/java/net/tfminecraft/bartershops/ShopMain.java index 47e775e..0f8cdd8 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopMain.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopMain.java @@ -1,6 +1,7 @@ package net.tfminecraft.bartershops; import java.io.File; +import org.bukkit.command.PluginCommand; import org.bukkit.plugin.java.JavaPlugin; public class ShopMain extends JavaPlugin { @@ -21,5 +22,12 @@ public void onEnable() { if (!ready) ready = migration.runWhenReady(); }, 100L, 100L); getServer().getPluginManager().registerEvents(events, this); + PluginCommand command = getCommand("bartershops"); + if (command != null) { + IntentCommand intents = new IntentCommand(events.trades, + getDataFolder().toPath().resolve("logs").resolve("staff-audit.log"), getLogger()); + command.setExecutor(intents); + command.setTabCompleter(intents); + } } } diff --git a/src/main/java/net/tfminecraft/bartershops/ShopTrades.java b/src/main/java/net/tfminecraft/bartershops/ShopTrades.java index c5e152d..7470d91 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopTrades.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopTrades.java @@ -6,11 +6,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; import java.util.logging.Logger; import com.google.gson.Gson; import com.google.gson.GsonBuilder; +import com.google.gson.JsonParseException; import net.tfminecraft.denareconomy.api.Denars; import net.tfminecraft.denareconomy.api.Leg; import net.tfminecraft.denareconomy.api.Result; @@ -21,7 +23,7 @@ final class ShopTrades { enum Outcome { APPLIED, REFUSED, WAITING, REVIEW } record Request(String shopId, CharacterRef customer, CharacterRef owner, String direction, int quantity, long cents, String item) {} - private record Intent(String operationId, Request request, String state) {} + record Intent(String operationId, Request request, String state) {} private static final Gson JSON = new GsonBuilder().setPrettyPrinting().create(); private final Path directory; private final Logger logger; @@ -32,6 +34,37 @@ private record Intent(String operationId, Request request, String state) {} this.logger = logger; } + /** Whether a trade at this shop is unfinished, in this process or from an earlier one. */ + boolean hasIntent(String shopId) { + return active.containsKey(shopId) || Files.exists(intentFile(shopId)); + } + + /** The unfinished trade at this shop, as this process last left it. Unreadable records throw. */ + Optional inspect(String shopId) throws IOException { + Intent intent = active.get(shopId); + if (intent != null) return Optional.of(intent); + Path file = intentFile(shopId); + if (!Files.exists(file)) return Optional.empty(); + try { + intent = JSON.fromJson(Files.readString(file), Intent.class); + } catch (JsonParseException ex) { + throw new IOException("Unreadable trade record " + file, ex); + } + if (intent == null || intent.operationId() == null || intent.state() == null || intent.request() == null) + throw new IOException("Incomplete trade record " + file); + return Optional.of(intent); + } + + /** Removes the record and this process's copy, so the shop trades again. Staff only. */ + void clear(String shopId) throws IOException { + DurableFiles.delete(intentFile(shopId)); + active.remove(shopId); + } + + private Path intentFile(String shopId) { + return directory.resolve(UUID.fromString(shopId) + ".json"); + } + Outcome trade(Request request, Runnable deliver) { Path file; try { diff --git a/src/main/resources/plugin.yml b/src/main/resources/plugin.yml index da2cc45..7e36cf7 100644 --- a/src/main/resources/plugin.yml +++ b/src/main/resources/plugin.yml @@ -7,7 +7,13 @@ author: Drefvelin depend: [MMOItems, MythicLib, DenarEconomy, TLibs] softdepend: [SimpleFactions] +commands: + bartershops: + description: Inspect and clear a shop's unfinished trade. + usage: /bartershops intent [clear [--confirm]] + permission: bartershops.admin + permissions: bartershops.admin: - description: Break shops owned by other players. + description: Break shops owned by other players and clear unfinished trades. default: op diff --git a/src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java b/src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java new file mode 100644 index 0000000..7d13ff2 --- /dev/null +++ b/src/test/java/net/tfminecraft/bartershops/IntentCommandTest.java @@ -0,0 +1,269 @@ +package net.tfminecraft.bartershops; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.logging.Logger; + +import org.bukkit.command.Command; +import org.bukkit.command.CommandSender; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedStatic; + +import com.google.gson.Gson; + +import net.tfminecraft.denareconomy.api.Denars; +import net.tfminecraft.denareconomy.api.Result; +import net.tfminecraft.tlibs.character.CharacterRef; + +class IntentCommandTest { + private static final CharacterRef CUSTOMER = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final CharacterRef OWNER = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final String OPERATION = "shop:op"; + + @TempDir Path directory; + private final String shopId = UUID.randomUUID().toString(); + private ShopTrades trades; + private IntentCommand command; + private CommandSender staff; + private final List messages = new ArrayList<>(); + private MockedStatic denars; + + @BeforeEach + void setUp() { + trades = new ShopTrades(directory.resolve("trades"), Logger.getAnonymousLogger()); + command = new IntentCommand(trades, audit(), Logger.getAnonymousLogger()); + staff = mock(CommandSender.class); + when(staff.getName()).thenReturn("Moderator"); + when(staff.hasPermission(ShopEvents.ADMIN_PERMISSION)).thenReturn(true); + doAnswer(call -> messages.add(call.getArgument(0))).when(staff).sendMessage(anyString()); + denars = mockStatic(Denars.class); + denars.when(Denars::ready).thenReturn(true); + denars.when(() -> Denars.pouch(any())).thenReturn("pouch"); + denars.when(() -> Denars.bank(any())).thenReturn("bank"); + } + + @AfterEach + void tearDown() { + denars.close(); + } + + @ParameterizedTest + @CsvSource({ + "prepared,,true,YES", "prepared,REFUSED,true,YES", + "prepared,APPLIED,true,CONFIRM", "prepared,CONFLICT,true,CONFIRM", + "delivering,APPLIED,true,CONFIRM", "delivering,,true,CONFIRM", "conflict,CONFLICT,true,CONFIRM", + "conflict,,true,CONFIRM", "prepared,,false,LEDGER", "delivering,APPLIED,false,LEDGER" + }) + void clearingFollowsTheIntentStatePostingAndLedger(String state, Result posting, boolean ready, IntentCommand.Clear expected) { + assertEquals(expected, IntentCommand.clearable(state, Optional.ofNullable(posting), ready)); + } + + @Test + void preparedTradeThatMovedNoMoneyClearsFileAndMemory() throws Exception { + intentInMemory(Result.UNAVAILABLE); + denars.when(() -> Denars.status(anyString())).thenReturn(Optional.empty()); + assertTrue(Files.exists(intentFile())); + + run("intent", shopId, "clear"); + + assertFalse(Files.exists(intentFile())); + assertFalse(trades.hasIntent(shopId)); + assertTrue(messages.stream().anyMatch(message -> message.contains("it trades again"))); + assertTrue(Files.readString(audit()).contains("Moderator cleared the trade at shop " + shopId + " state prepared, posting absent")); + // The shop trades again with a fresh operation. + denars.when(() -> Denars.trade(anyString(), anyString(), anyString(), any(), anyString(), anyLong(), anyBoolean())) + .thenReturn(Result.APPLIED); + assertEquals(ShopTrades.Outcome.APPLIED, trades.trade(request(), () -> {})); + } + + @Test + void preparedTradeWithARefusedPostingClearsFromTheFile() throws Exception { + writeIntent("prepared"); + denars.when(() -> Denars.status(OPERATION)).thenReturn(Optional.of(Result.REFUSED)); + + run("intent", shopId, "clear"); + + assertFalse(Files.exists(intentFile())); + } + + @ParameterizedTest + @CsvSource({"prepared,APPLIED", "delivering,APPLIED", "conflict,CONFLICT", "delivering,"}) + void tradesThatMayHaveMovedSomethingNeedConfirmation(String state, Result posting) throws Exception { + writeIntent(state); + denars.when(() -> Denars.status(OPERATION)).thenReturn(Optional.ofNullable(posting)); + + run("intent", shopId, "clear"); + + assertTrue(Files.exists(intentFile())); + assertTrue(trades.hasIntent(shopId)); + assertTrue(messages.stream().anyMatch(message -> message.contains("clear --confirm"))); + + run("intent", shopId, "clear", "--confirm"); + + assertFalse(Files.exists(intentFile())); + assertTrue(Files.readString(audit()).contains("(confirmed)")); + } + + @Test + void appliedPaymentTellsStaffToHandOverTheItems() throws Exception { + writeIntent("prepared"); + denars.when(() -> Denars.status(OPERATION)).thenReturn(Optional.of(Result.APPLIED)); + + run("intent", shopId); + + assertTrue(messages.get(0).contains("state prepared, posting APPLIED")); + assertTrue(messages.stream().anyMatch(message -> message.contains("Hand over the items"))); + assertTrue(Files.exists(intentFile())); + assertTrue(Files.readString(audit()).contains("Moderator viewed the trade at shop " + shopId)); + } + + @Test + void deliveringTradeIsShownWithItsRequest() throws Exception { + writeIntent("delivering"); + denars.when(() -> Denars.status(OPERATION)).thenReturn(Optional.of(Result.APPLIED)); + + run("intent", shopId); + + assertTrue(messages.stream().anyMatch(message -> message.contains("Operation " + OPERATION))); + assertTrue(messages.stream().anyMatch(message -> message.contains("buy of 5 for 1000 cents, customer " + CUSTOMER.characterId()))); + assertTrue(messages.stream().anyMatch(message -> message.contains("may already have moved"))); + } + + @ParameterizedTest + @ValueSource(strings = {"", "--confirm"}) + void nothingClearsWhileTheLedgerIsNotReady(String confirm) throws Exception { + writeIntent("prepared"); + denars.when(Denars::ready).thenReturn(false); + + run(confirm.isEmpty() ? new String[] {"intent", shopId, "clear"} : new String[] {"intent", shopId, "clear", confirm}); + + assertTrue(Files.exists(intentFile())); + assertTrue(messages.stream().anyMatch(message -> message.contains("ledger is not ready"))); + denars.verify(() -> Denars.status(anyString()), never()); + } + + @Test + void ledgerNotReadyIsShownWhenViewing() throws Exception { + writeIntent("prepared"); + denars.when(Denars::ready).thenReturn(false); + + run("intent", shopId); + + assertTrue(messages.get(0).contains("posting unknown (ledger not ready)")); + assertTrue(messages.stream().anyMatch(message -> message.contains("nothing can be cleared yet"))); + } + + @Test + void unreadableRecordIsShownAndClearsOnlyWithConfirmation() throws Exception { + Files.createDirectories(intentFile().getParent()); + Files.writeString(intentFile(), "{ broken"); + + run("intent", shopId, "clear"); + assertTrue(Files.exists(intentFile())); + assertTrue(messages.stream().anyMatch(message -> message.contains("cannot be read"))); + + run("intent", shopId, "clear", "--confirm"); + assertFalse(Files.exists(intentFile())); + assertTrue(Files.readString(audit()).contains("state unreadable, posting unknown")); + } + + @Test + void shopWithoutATradeSaysSo() { + run("intent", shopId); + assertEquals(List.of("§aShop " + shopId + " has no unfinished trade."), messages); + } + + @Test + void failedRemovalKeepsTheRecord() throws Exception { + writeIntent("prepared"); + denars.when(() -> Denars.status(OPERATION)).thenReturn(Optional.empty()); + Path file = intentFile(); + // A non-empty directory where the file should be cannot be deleted. + Files.delete(file); + Files.createDirectories(file.resolve("x")); + ShopTrades broken = spy(trades); + doReturn(Optional.of(new ShopTrades.Intent(OPERATION, request(), "prepared"))).when(broken).inspect(shopId); + command = new IntentCommand(broken, audit(), Logger.getAnonymousLogger()); + + run("intent", shopId, "clear"); + + assertTrue(messages.stream().anyMatch(message -> message.startsWith("§cCould not remove the trade record"))); + assertFalse(Files.exists(audit())); + } + + @Test + void staffOnly() throws Exception { + writeIntent("prepared"); + when(staff.hasPermission(ShopEvents.ADMIN_PERMISSION)).thenReturn(false); + + run("intent", shopId, "clear"); + + assertTrue(Files.exists(intentFile())); + assertEquals(List.of("§cYou do not have permission to use this command."), messages); + assertEquals(List.of(), command.onTabComplete(staff, mock(Command.class), "bartershops", new String[] {""})); + } + + @ParameterizedTest + @ValueSource(strings = {"", "list", "intent", "intent x y", "intent x clear --force", "intent x clear --confirm more"}) + void malformedCommandsShowUsage(String line) { + run(line.isEmpty() ? new String[0] : line.replace("x", shopId).split(" ")); + assertEquals(List.of(IntentCommand.USAGE), messages); + } + + @Test + void shopIdMustBeAUuid() { + run("intent", "../trades/x"); + assertEquals(List.of("§cA shop ID is a UUID, the name of its file in Data/."), messages); + } + + @Test + void tabCompletesTheCommandWords() { + Command bukkit = mock(Command.class); + assertEquals(List.of("intent"), command.onTabComplete(staff, bukkit, "bartershops", new String[] {""})); + assertEquals(List.of(), command.onTabComplete(staff, bukkit, "bartershops", new String[] {"intent", ""})); + assertEquals(List.of("clear"), command.onTabComplete(staff, bukkit, "bartershops", new String[] {"intent", shopId, ""})); + assertEquals(List.of("--confirm"), command.onTabComplete(staff, bukkit, "bartershops", new String[] {"intent", shopId, "clear", ""})); + assertEquals(List.of(), command.onTabComplete(staff, bukkit, "bartershops", new String[] {"intent", shopId, "x", ""})); + } + + private void run(String... args) { + assertTrue(command.onCommand(staff, mock(Command.class), "bartershops", args)); + } + + private ShopTrades.Request request() { + return new ShopTrades.Request(shopId, CUSTOMER, OWNER, "buy", 5, 1000, "DIRT"); + } + + private void intentInMemory(Result posting) { + denars.when(() -> Denars.status(anyString())).thenReturn(Optional.empty()); + denars.when(() -> Denars.trade(anyString(), anyString(), anyString(), any(), anyString(), anyLong(), anyBoolean())) + .thenReturn(posting); + assertEquals(ShopTrades.Outcome.WAITING, trades.trade(request(), () -> fail("delivery"))); + } + + private void writeIntent(String state) throws Exception { + Files.createDirectories(intentFile().getParent()); + Files.writeString(intentFile(), new Gson().toJson(new ShopTrades.Intent(OPERATION, request(), state))); + } + + private Path intentFile() { + return directory.resolve("trades").resolve(shopId + ".json"); + } + + private Path audit() { + return directory.resolve("logs").resolve("staff-audit.log"); + } +} diff --git a/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java b/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java index ce60a56..3102754 100644 --- a/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java +++ b/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java @@ -5,6 +5,7 @@ import java.nio.file.Path; import java.util.logging.Logger; import org.bukkit.Server; +import org.bukkit.command.PluginCommand; import org.bukkit.plugin.PluginManager; import org.bukkit.scheduler.BukkitScheduler; import org.junit.jupiter.api.Test; @@ -25,6 +26,8 @@ void enablesProtectionButWaitsForHeirsBeforeOpeningShops() { when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); when(server.getPluginManager()).thenReturn(plugins); when(server.getScheduler()).thenReturn(scheduler); + PluginCommand command = mock(PluginCommand.class); + when(plugin.getCommand("bartershops")).thenReturn(command); plugin.events = new ShopEvents(); doCallRealMethod().when(plugin).onEnable(); ShopMain previous = ShopMain.plugin; @@ -39,6 +42,8 @@ void enablesProtectionButWaitsForHeirsBeforeOpeningShops() { characters.when(Characters::legacyHeirsReady).thenReturn(true); task.getValue().run(); assertTrue(plugin.events.ready.getAsBoolean()); + verify(command).setExecutor(any(IntentCommand.class)); + verify(command).setTabCompleter(any(IntentCommand.class)); } finally { ShopMain.plugin = previous; } diff --git a/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java b/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java index e4f9085..fc49059 100644 --- a/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java +++ b/src/test/java/net/tfminecraft/bartershops/ShopTradesTest.java @@ -172,4 +172,38 @@ void conflictingPostingCannotBecomeDeliveryOnARetry() { denars.verify(() -> Denars.status(anyString()), times(1)); } } + @Test + void inspectionPrefersThisProcessesStateAndReadsEarlierRecords() throws Exception { + ShopTrades trades = new ShopTrades(directory, logger); + assertFalse(trades.hasIntent(shopId)); + assertEquals(Optional.empty(), trades.inspect(shopId)); + try (var denars = mockStatic(Denars.class)) { + denars.when(() -> Denars.status(anyString())).thenReturn(Optional.of(Result.APPLIED)); + assertEquals(ShopTrades.Outcome.REVIEW, trades.trade(request(), () -> { throw new IllegalStateException(); })); + } + assertTrue(trades.hasIntent(shopId)); + assertEquals("delivering", trades.inspect(shopId).orElseThrow().state()); + ShopTrades restarted = new ShopTrades(directory, logger); + assertTrue(restarted.hasIntent(shopId)); + ShopTrades.Intent read = restarted.inspect(shopId).orElseThrow(); + assertEquals("delivering", read.state()); + assertEquals(request(), read.request()); + assertTrue(read.operationId().startsWith("shop:" + shopId + ":")); + + restarted.clear(shopId); + assertFalse(Files.exists(intent())); + assertFalse(restarted.hasIntent(shopId)); + } + + @Test + void unreadableOrIncompleteRecordsCannotBeInspected() throws Exception { + ShopTrades trades = new ShopTrades(directory, logger); + Files.writeString(intent(), "{ broken"); + assertThrows(IOException.class, () -> trades.inspect(shopId)); + Files.writeString(intent(), "{\"state\": \"prepared\"}"); + assertThrows(IOException.class, () -> trades.inspect(shopId)); + Files.writeString(intent(), ""); + assertThrows(IOException.class, () -> trades.inspect(shopId)); + assertTrue(trades.hasIntent(shopId)); + } } From 9cbe09c75226158be4ee71d6759d6df062287f3d Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:05:59 +0000 Subject: [PATCH 3/5] feat: pass a dead character's shops on with the estate The bartershops estate participant gives each shop of the testator to the property recipient, character and account, rewriting Data/.json in place through DurableFiles. A journal per death (estates/.json) is written before the first shop changes, so a crash is finished at the next settle and DONE always carries every moved shop ID. Shops with an unfinished trade stay and answer BLOCKED; waiting and frozen property answers DONE with nothing moved. Settling waits for the shop migration and acts only while the estate guard holds. describe() lists the character's shops. The participant registers only when TLibs has the estate API, reconciles on the first tick and hourly over decided estates, and reports changed outcomes with the revision answered. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +- .../net/tfminecraft/bartershops/Database.java | 71 +++ .../tfminecraft/bartershops/EstateProbe.java | 23 + .../tfminecraft/bartershops/ShopEstate.java | 119 +++++ .../net/tfminecraft/bartershops/ShopMain.java | 5 + .../bartershops/ShopTransfers.java | 127 ++++++ .../bartershops/ShopEstateTest.java | 416 ++++++++++++++++++ .../tfminecraft/bartershops/ShopMainTest.java | 7 + 8 files changed, 771 insertions(+), 1 deletion(-) create mode 100644 src/main/java/net/tfminecraft/bartershops/EstateProbe.java create mode 100644 src/main/java/net/tfminecraft/bartershops/ShopEstate.java create mode 100644 src/main/java/net/tfminecraft/bartershops/ShopTransfers.java create mode 100644 src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java diff --git a/README.md b/README.md index f2345a4..70b631f 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ BarterShops turns a sign and a stocked chest into a place to trade. Players can - **Owner-set trades** — choose the quantity and price for each transaction through an in-game creation flow. - **Denar payments** — connect customer spending and shop-owner bank balances through DenarEconomy. - **Diplomatic consequences** — respect trade embargoes between nations when SimpleFactions is available. +- **Shops pass on at death** — a dead owner's shop closes, then passes with the estate to the will heir or the guild leader. ## Part of the economy @@ -27,7 +28,8 @@ Technical documentation is maintained in [TF-Minecraft/Docs](https://github.com/ With Java 21 and the [build dependencies](https://github.com/TF-Minecraft/Docs/blob/main/projects/BarterShops/README.md#build-and-dependencies) prepared, run `mvn clean verify`. -JUnit 5 and Mockito cover sign creation, trades, storage and embargo handling. +JUnit 5 and Mockito cover sign creation, trades, storage, embargo handling, +estate transfers and the staff trade command. Surefire writes results to `target/surefire-reports/`, which the Build workflow uploads. No coverage report or threshold is configured. The suite runs without a live Minecraft server or the real economy and faction plugins. diff --git a/src/main/java/net/tfminecraft/bartershops/Database.java b/src/main/java/net/tfminecraft/bartershops/Database.java index abaca72..5dc1b48 100644 --- a/src/main/java/net/tfminecraft/bartershops/Database.java +++ b/src/main/java/net/tfminecraft/bartershops/Database.java @@ -3,9 +3,13 @@ import java.io.File; import java.io.FileInputStream; import java.io.InputStreamReader; +import java.io.IOException; import java.math.BigDecimal; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.TreeMap; import java.util.UUID; @@ -20,6 +24,8 @@ import com.google.gson.GsonBuilder; import com.google.gson.ToNumberPolicy; +import net.tfminecraft.tlibs.character.CharacterRef; + public class Database { static final Gson SHOP_READER = new GsonBuilder() .setObjectToNumberStrategy(ToNumberPolicy.BIG_DECIMAL).create(); @@ -81,6 +87,71 @@ static String readCharacter(Object value) { } } + /** One shop file's owner and sign, as estates need them. */ + public record ShopRecord(String shopId, String ownerCharacter, String type, String world, double x, double y, double z) { + String label() { + return (type == null || type.isEmpty() ? "Shop" : type.substring(0, 1).toUpperCase() + type.substring(1) + " shop") + + " at " + world + " " + (int) Math.floor(x) + ", " + (int) Math.floor(y) + ", " + (int) Math.floor(z); + } + } + + /** Every readable shop file. Unreadable files are skipped, as the sign lookups skip them. */ + public List shops() { + List shops = new ArrayList<>(); + for (File file : shopFiles()) { + try { + Map shopJson = read(file); + shops.add(new ShopRecord(shopId(file), readCharacter(shopJson.get(OWNER_CHARACTER)), + shopJson.get("type") instanceof String type ? type : null, String.valueOf(shopJson.get("sign world")), + number(shopJson.get("sign xPos")), number(shopJson.get("sign yPos")), number(shopJson.get("sign zPos")))); + } catch (IOException ex) { + ex.printStackTrace(); + } + } + return shops; + } + + /** The owning character of one shop; empty when the shop is gone or has no character. */ + public Optional ownerCharacter(String shopId) throws IOException { + File file = shopFile(shopId); + if (!file.isFile()) return Optional.empty(); + return Optional.ofNullable(readCharacter(read(file).get(OWNER_CHARACTER))); + } + + /** + * Gives one shop to another character, rewriting its file in place and keeping every other value. + * False when the shop is gone. + */ + public boolean setOwner(String shopId, CharacterRef owner) throws IOException { + File file = shopFile(shopId); + if (!file.isFile()) return false; + Map shopJson = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + read(file).forEach((key, value) -> shopJson.put(String.valueOf(key), value)); + shopJson.put(OWNER, owner.ownerId().toString()); + shopJson.put(OWNER_CHARACTER, owner.characterId().toString()); + DurableFiles.write(file.toPath(), new GsonBuilder().setPrettyPrinting().create().toJson(shopJson)); + return true; + } + + // Shop IDs are file names, so only a UUID may name one. + private File shopFile(String shopId) { + return new File(dataFolder, UUID.fromString(shopId) + ".json"); + } + + private static Map read(File file) throws IOException { + try (InputStreamReader reader = new InputStreamReader(new FileInputStream(file), "UTF-8")) { + Map shopJson = SHOP_READER.fromJson(reader, Map.class); + if (shopJson == null) throw new IOException("Empty shop file " + file); + return shopJson; + } catch (RuntimeException ex) { + throw new IOException("Unreadable shop file " + file, ex); + } + } + + private static double number(Object value) { + return value instanceof Number number ? number.doubleValue() : 0; + } + // The data folder only exists once a shop has been saved. private File[] shopFiles() { File[] files = dataFolder.listFiles((folder, name) -> name.endsWith(".json")); diff --git a/src/main/java/net/tfminecraft/bartershops/EstateProbe.java b/src/main/java/net/tfminecraft/bartershops/EstateProbe.java new file mode 100644 index 0000000..cadb3c4 --- /dev/null +++ b/src/main/java/net/tfminecraft/bartershops/EstateProbe.java @@ -0,0 +1,23 @@ +package net.tfminecraft.bartershops; + +/** Whether TLibs has the estate API (2.6.0). Older TLibs leaves shops with dead owners. */ +final class EstateProbe { + static final String[] CLASSES = { + "net.tfminecraft.tlibs.character.estate.Estates", + "net.tfminecraft.tlibs.character.estate.EstateParticipant", + "net.tfminecraft.tlibs.character.estate.EstateDecision", + "net.tfminecraft.tlibs.character.estate.Outcome", + "net.tfminecraft.tlibs.character.estate.Holdings", + }; + + private EstateProbe() {} + + static boolean available(String... classes) { + try { + for (String name : classes) Class.forName(name); + return true; + } catch (ReflectiveOperationException | LinkageError ex) { + return false; + } + } +} diff --git a/src/main/java/net/tfminecraft/bartershops/ShopEstate.java b/src/main/java/net/tfminecraft/bartershops/ShopEstate.java new file mode 100644 index 0000000..6f13525 --- /dev/null +++ b/src/main/java/net/tfminecraft/bartershops/ShopEstate.java @@ -0,0 +1,119 @@ +package net.tfminecraft.bartershops; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.UUID; +import java.util.function.BooleanSupplier; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.logging.Logger; + +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.ServicePriority; + +import net.tfminecraft.tlibs.character.CharacterRef; +import net.tfminecraft.tlibs.character.estate.EstateDecision; +import net.tfminecraft.tlibs.character.estate.EstateParticipant; +import net.tfminecraft.tlibs.character.estate.Estates; +import net.tfminecraft.tlibs.character.estate.Holdings; +import net.tfminecraft.tlibs.character.estate.Outcome; + +/** + * The {@code bartershops} estate participant: a dead character's shops go to the estate's property + * recipient. Loaded only after {@link EstateProbe} finds the TLibs 2.6.0 estate API. + */ +final class ShopEstate implements EstateParticipant { + static final String ID = "bartershops"; + static final long HOUR_TICKS = 20L * 60 * 60; + + private final Database db; + private final ShopTransfers transfers; + private final BooleanSupplier ready; + private final Logger logger; + Predicate guard = Estates::guardHolds; + Supplier> decided = Estates::decided; + Reporter reporter = Estates::report; + // The last outcome reported per death, so the hourly pass reports only changes. + private final Map> reported = new HashMap<>(); + + interface Reporter { + boolean report(UUID deathId, String participantId, long revision, Outcome outcome); + } + + ShopEstate(Database db, ShopTransfers transfers, BooleanSupplier ready, Logger logger) { + this.db = db; + this.transfers = transfers; + this.ready = ready; + this.logger = logger; + } + + /** Registers the participant and runs the reconcile on the first tick, then hourly. */ + static void start(Plugin plugin, Database db, ShopTrades trades, BooleanSupplier ready) { + ShopEstate estate = new ShopEstate(db, + new ShopTransfers(db, trades, plugin.getDataFolder().toPath().resolve("estates")), ready, + plugin.getLogger()); + plugin.getServer().getServicesManager().register(EstateParticipant.class, estate, plugin, ServicePriority.Normal); + plugin.getServer().getScheduler().runTaskTimer(plugin, estate::reconcile, 1L, HOUR_TICKS); + } + + @Override + public String id() { + return ID; + } + + @Override + public Outcome settle(EstateDecision decision) { + return settle(decision, db.shops()); + } + + private Outcome settle(EstateDecision decision, List shops) { + if (!ready.getAsBoolean()) return Outcome.retry("Shops are waiting for character data"); + if (!guard.test(decision)) return Outcome.retry("The estate guard does not hold"); + Optional recipient = decision.property().character(); + // Waiting and frozen property stays with the testator; there is nothing to move at this revision. + if (recipient.isEmpty()) return Outcome.done(); + String testator = decision.testator().characterId().toString(); + List owned = shops.stream().filter(shop -> testator.equals(shop.ownerCharacter())) + .map(Database.ShopRecord::shopId).sorted().toList(); + ShopTransfers.Result result; + try { + result = transfers.move(decision.deathId(), decision.testator().characterId(), recipient.get(), owned); + } catch (IOException | RuntimeException ex) { + logger.warning("Could not move the shops of estate " + decision.deathId() + ": " + ex); + return Outcome.retry("Could not save a shop: " + ex.getMessage()); + } + if (!result.blocked().isEmpty()) { + return Outcome.blocked("Unfinished trades at shops " + String.join(", ", result.blocked()) + + "; staff resolve them with /bartershops intent "); + } + return Outcome.done(Map.of("shops", result.moved().size()), result.moved()); + } + + @Override + public Holdings describe(UUID characterId) { + String character = characterId.toString(); + List labels = db.shops().stream().filter(shop -> character.equals(shop.ownerCharacter())) + .map(Database.ShopRecord::label).sorted().toList(); + return labels.isEmpty() ? Holdings.NONE : new Holdings(Map.of("shops", labels.size()), labels); + } + + /** Settles every decided estate whose guard holds and reports outcomes that changed. */ + void reconcile() { + if (!ready.getAsBoolean()) return; + List shops = db.shops(); + for (EstateDecision decision : decided.get()) { + if (!guard.test(decision)) continue; + Outcome outcome = settle(decision, shops); + Map.Entry last = Map.entry(decision.revision(), outcome); + if (last.equals(reported.get(decision.deathId()))) continue; + if (reporter.report(decision.deathId(), ID, decision.revision(), outcome)) { + reported.put(decision.deathId(), last); + } + // A recipient may also be a later testator, so read the shops again after a move. + if (outcome instanceof Outcome.Done done && !done.movedIds().isEmpty()) shops = db.shops(); + } + } +} diff --git a/src/main/java/net/tfminecraft/bartershops/ShopMain.java b/src/main/java/net/tfminecraft/bartershops/ShopMain.java index 0f8cdd8..32017a6 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopMain.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopMain.java @@ -29,5 +29,10 @@ public void onEnable() { command.setExecutor(intents); command.setTabCompleter(intents); } + if (EstateProbe.available(EstateProbe.CLASSES)) { + ShopEstate.start(this, events.db, events.trades, () -> ready); + } else { + getLogger().warning("TLibs has no estate API (2.6.0 or later); shops of dead characters stay with them."); + } } } diff --git a/src/main/java/net/tfminecraft/bartershops/ShopTransfers.java b/src/main/java/net/tfminecraft/bartershops/ShopTransfers.java new file mode 100644 index 0000000..3f00fca --- /dev/null +++ b/src/main/java/net/tfminecraft/bartershops/ShopTransfers.java @@ -0,0 +1,127 @@ +package net.tfminecraft.bartershops; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.UUID; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonParseException; + +import net.tfminecraft.tlibs.character.CharacterRef; + +/** + * Gives a dead character's shops to the estate's recipient. Each death keeps a journal in + * {@code estates/.json}, written before the first shop file changes, so a crash between + * shops is finished at the next settle and every moved shop is still reported. + */ +final class ShopTransfers { + static final String PLANNED = "planned"; + static final String MOVED = "moved"; + // The shop was removed or given to someone else before this move reached it. + static final String SUPERSEDED = "superseded"; + + record Entry(String account, String character, String state) { + CharacterRef recipient() { + return new CharacterRef(UUID.fromString(account), UUID.fromString(character)); + } + } + + record Journal(String deathId, String testator, Map shops) {} + + /** + * @param moved every shop this estate has moved so far, in journal order + * @param blocked shops left with the testator because a trade there is unfinished + */ + record Result(List moved, List blocked) {} + + private static final Gson JSON = new GsonBuilder().setPrettyPrinting().create(); + private final Database db; + private final ShopTrades trades; + private final Path directory; + + ShopTransfers(Database db, ShopTrades trades, Path directory) { + this.db = db; + this.trades = trades; + this.directory = directory; + } + + /** + * Moves every shop in {@code owned} (the testator's today) and any shop planned before a crash. + * Shops with an unfinished trade stay where they are and are returned as blocked. + */ + Result move(UUID deathId, UUID testator, CharacterRef to, Collection owned) throws IOException { + Journal journal = read(deathId); + if (journal.testator() == null) journal = new Journal(deathId.toString(), testator.toString(), journal.shops()); + Map shops = journal.shops(); + List blocked = new ArrayList<>(); + Entry planned = new Entry(to.ownerId().toString(), to.characterId().toString(), PLANNED); + boolean changed = false; + for (String shopId : owned) { + if (blockedByTrade(shopId)) { + blocked.add(shopId); + continue; + } + shops.put(shopId, planned); + changed = true; + } + if (changed) write(deathId, journal); + changed = false; + try { + for (Map.Entry shop : shops.entrySet()) { + Entry entry = shop.getValue(); + if (!PLANNED.equals(entry.state())) continue; + Optional owner = db.ownerCharacter(shop.getKey()); + String state = SUPERSEDED; + if (owner.equals(Optional.of(testator.toString()))) { + state = db.setOwner(shop.getKey(), entry.recipient()) ? MOVED : SUPERSEDED; + } else if (owner.equals(Optional.of(entry.character()))) { + state = MOVED; // written before a crash cut the journal update short + } + shop.setValue(new Entry(entry.account(), entry.character(), state)); + changed = true; + } + } finally { + if (changed) write(deathId, journal); + } + List moved = shops.entrySet().stream().filter(shop -> MOVED.equals(shop.getValue().state())) + .map(Map.Entry::getKey).toList(); + return new Result(moved, blocked); + } + + // A shop file whose name is not a shop ID cannot trade or be rewritten; staff handle it. + private boolean blockedByTrade(String shopId) { + try { + return trades.hasIntent(shopId); + } catch (IllegalArgumentException ex) { + return true; + } + } + + private Journal read(UUID deathId) throws IOException { + Path file = file(deathId); + if (!Files.exists(file)) return new Journal(deathId.toString(), null, new LinkedHashMap<>()); + try { + Journal journal = JSON.fromJson(Files.readString(file), Journal.class); + if (journal == null || journal.shops() == null) throw new IOException("Incomplete estate journal " + file); + return new Journal(journal.deathId(), journal.testator(), new LinkedHashMap<>(journal.shops())); + } catch (JsonParseException ex) { + throw new IOException("Unreadable estate journal " + file, ex); + } + } + + private void write(UUID deathId, Journal journal) throws IOException { + DurableFiles.write(file(deathId), JSON.toJson(journal)); + } + + private Path file(UUID deathId) { + return directory.resolve(deathId + ".json"); + } +} diff --git a/src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java b/src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java new file mode 100644 index 0000000..c94b082 --- /dev/null +++ b/src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java @@ -0,0 +1,416 @@ +package net.tfminecraft.bartershops; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.UUID; +import java.util.logging.Logger; + +import org.bukkit.Server; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.ServicePriority; +import org.bukkit.plugin.ServicesManager; +import org.bukkit.scheduler.BukkitScheduler; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.google.gson.Gson; + +import net.tfminecraft.tlibs.character.CharacterRef; +import net.tfminecraft.tlibs.character.estate.EstateDecision; +import net.tfminecraft.tlibs.character.estate.EstateParticipant; +import net.tfminecraft.tlibs.character.estate.Holdings; +import net.tfminecraft.tlibs.character.estate.Outcome; +import net.tfminecraft.tlibs.character.estate.Recipient; + +class ShopEstateTest { + private static final CharacterRef TESTATOR = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final CharacterRef HEIR = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final CharacterRef OTHER = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final UUID DEATH = UUID.randomUUID(); + + @TempDir Path directory; + private Path data; + private Path tradesDirectory; + private Database db; + private ShopTrades trades; + private ShopTransfers transfers; + private ShopEstate estate; + private boolean ready = true; + private boolean guard = true; + + @BeforeEach + void setUp() { + data = directory.resolve("Data"); + tradesDirectory = directory.resolve("trades"); + db = new Database(data.toFile()); + trades = new ShopTrades(tradesDirectory, Logger.getAnonymousLogger()); + transfers = new ShopTransfers(db, trades, directory.resolve("estates")); + estate = new ShopEstate(db, transfers, () -> ready, Logger.getAnonymousLogger()); + estate.guard = decision -> guard; + } + + // --- Database --- + + @Test + void ownerIsRewrittenInPlaceKeepingEveryOtherValue() throws Exception { + String id = shop(TESTATOR, "buy", 10.5); + Path file = data.resolve(id + ".json"); + Map before = json(file); + + assertTrue(db.setOwner(id, HEIR)); + + assertEquals(List.of(file), files(data)); + Map after = json(file); + assertEquals(HEIR.ownerId().toString(), after.get(Database.OWNER)); + assertEquals(HEIR.characterId().toString(), after.get(Database.OWNER_CHARACTER)); + for (Object key : before.keySet()) { + if (key.equals(Database.OWNER) || key.equals(Database.OWNER_CHARACTER)) continue; + assertEquals(before.get(key), after.get(key), key.toString()); + } + assertEquals(Optional.of(HEIR.characterId().toString()), db.ownerCharacter(id)); + assertTrue(Files.readString(file).contains("\"sign xPos\": 10.5")); + } + + @Test + void missingShopsAreNotRewrittenOrCreated() throws Exception { + String id = UUID.randomUUID().toString(); + assertFalse(db.setOwner(id, HEIR)); + assertEquals(Optional.empty(), db.ownerCharacter(id)); + assertFalse(Files.exists(data.resolve(id + ".json"))); + assertThrows(IllegalArgumentException.class, () -> db.setOwner("../escape", HEIR)); + } + + @Test + void shopListingSkipsUnreadableFilesAndLabelsSigns() throws Exception { + String id = shop(TESTATOR, "sell", -3.2); + Files.writeString(data.resolve(UUID.randomUUID() + ".json"), "{ not json"); + Files.writeString(data.resolve(UUID.randomUUID() + ".json"), ""); + + List shops = db.shops(); + + assertEquals(1, shops.size()); + assertEquals(id, shops.get(0).shopId()); + assertEquals(TESTATOR.characterId().toString(), shops.get(0).ownerCharacter()); + assertEquals("Sell shop at world -4, 64, -4", shops.get(0).label()); + assertEquals("Shop at w 0, 0, 0", new Database.ShopRecord("x", null, null, "w", 0, 0, 0).label()); + assertEquals("Shop at w 0, 0, 0", new Database.ShopRecord("x", null, "", "w", 0, 0, 0).label()); + } + + // --- Transfers --- + + @Test + void decidedEstateGivesEveryShopOfTheTestatorToTheRecipient() throws Exception { + String first = shop(TESTATOR, "buy", 1); + String second = shop(TESTATOR, "sell", 2); + String others = shop(OTHER, "buy", 3); + + Outcome outcome = estate.settle(decision(Recipient.character(HEIR), 0)); + + List moved = new ArrayList<>(List.of(first, second)); + moved.sort(null); + assertEquals(Outcome.done(Map.of("shops", 2), moved), outcome); + assertEquals(Optional.of(HEIR.characterId().toString()), db.ownerCharacter(first)); + assertEquals(Optional.of(HEIR.characterId().toString()), db.ownerCharacter(second)); + assertEquals(HEIR.ownerId().toString(), json(data.resolve(first + ".json")).get(Database.OWNER)); + assertEquals(Optional.of(OTHER.characterId().toString()), db.ownerCharacter(others)); + } + + @Test + void settlingAgainMovesNothingTwiceAndStillReportsEveryMovedShop() throws Exception { + String id = shop(TESTATOR, "buy", 1); + Outcome first = estate.settle(decision(Recipient.character(HEIR), 0)); + String after = Files.readString(data.resolve(id + ".json")); + + Outcome again = estate.settle(decision(Recipient.character(HEIR), 1)); + + assertEquals(first, again); + assertEquals(Outcome.done(Map.of("shops", 1), List.of(id)), again); + assertEquals(after, Files.readString(data.resolve(id + ".json"))); + } + + @Test + void crashAfterThePlanFinishesTheMoveOnTheNextSettle() throws Exception { + String id = shop(TESTATOR, "buy", 1); + Path journal = directory.resolve("estates").resolve(DEATH + ".json"); + Files.createDirectories(journal.getParent()); + Files.writeString(journal, new Gson().toJson(new ShopTransfers.Journal(DEATH.toString(), + TESTATOR.characterId().toString(), Map.of(id, new ShopTransfers.Entry(HEIR.ownerId().toString(), + HEIR.characterId().toString(), ShopTransfers.PLANNED))))); + // A shop planned before the crash is finished even when the scan does not list it. + ShopTransfers.Result result = transfers.move(DEATH, TESTATOR.characterId(), HEIR, List.of()); + + assertEquals(List.of(id), result.moved()); + assertEquals(Optional.of(HEIR.characterId().toString()), db.ownerCharacter(id)); + assertTrue(Files.readString(journal).contains(ShopTransfers.MOVED)); + assertEquals(Outcome.done(Map.of("shops", 1), List.of(id)), estate.settle(decision(Recipient.character(HEIR), 0))); + } + + @Test + void crashAfterTheShopWriteCountsTheShopWithoutWritingItAgain() throws Exception { + String id = shop(HEIR, "buy", 1); + Path journal = directory.resolve("estates").resolve(DEATH + ".json"); + Files.createDirectories(journal.getParent()); + Files.writeString(journal, new Gson().toJson(new ShopTransfers.Journal(DEATH.toString(), + TESTATOR.characterId().toString(), Map.of(id, new ShopTransfers.Entry(HEIR.ownerId().toString(), + HEIR.characterId().toString(), ShopTransfers.PLANNED))))); + Database spy = spy(db); + ShopEstate replay = new ShopEstate(spy, new ShopTransfers(spy, trades, directory.resolve("estates")), + () -> true, Logger.getAnonymousLogger()); + replay.guard = decision -> true; + + assertEquals(Outcome.done(Map.of("shops", 1), List.of(id)), replay.settle(decision(Recipient.character(HEIR), 0))); + verify(spy, never()).setOwner(anyString(), any()); + } + + @Test + void shopsRemovedOrGivenAwayBeforeTheMoveAreLeftAlone() throws Exception { + String gone = UUID.randomUUID().toString(); + String givenAway = shop(OTHER, "buy", 1); + Path journal = directory.resolve("estates").resolve(DEATH + ".json"); + Files.createDirectories(journal.getParent()); + ShopTransfers.Entry planned = new ShopTransfers.Entry(HEIR.ownerId().toString(), HEIR.characterId().toString(), + ShopTransfers.PLANNED); + Files.writeString(journal, new Gson().toJson(new ShopTransfers.Journal(DEATH.toString(), + TESTATOR.characterId().toString(), Map.of(gone, planned, givenAway, planned)))); + + assertEquals(Outcome.done(Map.of("shops", 0), List.of()), estate.settle(decision(Recipient.character(HEIR), 0))); + assertEquals(Optional.of(OTHER.characterId().toString()), db.ownerCharacter(givenAway)); + assertFalse(Files.readString(journal).contains(ShopTransfers.PLANNED)); + } + + @Test + void shopWithAnUnfinishedTradeIsBlockedAndKeptWhileTheOthersMove() throws Exception { + String stuck = shop(TESTATOR, "buy", 1); + String free = shop(TESTATOR, "buy", 2); + Files.createDirectories(tradesDirectory); + Files.writeString(tradesDirectory.resolve(stuck + ".json"), "{}"); + + Outcome outcome = estate.settle(decision(Recipient.character(HEIR), 0)); + + Outcome.Blocked blocked = assertInstanceOf(Outcome.Blocked.class, outcome); + assertTrue(blocked.reason().contains(stuck)); + assertTrue(blocked.reason().contains("/bartershops intent")); + assertFalse(blocked.reason().contains(free)); + assertEquals(Optional.of(TESTATOR.characterId().toString()), db.ownerCharacter(stuck)); + assertEquals(Optional.of(HEIR.characterId().toString()), db.ownerCharacter(free)); + + // Staff clear the trade; the next settle moves the shop and reports both. + trades.clear(stuck); + Outcome.Done done = assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.character(HEIR), 0))); + assertEquals(Map.of("shops", 2), done.counts()); + assertEquals(java.util.Set.of(stuck, free), java.util.Set.copyOf(done.movedIds())); + } + + @Test + void waitingAndFrozenPropertyMovesNothingAndAnswersDone() throws Exception { + String id = shop(TESTATOR, "buy", 1); + assertEquals(Outcome.done(), estate.settle(decision(Recipient.WAITING, 0))); + assertEquals(Outcome.done(), estate.settle(decision(Recipient.FROZEN, 1))); + assertEquals(Optional.of(TESTATOR.characterId().toString()), db.ownerCharacter(id)); + assertFalse(Files.exists(directory.resolve("estates"))); + } + + @Test + void failingGuardOrUnmigratedShopsMoveNothing() throws Exception { + String id = shop(TESTATOR, "buy", 1); + guard = false; + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.character(HEIR), 0)).kind()); + guard = true; + ready = false; + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.character(HEIR), 0)).kind()); + assertEquals(Optional.of(TESTATOR.characterId().toString()), db.ownerCharacter(id)); + } + + @Test + void failedWriteRetriesLater() throws Exception { + shop(TESTATOR, "buy", 1); + Files.writeString(directory.resolve("estates"), "not a directory"); + + Outcome.Retry retry = assertInstanceOf(Outcome.Retry.class, estate.settle(decision(Recipient.character(HEIR), 0))); + assertTrue(retry.reason().startsWith("Could not save a shop")); + } + + @Test + void unreadableJournalRetriesWithoutMoving() throws Exception { + String id = shop(TESTATOR, "buy", 1); + Files.createDirectories(directory.resolve("estates")); + Files.writeString(directory.resolve("estates").resolve(DEATH + ".json"), "{ broken"); + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.character(HEIR), 0)).kind()); + Files.writeString(directory.resolve("estates").resolve(DEATH + ".json"), "{}"); + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.character(HEIR), 0)).kind()); + assertEquals(Optional.of(TESTATOR.characterId().toString()), db.ownerCharacter(id)); + } + + @Test + void shopFileNotNamedByAShopIdIsBlocked() throws Exception { + Files.createDirectories(data); + Files.writeString(data.resolve("odd.json"), shopJson(TESTATOR, "buy", 1)); + Outcome.Blocked blocked = assertInstanceOf(Outcome.Blocked.class, estate.settle(decision(Recipient.character(HEIR), 0))); + assertTrue(blocked.reason().contains("odd")); + } + + // --- Describe and reconcile --- + + @Test + void describeListsTheCharactersShops() throws Exception { + shop(TESTATOR, "buy", 1); + shop(TESTATOR, "sell", 2); + shop(OTHER, "buy", 3); + + Holdings holdings = estate.describe(TESTATOR.characterId()); + + assertEquals(Map.of("shops", 2), holdings.counts()); + assertEquals(List.of("Buy shop at world 1, 64, 1", "Sell shop at world 2, 64, 2"), holdings.labels()); + assertSame(Holdings.NONE, estate.describe(HEIR.characterId())); + } + + @Test + void reconcileSettlesDecidedEstatesAndReportsOnlyChanges() throws Exception { + String id = shop(TESTATOR, "buy", 1); + EstateDecision decided = decision(Recipient.character(HEIR), 0); + EstateDecision reverted = new EstateDecision(UUID.randomUUID(), OTHER, Optional.empty(), Instant.EPOCH, + Optional.empty(), Optional.empty(), Optional.empty(), Recipient.FROZEN, Recipient.character(HEIR), + Recipient.FROZEN, Recipient.SENDER, 0); + shop(OTHER, "buy", 2); + estate.guard = decision -> decision != reverted; + estate.decided = () -> List.of(reverted, decided); + List reports = new ArrayList<>(); + estate.reporter = (death, participant, revision, outcome) -> reports.add(new Object[] {death, participant, revision, outcome}); + + estate.reconcile(); + estate.reconcile(); + + assertEquals(1, reports.size()); + assertEquals(DEATH, reports.get(0)[0]); + assertEquals("bartershops", reports.get(0)[1]); + assertEquals(0L, reports.get(0)[2]); + assertEquals(Outcome.done(Map.of("shops", 1), List.of(id)), reports.get(0)[3]); + assertEquals(Optional.of(OTHER.characterId().toString()), db.ownerCharacter(shopOf(OTHER))); + + // A new revision is reported again. + estate.decided = () -> List.of(decision(Recipient.character(HEIR), 1)); + estate.reconcile(); + assertEquals(2, reports.size()); + assertEquals(1L, reports.get(1)[2]); + } + + @Test + void reconcileReportsAgainWhenNoServiceTookTheReport() throws Exception { + shop(TESTATOR, "buy", 1); + estate.decided = () -> List.of(decision(Recipient.character(HEIR), 0)); + List reports = new ArrayList<>(); + estate.reporter = (death, participant, revision, outcome) -> reports.add(outcome) && false; + + estate.reconcile(); + estate.reconcile(); + + assertEquals(2, reports.size()); + } + + @Test + void reconcileWaitsForMigration() { + ready = false; + estate.decided = () -> fail("read before shops were ready"); + estate.reconcile(); + } + + @Test + void shopsReceivedFromOneEstateFollowTheRecipientsOwnEstate() throws Exception { + String id = shop(TESTATOR, "buy", 1); + UUID heirDeath = UUID.randomUUID(); + EstateDecision heirs = new EstateDecision(heirDeath, HEIR, Optional.empty(), Instant.EPOCH, Optional.empty(), + Optional.empty(), Optional.empty(), Recipient.FROZEN, Recipient.character(OTHER), Recipient.FROZEN, + Recipient.SENDER, 0); + estate.decided = () -> List.of(decision(Recipient.character(HEIR), 0), heirs); + estate.reporter = (death, participant, revision, outcome) -> true; + + estate.reconcile(); + + assertEquals(Optional.of(OTHER.characterId().toString()), db.ownerCharacter(id)); + } + + @Test + void startRegistersTheParticipantAndReconcilesOnTheFirstTickThenHourly() { + Plugin plugin = mock(Plugin.class); + Server server = mock(Server.class); + ServicesManager services = mock(ServicesManager.class); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getServer()).thenReturn(server); + when(plugin.getDataFolder()).thenReturn(directory.toFile()); + when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + when(server.getServicesManager()).thenReturn(services); + when(server.getScheduler()).thenReturn(scheduler); + + ShopEstate.start(plugin, db, trades, () -> true); + + var participant = org.mockito.ArgumentCaptor.forClass(EstateParticipant.class); + verify(services).register(eq(EstateParticipant.class), participant.capture(), eq(plugin), eq(ServicePriority.Normal)); + assertEquals("bartershops", participant.getValue().id()); + verify(scheduler).runTaskTimer(eq(plugin), any(Runnable.class), eq(1L), eq(72_000L)); + } + + @Test + void probeFindsTheEstateApiAndMissesAbsentClasses() { + assertTrue(EstateProbe.available(EstateProbe.CLASSES)); + assertFalse(EstateProbe.available("net.tfminecraft.tlibs.character.estate.Missing")); + } + + // --- Helpers --- + + private EstateDecision decision(Recipient property, long revision) { + return new EstateDecision(DEATH, TESTATOR, Optional.of(Instant.EPOCH), Instant.EPOCH, Optional.empty(), + Optional.empty(), Optional.empty(), Recipient.FROZEN, property, Recipient.FROZEN, Recipient.SENDER, revision); + } + + private String shopOf(CharacterRef owner) { + return db.shops().stream().filter(shop -> owner.characterId().toString().equals(shop.ownerCharacter())) + .findFirst().orElseThrow().shopId(); + } + + private String shop(CharacterRef owner, String type, double x) throws IOException { + String id = UUID.randomUUID().toString(); + Files.createDirectories(data); + Files.writeString(data.resolve(id + ".json"), shopJson(owner, type, x)); + return id; + } + + private static String shopJson(CharacterRef owner, String type, double x) { + return """ + { + "barter amount": 5.0, + "owner": "%s", + "owner character": "%s", + "payment item": "MATERIAL.COIN", + "price": 10.0, + "sign world": "world", + "sign xPos": %s, + "sign yPos": 64.0, + "sign zPos": %s, + "storage world": "world", + "storage xPos": 0.0, + "storage yPos": 63.0, + "storage zPos": 0.0, + "type": "%s" + }""".formatted(owner.ownerId(), owner.characterId(), x, x, type); + } + + private static Map json(Path file) throws IOException { + return Database.SHOP_READER.fromJson(Files.readString(file), Map.class); + } + + private static List files(Path folder) throws IOException { + try (var stream = Files.list(folder)) { + return stream.toList(); + } + } +} diff --git a/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java b/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java index 3102754..260209c 100644 --- a/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java +++ b/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java @@ -7,10 +7,13 @@ import org.bukkit.Server; import org.bukkit.command.PluginCommand; import org.bukkit.plugin.PluginManager; +import org.bukkit.plugin.ServicePriority; +import org.bukkit.plugin.ServicesManager; import org.bukkit.scheduler.BukkitScheduler; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import net.tfminecraft.tlibs.character.Characters; +import net.tfminecraft.tlibs.character.estate.EstateParticipant; class ShopMainTest { @TempDir Path directory; @@ -26,6 +29,8 @@ void enablesProtectionButWaitsForHeirsBeforeOpeningShops() { when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); when(server.getPluginManager()).thenReturn(plugins); when(server.getScheduler()).thenReturn(scheduler); + ServicesManager services = mock(ServicesManager.class); + when(server.getServicesManager()).thenReturn(services); PluginCommand command = mock(PluginCommand.class); when(plugin.getCommand("bartershops")).thenReturn(command); plugin.events = new ShopEvents(); @@ -44,6 +49,8 @@ void enablesProtectionButWaitsForHeirsBeforeOpeningShops() { assertTrue(plugin.events.ready.getAsBoolean()); verify(command).setExecutor(any(IntentCommand.class)); verify(command).setTabCompleter(any(IntentCommand.class)); + verify(services).register(eq(EstateParticipant.class), any(ShopEstate.class), eq(plugin), eq(ServicePriority.Normal)); + verify(scheduler).runTaskTimer(eq(plugin), any(Runnable.class), eq(1L), eq(ShopEstate.HOUR_TICKS)); } finally { ShopMain.plugin = previous; } From b995f6f9bd9c99e4427cdd802aa0bca914e7e709 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:33:02 +0000 Subject: [PATCH 4/5] fix: tighten the shop estate journal, logging and reconcile A shop planned before a crash is checked for an unfinished trade again before it moves, and is blocked if it has one. A journal whose death or testator does not match the estate is refused (RETRY), so another death's shops cannot join this one. Unreadable shop files are reported once each through the plugin logger instead of a stack trace on every pass, and one failing estate no longer stops the hourly reconcile of the others. Co-Authored-By: Claude Opus 5.5 --- .../net/tfminecraft/bartershops/Database.java | 9 ++- .../tfminecraft/bartershops/ShopEstate.java | 21 ++++--- .../net/tfminecraft/bartershops/ShopMain.java | 1 + .../bartershops/ShopTransfers.java | 15 +++-- .../bartershops/ShopEstateTest.java | 63 +++++++++++++++++++ 5 files changed, 96 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/tfminecraft/bartershops/Database.java b/src/main/java/net/tfminecraft/bartershops/Database.java index 5dc1b48..aa41717 100644 --- a/src/main/java/net/tfminecraft/bartershops/Database.java +++ b/src/main/java/net/tfminecraft/bartershops/Database.java @@ -7,11 +7,14 @@ import java.math.BigDecimal; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.TreeMap; import java.util.UUID; +import java.util.logging.Logger; import org.bukkit.Bukkit; import org.bukkit.ChatColor; @@ -33,6 +36,9 @@ public class Database { static final String OWNER = "owner"; static final String OWNER_CHARACTER = "owner character"; private final File dataFolder; + Logger logger = Logger.getLogger("BarterShops"); + // Unreadable shop files already reported, so the hourly estate pass warns once per file. + private final Set unreadable = new HashSet<>(); public Database() { this(new File("plugins/BarterShops/Data")); @@ -105,7 +111,8 @@ public List shops() { shopJson.get("type") instanceof String type ? type : null, String.valueOf(shopJson.get("sign world")), number(shopJson.get("sign xPos")), number(shopJson.get("sign yPos")), number(shopJson.get("sign zPos")))); } catch (IOException ex) { - ex.printStackTrace(); + if (unreadable.add(file.getName())) + logger.warning("Skipping unreadable shop file " + file.getName() + ": " + ex.getMessage()); } } return shops; diff --git a/src/main/java/net/tfminecraft/bartershops/ShopEstate.java b/src/main/java/net/tfminecraft/bartershops/ShopEstate.java index 6f13525..7fdd02a 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopEstate.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopEstate.java @@ -105,15 +105,20 @@ void reconcile() { if (!ready.getAsBoolean()) return; List shops = db.shops(); for (EstateDecision decision : decided.get()) { - if (!guard.test(decision)) continue; - Outcome outcome = settle(decision, shops); - Map.Entry last = Map.entry(decision.revision(), outcome); - if (last.equals(reported.get(decision.deathId()))) continue; - if (reporter.report(decision.deathId(), ID, decision.revision(), outcome)) { - reported.put(decision.deathId(), last); + // One estate failing must not stop the others until the next hour. + try { + if (!guard.test(decision)) continue; + Outcome outcome = settle(decision, shops); + Map.Entry last = Map.entry(decision.revision(), outcome); + if (last.equals(reported.get(decision.deathId()))) continue; + if (reporter.report(decision.deathId(), ID, decision.revision(), outcome)) { + reported.put(decision.deathId(), last); + } + // A recipient may also be a later testator, so read the shops again after a move. + if (outcome instanceof Outcome.Done done && !done.movedIds().isEmpty()) shops = db.shops(); + } catch (RuntimeException ex) { + logger.warning("Could not reconcile the shops of estate " + decision.deathId() + ": " + ex); } - // A recipient may also be a later testator, so read the shops again after a move. - if (outcome instanceof Outcome.Done done && !done.movedIds().isEmpty()) shops = db.shops(); } } } diff --git a/src/main/java/net/tfminecraft/bartershops/ShopMain.java b/src/main/java/net/tfminecraft/bartershops/ShopMain.java index 32017a6..76a12df 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopMain.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopMain.java @@ -13,6 +13,7 @@ public class ShopMain extends JavaPlugin { public void onEnable() { plugin = this; events.db = new Database(new File(getDataFolder(), "Data")); + events.db.logger = getLogger(); events.trades = new ShopTrades(getDataFolder().toPath().resolve("trades"), getLogger()); events.ready = () -> ready; ShopMigration migration = new ShopMigration(new File(getDataFolder(), "Data"), diff --git a/src/main/java/net/tfminecraft/bartershops/ShopTransfers.java b/src/main/java/net/tfminecraft/bartershops/ShopTransfers.java index 3f00fca..f24f8a2 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopTransfers.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopTransfers.java @@ -58,8 +58,7 @@ record Result(List moved, List blocked) {} * Shops with an unfinished trade stay where they are and are returned as blocked. */ Result move(UUID deathId, UUID testator, CharacterRef to, Collection owned) throws IOException { - Journal journal = read(deathId); - if (journal.testator() == null) journal = new Journal(deathId.toString(), testator.toString(), journal.shops()); + Journal journal = read(deathId, testator); Map shops = journal.shops(); List blocked = new ArrayList<>(); Entry planned = new Entry(to.ownerId().toString(), to.characterId().toString(), PLANNED); @@ -78,6 +77,11 @@ Result move(UUID deathId, UUID testator, CharacterRef to, Collection own for (Map.Entry shop : shops.entrySet()) { Entry entry = shop.getValue(); if (!PLANNED.equals(entry.state())) continue; + // A shop never moves with an unfinished trade, even one planned before. + if (blockedByTrade(shop.getKey())) { + if (!blocked.contains(shop.getKey())) blocked.add(shop.getKey()); + continue; + } Optional owner = db.ownerCharacter(shop.getKey()); String state = SUPERSEDED; if (owner.equals(Optional.of(testator.toString()))) { @@ -105,12 +109,15 @@ private boolean blockedByTrade(String shopId) { } } - private Journal read(UUID deathId) throws IOException { + private Journal read(UUID deathId, UUID testator) throws IOException { Path file = file(deathId); - if (!Files.exists(file)) return new Journal(deathId.toString(), null, new LinkedHashMap<>()); + if (!Files.exists(file)) return new Journal(deathId.toString(), testator.toString(), new LinkedHashMap<>()); try { Journal journal = JSON.fromJson(Files.readString(file), Journal.class); if (journal == null || journal.shops() == null) throw new IOException("Incomplete estate journal " + file); + // A journal copied from another death must not add its shops to this estate. + if (!deathId.toString().equals(journal.deathId()) || !testator.toString().equals(journal.testator())) + throw new IOException("Estate journal " + file + " belongs to another death"); return new Journal(journal.deathId(), journal.testator(), new LinkedHashMap<>(journal.shops())); } catch (JsonParseException ex) { throw new IOException("Unreadable estate journal " + file, ex); diff --git a/src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java b/src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java index c94b082..107fea3 100644 --- a/src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java +++ b/src/test/java/net/tfminecraft/bartershops/ShopEstateTest.java @@ -95,8 +95,14 @@ void shopListingSkipsUnreadableFilesAndLabelsSigns() throws Exception { String id = shop(TESTATOR, "sell", -3.2); Files.writeString(data.resolve(UUID.randomUUID() + ".json"), "{ not json"); Files.writeString(data.resolve(UUID.randomUUID() + ".json"), ""); + Logger logger = mock(Logger.class); + db.logger = logger; List shops = db.shops(); + db.shops(); + + // Each unreadable file is reported once, through the plugin logger. + verify(logger, times(2)).warning(startsWith("Skipping unreadable shop file ")); assertEquals(1, shops.size()); assertEquals(id, shops.get(0).shopId()); @@ -211,6 +217,63 @@ void shopWithAnUnfinishedTradeIsBlockedAndKeptWhileTheOthersMove() throws Except assertEquals(java.util.Set.of(stuck, free), java.util.Set.copyOf(done.movedIds())); } + @Test + void aPlannedShopThatGainedAnUnfinishedTradeIsBlockedNotMoved() throws Exception { + String id = shop(TESTATOR, "buy", 1); + Path journal = directory.resolve("estates").resolve(DEATH + ".json"); + Files.createDirectories(journal.getParent()); + Files.writeString(journal, new Gson().toJson(new ShopTransfers.Journal(DEATH.toString(), + TESTATOR.characterId().toString(), Map.of(id, new ShopTransfers.Entry(HEIR.ownerId().toString(), + HEIR.characterId().toString(), ShopTransfers.PLANNED))))); + Files.createDirectories(tradesDirectory); + Files.writeString(tradesDirectory.resolve(id + ".json"), "{}"); + + ShopTransfers.Result result = transfers.move(DEATH, TESTATOR.characterId(), HEIR, List.of()); + + assertEquals(List.of(id), result.blocked()); + assertEquals(List.of(), result.moved()); + assertEquals(Optional.of(TESTATOR.characterId().toString()), db.ownerCharacter(id)); + assertTrue(Files.readString(journal).contains(ShopTransfers.PLANNED)); + } + + @Test + void aJournalFromAnotherDeathIsRefused() throws Exception { + String mine = shop(TESTATOR, "buy", 1); + String theirs = shop(OTHER, "buy", 2); + Path journal = directory.resolve("estates").resolve(DEATH + ".json"); + Files.createDirectories(journal.getParent()); + ShopTransfers.Entry planned = new ShopTransfers.Entry(HEIR.ownerId().toString(), HEIR.characterId().toString(), + ShopTransfers.PLANNED); + Files.writeString(journal, new Gson().toJson(new ShopTransfers.Journal(UUID.randomUUID().toString(), + TESTATOR.characterId().toString(), Map.of(theirs, planned)))); + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.character(HEIR), 0)).kind()); + Files.writeString(journal, new Gson().toJson(new ShopTransfers.Journal(DEATH.toString(), + OTHER.characterId().toString(), Map.of(theirs, planned)))); + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.character(HEIR), 0)).kind()); + assertEquals(Optional.of(TESTATOR.characterId().toString()), db.ownerCharacter(mine)); + assertEquals(Optional.of(OTHER.characterId().toString()), db.ownerCharacter(theirs)); + } + + @Test + void oneFailingEstateDoesNotStopTheReconcileOfTheOthers() throws Exception { + String id = shop(TESTATOR, "buy", 1); + EstateDecision broken = new EstateDecision(UUID.randomUUID(), OTHER, Optional.empty(), Instant.EPOCH, + Optional.empty(), Optional.empty(), Optional.empty(), Recipient.FROZEN, Recipient.FROZEN, + Recipient.FROZEN, Recipient.SENDER, 0); + estate.guard = decision -> { + if (decision == broken) throw new IllegalStateException("service failed"); + return true; + }; + estate.decided = () -> List.of(broken, decision(Recipient.character(HEIR), 0)); + List reported = new ArrayList<>(); + estate.reporter = (death, participant, revision, outcome) -> reported.add(death); + + estate.reconcile(); + + assertEquals(List.of(DEATH), reported); + assertEquals(Optional.of(HEIR.characterId().toString()), db.ownerCharacter(id)); + } + @Test void waitingAndFrozenPropertyMovesNothingAndAnswersDone() throws Exception { String id = shop(TESTATOR, "buy", 1); From 7f37cabd10d827e3087aaef8130524fe9d7f1e0b Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:07:12 +0000 Subject: [PATCH 5/5] fix: reconcile shop estates as soon as shops are ready Under Main's load order BarterShops enables before RPCharacters and turns ready on its five-second poll, after the first-tick estate reconcile has already returned early. Shops of estates decided at start-up then waited for the hourly pass. The poll now runs the reconcile once when it finds shops ready; the first-tick and hourly runs stay. Co-Authored-By: Claude Opus 5.5 --- .../tfminecraft/bartershops/ShopEstate.java | 8 +++- .../net/tfminecraft/bartershops/ShopMain.java | 13 +++++- .../tfminecraft/bartershops/ShopMainTest.java | 43 +++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/tfminecraft/bartershops/ShopEstate.java b/src/main/java/net/tfminecraft/bartershops/ShopEstate.java index 7fdd02a..0044918 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopEstate.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopEstate.java @@ -50,13 +50,17 @@ interface Reporter { this.logger = logger; } - /** Registers the participant and runs the reconcile on the first tick, then hourly. */ - static void start(Plugin plugin, Database db, ShopTrades trades, BooleanSupplier ready) { + /** + * Registers the participant and runs the reconcile on the first tick, then hourly. Returns the + * reconcile, for the caller to run once shops become ready. + */ + static Runnable start(Plugin plugin, Database db, ShopTrades trades, BooleanSupplier ready) { ShopEstate estate = new ShopEstate(db, new ShopTransfers(db, trades, plugin.getDataFolder().toPath().resolve("estates")), ready, plugin.getLogger()); plugin.getServer().getServicesManager().register(EstateParticipant.class, estate, plugin, ServicePriority.Normal); plugin.getServer().getScheduler().runTaskTimer(plugin, estate::reconcile, 1L, HOUR_TICKS); + return estate::reconcile; } @Override diff --git a/src/main/java/net/tfminecraft/bartershops/ShopMain.java b/src/main/java/net/tfminecraft/bartershops/ShopMain.java index 76a12df..2d0b93a 100644 --- a/src/main/java/net/tfminecraft/bartershops/ShopMain.java +++ b/src/main/java/net/tfminecraft/bartershops/ShopMain.java @@ -8,6 +8,8 @@ public class ShopMain extends JavaPlugin { public static ShopMain plugin; ShopEvents events = new ShopEvents(); private boolean ready; + // Runs once when shops become ready, so estates decided during startup need not wait an hour. + private Runnable onReady = () -> {}; @Override public void onEnable() { @@ -20,7 +22,14 @@ public void onEnable() { new File(getDataFolder(), "migrations"), getLogger()); ready = migration.runWhenReady(); getServer().getScheduler().runTaskTimer(this, () -> { - if (!ready) ready = migration.runWhenReady(); + if (ready) return; + ready = migration.runWhenReady(); + if (!ready) return; + try { + onReady.run(); + } catch (RuntimeException ex) { + getLogger().warning("Could not reconcile shop estates once shops were ready: " + ex); + } }, 100L, 100L); getServer().getPluginManager().registerEvents(events, this); PluginCommand command = getCommand("bartershops"); @@ -31,7 +40,7 @@ public void onEnable() { command.setTabCompleter(intents); } if (EstateProbe.available(EstateProbe.CLASSES)) { - ShopEstate.start(this, events.db, events.trades, () -> ready); + onReady = ShopEstate.start(this, events.db, events.trades, () -> ready); } else { getLogger().warning("TLibs has no estate API (2.6.0 or later); shops of dead characters stay with them."); } diff --git a/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java b/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java index 260209c..d136357 100644 --- a/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java +++ b/src/test/java/net/tfminecraft/bartershops/ShopMainTest.java @@ -55,4 +55,47 @@ void enablesProtectionButWaitsForHeirsBeforeOpeningShops() { ShopMain.plugin = previous; } } + + @Test + void estatesReconcileAsSoonAsShopsBecomeReadyNotAnHourLater() { + ShopMain plugin = mock(ShopMain.class); + Server server = mock(Server.class); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + Logger logger = mock(Logger.class); + when(plugin.getServer()).thenReturn(server); + when(plugin.getDataFolder()).thenReturn(directory.toFile()); + when(plugin.getLogger()).thenReturn(logger); + when(server.getPluginManager()).thenReturn(mock(PluginManager.class)); + when(server.getScheduler()).thenReturn(scheduler); + plugin.events = new ShopEvents(); + doCallRealMethod().when(plugin).onEnable(); + ShopMain previous = ShopMain.plugin; + java.util.concurrent.atomic.AtomicInteger reconciles = new java.util.concurrent.atomic.AtomicInteger(); + try (var characters = mockStatic(Characters.class); var estates = mockStatic(ShopEstate.class)) { + estates.when(() -> ShopEstate.start(any(), any(), any(), any())).thenReturn((Runnable) () -> { + if (reconciles.incrementAndGet() == 1) throw new IllegalStateException("estate service failed"); + }); + plugin.onEnable(); + var poll = org.mockito.ArgumentCaptor.forClass(Runnable.class); + verify(scheduler).runTaskTimer(eq(plugin), poll.capture(), eq(100L), eq(100L)); + + // Still waiting for heirs: no reconcile yet. + poll.getValue().run(); + assertEquals(0, reconciles.get()); + + // The poll that finds shops ready reconciles at once; a failure is logged, shops stay ready. + characters.when(Characters::available).thenReturn(true); + characters.when(Characters::legacyHeirsReady).thenReturn(true); + poll.getValue().run(); + assertEquals(1, reconciles.get()); + assertTrue(plugin.events.ready.getAsBoolean()); + verify(logger).warning(startsWith("Could not reconcile shop estates once shops were ready")); + + // Later polls leave it to the hourly reconcile. + poll.getValue().run(); + assertEquals(1, reconciles.get()); + } finally { + ShopMain.plugin = previous; + } + } }