From a3993fb1789c1e30bd68aeb5304e040b798f007b Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:48:43 +0000 Subject: [PATCH 1/5] feat: record the writing character and keep letters for estates New letters record the sending character (senderCharacterId) in flight and in the pending queue, and queued letters keep the mail ID. The store gains what estates need: stored IDs for letters queued earlier, moves to an heir or back to the writer, and an account-keyed returned queue in returned_mail.yml for letters that never recorded their writer. A letter coming back because its recipient died says so; one returned to an account arrives at that account's next character session, marked as returned. The account queue is saved before the dead character's queue drops a letter, and load discards a copy left behind by a crash in between. Co-Authored-By: Claude Opus 5.5 --- .../tfminecraft/birdmessenger/BirdConfig.java | 9 + .../birdmessenger/mail/MailDelivery.java | 15 + .../birdmessenger/mail/MailService.java | 8 +- .../birdmessenger/mail/MailStore.java | 311 ++++++++++++++++-- .../birdmessenger/mail/StoredMail.java | 19 ++ src/main/resources/config.yml | 2 + .../birdmessenger/BirdConfigCoverageTest.java | 2 + .../mail/DeliverySessionCoverageTest.java | 24 ++ .../mail/EstateMailStoreTest.java | 277 ++++++++++++++++ .../mail/MailServiceCoverageTest.java | 30 ++ 10 files changed, 666 insertions(+), 31 deletions(-) create mode 100644 src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java diff --git a/src/main/java/net/tfminecraft/birdmessenger/BirdConfig.java b/src/main/java/net/tfminecraft/birdmessenger/BirdConfig.java index 921cc26..7259226 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/BirdConfig.java +++ b/src/main/java/net/tfminecraft/birdmessenger/BirdConfig.java @@ -116,6 +116,15 @@ public String msgUnsentReturned() { return message("unsent-returned", "&bA bird lands at your feet and returns your unsent letter."); } + public String msgReturnedRecipientDied() { + return message("returned-recipient-died", + "&eA bird brings back a letter you sent. Returned: the recipient has died."); + } + + public String msgReturnedToSender() { + return message("returned-to-sender", "&eA bird brings back a letter you sent. Returned."); + } + public String msgLetterCancelled() { return message("letter-cancelled", "&cLetter cancelled."); } diff --git a/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java b/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java index 829b01b..4edd837 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java +++ b/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java @@ -67,6 +67,21 @@ public void returnUnsent(Player owner, ItemStack letter) { }); } + /** + * Hands back a sent letter that its recipient never received. {@code recipientDied} names the reason; + * letters returned to an account say only that they were returned. + */ + public void returnToSender(Player owner, ItemStack letter, boolean recipientDied) { + if (owner == null || letter == null) { + return; + } + ItemGive.giveOrDrop(owner, letter.clone()); + notifyQuietly(() -> { + owner.sendMessage(recipientDied ? config.msgReturnedRecipientDied() : config.msgReturnedToSender()); + owner.playSound(owner.getLocation(), Sound.ENTITY_PARROT_FLY, 1f, 1f); + }); + } + private static void notifyQuietly(Runnable notification) { try { notification.run(); diff --git a/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java b/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java index 5a5645b..031f90d 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java +++ b/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java @@ -76,7 +76,8 @@ public boolean trySend(Player sender, CharacterSession origin, ItemStack letter, target.getCharacterId(), target.getDisplayTab(), letter.clone(), - deliveryTime); + deliveryTime, + origin.ref().characterId().toString()); if (!SendSession.isCurrent(sender, origin)) { refundLetter(sender, origin, letter); return false; @@ -136,10 +137,15 @@ public void tryDeliverPending(Player owner, CharacterSession session) { store.drainPending(session.ref(), () -> SendSession.isCurrent(owner, session), pending -> { if (pending.returned) { delivery.returnUnsent(owner, pending.item); + } else if (pending.recipientDied) { + delivery.returnToSender(owner, pending.item, true); } else { delivery.deliver(owner, pending.item, pending.senderUuid, pending.addresseeDisplayTab); } }); + // Letters returned to the account, whichever of its characters is played. + store.drainReturned(session.ref().ownerId(), () -> SendSession.isCurrent(owner, session), + pending -> delivery.returnToSender(owner, pending.item, false)); } private void schedule(StoredMail mail) { diff --git a/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java b/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java index f5b020c..72c7f6b 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java +++ b/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java @@ -30,8 +30,11 @@ public final class MailStore { private final BirdMessenger plugin; private final File inFlightFile; private final File pendingFile; + private final File returnedFile; private final Map inFlight = new ConcurrentHashMap<>(); private final Map> pending = new ConcurrentHashMap<>(); + // Letters sent back to an account because their writer's character was not recorded. + private final Map> returned = new ConcurrentHashMap<>(); private final Set recoveryRequired = new HashSet<>(); public MailStore(BirdMessenger plugin) { @@ -42,6 +45,7 @@ public MailStore(BirdMessenger plugin) { } this.inFlightFile = new File(data, "in_flight_mail.yml"); this.pendingFile = new File(data, "pending_mail.yml"); + this.returnedFile = new File(data, "returned_mail.yml"); } public Map inFlight() { @@ -127,11 +131,8 @@ public void addPending(StoredMail mail) { if (mail == null || mail.getCharacterId() == null) { return; } - addPending(mail.getCharacterId(), new PendingLetter( - mail.getOwnerUuid(), - mail.getSenderUuid(), - mail.getAddresseeDisplayTab(), - mail.getItem())); + addPending(mail.getCharacterId(), new PendingLetter(mail.getId(), mail.getOwnerUuid(), mail.getSenderUuid(), + mail.getSenderCharacterId(), mail.getAddresseeDisplayTab(), mail.getItem(), false, false)); } public void addPending(String characterId, PendingLetter letter) { @@ -140,32 +141,196 @@ public void addPending(String characterId, PendingLetter letter) { } requireRecovered(pendingFile); Objects.requireNonNull(letter.ownerUuid, "Pending letter owner is required"); - PendingLetter snapshot = new PendingLetter(letter.ownerUuid, letter.senderUuid, - letter.addresseeDisplayTab, letter.item.clone(), letter.returned); + enqueue(characterId, letter.copy(letter.id, letter.ownerUuid)); + savePending(); + } + + private void enqueue(String characterId, PendingLetter snapshot) { // Publish only complete entries: this private map always contains nonempty lists. pending.compute(characterId, (key, letters) -> { if (letters == null) { letters = new ArrayList<>(); } for (PendingLetter existing : letters) { - if (!letter.ownerUuid.equals(existing.ownerUuid)) { + if (!snapshot.ownerUuid.equals(existing.ownerUuid)) { throw new IllegalStateException("Pending character group belongs to another owner"); } } letters.add(snapshot); return letters; }); + } + + /** Where an estate sends one of the dead character's waiting letters. */ + public enum Destination { + /** To the will heir, as ordinary mail. */ + HEIR, + /** Back to the character that wrote it, marked as returned because the recipient died. */ + SENDER_CHARACTER, + /** Back to the writer's account, for letters that never recorded their character. */ + SENDER_ACCOUNT + } + + /** + * @param characterId the receiving character; unused for {@link Destination#SENDER_ACCOUNT} + * @param ownerUuid the receiving account + */ + public record Move(UUID letterId, Destination destination, String characterId, UUID ownerUuid) { + } + + /** A copy of this character's waiting letters, oldest first. */ + public List pendingFor(String characterId) { + List letters = new ArrayList<>(); + for (Map.Entry> entry : pending.entrySet()) { + if (entry.getKey().equalsIgnoreCase(characterId)) { + letters.addAll(entry.getValue()); + } + } + return letters; + } + + /** + * This character's waiting letters, each with a stored ID. Letters queued before IDs were kept are + * given one and saved first, so a move interrupted between files can be recognised. + */ + public List identifyPending(String characterId) { + boolean changed = false; + for (Map.Entry> entry : pending.entrySet()) { + if (!entry.getKey().equalsIgnoreCase(characterId)) { + continue; + } + List letters = entry.getValue(); + for (int i = 0; i < letters.size(); i++) { + if (letters.get(i).id == null) { + requireRecovered(pendingFile); + letters.set(i, letters.get(i).copy(UUID.randomUUID(), letters.get(i).ownerUuid)); + changed = true; + } + } + } + if (changed) { + savePending(); + } + return pendingFor(characterId); + } + + /** + * Moves the named letters out of the dead character's queue. Letters for an account queue are saved + * there before the character queue drops them; {@link #load()} discards a copy left behind by a crash + * in between. Returns the IDs moved now. + */ + public List moveEstateLetters(String characterId, List moves) { + requireRecovered(pendingFile); + requireRecovered(returnedFile); + Map byId = new LinkedHashMap<>(); + for (Move move : moves) { + byId.put(move.letterId(), move); + } + List taken = new ArrayList<>(); + for (PendingLetter letter : pendingFor(characterId)) { + if (letter.id != null && byId.containsKey(letter.id)) { + taken.add(letter); + } + } + if (taken.isEmpty()) { + return List.of(); + } + boolean toAccounts = false; + for (PendingLetter letter : taken) { + Move move = byId.get(letter.id); + if (move.destination() == Destination.SENDER_ACCOUNT) { + returned.computeIfAbsent(move.ownerUuid(), ignored -> new ArrayList<>()) + .add(letter.copy(letter.id, move.ownerUuid())); + toAccounts = true; + } + } + if (toAccounts) { + saveReturned(); + } + List moved = new ArrayList<>(); + for (PendingLetter letter : taken) { + Move move = byId.get(letter.id); + if (move.destination() == Destination.HEIR) { + enqueue(move.characterId(), new PendingLetter(letter.id, move.ownerUuid(), letter.senderUuid, + letter.senderCharacterId, "", letter.item, false, false)); + } else if (move.destination() == Destination.SENDER_CHARACTER) { + enqueue(move.characterId(), new PendingLetter(letter.id, move.ownerUuid(), letter.senderUuid, + letter.senderCharacterId, letter.addresseeDisplayTab, letter.item, false, true)); + } + moved.add(letter.id); + } + pending.entrySet().removeIf(entry -> { + if (!entry.getKey().equalsIgnoreCase(characterId)) { + return false; + } + entry.getValue().removeIf(letter -> letter.id != null && byId.containsKey(letter.id)); + return entry.getValue().isEmpty(); + }); savePending(); + return moved; + } + + /** + * Hands this account's returned letters to {@code deliver} while {@code current} holds, then saves + * once. A letter goes back to the head of the queue if the hand-over throws. + */ + public void drainReturned(UUID account, BooleanSupplier current, Consumer deliver) { + if (account == null) { + return; + } + boolean taken = false; + try { + while (current.getAsBoolean()) { + List letters = returned.get(account); + if (letters == null) { + return; + } + requireRecovered(returnedFile); + PendingLetter letter = letters.removeFirst(); + if (letters.isEmpty()) { + returned.remove(account, letters); + } + taken = true; + try { + deliver.accept(letter); + } catch (RuntimeException failure) { + returned.computeIfAbsent(account, ignored -> new ArrayList<>()).addFirst(letter); + throw failure; + } + } + } finally { + if (taken) { + saveReturned(); + } + } } public void load() { loadInFlight(); loadPending(); + loadReturned(); + dropMovedCopies(); } public void saveAll() { saveInFlight(); savePending(); + saveReturned(); + } + + // A crash after an estate saved a letter to an account queue, but before the dead character's queue + // dropped it, leaves the same letter in both. The account copy is the moved one. + private void dropMovedCopies() { + Set ids = new HashSet<>(); + returned.values().forEach(letters -> letters.forEach(letter -> ids.add(letter.id))); + ids.remove(null); + int before = pending.values().stream().mapToInt(List::size).sum(); + pending.values().forEach(letters -> letters.removeIf(letter -> ids.contains(letter.id))); + pending.values().removeIf(List::isEmpty); + if (pending.values().stream().mapToInt(List::size).sum() != before) { + plugin.getLogger().warning("Dropped pending letters already returned to their senders' accounts."); + savePending(); + } } private void saveInFlight() { @@ -176,6 +341,7 @@ private void saveInFlight() { config.set(path + ".sender", mail.getSenderUuid().toString()); config.set(path + ".owner", mail.getOwnerUuid().toString()); config.set(path + ".character-id", mail.getCharacterId()); + config.set(path + ".sender-character", mail.getSenderCharacterId()); config.set(path + ".addressee-display", mail.getAddresseeDisplayTab()); config.set(path + ".delivery-time", mail.getDeliveryTime()); config.set(path + ".item", mail.getItem().serialize()); @@ -234,7 +400,8 @@ private void loadInFlight() { characterId, addresseeDisplay, item, - deliveryTime)); + deliveryTime, + config.getString("mail." + key + ".sender-character"))); } catch (Exception ex) { damaged = true; plugin.getLogger().warning("Could not load in-flight mail " + key); @@ -250,22 +417,7 @@ private void savePending() { String path = "characters." + entry.getKey(); List letters = entry.getValue(); config.set(path + ".owner", letters.get(0).ownerUuid.toString()); - List> letterMaps = new ArrayList<>(); - for (PendingLetter letter : letters) { - Map map = new java.util.LinkedHashMap<>(); - if (letter.senderUuid != null) { - map.put("sender", letter.senderUuid.toString()); - } - if (letter.addresseeDisplayTab != null && !letter.addresseeDisplayTab.isBlank()) { - map.put("addressee-display", letter.addresseeDisplayTab); - } - if (letter.returned) { - map.put("returned", true); - } - map.put("item", letter.item.serialize()); - letterMaps.add(map); - } - config.set(path + ".letters", letterMaps); + config.set(path + ".letters", letterMaps(letters)); } try { config.save(pendingFile); @@ -274,6 +426,88 @@ private void savePending() { } } + private static List> letterMaps(List letters) { + List> letterMaps = new ArrayList<>(); + for (PendingLetter letter : letters) { + Map map = new java.util.LinkedHashMap<>(); + if (letter.id != null) { + map.put("id", letter.id.toString()); + } + if (letter.senderUuid != null) { + map.put("sender", letter.senderUuid.toString()); + } + if (letter.senderCharacterId != null) { + map.put("sender-character", letter.senderCharacterId); + } + if (letter.addresseeDisplayTab != null && !letter.addresseeDisplayTab.isBlank()) { + map.put("addressee-display", letter.addresseeDisplayTab); + } + if (letter.returned) { + map.put("returned", true); + } + if (letter.recipientDied) { + map.put("recipient-died", true); + } + map.put("item", letter.item.serialize()); + letterMaps.add(map); + } + return letterMaps; + } + + private void saveReturned() { + requireRecovered(returnedFile); + FileConfiguration config = new YamlConfiguration(); + for (Map.Entry> entry : returned.entrySet()) { + config.set("accounts." + entry.getKey() + ".letters", letterMaps(entry.getValue())); + } + try { + config.save(returnedFile); + } catch (IOException ex) { + plugin.getLogger().log(Level.SEVERE, "Could not save returned mail", ex); + } + } + + @SuppressWarnings("unchecked") + private void loadReturned() { + returned.clear(); + if (!returnedFile.exists()) { + recoveryRequired.remove(returnedFile); + return; + } + FileConfiguration config = loadFile(returnedFile); + if (config == null) { + return; + } + recoveryRequired.remove(returnedFile); + ConfigurationSection section = config.getConfigurationSection("accounts"); + if (section == null) { + if (config.contains("accounts")) preserveDamagedFile(returnedFile); + return; + } + boolean damaged = false; + for (String key : section.getKeys(false)) { + try { + UUID account = UUID.fromString(key); + List letters = new ArrayList<>(); + for (Object raw : (List) config.get("accounts." + key + ".letters")) { + try { + letters.add(parsePendingLetter(account, (Map) raw)); + } catch (Exception ex) { + damaged = true; + plugin.getLogger().warning("Could not load a returned letter for " + key); + } + } + if (!letters.isEmpty()) { + returned.put(account, letters); + } + } catch (Exception ex) { + damaged = true; + plugin.getLogger().warning("Could not load returned mail for " + key); + } + } + if (damaged) preserveDamagedFile(returnedFile); + } + @SuppressWarnings("unchecked") private void loadPending() { if (!pendingFile.exists()) { @@ -385,7 +619,10 @@ private static PendingLetter parsePendingLetter(UUID ownerUuid, Map ItemGive.giveOrDrop(owner, copy), times(2)); + } + var order = inOrder(owner); + order.verify(owner).sendMessage("Recipient died"); + order.verify(owner).sendMessage("Returned"); + verify(owner, times(2)).playSound((Location) isNull(), eq(Sound.ENTITY_PARROT_FLY), eq(1f), eq(1f)); + } + @Test void returnedLetterArrivesAsTheWritersOwnUnsentLetter() { BirdConfig config = mock(BirdConfig.class); diff --git a/src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java b/src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java new file mode 100644 index 0000000..affd2a8 --- /dev/null +++ b/src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java @@ -0,0 +1,277 @@ +package net.tfminecraft.birdmessenger.mail; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.*; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.logging.Logger; + +import org.bukkit.Bukkit; +import org.bukkit.UnsafeValues; +import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.inventory.ItemStack; +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.mockito.MockedStatic; + +import net.tfminecraft.birdmessenger.BirdMessenger; +import net.tfminecraft.birdmessenger.mail.MailStore.Destination; +import net.tfminecraft.birdmessenger.mail.MailStore.Move; +import net.tfminecraft.birdmessenger.mail.MailStore.PendingLetter; +import net.tfminecraft.tlibs.character.CharacterRef; + +/** Sending characters, letter IDs and the moves an estate makes in the mail store. */ +class EstateMailStoreTest { + private static final UUID DEAD_ACCOUNT = UUID.randomUUID(); + private static final String DEAD = UUID.randomUUID().toString(); + private static final CharacterRef HEIR = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final CharacterRef WRITER = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + @TempDir Path root; + private BirdMessenger plugin; + private Logger logger; + private MockedStatic bukkit; + + @BeforeEach + void setup() { + plugin = mock(BirdMessenger.class); + logger = mock(Logger.class); + when(plugin.getDataFolder()).thenReturn(root.toFile()); + when(plugin.getLogger()).thenReturn(logger); + UnsafeValues unsafe = mock(UnsafeValues.class); + bukkit = mockStatic(Bukkit.class); + bukkit.when(Bukkit::getUnsafe).thenReturn(unsafe); + when(unsafe.deserializeStack(anyMap())).thenAnswer(call -> item(call.getArgument(0))); + } + + @AfterEach + void close() { + bukkit.close(); + } + + @Test + void theSendingCharacterSurvivesFlightAndTheQueueWithTheMailId() { + UUID id = UUID.randomUUID(); + StoredMail mail = new StoredMail(id, WRITER.ownerId(), DEAD_ACCOUNT, DEAD, "Dead", letter("A"), 5L, + WRITER.characterId().toString()); + MailStore first = new MailStore(plugin); + first.putInFlight(mail); + MailStore restored = reload(); + assertEquals(WRITER.characterId().toString(), restored.inFlight().get(id).getSenderCharacterId()); + + restored.addPending(restored.inFlight().get(id)); + PendingLetter queued = reload().pendingFor(DEAD).getFirst(); + assertEquals(id, queued.id); + assertEquals(WRITER.characterId().toString(), queued.senderCharacterId); + assertEquals(WRITER.ownerId(), queued.senderUuid); + assertNull(new StoredMail(id, null, DEAD_ACCOUNT, DEAD, "", letter("B"), 1L).getSenderCharacterId()); + } + + @Test + void lettersQueuedBeforeIdsGetOneStoredBeforeAnyMove() { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + assertNull(store.pendingFor(DEAD).getFirst().id); + + UUID id = store.identifyPending(DEAD).getFirst().id; + + assertNotNull(id); + assertEquals(id, reload().pendingFor(DEAD).getFirst().id); + assertEquals(List.of(id), store.identifyPending(DEAD).stream().map(letter -> letter.id).toList()); + assertEquals(List.of(), store.identifyPending(UUID.randomUUID().toString())); + } + + @Test + void heirLettersBecomeOrdinaryMailForTheHeir() { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("To heir"))); + store.addPending(DEAD, PendingLetter.returned(DEAD_ACCOUNT, letter("Own unsent"))); + List letters = store.identifyPending(DEAD); + + List moved = store.moveEstateLetters(DEAD, letters.stream() + .map(letter -> new Move(letter.id, Destination.HEIR, HEIR.characterId().toString(), HEIR.ownerId())).toList()); + + assertEquals(letters.stream().map(letter -> letter.id).toList(), moved); + assertEquals(List.of(), store.pendingFor(DEAD)); + List heirs = reload().pendingFor(HEIR.characterId().toString()); + assertEquals(2, heirs.size()); + for (PendingLetter letter : heirs) { + assertEquals(HEIR.ownerId(), letter.ownerUuid); + assertEquals("", letter.addresseeDisplayTab); + assertFalse(letter.returned); + assertFalse(letter.recipientDied); + } + assertEquals(WRITER.ownerId(), heirs.getFirst().senderUuid); + } + + @Test + void letterGoesBackToTheWritingCharacterMarkedAsReturned() { + MailStore store = new MailStore(plugin); + store.addPending(new StoredMail(UUID.randomUUID(), WRITER.ownerId(), DEAD_ACCOUNT, DEAD, "Dead", + letter("Back"), 1L, WRITER.characterId().toString())); + PendingLetter letter = store.identifyPending(DEAD).getFirst(); + + store.moveEstateLetters(DEAD, List.of(new Move(letter.id, Destination.SENDER_CHARACTER, + WRITER.characterId().toString(), WRITER.ownerId()))); + + PendingLetter back = reload().pendingFor(WRITER.characterId().toString()).getFirst(); + assertEquals(letter.id, back.id); + assertEquals(WRITER.ownerId(), back.ownerUuid); + assertTrue(back.recipientDied); + assertEquals("Dead", back.addresseeDisplayTab); + assertEquals(List.of(), reload().pendingFor(DEAD)); + } + + @Test + void olderLettersGoToTheWritersAccountQueueAndArriveOnAnyCharacter() { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Held"))); + List letters = store.identifyPending(DEAD); + + store.moveEstateLetters(DEAD, List.of( + new Move(letters.getFirst().id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId()))); + + MailStore restored = reload(); + assertEquals(List.of(letters.get(1).id), restored.pendingFor(DEAD).stream().map(letter -> letter.id).toList()); + List handed = new ArrayList<>(); + restored.drainReturned(UUID.randomUUID(), () -> true, handed::add); + restored.drainReturned(null, () -> true, handed::add); + restored.drainReturned(WRITER.ownerId(), () -> false, handed::add); + assertEquals(List.of(), handed); + restored.drainReturned(WRITER.ownerId(), () -> true, handed::add); + assertEquals(1, handed.size()); + assertEquals(letters.getFirst().id, handed.getFirst().id); + assertEquals(WRITER.ownerId(), handed.getFirst().ownerUuid); + restored.drainReturned(WRITER.ownerId(), () -> true, handed::add); + assertEquals(1, reload().pendingFor(DEAD).size()); + reload().drainReturned(WRITER.ownerId(), () -> true, letter -> fail("delivered twice")); + } + + @Test + void aFailedHandOverKeepsTheReturnedLetterFirstInLine() { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + UUID id = store.identifyPending(DEAD).getFirst().id; + store.moveEstateLetters(DEAD, List.of(new Move(id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId()))); + + assertThrows(IllegalStateException.class, () -> store.drainReturned(WRITER.ownerId(), () -> true, letter -> { + throw new IllegalStateException("inventory closed"); + })); + + List handed = new ArrayList<>(); + reload().drainReturned(WRITER.ownerId(), () -> true, handed::add); + assertEquals(List.of(id), handed.stream().map(letter -> letter.id).toList()); + } + + @Test + void crashBetweenTheTwoFilesKeepsOnlyTheAccountCopy() throws Exception { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Other"))); + List letters = store.identifyPending(DEAD); + String pendingBefore = Files.readString(root.resolve("pending_mail.yml")); + store.moveEstateLetters(DEAD, List.of( + new Move(letters.getFirst().id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId()))); + // The account queue was saved, then the process died before the dead character's queue was. + Files.writeString(root.resolve("pending_mail.yml"), pendingBefore); + + MailStore restored = reload(); + + assertEquals(List.of(letters.get(1).id), restored.pendingFor(DEAD).stream().map(letter -> letter.id).toList()); + verify(logger).warning(contains("already returned")); + assertEquals(1, reload().pendingFor(DEAD).size()); + } + + @Test + void movesNamingNoQueuedLetterChangeNothing() throws Exception { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + String before = Files.readString(root.resolve("pending_mail.yml")); + + assertEquals(List.of(), store.moveEstateLetters(DEAD, List.of( + new Move(UUID.randomUUID(), Destination.HEIR, HEIR.characterId().toString(), HEIR.ownerId())))); + + assertEquals(before, Files.readString(root.resolve("pending_mail.yml"))); + assertFalse(Files.exists(root.resolve("returned_mail.yml"))); + } + + @Test + void damagedReturnedQueueIsPreservedAndGoodLettersLoad() throws Exception { + YamlConfiguration returned = new YamlConfiguration(); + returned.set("accounts." + WRITER.ownerId() + ".letters", + List.of(Map.of("id", UUID.randomUUID().toString(), "item", serialized("Good")), Map.of("item", "bad"))); + returned.set("accounts.not-a-uuid.letters", List.of(Map.of("item", serialized("Lost")))); + returned.set("accounts." + UUID.randomUUID() + ".letters", List.of(Map.of("item", "bad"))); + returned.save(root.resolve("returned_mail.yml").toFile()); + + MailStore store = reload(); + + List handed = new ArrayList<>(); + store.drainReturned(WRITER.ownerId(), () -> true, handed::add); + assertEquals(1, handed.size()); + try (var files = Files.list(root)) { + assertTrue(files.anyMatch(file -> file.getFileName().toString().startsWith("returned_mail.yml.corrupt-"))); + } + } + + @Test + void unreadableOrMisshapenReturnedQueueIsPreservedBeforeAnyWrite() throws Exception { + Files.writeString(root.resolve("returned_mail.yml"), "accounts: [not, a, section]\n"); + reload(); + Files.writeString(root.resolve("returned_mail.yml"), "accounts: {\n"); + MailStore store = reload(); + Files.writeString(root.resolve("returned_mail.yml"), "other: 1\n"); + reload(); + try (var files = Files.list(root)) { + assertEquals(2, files.filter(file -> file.getFileName().toString().startsWith("returned_mail.yml.corrupt-")).count()); + } + // Once preserved, the queue can be written again. + assertDoesNotThrow(store::saveAll); + } + + @Test + void failedSaveOfTheReturnedQueueIsLogged() throws Exception { + Files.createDirectories(root.resolve("returned_mail.yml")); + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + UUID id = store.identifyPending(DEAD).getFirst().id; + store.moveEstateLetters(DEAD, List.of(new Move(id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId()))); + verify(logger).log(eq(java.util.logging.Level.SEVERE), eq("Could not save returned mail"), any(Throwable.class)); + } + + private MailStore reload() { + MailStore store = new MailStore(plugin); + store.load(); + return store; + } + + private static ItemStack letter(String title) { + return item(serialized(title)); + } + + private static ItemStack item(Map serialized) { + ItemStack item = mock(ItemStack.class); + when(item.serialize()).thenReturn(serialized); + when(item.clone()).thenAnswer(invocation -> item(serialized)); + return item; + } + + private static Map serialized(String title) { + Map map = new LinkedHashMap<>(); + map.put("schema_version", 1); + map.put("DataVersion", 4556); + map.put("id", "minecraft:written_book"); + map.put("count", 1); + map.put("title", title); + return map; + } +} diff --git a/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java b/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java index 5e46e8d..2c0b190 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java @@ -57,6 +57,7 @@ void acceptedMailOwnsACloneAndScheduledCompletionIsIdempotent() { assertEquals(f.ownerId, mail.getOwnerUuid()); assertEquals(f.characterId.toString(), mail.getCharacterId()); assertEquals("Recipient", mail.getAddresseeDisplayTab()); + assertEquals(f.origin.ref().characterId().toString(), mail.getSenderCharacterId()); assertTrue(mail.getDeliveryTime() >= before + 10_000); assertTrue(mail.getDeliveryTime() <= System.currentTimeMillis() + 10_000); verify(f.sender).sendMessage("Departed"); @@ -209,6 +210,35 @@ void pendingDrainStopsAtACharacterSwitchAndOldTokenCannotResumeAfterSwitchingBac } } + @Test + void lettersReturnedByAnEstateComeBackToTheWritingCharacterOrItsAccount() { + try (Fixture f = new Fixture()) { + f.activeOwner(); + ItemStack died = mock(ItemStack.class); + ItemStack toAccount = mock(ItemStack.class); + f.queuePending(new MailStore.PendingLetter(UUID.randomUUID(), f.ownerId, f.ownerId, + f.characterId.toString(), "Dead friend", died, false, true)); + List drained = new ArrayList<>(); + doAnswer(call -> { + drained.add(call.getArgument(0)); + BooleanSupplier stillCurrent = call.getArgument(1); + Consumer deliver = call.getArgument(2); + if (stillCurrent.getAsBoolean()) { + deliver.accept(new MailStore.PendingLetter(f.ownerId, f.ownerId, "", toAccount)); + } + return null; + }).when(f.store).drainReturned(any(), any(), any()); + + f.service.tryDeliverPending(f.owner, f.ownerSession); + + var order = inOrder(f.delivery); + order.verify(f.delivery).returnToSender(f.owner, died, true); + order.verify(f.delivery).returnToSender(f.owner, toAccount, false); + verifyNoMoreInteractions(f.delivery); + assertEquals(List.of(f.ownerId), drained); + } + } + @Test void returnedLettersComeBackWithTheirOwnMessageInsteadOfAnAddressee() { try (Fixture f = new Fixture()) { From e1d26823a76301bf6b6741ca16a4faa81ec7eeb9 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:51:41 +0000 Subject: [PATCH 2/5] feat: send a dead character's waiting letters on with the estate The birdmessenger estate participant moves the testator's pending letters. With a will heir they are re-keyed (character and account) to the heir and delivered as normal mail. Without one, each goes back to the character that wrote it, marked "Returned: the recipient has died"; a letter from before writers were recorded goes to the writer's account queue; a letter whose writer is dead or unknown, or the testator's own unsent letter, stays and is counted as held for staff (it goes back if the writer lives again). Each moved letter carries a stored ID; a journal per death (estates/.yml) lists them before the queue changes, so DONE always carries every moved ID. In-flight letters are left alone and moved by the hourly reconcile once they land. The participant registers after mail loads, 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. Pins TLibs 2.6.0. Co-Authored-By: Claude Opus 5.5 --- pom.xml | 2 +- .../birdmessenger/BirdMessenger.java | 7 + .../birdmessenger/estate/EstateProbe.java | 27 ++ .../birdmessenger/estate/LetterEstate.java | 214 +++++++++++++ .../PluginLifecycleCoverageTest.java | 12 + .../estate/LetterEstateTest.java | 281 ++++++++++++++++++ 6 files changed, 542 insertions(+), 1 deletion(-) create mode 100644 src/main/java/net/tfminecraft/birdmessenger/estate/EstateProbe.java create mode 100644 src/main/java/net/tfminecraft/birdmessenger/estate/LetterEstate.java create mode 100644 src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java diff --git a/pom.xml b/pom.xml index 524466b..ccdb3da 100644 --- a/pom.xml +++ b/pom.xml @@ -14,7 +14,7 @@ UTF-8 full 2.16.0 - 2.3.0 + 2.6.0 diff --git a/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java b/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java index 0df8190..c4f4c31 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java +++ b/src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java @@ -7,6 +7,8 @@ import net.tfminecraft.rpcharacters.RPCharacters; import net.tfminecraft.birdmessenger.command.BirdMessengerCommand; import net.tfminecraft.birdmessenger.gui.CharacterPickerGui; +import net.tfminecraft.birdmessenger.estate.EstateProbe; +import net.tfminecraft.birdmessenger.estate.LetterEstate; import net.tfminecraft.birdmessenger.listener.CoopListener; import net.tfminecraft.birdmessenger.listener.CoopFurnitureListener; import net.tfminecraft.birdmessenger.listener.GuiListener; @@ -48,6 +50,11 @@ public void onEnable() { MailStore store = new MailStore(this); store.load(); mail = new MailService(this, store); + if (EstateProbe.available(EstateProbe.CLASSES)) { + LetterEstate.start(this, store); + } else { + getLogger().warning("TLibs has no estate API (2.6.0 or later); letters for dead characters keep waiting."); + } Bukkit.getPluginManager().registerEvents(new CoopListener(this), this); if (Bukkit.getPluginManager().getPlugin("ItemsAdder") != null) { Bukkit.getPluginManager().registerEvents(new CoopFurnitureListener(this), this); diff --git a/src/main/java/net/tfminecraft/birdmessenger/estate/EstateProbe.java b/src/main/java/net/tfminecraft/birdmessenger/estate/EstateProbe.java new file mode 100644 index 0000000..7145897 --- /dev/null +++ b/src/main/java/net/tfminecraft/birdmessenger/estate/EstateProbe.java @@ -0,0 +1,27 @@ +package net.tfminecraft.birdmessenger.estate; + +/** Whether TLibs has the estate API (2.6.0). On older TLibs letters wait for dead characters as before. */ +public final class EstateProbe { + + public 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() { + } + + public 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/birdmessenger/estate/LetterEstate.java b/src/main/java/net/tfminecraft/birdmessenger/estate/LetterEstate.java new file mode 100644 index 0000000..581cd5c --- /dev/null +++ b/src/main/java/net/tfminecraft/birdmessenger/estate/LetterEstate.java @@ -0,0 +1,214 @@ +package net.tfminecraft.birdmessenger.estate; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.logging.Logger; + +import org.bukkit.configuration.InvalidConfigurationException; +import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.plugin.Plugin; +import org.bukkit.plugin.ServicePriority; + +import net.tfminecraft.birdmessenger.mail.MailStore; +import net.tfminecraft.birdmessenger.mail.MailStore.Destination; +import net.tfminecraft.birdmessenger.mail.MailStore.Move; +import net.tfminecraft.birdmessenger.mail.MailStore.PendingLetter; +import net.tfminecraft.tlibs.character.CharacterInfo; +import net.tfminecraft.tlibs.character.CharacterRef; +import net.tfminecraft.tlibs.character.Characters; +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 birdmessenger} estate participant: letters waiting for a dead character go to the will + * heir, or back to whoever wrote them. Loaded only after {@link EstateProbe} finds the TLibs 2.6.0 + * estate API. Letters still in flight are left alone; they join the queue on landing and the hourly + * reconcile moves them. + */ +public final class LetterEstate implements EstateParticipant { + static final String ID = "birdmessenger"; + static final long HOUR_TICKS = 20L * 60 * 60; + + interface Reporter { + boolean report(UUID deathId, String participantId, long revision, Outcome outcome); + } + + private final MailStore store; + private final Path journals; + private final Logger logger; + Predicate guard = Estates::guardHolds; + Supplier> decided = Estates::decided; + Reporter reporter = Estates::report; + Function> characters = Characters::info; + // The last outcome reported per death, so the hourly pass reports only changes. + private final Map> reported = new HashMap<>(); + + LetterEstate(MailStore store, Path journals, Logger logger) { + this.store = store; + this.journals = journals; + this.logger = logger; + } + + /** Registers the participant once mail is loaded; reconciles on the first tick, then hourly. */ + public static void start(Plugin plugin, MailStore store) { + LetterEstate estate = new LetterEstate(store, plugin.getDataFolder().toPath().resolve("estates"), + 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) { + if (!guard.test(decision)) { + return Outcome.retry("The estate guard does not hold"); + } + String testator = decision.testator().characterId().toString(); + Optional heir = decision.letters().character(); + try { + List moves = new ArrayList<>(); + int held = 0; + for (PendingLetter letter : store.identifyPending(testator)) { + Move move = heir.isPresent() + ? new Move(letter.id, Destination.HEIR, heir.get().characterId().toString(), heir.get().ownerId()) + : toSender(letter); + if (move == null) { + held++; + } else { + moves.add(move); + } + } + Set planned = readJournal(decision.deathId()); + boolean planning = false; + for (Move move : moves) { + planning |= planned.add(move.letterId().toString()); + } + if (planning) { + writeJournal(decision.deathId(), planned); + } + store.moveEstateLetters(testator, moves); + Set waiting = new HashSet<>(); + store.pendingFor(testator).forEach(letter -> waiting.add(String.valueOf(letter.id))); + List moved = planned.stream().filter(id -> !waiting.contains(id)).toList(); + return Outcome.done(Map.of("letters", moved.size(), "held", held), moved); + } catch (IOException | RuntimeException ex) { + logger.warning("Could not move the letters of estate " + decision.deathId() + ": " + ex); + return Outcome.retry("Could not move a letter: " + ex.getMessage()); + } + } + + /** + * Where a letter goes when it returns to its writer, or null to hold it for staff: the writer's + * character is dead or unknown, or the letter is the dead character's own unsent one. + */ + private Move toSender(PendingLetter letter) { + if (letter.returned) { + return null; + } + if (letter.senderCharacterId == null) { + return letter.senderUuid == null ? null + : new Move(letter.id, Destination.SENDER_ACCOUNT, null, letter.senderUuid); + } + Optional writer; + try { + writer = characters.apply(UUID.fromString(letter.senderCharacterId)); + } catch (IllegalArgumentException ex) { + return null; + } + return writer.filter(CharacterInfo::alive) + .map(info -> new Move(letter.id, Destination.SENDER_CHARACTER, letter.senderCharacterId, + info.ref().ownerId())) + .orElse(null); + } + + @Override + public Holdings describe(UUID characterId) { + List labels = store.pendingFor(characterId.toString()).stream().map(LetterEstate::label).toList(); + return labels.isEmpty() ? Holdings.NONE : new Holdings(Map.of("letters", labels.size()), labels); + } + + private static String label(PendingLetter letter) { + String name = letter.id == null ? "Letter" : "Letter " + letter.id; + if (letter.returned) { + return name + ", this character's own unsent letter"; + } + if (letter.senderCharacterId != null) { + return name + " from character " + letter.senderCharacterId; + } + return letter.senderUuid == null ? name : name + " from account " + letter.senderUuid; + } + + /** Settles every decided estate whose guard holds and reports outcomes that changed. */ + void reconcile() { + for (EstateDecision decision : decided.get()) { + if (!guard.test(decision)) { + continue; + } + Outcome outcome = settle(decision); + Map.Entry last = Map.entry(decision.revision(), outcome); + if (!last.equals(reported.get(decision.deathId())) + && reporter.report(decision.deathId(), ID, decision.revision(), outcome)) { + reported.put(decision.deathId(), last); + } + } + } + + // Every letter ID this estate has planned to move, written before the queue changes. + private Set readJournal(UUID deathId) throws IOException { + Path file = journals.resolve(deathId + ".yml"); + Set ids = new LinkedHashSet<>(); + if (Files.exists(file)) { + YamlConfiguration journal = new YamlConfiguration(); + try { + journal.loadFromString(Files.readString(file)); + } catch (InvalidConfigurationException ex) { + throw new IOException("Unreadable estate journal " + file, ex); + } + ids.addAll(journal.getStringList("letters")); + } + return ids; + } + + private void writeJournal(UUID deathId, Set ids) throws IOException { + YamlConfiguration journal = new YamlConfiguration(); + journal.set("death", deathId.toString()); + journal.set("letters", new ArrayList<>(ids)); + Files.createDirectories(journals); + Path temp = Files.createTempFile(journals, ".pending-", ".tmp"); + try { + try (FileChannel channel = FileChannel.open(temp, StandardOpenOption.WRITE)) { + channel.write(ByteBuffer.wrap(journal.saveToString().getBytes(StandardCharsets.UTF_8))); + channel.force(true); + } + Files.move(temp, journals.resolve(deathId + ".yml"), StandardCopyOption.ATOMIC_MOVE, + StandardCopyOption.REPLACE_EXISTING); + } finally { + Files.deleteIfExists(temp); + } + } +} diff --git a/src/test/java/net/tfminecraft/birdmessenger/PluginLifecycleCoverageTest.java b/src/test/java/net/tfminecraft/birdmessenger/PluginLifecycleCoverageTest.java index 65c5416..2aea2fb 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/PluginLifecycleCoverageTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/PluginLifecycleCoverageTest.java @@ -26,6 +26,8 @@ import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.java.JavaPlugin; import org.junit.jupiter.api.Test; +import net.tfminecraft.birdmessenger.estate.EstateProbe; +import net.tfminecraft.birdmessenger.estate.LetterEstate; import org.junit.jupiter.api.io.TempDir; class PluginLifecycleCoverageTest { @@ -78,6 +80,8 @@ void lifecycleInitializesListenersMailAndOptionalIntegrationsAndClosesResources( plugin.onDisable(); try (var bukkit = mockStatic(Bukkit.class); var characters = mockStatic(net.tfminecraft.tlibs.character.Characters.class); + var probe = mockStatic(EstateProbe.class); + var estates = mockStatic(LetterEstate.class); var features = mockConstruction( LetterFeature.class, @@ -92,7 +96,15 @@ void lifecycleInitializesListenersMailAndOptionalIntegrationsAndClosesResources( (service, context) -> when(service.store()).thenReturn(stores.constructed().getFirst()))) { bukkit.when(Bukkit::getPluginManager).thenReturn(manager); + // Scenario 0 runs on TLibs with the estate API, the others on an older TLibs. + probe.when(() -> EstateProbe.available(any(String[].class))).thenReturn(scenario == 0); plugin.onEnable(); + if (scenario == 0) { + estates.verify(() -> LetterEstate.start(plugin, stores.constructed().getFirst())); + } else { + estates.verifyNoInteractions(); + verify(logger).warning(contains("no estate API")); + } assertNotNull(plugin.config()); assertNotNull(plugin.sessions()); assertSame(services.constructed().getFirst(), plugin.mail()); diff --git a/src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java b/src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java new file mode 100644 index 0000000..97f5354 --- /dev/null +++ b/src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java @@ -0,0 +1,281 @@ +package net.tfminecraft.birdmessenger.estate; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.*; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Instant; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.logging.Logger; + +import org.bukkit.Bukkit; +import org.bukkit.Server; +import org.bukkit.UnsafeValues; +import org.bukkit.inventory.ItemStack; +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.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.MockedStatic; + +import net.tfminecraft.birdmessenger.BirdMessenger; +import net.tfminecraft.birdmessenger.mail.MailStore; +import net.tfminecraft.birdmessenger.mail.MailStore.PendingLetter; +import net.tfminecraft.birdmessenger.mail.StoredMail; +import net.tfminecraft.tlibs.character.CharacterInfo; +import net.tfminecraft.tlibs.character.CharacterRef; +import net.tfminecraft.tlibs.character.CharacterStatus; +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 LetterEstateTest { + private static final CharacterRef DEAD = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final CharacterRef HEIR = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final CharacterRef WRITER = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final CharacterRef DEAD_WRITER = new CharacterRef(UUID.randomUUID(), UUID.randomUUID()); + private static final UUID DEATH = UUID.randomUUID(); + @TempDir Path root; + private MockedStatic bukkit; + private MailStore store; + private LetterEstate estate; + private boolean guard = true; + private final Map statuses = new HashMap<>(); + + @BeforeEach + void setup() { + BirdMessenger plugin = mock(BirdMessenger.class); + when(plugin.getDataFolder()).thenReturn(root.toFile()); + when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + UnsafeValues unsafe = mock(UnsafeValues.class); + bukkit = mockStatic(Bukkit.class); + bukkit.when(Bukkit::getUnsafe).thenReturn(unsafe); + when(unsafe.deserializeStack(anyMap())).thenAnswer(call -> item(call.getArgument(0))); + store = new MailStore(plugin); + estate = new LetterEstate(store, root.resolve("estates"), Logger.getAnonymousLogger()); + estate.guard = decision -> guard; + statuses.put(WRITER.characterId(), CharacterStatus.ALIVE); + statuses.put(DEAD_WRITER.characterId(), CharacterStatus.DEAD); + estate.characters = id -> Optional.ofNullable(statuses.get(id)).map(status -> new CharacterInfo( + id.equals(WRITER.characterId()) ? WRITER : DEAD_WRITER, "Writer", status, null, Set.of(), 0, null)); + } + + @AfterEach + void close() { + bukkit.close(); + } + + @Test + void willHeirReceivesEveryWaitingLetterAsOrdinaryMail() { + UUID sent = send(WRITER); + store.addPending(dead(), PendingLetter.returned(DEAD.ownerId(), letter("Own unsent"))); + + Outcome.Done done = assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.character(HEIR), 0))); + + assertEquals(Map.of("letters", 2, "held", 0), done.counts()); + assertTrue(done.movedIds().contains(sent.toString())); + assertEquals(List.of(), store.pendingFor(dead())); + List heirs = store.pendingFor(HEIR.characterId().toString()); + assertEquals(2, heirs.size()); + assertTrue(heirs.stream().allMatch(letter -> HEIR.ownerId().equals(letter.ownerUuid) && !letter.returned)); + } + + @Test + void withoutAWillLettersGoBackToTheirWritersOrWaitForStaff() { + UUID toWriter = send(WRITER); + store.addPending(dead(), new PendingLetter(DEAD.ownerId(), WRITER.ownerId(), "Dead", letter("Old"))); + UUID toDeadWriter = send(DEAD_WRITER); + store.addPending(new StoredMail(UUID.randomUUID(), WRITER.ownerId(), DEAD.ownerId(), dead(), "Dead", + letter("Unknown writer"), 1L, UUID.randomUUID().toString())); + store.addPending(new StoredMail(UUID.randomUUID(), WRITER.ownerId(), DEAD.ownerId(), dead(), "Dead", + letter("Garbled writer"), 1L, "not-a-uuid")); + store.addPending(dead(), PendingLetter.returned(DEAD.ownerId(), letter("Own unsent"))); + store.addPending(dead(), new PendingLetter(DEAD.ownerId(), null, "Dead", letter("No sender"))); + + Outcome.Done done = assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.SENDER, 0))); + + assertEquals(Map.of("letters", 2, "held", 5), done.counts()); + assertTrue(done.movedIds().contains(toWriter.toString())); + PendingLetter back = store.pendingFor(WRITER.characterId().toString()).getFirst(); + assertEquals(toWriter, back.id); + assertTrue(back.recipientDied); + assertEquals(WRITER.ownerId(), back.ownerUuid); + List account = new ArrayList<>(); + store.drainReturned(WRITER.ownerId(), () -> true, account::add); + assertEquals(1, account.size()); + assertEquals(5, store.pendingFor(dead()).size()); + assertTrue(store.pendingFor(dead()).stream().anyMatch(letter -> toDeadWriter.equals(letter.id))); + } + + @Test + void settlingAgainMovesNothingTwiceAndReportsEveryLetterMovedSoFar() { + UUID first = send(WRITER); + Outcome before = estate.settle(decision(Recipient.SENDER, 0)); + assertEquals(before, estate.settle(decision(Recipient.SENDER, 1))); + assertEquals(1, store.pendingFor(WRITER.characterId().toString()).size()); + + // A letter that was in flight lands later and the next settle returns it too. + UUID landed = send(WRITER); + Outcome.Done done = assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.SENDER, 1))); + assertEquals(Set.of(first.toString(), landed.toString()), Set.copyOf(done.movedIds())); + assertEquals(2, store.pendingFor(WRITER.characterId().toString()).size()); + } + + @Test + void heldLetterGoesBackOnceItsWriterLivesAgain() { + UUID held = send(DEAD_WRITER); + assertEquals(Map.of("letters", 0, "held", 1), + assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.SENDER, 0))).counts()); + + statuses.put(DEAD_WRITER.characterId(), CharacterStatus.ALIVE); + Outcome.Done done = assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.SENDER, 0))); + + assertEquals(List.of(held.toString()), done.movedIds()); + assertEquals(DEAD_WRITER.ownerId(), store.pendingFor(DEAD_WRITER.characterId().toString()).getFirst().ownerUuid); + } + + @Test + void failingGuardMovesNothing() { + send(WRITER); + guard = false; + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.SENDER, 0)).kind()); + assertEquals(1, store.pendingFor(dead()).size()); + } + + @Test + void journalProblemsRetryWithoutMovingLetters() throws Exception { + send(WRITER); + Files.createDirectories(root.resolve("estates")); + Files.writeString(root.resolve("estates").resolve(DEATH + ".yml"), "letters: [\n"); + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.SENDER, 0)).kind()); + Files.delete(root.resolve("estates").resolve(DEATH + ".yml")); + Files.delete(root.resolve("estates")); + Files.writeString(root.resolve("estates"), "not a folder"); + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.SENDER, 0)).kind()); + assertEquals(1, store.pendingFor(dead()).size()); + } + + @Test + void describeListsWaitingLettersAndWhoWroteThem() { + UUID id = send(WRITER); + store.addPending(dead(), new PendingLetter(DEAD.ownerId(), WRITER.ownerId(), "Dead", letter("Old"))); + store.addPending(dead(), PendingLetter.returned(DEAD.ownerId(), letter("Own"))); + store.addPending(dead(), new PendingLetter(DEAD.ownerId(), null, "Dead", letter("Anonymous"))); + + Holdings holdings = estate.describe(DEAD.characterId()); + + assertEquals(Map.of("letters", 4), holdings.counts()); + assertEquals(List.of("Letter " + id + " from character " + WRITER.characterId(), + "Letter from account " + WRITER.ownerId(), "Letter, this character's own unsent letter", "Letter"), + holdings.labels()); + assertSame(Holdings.NONE, estate.describe(HEIR.characterId())); + } + + @Test + void reconcileSettlesDecidedEstatesAndReportsOnlyChanges() { + send(WRITER); + EstateDecision other = new EstateDecision(UUID.randomUUID(), HEIR, Optional.empty(), Instant.EPOCH, + Optional.empty(), Optional.empty(), Optional.empty(), Recipient.FROZEN, Recipient.FROZEN, Recipient.FROZEN, + Recipient.SENDER, 0); + estate.guard = decision -> decision != other; + estate.decided = () -> List.of(other, decision(Recipient.SENDER, 0)); + 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.getFirst()[0]); + assertEquals("birdmessenger", reports.getFirst()[1]); + assertEquals(0L, reports.getFirst()[2]); + + estate.decided = () -> List.of(decision(Recipient.SENDER, 1)); + estate.reconcile(); + assertEquals(2, reports.size()); + } + + @Test + void reconcileReportsAgainWhenNoServiceTookTheReport() { + estate.decided = () -> List.of(decision(Recipient.SENDER, 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 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(root.toFile()); + when(server.getServicesManager()).thenReturn(services); + when(server.getScheduler()).thenReturn(scheduler); + + LetterEstate.start(plugin, store); + + var participant = org.mockito.ArgumentCaptor.forClass(EstateParticipant.class); + verify(services).register(eq(EstateParticipant.class), participant.capture(), eq(plugin), eq(ServicePriority.Normal)); + assertEquals("birdmessenger", 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")); + } + + private UUID send(CharacterRef writer) { + UUID id = UUID.randomUUID(); + store.addPending(new StoredMail(id, writer.ownerId(), DEAD.ownerId(), dead(), "Dead", letter("Letter " + id), 1L, + writer.characterId().toString())); + return id; + } + + private static String dead() { + return DEAD.characterId().toString(); + } + + private EstateDecision decision(Recipient letters, long revision) { + return new EstateDecision(DEATH, DEAD, Optional.of(Instant.EPOCH), Instant.EPOCH, Optional.empty(), + Optional.empty(), Optional.empty(), Recipient.FROZEN, Recipient.FROZEN, Recipient.FROZEN, letters, revision); + } + + private static ItemStack letter(String title) { + Map map = new LinkedHashMap<>(); + map.put("schema_version", 1); + map.put("DataVersion", 4556); + map.put("id", "minecraft:written_book"); + map.put("title", title); + return item(map); + } + + private static ItemStack item(Map serialized) { + ItemStack item = mock(ItemStack.class); + when(item.serialize()).thenReturn(serialized); + when(item.clone()).thenAnswer(invocation -> item(serialized)); + return item; + } +} From 6a1448bf0c727dffd17bf05213e5a512192dc5af Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:52:31 +0000 Subject: [PATCH 3/5] docs: mention estates in the README Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a86d104..5a584b7 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ BirdMessenger turns written correspondence into an in-world activity. Players se - **Travel time** — delivery time follows the distance to the recipient's character location, with an estimated arrival shown to the sender. - **Pending deliveries** — letters wait until their recipient is online and using the correct character; in-flight mail resumes after a restart. - **Delivery feedback** — sender and recipient receive messages and bird-themed sound feedback as correspondence progresses. +- **Letters after a death** — letters waiting for a dead character go to their will heir, or back to whoever wrote them. - **Optional Discord notices** — TFMCWeb integration can notify linked recipients when a letter's flight completes. The mailbox interaction protects against accidental ordinary left-click breaking, while deliberate sneak-breaking remains available. Sending, recipient selection, and confirmation all happen in game, keeping correspondence close to the roleplay world. @@ -28,8 +29,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/BirdMessenger/README.md#build-and-dependencies) prepared, run `mvn clean verify`. -JUnit 5 and Mockito cover letters, recipient selection, delivery, persistence and -plugin boundaries without starting a live Minecraft server. +JUnit 5 and Mockito cover letters, recipient selection, delivery, persistence, estates +and plugin boundaries without starting a live Minecraft server. `mvn clean verify` requires 100% production line coverage with no class or package exclusions. JaCoCo writes `target/site/jacoco/index.html` and `target/site/jacoco/jacoco.xml`; Surefire results are in `target/surefire-reports/`. From 5cf2d2bc9b8ddd80f4e16b8453dd689ae7a8d10d Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:30:35 +0000 Subject: [PATCH 4/5] fix: estate letter moves fail loudly and leave nothing half-moved Mail queues are now written whole: a flushed temporary file moved over the queue, then the folder flushed. Estate moves no longer use the saves that only log a failure: identifyPending and moveEstateLetters throw when a queue cannot be saved, and put the queues back in memory first, so the participant answers RETRY instead of DONE for letters that never reached disk. Every receiving queue is checked before anything changes, and each moved letter gets its own copy of the item. Co-Authored-By: Claude Opus 5.5 --- .../birdmessenger/mail/MailStore.java | 135 +++++++++++------- .../birdmessenger/util/DurableFiles.java | 38 +++++ .../mail/EstateMailStoreTest.java | 114 +++++++++++++-- 3 files changed, 231 insertions(+), 56 deletions(-) create mode 100644 src/main/java/net/tfminecraft/birdmessenger/util/DurableFiles.java diff --git a/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java b/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java index 72c7f6b..b624401 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java +++ b/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java @@ -23,6 +23,7 @@ import org.bukkit.inventory.ItemStack; import net.tfminecraft.birdmessenger.BirdMessenger; +import net.tfminecraft.birdmessenger.util.DurableFiles; import net.tfminecraft.tlibs.character.CharacterRef; public final class MailStore { @@ -191,9 +192,11 @@ public List pendingFor(String characterId) { /** * This character's waiting letters, each with a stored ID. Letters queued before IDs were kept are - * given one and saved first, so a move interrupted between files can be recognised. + * given one and saved first, so a move interrupted between files can be recognised. Throws, with + * nothing changed, when that save fails. */ - public List identifyPending(String characterId) { + public List identifyPending(String characterId) throws IOException { + Map> before = snapshot(pending); boolean changed = false; for (Map.Entry> entry : pending.entrySet()) { if (!entry.getKey().equalsIgnoreCase(characterId)) { @@ -209,22 +212,34 @@ public List identifyPending(String characterId) { } } if (changed) { - savePending(); + try { + writePending(); + } catch (IOException | RuntimeException failure) { + restore(pending, before); + throw failure; + } } return pendingFor(characterId); } /** - * Moves the named letters out of the dead character's queue. Letters for an account queue are saved - * there before the character queue drops them; {@link #load()} discards a copy left behind by a crash - * in between. Returns the IDs moved now. + * Moves the named letters out of the dead character's queue, all or nothing. Every receiving queue is + * checked first. Letters for an account queue are saved there before the character queue drops them; + * {@link #load()} discards a copy left behind by a crash in between. A failed save puts both queues + * back as they were in memory and throws. Returns the IDs moved now. */ - public List moveEstateLetters(String characterId, List moves) { + public List moveEstateLetters(String characterId, List moves) throws IOException { requireRecovered(pendingFile); requireRecovered(returnedFile); Map byId = new LinkedHashMap<>(); for (Move move : moves) { byId.put(move.letterId(), move); + if (move.destination() != Destination.SENDER_ACCOUNT) { + List queue = pending.get(move.characterId()); + if (queue != null && !queue.getFirst().ownerUuid.equals(move.ownerUuid())) { + throw new IllegalStateException("Pending queue of " + move.characterId() + " belongs to another owner"); + } + } } List taken = new ArrayList<>(); for (PendingLetter letter : pendingFor(characterId)) { @@ -235,39 +250,55 @@ public List moveEstateLetters(String characterId, List moves) { if (taken.isEmpty()) { return List.of(); } - boolean toAccounts = false; - for (PendingLetter letter : taken) { - Move move = byId.get(letter.id); - if (move.destination() == Destination.SENDER_ACCOUNT) { - returned.computeIfAbsent(move.ownerUuid(), ignored -> new ArrayList<>()) - .add(letter.copy(letter.id, move.ownerUuid())); - toAccounts = true; + Map> pendingBefore = snapshot(pending); + Map> returnedBefore = snapshot(returned); + try { + boolean toAccounts = false; + List moved = new ArrayList<>(); + for (PendingLetter letter : taken) { + Move move = byId.get(letter.id); + switch (move.destination()) { + case SENDER_ACCOUNT -> { + returned.computeIfAbsent(move.ownerUuid(), ignored -> new ArrayList<>()) + .add(letter.copy(letter.id, move.ownerUuid())); + toAccounts = true; + } + case HEIR -> enqueue(move.characterId(), new PendingLetter(letter.id, move.ownerUuid(), + letter.senderUuid, letter.senderCharacterId, "", letter.item.clone(), false, false)); + case SENDER_CHARACTER -> enqueue(move.characterId(), new PendingLetter(letter.id, move.ownerUuid(), + letter.senderUuid, letter.senderCharacterId, letter.addresseeDisplayTab, letter.item.clone(), + false, true)); + } + moved.add(letter.id); } - } - if (toAccounts) { - saveReturned(); - } - List moved = new ArrayList<>(); - for (PendingLetter letter : taken) { - Move move = byId.get(letter.id); - if (move.destination() == Destination.HEIR) { - enqueue(move.characterId(), new PendingLetter(letter.id, move.ownerUuid(), letter.senderUuid, - letter.senderCharacterId, "", letter.item, false, false)); - } else if (move.destination() == Destination.SENDER_CHARACTER) { - enqueue(move.characterId(), new PendingLetter(letter.id, move.ownerUuid(), letter.senderUuid, - letter.senderCharacterId, letter.addresseeDisplayTab, letter.item, false, true)); + pending.entrySet().removeIf(entry -> { + if (!entry.getKey().equalsIgnoreCase(characterId)) { + return false; + } + entry.getValue().removeIf(letter -> letter.id != null && byId.containsKey(letter.id)); + return entry.getValue().isEmpty(); + }); + if (toAccounts) { + writeReturned(); } - moved.add(letter.id); + writePending(); + return moved; + } catch (IOException | RuntimeException failure) { + restore(pending, pendingBefore); + restore(returned, returnedBefore); + throw failure; } - pending.entrySet().removeIf(entry -> { - if (!entry.getKey().equalsIgnoreCase(characterId)) { - return false; - } - entry.getValue().removeIf(letter -> letter.id != null && byId.containsKey(letter.id)); - return entry.getValue().isEmpty(); - }); - savePending(); - return moved; + } + + private static Map> snapshot(Map> queues) { + Map> copy = new LinkedHashMap<>(); + queues.forEach((key, letters) -> copy.put(key, new ArrayList<>(letters))); + return copy; + } + + private static void restore(Map> queues, Map> snapshot) { + queues.clear(); + queues.putAll(snapshot); } /** @@ -347,7 +378,7 @@ private void saveInFlight() { config.set(path + ".item", mail.getItem().serialize()); } try { - config.save(inFlightFile); + DurableFiles.write(inFlightFile.toPath(), config.saveToString()); } catch (IOException ex) { plugin.getLogger().log(Level.SEVERE, "Could not save in-flight mail", ex); } @@ -411,6 +442,14 @@ private void loadInFlight() { } private void savePending() { + try { + writePending(); + } catch (IOException ex) { + plugin.getLogger().log(Level.SEVERE, "Could not save pending mail", ex); + } + } + + private void writePending() throws IOException { requireRecovered(pendingFile); FileConfiguration config = new YamlConfiguration(); for (Map.Entry> entry : pending.entrySet()) { @@ -419,11 +458,7 @@ private void savePending() { config.set(path + ".owner", letters.get(0).ownerUuid.toString()); config.set(path + ".letters", letterMaps(letters)); } - try { - config.save(pendingFile); - } catch (IOException ex) { - plugin.getLogger().log(Level.SEVERE, "Could not save pending mail", ex); - } + DurableFiles.write(pendingFile.toPath(), config.saveToString()); } private static List> letterMaps(List letters) { @@ -455,16 +490,20 @@ private static List> letterMaps(List letters) } private void saveReturned() { + try { + writeReturned(); + } catch (IOException ex) { + plugin.getLogger().log(Level.SEVERE, "Could not save returned mail", ex); + } + } + + private void writeReturned() throws IOException { requireRecovered(returnedFile); FileConfiguration config = new YamlConfiguration(); for (Map.Entry> entry : returned.entrySet()) { config.set("accounts." + entry.getKey() + ".letters", letterMaps(entry.getValue())); } - try { - config.save(returnedFile); - } catch (IOException ex) { - plugin.getLogger().log(Level.SEVERE, "Could not save returned mail", ex); - } + DurableFiles.write(returnedFile.toPath(), config.saveToString()); } @SuppressWarnings("unchecked") diff --git a/src/main/java/net/tfminecraft/birdmessenger/util/DurableFiles.java b/src/main/java/net/tfminecraft/birdmessenger/util/DurableFiles.java new file mode 100644 index 0000000..ad31cf9 --- /dev/null +++ b/src/main/java/net/tfminecraft/birdmessenger/util/DurableFiles.java @@ -0,0 +1,38 @@ +package net.tfminecraft.birdmessenger.util; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; + +/** Whole files only: a temporary file is flushed, then moved over the target, then the folder is flushed. */ +public final class DurableFiles { + + private DurableFiles() { + } + + public static void write(Path target, String text) throws IOException { + Path folder = target.toAbsolutePath().getParent(); + Files.createDirectories(folder); + Path temp = Files.createTempFile(folder, ".pending-", ".tmp"); + try { + try (FileChannel channel = FileChannel.open(temp, StandardOpenOption.WRITE)) { + ByteBuffer buffer = ByteBuffer.wrap(text.getBytes(StandardCharsets.UTF_8)); + while (buffer.hasRemaining()) { + channel.write(buffer); + } + channel.force(true); + } + Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + try (FileChannel channel = FileChannel.open(folder, StandardOpenOption.READ)) { + channel.force(true); + } + } finally { + Files.deleteIfExists(temp); + } + } +} diff --git a/src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java b/src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java index affd2a8..fb70b79 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/mail/EstateMailStoreTest.java @@ -58,7 +58,7 @@ void close() { } @Test - void theSendingCharacterSurvivesFlightAndTheQueueWithTheMailId() { + void theSendingCharacterSurvivesFlightAndTheQueueWithTheMailId() throws Exception { UUID id = UUID.randomUUID(); StoredMail mail = new StoredMail(id, WRITER.ownerId(), DEAD_ACCOUNT, DEAD, "Dead", letter("A"), 5L, WRITER.characterId().toString()); @@ -76,7 +76,7 @@ void theSendingCharacterSurvivesFlightAndTheQueueWithTheMailId() { } @Test - void lettersQueuedBeforeIdsGetOneStoredBeforeAnyMove() { + void lettersQueuedBeforeIdsGetOneStoredBeforeAnyMove() throws Exception { MailStore store = new MailStore(plugin); store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); assertNull(store.pendingFor(DEAD).getFirst().id); @@ -90,7 +90,7 @@ void lettersQueuedBeforeIdsGetOneStoredBeforeAnyMove() { } @Test - void heirLettersBecomeOrdinaryMailForTheHeir() { + void heirLettersBecomeOrdinaryMailForTheHeir() throws Exception { MailStore store = new MailStore(plugin); store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("To heir"))); store.addPending(DEAD, PendingLetter.returned(DEAD_ACCOUNT, letter("Own unsent"))); @@ -113,7 +113,7 @@ void heirLettersBecomeOrdinaryMailForTheHeir() { } @Test - void letterGoesBackToTheWritingCharacterMarkedAsReturned() { + void letterGoesBackToTheWritingCharacterMarkedAsReturned() throws Exception { MailStore store = new MailStore(plugin); store.addPending(new StoredMail(UUID.randomUUID(), WRITER.ownerId(), DEAD_ACCOUNT, DEAD, "Dead", letter("Back"), 1L, WRITER.characterId().toString())); @@ -131,7 +131,7 @@ void letterGoesBackToTheWritingCharacterMarkedAsReturned() { } @Test - void olderLettersGoToTheWritersAccountQueueAndArriveOnAnyCharacter() { + void olderLettersGoToTheWritersAccountQueueAndArriveOnAnyCharacter() throws Exception { MailStore store = new MailStore(plugin); store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Held"))); @@ -157,7 +157,7 @@ void olderLettersGoToTheWritersAccountQueueAndArriveOnAnyCharacter() { } @Test - void aFailedHandOverKeepsTheReturnedLetterFirstInLine() { + void aFailedHandOverKeepsTheReturnedLetterFirstInLine() throws Exception { MailStore store = new MailStore(plugin); store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); UUID id = store.identifyPending(DEAD).getFirst().id; @@ -239,15 +239,113 @@ void unreadableOrMisshapenReturnedQueueIsPreservedBeforeAnyWrite() throws Except } @Test - void failedSaveOfTheReturnedQueueIsLogged() throws Exception { - Files.createDirectories(root.resolve("returned_mail.yml")); + void aReturnedQueueThatCannotBeSavedMovesNothing() throws Exception { + Files.createDirectories(root.resolve("returned_mail.yml").resolve("blocked")); + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + UUID id = store.identifyPending(DEAD).getFirst().id; + + assertThrows(java.io.IOException.class, () -> store.moveEstateLetters(DEAD, + List.of(new Move(id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId())))); + + assertEquals(List.of(id), store.pendingFor(DEAD).stream().map(letter -> letter.id).toList()); + List handed = new ArrayList<>(); + store.drainReturned(WRITER.ownerId(), () -> true, handed::add); + assertEquals(List.of(), handed); + } + + @Test + void aPendingQueueThatCannotBeSavedPutsEveryQueueBack() throws Exception { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("To heir"))); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("To account"))); + List letters = store.identifyPending(DEAD); + Path pending = root.resolve("pending_mail.yml"); + Files.delete(pending); + Files.createDirectories(pending.resolve("blocked")); + + assertThrows(java.io.IOException.class, () -> store.moveEstateLetters(DEAD, List.of( + new Move(letters.get(0).id, Destination.HEIR, HEIR.characterId().toString(), HEIR.ownerId()), + new Move(letters.get(1).id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId())))); + + assertEquals(2, store.pendingFor(DEAD).size()); + assertEquals(List.of(), store.pendingFor(HEIR.characterId().toString())); + List handed = new ArrayList<>(); + store.drainReturned(WRITER.ownerId(), () -> true, handed::add); + assertEquals(List.of(), handed); + } + + @Test + void aDeliveryThatCannotBeSavedIsLogged() throws Exception { MailStore store = new MailStore(plugin); store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); UUID id = store.identifyPending(DEAD).getFirst().id; store.moveEstateLetters(DEAD, List.of(new Move(id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId()))); + Path returned = root.resolve("returned_mail.yml"); + Files.delete(returned); + Files.createDirectories(returned.resolve("blocked")); + + List handed = new ArrayList<>(); + store.drainReturned(WRITER.ownerId(), () -> true, handed::add); + + assertEquals(1, handed.size()); verify(logger).log(eq(java.util.logging.Level.SEVERE), eq("Could not save returned mail"), any(Throwable.class)); } + @Test + void idsThatCannotBeSavedAreNotKept() throws Exception { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("Old"))); + Path pending = root.resolve("pending_mail.yml"); + Files.delete(pending); + Files.createDirectories(pending.resolve("blocked")); + + assertThrows(java.io.IOException.class, () -> store.identifyPending(DEAD)); + + assertNull(store.pendingFor(DEAD).getFirst().id); + } + + @Test + void aReceivingQueueOwnedByAnotherAccountStopsTheWholeMove() throws Exception { + MailStore store = new MailStore(plugin); + store.addPending(HEIR.characterId().toString(), + new PendingLetter(UUID.randomUUID(), WRITER.ownerId(), "Heir", letter("Odd"))); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("A"))); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("B"))); + List letters = store.identifyPending(DEAD); + String before = Files.readString(root.resolve("pending_mail.yml")); + + assertThrows(IllegalStateException.class, () -> store.moveEstateLetters(DEAD, List.of( + new Move(letters.get(0).id, Destination.SENDER_ACCOUNT, null, WRITER.ownerId()), + new Move(letters.get(1).id, Destination.HEIR, HEIR.characterId().toString(), HEIR.ownerId())))); + + assertEquals(2, store.pendingFor(DEAD).size()); + assertEquals(1, store.pendingFor(HEIR.characterId().toString()).size()); + assertEquals(before, Files.readString(root.resolve("pending_mail.yml"))); + assertFalse(Files.exists(root.resolve("returned_mail.yml"))); + } + + @Test + void movedLettersDoNotShareTheirItemWithTheOriginal() throws Exception { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("A"))); + PendingLetter original = store.identifyPending(DEAD).getFirst(); + + store.moveEstateLetters(DEAD, List.of( + new Move(original.id, Destination.HEIR, HEIR.characterId().toString(), HEIR.ownerId()))); + + assertNotSame(original.item, store.pendingFor(HEIR.characterId().toString()).getFirst().item); + } + + @Test + void queuesAreReplacedWholeWithNoTemporaryFilesLeft() throws Exception { + MailStore store = new MailStore(plugin); + store.addPending(DEAD, new PendingLetter(DEAD_ACCOUNT, WRITER.ownerId(), "Dead", letter("A"))); + try (var files = Files.list(root)) { + assertEquals(List.of(root.resolve("pending_mail.yml")), files.toList()); + } + } + private MailStore reload() { MailStore store = new MailStore(plugin); store.load(); From 48784d5a73e81fbfdc60d942dd284859f3d2b101 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:30:35 +0000 Subject: [PATCH 5/5] fix: count only moved letters and name writers in estate labels DONE carries only the letters count, so the site memorial never shows a held count as something received; letters held for staff are listed by describe(). Labels name the writer's character, or say the letter predates recorded writers, instead of showing account or character UUIDs. The estate journal is written through the same durable path, folder flush included, and one failing estate no longer stops the hourly reconcile of the others. Co-Authored-By: Claude Opus 5.5 --- .../birdmessenger/estate/LetterEstate.java | 68 +++++++++---------- .../estate/LetterEstateTest.java | 63 +++++++++++++++-- 2 files changed, 88 insertions(+), 43 deletions(-) diff --git a/src/main/java/net/tfminecraft/birdmessenger/estate/LetterEstate.java b/src/main/java/net/tfminecraft/birdmessenger/estate/LetterEstate.java index 581cd5c..dea2455 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/estate/LetterEstate.java +++ b/src/main/java/net/tfminecraft/birdmessenger/estate/LetterEstate.java @@ -1,13 +1,8 @@ package net.tfminecraft.birdmessenger.estate; import java.io.IOException; -import java.nio.ByteBuffer; -import java.nio.channels.FileChannel; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; -import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -28,6 +23,7 @@ import org.bukkit.plugin.ServicePriority; import net.tfminecraft.birdmessenger.mail.MailStore; +import net.tfminecraft.birdmessenger.util.DurableFiles; import net.tfminecraft.birdmessenger.mail.MailStore.Destination; import net.tfminecraft.birdmessenger.mail.MailStore.Move; import net.tfminecraft.birdmessenger.mail.MailStore.PendingLetter; @@ -91,15 +87,13 @@ public Outcome settle(EstateDecision decision) { String testator = decision.testator().characterId().toString(); Optional heir = decision.letters().character(); try { + // Letters with no move stay with the testator for staff; describe() lists them. List moves = new ArrayList<>(); - int held = 0; for (PendingLetter letter : store.identifyPending(testator)) { Move move = heir.isPresent() ? new Move(letter.id, Destination.HEIR, heir.get().characterId().toString(), heir.get().ownerId()) : toSender(letter); - if (move == null) { - held++; - } else { + if (move != null) { moves.add(move); } } @@ -115,7 +109,7 @@ public Outcome settle(EstateDecision decision) { Set waiting = new HashSet<>(); store.pendingFor(testator).forEach(letter -> waiting.add(String.valueOf(letter.id))); List moved = planned.stream().filter(id -> !waiting.contains(id)).toList(); - return Outcome.done(Map.of("letters", moved.size(), "held", held), moved); + return Outcome.done(Map.of("letters", moved.size()), moved); } catch (IOException | RuntimeException ex) { logger.warning("Could not move the letters of estate " + decision.deathId() + ": " + ex); return Outcome.retry("Could not move a letter: " + ex.getMessage()); @@ -148,32 +142,45 @@ private Move toSender(PendingLetter letter) { @Override public Holdings describe(UUID characterId) { - List labels = store.pendingFor(characterId.toString()).stream().map(LetterEstate::label).toList(); + List labels = store.pendingFor(characterId.toString()).stream().map(this::label).toList(); return labels.isEmpty() ? Holdings.NONE : new Holdings(Map.of("letters", labels.size()), labels); } - private static String label(PendingLetter letter) { - String name = letter.id == null ? "Letter" : "Letter " + letter.id; + // Names the writer where it can, never an account or character UUID. + private String label(PendingLetter letter) { + String name = letter.id == null ? "Letter" : "Letter " + letter.id.toString().substring(0, 8); if (letter.returned) { return name + ", this character's own unsent letter"; } - if (letter.senderCharacterId != null) { - return name + " from character " + letter.senderCharacterId; + if (letter.senderCharacterId == null) { + return letter.senderUuid == null ? name + " with no sender" : name + " from before writers were recorded"; } - return letter.senderUuid == null ? name : name + " from account " + letter.senderUuid; + Optional writer; + try { + writer = characters.apply(UUID.fromString(letter.senderCharacterId)); + } catch (IllegalArgumentException ex) { + writer = Optional.empty(); + } + return writer.map(info -> name + " from " + info.name() + (info.alive() ? "" : " (dead)")) + .orElse(name + " from an unknown writer"); } /** Settles every decided estate whose guard holds and reports outcomes that changed. */ void reconcile() { for (EstateDecision decision : decided.get()) { - if (!guard.test(decision)) { - continue; - } - Outcome outcome = settle(decision); - Map.Entry last = Map.entry(decision.revision(), outcome); - if (!last.equals(reported.get(decision.deathId())) - && 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); + Map.Entry last = Map.entry(decision.revision(), outcome); + if (!last.equals(reported.get(decision.deathId())) + && reporter.report(decision.deathId(), ID, decision.revision(), outcome)) { + reported.put(decision.deathId(), last); + } + } catch (RuntimeException ex) { + logger.warning("Could not reconcile the letters of estate " + decision.deathId() + ": " + ex); } } } @@ -198,17 +205,6 @@ private void writeJournal(UUID deathId, Set ids) throws IOException { YamlConfiguration journal = new YamlConfiguration(); journal.set("death", deathId.toString()); journal.set("letters", new ArrayList<>(ids)); - Files.createDirectories(journals); - Path temp = Files.createTempFile(journals, ".pending-", ".tmp"); - try { - try (FileChannel channel = FileChannel.open(temp, StandardOpenOption.WRITE)) { - channel.write(ByteBuffer.wrap(journal.saveToString().getBytes(StandardCharsets.UTF_8))); - channel.force(true); - } - Files.move(temp, journals.resolve(deathId + ".yml"), StandardCopyOption.ATOMIC_MOVE, - StandardCopyOption.REPLACE_EXISTING); - } finally { - Files.deleteIfExists(temp); - } + DurableFiles.write(journals.resolve(deathId + ".yml"), journal.saveToString()); } } diff --git a/src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java b/src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java index 97f5354..db18b4b 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/estate/LetterEstateTest.java @@ -87,7 +87,7 @@ void willHeirReceivesEveryWaitingLetterAsOrdinaryMail() { Outcome.Done done = assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.character(HEIR), 0))); - assertEquals(Map.of("letters", 2, "held", 0), done.counts()); + assertEquals(Map.of("letters", 2), done.counts()); assertTrue(done.movedIds().contains(sent.toString())); assertEquals(List.of(), store.pendingFor(dead())); List heirs = store.pendingFor(HEIR.characterId().toString()); @@ -109,7 +109,7 @@ void withoutAWillLettersGoBackToTheirWritersOrWaitForStaff() { Outcome.Done done = assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.SENDER, 0))); - assertEquals(Map.of("letters", 2, "held", 5), done.counts()); + assertEquals(Map.of("letters", 2), done.counts()); assertTrue(done.movedIds().contains(toWriter.toString())); PendingLetter back = store.pendingFor(WRITER.characterId().toString()).getFirst(); assertEquals(toWriter, back.id); @@ -139,7 +139,7 @@ void settlingAgainMovesNothingTwiceAndReportsEveryLetterMovedSoFar() { @Test void heldLetterGoesBackOnceItsWriterLivesAgain() { UUID held = send(DEAD_WRITER); - assertEquals(Map.of("letters", 0, "held", 1), + assertEquals(Map.of("letters", 0), assertInstanceOf(Outcome.Done.class, estate.settle(decision(Recipient.SENDER, 0))).counts()); statuses.put(DEAD_WRITER.characterId(), CharacterStatus.ALIVE); @@ -170,6 +170,38 @@ void journalProblemsRetryWithoutMovingLetters() throws Exception { assertEquals(1, store.pendingFor(dead()).size()); } + @Test + void aQueueThatCannotBeSavedRetriesAndMovesNothing() throws Exception { + UUID id = send(WRITER); + Path pending = root.resolve("pending_mail.yml"); + Files.delete(pending); + Files.createDirectories(pending.resolve("blocked")); + + assertEquals(Outcome.Kind.RETRY, estate.settle(decision(Recipient.character(HEIR), 0)).kind()); + + assertEquals(List.of(id), store.pendingFor(dead()).stream().map(letter -> letter.id).toList()); + assertEquals(List.of(), store.pendingFor(HEIR.characterId().toString())); + } + + @Test + void oneFailingEstateDoesNotStopTheReconcileOfTheOthers() { + send(WRITER); + EstateDecision broken = new EstateDecision(UUID.randomUUID(), HEIR, 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.SENDER, 0)); + List reported = new ArrayList<>(); + estate.reporter = (death, participant, revision, outcome) -> reported.add(death); + + estate.reconcile(); + + assertEquals(List.of(DEATH), reported); + } + @Test void describeListsWaitingLettersAndWhoWroteThem() { UUID id = send(WRITER); @@ -178,11 +210,28 @@ void describeListsWaitingLettersAndWhoWroteThem() { store.addPending(dead(), new PendingLetter(DEAD.ownerId(), null, "Dead", letter("Anonymous"))); Holdings holdings = estate.describe(DEAD.characterId()); - assertEquals(Map.of("letters", 4), holdings.counts()); - assertEquals(List.of("Letter " + id + " from character " + WRITER.characterId(), - "Letter from account " + WRITER.ownerId(), "Letter, this character's own unsent letter", "Letter"), - holdings.labels()); + + send(DEAD_WRITER); + store.addPending(new StoredMail(UUID.randomUUID(), WRITER.ownerId(), DEAD.ownerId(), dead(), "Dead", + letter("Unknown"), 1L, UUID.randomUUID().toString())); + store.addPending(new StoredMail(UUID.randomUUID(), WRITER.ownerId(), DEAD.ownerId(), dead(), "Dead", + letter("Garbled"), 1L, "not-a-uuid")); + + holdings = estate.describe(DEAD.characterId()); + + assertEquals(Map.of("letters", 7), holdings.counts()); + List labels = holdings.labels(); + assertEquals("Letter " + id.toString().substring(0, 8) + " from Writer", labels.get(0)); + assertEquals(List.of("Letter from before writers were recorded", "Letter, this character's own unsent letter", + "Letter with no sender"), labels.subList(1, 4)); + assertTrue(labels.get(4).endsWith(" from Writer (dead)")); + assertTrue(labels.get(5).endsWith(" from an unknown writer")); + assertTrue(labels.get(6).endsWith(" from an unknown writer")); + // Labels never show account or character UUIDs. + for (String label : labels) { + assertFalse(label.contains(WRITER.ownerId().toString()) || label.contains(WRITER.characterId().toString())); + } assertSame(Holdings.NONE, estate.describe(HEIR.characterId())); }