diff --git a/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java b/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java index 4edd837..5764c3f 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java +++ b/src/main/java/net/tfminecraft/birdmessenger/mail/MailDelivery.java @@ -23,6 +23,24 @@ public MailDelivery(BirdConfig config) { this.config = config; } + /** Checks storage slots without adding items or counting armour and offhand slots. */ + public static boolean hasRoom(Player owner, ItemStack letter) { + var inventory = owner.getInventory(); + int remaining = letter.getAmount(); + int stackLimit = Math.min(letter.getMaxStackSize(), inventory.getMaxStackSize()); + for (ItemStack slot : inventory.getStorageContents()) { + if (slot == null || slot.getType().isAir()) { + remaining -= stackLimit; + } else if (letter.isSimilar(slot)) { + remaining -= Math.max(0, stackLimit - slot.getAmount()); + } + if (remaining <= 0) { + return true; + } + } + return false; + } + public static boolean isActiveCharacter(Player owner, String characterId) { if (owner == null || characterId == null) { return false; diff --git a/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java b/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java index 031f90d..c2da9ef 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java +++ b/src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java @@ -119,6 +119,7 @@ public void resume() { if (!store.inFlight().isEmpty()) { plugin.getLogger().info("Resumed " + store.inFlight().size() + " in-flight letter(s)."); } + Bukkit.getScheduler().runTaskTimer(plugin, this::flushPendingForOnlinePlayers, 20L, 20L); } public void flushPendingForOnlinePlayers() { @@ -134,7 +135,8 @@ public void tryDeliverPending(Player owner, CharacterSession session) { if (!SendSession.isCurrent(owner, session)) { return; } - store.drainPending(session.ref(), () -> SendSession.isCurrent(owner, session), pending -> { + store.drainPending(session.ref(), () -> SendSession.isCurrent(owner, session), + pending -> MailDelivery.hasRoom(owner, pending.item), pending -> { if (pending.returned) { delivery.returnUnsent(owner, pending.item); } else if (pending.recipientDied) { @@ -145,6 +147,7 @@ public void tryDeliverPending(Player owner, CharacterSession session) { }); // Letters returned to the account, whichever of its characters is played. store.drainReturned(session.ref().ownerId(), () -> SendSession.isCurrent(owner, session), + pending -> MailDelivery.hasRoom(owner, pending.item), pending -> delivery.returnToSender(owner, pending.item, false)); } @@ -163,7 +166,8 @@ private void complete(StoredMail mail) { return; } Player owner = Bukkit.getPlayer(mail.getOwnerUuid()); - if (owner != null && owner.isOnline() && MailDelivery.isActiveCharacter(owner, mail.getCharacterId())) { + if (owner != null && owner.isOnline() && MailDelivery.isActiveCharacter(owner, mail.getCharacterId()) + && MailDelivery.hasRoom(owner, mail.getItem())) { store.removeInFlight(mail.getId()); delivery.deliver(owner, mail); DiscordNotifyService.notifyFlightComplete(plugin, mail); diff --git a/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java b/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java index b624401..616730f 100644 --- a/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java +++ b/src/main/java/net/tfminecraft/birdmessenger/mail/MailStore.java @@ -14,6 +14,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.BooleanSupplier; import java.util.function.Consumer; +import java.util.function.Predicate; import java.util.logging.Level; import org.bukkit.configuration.ConfigurationSection; @@ -59,13 +60,19 @@ public Map inFlight() { * goes back to the head of its queue if the hand-over throws. */ public void drainPending(CharacterRef ref, BooleanSupplier current, Consumer deliver) { + drainPending(ref, current, letter -> true, deliver); + } + + /** Leaves the next letter queued when its complete stack cannot be accepted. */ + public void drainPending(CharacterRef ref, BooleanSupplier current, Predicate accepts, + Consumer deliver) { if (ref == null) { return; } boolean taken = false; try { while (current.getAsBoolean()) { - Taken next = removePending(ref); + Taken next = removePending(ref, accepts); if (next == null) { return; } @@ -87,7 +94,7 @@ public void drainPending(CharacterRef ref, BooleanSupplier current, Consumer accepts) { String characterId = ref.characterId().toString(); for (Map.Entry> entry : pending.entrySet()) { if (!entry.getKey().equalsIgnoreCase(characterId)) { @@ -98,6 +105,9 @@ private Taken removePending(CharacterRef ref) { if (!ref.ownerId().equals(letters.get(i).ownerUuid)) { continue; } + if (!accepts.test(letters.get(i))) { + return null; + } requireRecovered(pendingFile); PendingLetter letter = letters.remove(i); if (letters.isEmpty()) { @@ -306,6 +316,12 @@ private static void restore(Map> queues, Map deliver) { + drainReturned(account, current, letter -> true, deliver); + } + + /** Leaves the next returned letter queued until its complete stack can be accepted. */ + public void drainReturned(UUID account, BooleanSupplier current, Predicate accepts, + Consumer deliver) { if (account == null) { return; } @@ -316,6 +332,9 @@ public void drainReturned(UUID account, BooleanSupplier current, Consumer true, handed::add); restored.drainReturned(null, () -> true, handed::add); restored.drainReturned(WRITER.ownerId(), () -> false, handed::add); + restored.drainReturned(WRITER.ownerId(), () -> true, letter -> false, handed::add); assertEquals(List.of(), handed); + restored = reload(); restored.drainReturned(WRITER.ownerId(), () -> true, handed::add); assertEquals(1, handed.size()); assertEquals(letters.getFirst().id, handed.getFirst().id); diff --git a/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java b/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java index 2c0b190..e86f3c1 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/mail/MailServiceCoverageTest.java @@ -14,6 +14,7 @@ import java.util.UUID; import java.util.function.BooleanSupplier; import java.util.function.Consumer; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import net.tfminecraft.birdmessenger.BirdConfig; @@ -28,6 +29,7 @@ import org.bukkit.Bukkit; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; import org.bukkit.plugin.PluginManager; import org.bukkit.scheduler.BukkitScheduler; import org.junit.jupiter.api.Test; @@ -36,6 +38,136 @@ import org.mockito.ArgumentCaptor; class MailServiceCoverageTest { + @Test + void anOversizedLetterWaitsEvenWhenOneSlotIsEmpty() { + try (Fixture f = new Fixture()) { + f.activeOwner(); + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[1]); + StoredMail mail = f.mail(0); + when(mail.getItem().getAmount()).thenReturn(2); + when(mail.getItem().getMaxStackSize()).thenReturn(1); + f.inFlight.put(mail.getId(), mail); + f.service.resume(); + verify(f.store).addPending(mail); + verifyNoInteractions(f.delivery); + + f.queuePending(new MailStore.PendingLetter(f.ownerId, f.senderId, "Recipient", mail.getItem())); + f.service.tryDeliverPending(f.owner, f.ownerSession); + assertEquals(1, f.pending.get(f.ownerSession.ref()).size()); + verifyNoInteractions(f.delivery); + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[2]); + f.service.tryDeliverPending(f.owner, f.ownerSession); + verify(f.delivery).deliver(f.owner, mail.getItem(), f.senderId, "Recipient"); + assertTrue(f.pending.get(f.ownerSession.ref()).isEmpty()); + } + } + + @Test + void fullRecipientInventoryQueuesArrivingMailUntilTheAutomaticRetryFindsRoom() { + try (Fixture f = new Fixture()) { + f.activeOwner(); + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[0]); + f.bukkit.when(Bukkit::getOnlinePlayers).thenReturn(List.of(f.owner)); + StoredMail mail = f.mail(0); + f.inFlight.put(mail.getId(), mail); + doAnswer(call -> { + StoredMail queued = call.getArgument(0); + f.queuePending(new MailStore.PendingLetter(queued.getId(), queued.getOwnerUuid(), + queued.getSenderUuid(), queued.getSenderCharacterId(), queued.getAddresseeDisplayTab(), + queued.getItem(), false, false)); + return null; + }).when(f.store).addPending(any(StoredMail.class)); + + f.service.resume(); + + var order = inOrder(f.store); + order.verify(f.store).addPending(mail); + order.verify(f.store).removeInFlight(mail.getId()); + assertTrue(f.inFlight.isEmpty()); + verifyNoInteractions(f.delivery); + ArgumentCaptor retry = ArgumentCaptor.forClass(Runnable.class); + verify(f.scheduler).runTaskTimer(eq(f.plugin), retry.capture(), eq(20L), eq(20L)); + retry.getValue().run(); + assertEquals(1, f.pending.get(f.ownerSession.ref()).size()); + verifyNoInteractions(f.delivery); + + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[36]); + retry.getValue().run(); + retry.getValue().run(); + + assertTrue(f.pending.get(f.ownerSession.ref()).isEmpty()); + verify(f.delivery, times(1)).deliver(f.owner, mail.getItem(), f.senderId, "Recipient"); + verifyNoMoreInteractions(f.delivery); + f.give.verifyNoInteractions(); + } + } + + @Test + void pendingBatchStopsWhenTheLastFreeSlotIsFilledAndResumesWithoutDuplicates() { + try (Fixture f = new Fixture()) { + f.activeOwner(); + ItemStack first = mock(ItemStack.class); + ItemStack second = mock(ItemStack.class); + f.queuePending(new MailStore.PendingLetter(f.ownerId, f.senderId, "First", first)); + f.queuePending(new MailStore.PendingLetter(f.ownerId, f.senderId, "Second", second)); + doAnswer(call -> { + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[0]); + return null; + }).when(f.delivery).deliver(f.owner, first, f.senderId, "First"); + + f.service.tryDeliverPending(f.owner, f.ownerSession); + f.service.tryDeliverPending(f.owner, f.ownerSession); + + verify(f.delivery, times(1)).deliver(f.owner, first, f.senderId, "First"); + verify(f.delivery, never()).deliver(f.owner, second, f.senderId, "Second"); + assertSame(second, f.pending.get(f.ownerSession.ref()).getFirst().item); + + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[36]); + f.service.tryDeliverPending(f.owner, f.ownerSession); + assertTrue(f.pending.get(f.ownerSession.ref()).isEmpty()); + verify(f.delivery, times(1)).deliver(f.owner, second, f.senderId, "Second"); + } + } + + @Test + void accountReturnsAlsoWaitForRoomAndStopWhenTheInventoryFills() { + try (Fixture f = new Fixture()) { + f.activeOwner(); + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[0]); + ItemStack first = mock(ItemStack.class); + ItemStack second = mock(ItemStack.class); + Deque returned = new ArrayDeque<>(List.of( + new MailStore.PendingLetter(f.ownerId, f.senderId, "", first), + new MailStore.PendingLetter(f.ownerId, f.senderId, "", second))); + doAnswer(call -> { + BooleanSupplier ready = call.getArgument(1); + Predicate accepts = call.getArgument(2); + Consumer deliver = call.getArgument(3); + while (ready.getAsBoolean() && !returned.isEmpty() && accepts.test(returned.getFirst())) { + deliver.accept(returned.removeFirst()); + } + return null; + }).when(f.store).drainReturned(eq(f.ownerId), any(), any(), any()); + doAnswer(call -> { + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[0]); + return null; + }).when(f.delivery).returnToSender(f.owner, first, false); + + f.service.tryDeliverPending(f.owner, f.ownerSession); + assertEquals(2, returned.size()); + verifyNoInteractions(f.delivery); + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[36]); + f.service.tryDeliverPending(f.owner, f.ownerSession); + assertEquals(1, returned.size()); + verify(f.delivery, times(1)).returnToSender(f.owner, first, false); + verify(f.delivery, never()).returnToSender(f.owner, second, false); + when(f.inventory.getStorageContents()).thenReturn(new ItemStack[36]); + f.service.tryDeliverPending(f.owner, f.ownerSession); + assertTrue(returned.isEmpty()); + verify(f.delivery, times(1)).returnToSender(f.owner, second, false); + } + } + @Test void acceptedMailOwnsACloneAndScheduledCompletionIsIdempotent() { try (Fixture f = new Fixture()) { @@ -168,13 +300,13 @@ void pendingDeliveryRequiresOnlineMatchingCharacterAndFlushChecksEachPlayer() { f.service.tryDeliverPending(f.owner, null); f.service.tryDeliverPending(f.owner, f.origin); f.service.tryDeliverPending(f.owner, f.ownerSession); - verify(f.store, never()).drainPending(any(), any(), any()); + verify(f.store, never()).drainPending(any(), any(), any(), any()); f.activeOwner(); ItemStack letter = mock(ItemStack.class); f.queuePending(new MailStore.PendingLetter(f.ownerId, f.senderId, "Recipient", letter)); f.bukkit.when(Bukkit::getOnlinePlayers).thenReturn(Arrays.asList(null, f.sender, f.owner)); f.service.flushPendingForOnlinePlayers(); - verify(f.store).drainPending(eq(f.ownerSession.ref()), any(), any()); + verify(f.store).drainPending(eq(f.ownerSession.ref()), any(), any(), any()); verify(f.delivery).deliver(f.owner, letter, f.senderId, "Recipient"); } } @@ -197,13 +329,13 @@ void pendingDrainStopsAtACharacterSwitchAndOldTokenCannotResumeAfterSwitchingBac verify(f.delivery).deliver(f.owner, first, f.senderId, "First"); verify(f.delivery, never()).deliver(f.owner, second, f.senderId, "Second"); assertSame(second, f.pending.get(f.ownerSession.ref()).getFirst().item); - verify(f.store).drainPending(eq(f.ownerSession.ref()), any(), any()); + verify(f.store).drainPending(eq(f.ownerSession.ref()), any(), any(), any()); CharacterSession returned = new CharacterSession(f.ownerSession.ref(), 4L); f.current.put(f.ownerId, returned); f.service.tryDeliverPending(f.owner, f.ownerSession); assertEquals(1, f.pending.get(f.ownerSession.ref()).size()); - verify(f.store).drainPending(eq(f.ownerSession.ref()), any(), any()); + verify(f.store).drainPending(eq(f.ownerSession.ref()), any(), any(), any()); f.service.tryDeliverPending(f.owner, returned); verify(f.delivery).deliver(f.owner, second, f.senderId, "Second"); assertTrue(f.pending.get(f.ownerSession.ref()).isEmpty()); @@ -222,12 +354,13 @@ void lettersReturnedByAnEstateComeBackToTheWritingCharacterOrItsAccount() { doAnswer(call -> { drained.add(call.getArgument(0)); BooleanSupplier stillCurrent = call.getArgument(1); - Consumer deliver = call.getArgument(2); + Predicate accepts = call.getArgument(2); + Consumer deliver = call.getArgument(3); if (stillCurrent.getAsBoolean()) { deliver.accept(new MailStore.PendingLetter(f.ownerId, f.ownerId, "", toAccount)); } return null; - }).when(f.store).drainReturned(any(), any(), any()); + }).when(f.store).drainReturned(any(), any(), any(), any()); f.service.tryDeliverPending(f.owner, f.ownerSession); @@ -266,7 +399,7 @@ void pendingDrainRejectsAnOldBukkitConnectionEvenWhenTheProviderTokenStillMatche when(replacement.isOnline()).thenReturn(true); f.bukkit.when(() -> Bukkit.getPlayer(f.ownerId)).thenReturn(replacement); f.service.tryDeliverPending(f.owner, f.ownerSession); - verify(f.store, never()).drainPending(any(), any(), any()); + verify(f.store, never()).drainPending(any(), any(), any(), any()); verifyNoInteractions(f.delivery); } } @@ -530,6 +663,7 @@ private static final class Fixture implements AutoCloseable { new CharacterSession(new CharacterRef(ownerId, characterId), 2L); final Player sender = mock(Player.class); final Player owner = mock(Player.class); + final PlayerInventory inventory = mock(PlayerInventory.class); final SelectedTarget target = new SelectedTarget(ownerId, characterId.toString(), "Recipient", "Recipient", null, null); final Map inFlight = new HashMap<>(); @@ -552,7 +686,13 @@ private static final class Fixture implements AutoCloseable { when(plugin.getLogger()).thenReturn(logger); when(sender.getUniqueId()).thenReturn(senderId); when(sender.isOnline()).thenReturn(true); + PlayerInventory senderInventory = mock(PlayerInventory.class); + when(sender.getInventory()).thenReturn(senderInventory); + when(senderInventory.getStorageContents()).thenReturn(new ItemStack[36]); when(owner.getUniqueId()).thenReturn(ownerId); + when(owner.getInventory()).thenReturn(inventory); + when(inventory.getStorageContents()).thenReturn(new ItemStack[36]); + when(inventory.getMaxStackSize()).thenReturn(64); when(config.msgCannotSendToSelf()).thenReturn("Self forbidden"); when(config.msgRecipientUnavailable()).thenReturn("Unavailable"); when(config.msgDifferentWorld()).thenReturn("Different world"); @@ -574,12 +714,14 @@ private static final class Fixture implements AutoCloseable { doAnswer(call -> { Deque letters = pending.get(call.getArgument(0)); BooleanSupplier stillCurrent = call.getArgument(1); - Consumer deliver = call.getArgument(2); - while (stillCurrent.getAsBoolean() && letters != null && !letters.isEmpty()) { + Predicate accepts = call.getArgument(2); + Consumer deliver = call.getArgument(3); + while (stillCurrent.getAsBoolean() && letters != null && !letters.isEmpty() + && accepts.test(letters.getFirst())) { deliver.accept(letters.pollFirst()); } return null; - }).when(store).drainPending(any(), any(), any()); + }).when(store).drainPending(any(), any(), any(), any()); current.put(senderId, origin); identities.when(() -> Characters.currentSession(any(Player.class))).thenAnswer(call -> { Player player = call.getArgument(0); diff --git a/src/test/java/net/tfminecraft/birdmessenger/mail/MailStoreCoverageTest.java b/src/test/java/net/tfminecraft/birdmessenger/mail/MailStoreCoverageTest.java index 4b639ff..e6c7415 100644 --- a/src/test/java/net/tfminecraft/birdmessenger/mail/MailStoreCoverageTest.java +++ b/src/test/java/net/tfminecraft/birdmessenger/mail/MailStoreCoverageTest.java @@ -347,6 +347,26 @@ void drainSavesOnceAfterHandingOverEveryLetterAndKeepsTheReturnedFlag() { assertEquals(serializedItem("Unsent"), handed.get(1).item.serialize()); } + @Test + void rejectedCapacityKeepsTheWholeLetterSavedAndStopsTheBatch() { + MailStore store = new MailStore(plugin); + store.addPending(CHARACTER, new MailStore.PendingLetter(OWNER, SENDER, "A", item(serializedItem("A")))); + store.addPending(CHARACTER, new MailStore.PendingLetter(OWNER, SENDER, "B", item(serializedItem("B")))); + List handed = new ArrayList<>(); + store.drainPending(REF, () -> true, letter -> false, handed::add); + assertTrue(handed.isEmpty()); + assertEquals(2, savedLetters()); + MailStore restored = new MailStore(plugin); + restored.load(); + restored.drainPending(REF, () -> true, letter -> letter.addresseeDisplayTab.equals("A"), handed::add); + assertEquals(1, handed.size()); + assertEquals(1, savedLetters()); + assertEquals("B", restored.pendingFor(CHARACTER).getFirst().addresseeDisplayTab); + restored.load(); + restored.drainPending(REF, () -> true, handed::add); + assertEquals(List.of("A", "B"), handed.stream().map(letter -> letter.addresseeDisplayTab).toList()); + } + @Test void failedHandOverPutsTheLetterBackAtTheHeadOfItsQueue() { MailStore store = new MailStore(plugin);