From 154c447babf9024045ebe600a28cabc9347808e7 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:10:55 +0000 Subject: [PATCH 1/3] fix: complete accessible studio projects and usable instrument actions --- .../commands/InstrumentCommand.java | 12 +- .../studio/CharacterStudioBridge.java | 6 + .../studio/StudioCommand.java | 5 +- .../studio/StudioService.java | 52 ++++++- .../commands/InstrumentCommandTest.java | 12 ++ .../studio/StudioBridgeTest.java | 5 + .../studio/StudioCommandWorkflowTest.java | 6 +- .../studio/StudioCompletionTest.java | 138 ++++++++++++++++++ 8 files changed, 227 insertions(+), 9 deletions(-) create mode 100644 src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java diff --git a/src/main/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommand.java b/src/main/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommand.java index 34eca73..1d5b8a0 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommand.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommand.java @@ -205,11 +205,13 @@ public List onTabComplete(CommandSender sender, Command command, String if (args.length == 1) { List subcommands = new ArrayList<>(); if (sender.hasPermission("instruments.use")) { - subcommands.add("play"); - subcommands.add("keybinds"); + if (sender instanceof Player) { + subcommands.add("play"); + subcommands.add("keybinds"); + } subcommands.add("list"); } - if (sender.hasPermission("instruments.give")) { + if (sender instanceof Player && sender.hasPermission("instruments.give")) { subcommands.add("give"); } if (sender.hasPermission("instruments.reload")) { @@ -219,7 +221,9 @@ public List onTabComplete(CommandSender sender, Command command, String return filterPrefix(subcommands, args[0]); } - if (args.length == 2 && (args[0].equalsIgnoreCase("give") || args[0].equalsIgnoreCase("play")) + if (sender instanceof Player && args.length == 2 + && (args[0].equalsIgnoreCase("give") + || args[0].equalsIgnoreCase("play") && sender.hasPermission("instruments.use")) && sender.hasPermission("instruments.give")) { return filterPrefix(new ArrayList<>(manager.getAllInstruments()), args[1]); } diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java index 7ecf9c6..54d1b18 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java @@ -10,6 +10,7 @@ import org.bukkit.plugin.Plugin; import net.tfminecraft.tlibs.character.CharacterSession; import net.tfminecraft.tlibs.character.CharacterSessionEndEvent; +import net.tfminecraft.tlibs.character.CharacterSessionStartEvent; import net.tfminecraft.tlibs.character.Characters; /** Loaded only after the optional TLibs character API has been probed. */ @@ -42,6 +43,10 @@ public Optional heir(UUID account) { public static void start(Plugin plugin, StudioService studio, StudioMenu menu) { plugin.getServer().getPluginManager().registerEvents(new Listener() { + @EventHandler(priority = EventPriority.MONITOR) + public void start(CharacterSessionStartEvent event) { + studio.completionProjectIds(event.getPlayer()); + } @EventHandler(priority = EventPriority.LOWEST) public void end(CharacterSessionEndEvent event) { // The event's session is already noncurrent; finish with its captured owner. @@ -49,5 +54,6 @@ public void end(CharacterSessionEndEvent event) { menu.end(event.getPlayer()); } }, plugin); + Bukkit.getOnlinePlayers().forEach(studio::completionProjectIds); } } diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java index 3ca005d..43f49eb 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java @@ -193,6 +193,7 @@ private float gain(String value) { @Override public List onTabComplete(CommandSender sender, Command command, String alias, String[] args) { + if (!(sender instanceof Player player) || args.length == 0) return List.of(); List options = List.of(); if (args.length == 1) { var actions = new java.util.ArrayList(); @@ -206,8 +207,10 @@ public List onTabComplete(CommandSender sender, Command command, String options = actions; } else if (args.length == 2 && sender.hasPermission("instruments.record")) { options = switch (args[0].toLowerCase(Locale.ROOT)) { - case "record", "volume", "mute", "remove" -> java.util.stream.IntStream.rangeClosed(1, studio.settings().tracks()) + case "record" -> java.util.stream.IntStream.rangeClosed(1, studio.settings().tracks()) .mapToObj(Integer::toString).toList(); + case "open" -> studio.completionProjectIds(player); + case "volume", "mute", "remove" -> studio.completionTracks(player); case "preview" -> List.of("take"); case "metronome" -> List.of("on", "off"); case "reset" -> List.of("confirm"); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java index c907ece..c6578b3 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java @@ -50,6 +50,8 @@ public final class StudioService { private final StudioStation station; private final DiscItems discs; private final Map projects = new HashMap<>(); + private final Map> completionIds = new HashMap<>(); + private final Set completionLoaded = new HashSet<>(); private final Map recordings = new HashMap<>(); private final Map previews = new HashMap<>(); private final Map> privateSounds = new HashMap<>(); @@ -188,7 +190,10 @@ boolean quiescent(UUID character) { && recordings.values().stream().noneMatch(capture -> capture.token.owner().character().equals(character)); } - void transferred(UUID character) { projects.remove(character); owners.remove(character); } + void transferred(UUID character) { + projects.remove(character); owners.remove(character); + completionIds.remove(character); completionLoaded.remove(character); + } public record Activity(int track, int seconds, int notes, int countdown, boolean previewing) { public boolean recording() { return track > 0; } @@ -274,6 +279,7 @@ public void deleteProject(Player player, Project expected) throws IOException { throw new IllegalArgumentException("The project changed. Open the delete confirmation again"); } store.delete(owner, id); + completionIds.computeIfPresent(owner, (key, ids) -> ids.stream().filter(candidate -> !candidate.equals(id)).toList()); Project current = projects.get(owner); if (current != null && current.song().id().equals(id)) { projects.remove(owner); @@ -292,7 +298,42 @@ private List listProjects(StudioIdentity.Token token) throws IOE // An uncached project matches its files; a cached one may be newer than them. Project active = projects.get(owner); if (active != null) all.put(active.song().id(), ProjectSummary.of(active)); - return all.values().stream().sorted(ProjectSummary.ORDER).toList(); + List summaries = all.values().stream().sorted(ProjectSummary.ORDER).toList(); + completionIds.put(owner, summaries.stream().map(ProjectSummary::id).toList()); + return summaries; + } + + // Completion reads memory only. Coalesce a cold owner lookup into one later server task. + private StudioIdentity.Token completionToken(Player player) { + StudioIdentity.Token token = identity.current(player).orElse(null); + if (closed || token == null || !current(token, player)) return null; + UUID owner = token.owner().character(); + if (completionLoaded.add(owner)) { + plugin.getServer().getScheduler().runTask(plugin, () -> { + if (closed || !current(token, player)) return; + try { + listProjects(player); + project(player); + } catch (IOException | IllegalArgumentException ex) { + completionIds.remove(owner); + plugin.getLogger().log(Level.FINE, "Studio completion data unavailable", ex); + } + }); + } + return token; + } + + List completionProjectIds(Player player) { + StudioIdentity.Token token = completionToken(player); + return token == null ? List.of() : completionIds.getOrDefault(token.owner().character(), List.of()) + .stream().map(UUID::toString).toList(); + } + + List completionTracks(Player player) { + StudioIdentity.Token token = completionToken(player); + Project project = token == null ? null : projects.get(token.owner().character()); + return project == null ? List.of() : project.song().tracks().stream() + .map(track -> Integer.toString(track.slot())).toList(); } public void selectProject(Player player, UUID id) throws IOException { @@ -318,6 +359,11 @@ private void save(Player player, StudioIdentity.Token token, Project project) th store.saveOwned(token.owner(), project); // Persist successfully before replacing the in-memory version. projects.put(token.owner().character(), project); unsaved.remove(token.owner().character()); + completionIds.computeIfPresent(token.owner().character(), (owner, ids) -> { + var updated = new java.util.LinkedHashSet<>(ids); + updated.add(project.song().id()); + return List.copyOf(updated); + }); } /** @@ -903,6 +949,7 @@ public void quit(Player player) { StudioIdentity.Token token = identity.current(player).orElse(null); if (token == null) return; UUID owner = token.owner().character(); + completionIds.remove(owner); completionLoaded.remove(owner); if (pendingSaves.containsKey(owner)) return; // completeSave evicts the project once it is written. if (unsaved.contains(owner)) queueSave(owners.get(owner), projects.get(owner)); // Retry a write that failed. else projects.remove(owner); // Its files are current; the next visit reloads it. @@ -913,6 +960,7 @@ public void end(Player player, StudioIdentity.Token ending) { if (capture != null && capture.token.equals(ending)) finish(player, capture, "Recording stopped when the character session ended"); stopPreview(player); UUID character = ending.owner().character(); + completionIds.remove(character); completionLoaded.remove(character); if (!pendingSaves.containsKey(character)) { if (unsaved.contains(character)) queueSave(ending.owner(), projects.get(character)); else projects.remove(character); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommandTest.java b/src/test/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommandTest.java index 524b825..86be240 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommandTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/commands/InstrumentCommandTest.java @@ -265,4 +265,16 @@ void completesInstrumentNamesForPlay() { assertEquals(List.of("celtic_harp"), complete(operator, "play", "c")); assertEquals(List.of(), complete(player, "play", "")); } + @Test void consoleAndUseDeniedPlayersOnlySeeUsableInstrumentCommands() { + loadInstruments("lute"); + assertEquals(List.of("list", "reload"), complete(console, "")); + assertEquals(List.of(), complete(console, "play", "")); + assertEquals(List.of(), complete(console, "give", "")); + player.addAttachment(MockBukkit.createMockPlugin(), "instruments.give", true); + player.addAttachment(MockBukkit.createMockPlugin(), "instruments.use", false); + assertFalse(complete(player, "").contains("play")); + assertEquals(List.of(), complete(player, "play", "L")); + assertEquals(List.of("lute"), complete(player, "give", "L")); + } + } diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java index afbabbf..c056f79 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java @@ -41,8 +41,13 @@ class StudioBridgeTest { Plugin plugin = mock(Plugin.class); var server = mock(org.bukkit.Server.class); var manager = mock(org.bukkit.plugin.PluginManager.class); when(plugin.getServer()).thenReturn(server); when(server.getPluginManager()).thenReturn(manager); StudioService studio = mock(StudioService.class); StudioMenu menu = mock(StudioMenu.class); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.List.of(player)); CharacterStudioBridge.start(plugin, studio, menu); + verify(studio).completionProjectIds(player); var listener = ArgumentCaptor.forClass(Listener.class); verify(manager).registerEvents(listener.capture(), eq(plugin)); + listener.getValue().getClass().getMethod("start", net.tfminecraft.tlibs.character.CharacterSessionStartEvent.class) + .invoke(listener.getValue(), new net.tfminecraft.tlibs.character.CharacterSessionStartEvent(player, session)); + verify(studio, times(2)).completionProjectIds(player); characters.when(() -> Characters.isCurrent(session)).thenReturn(false); listener.getValue().getClass().getMethod("end", CharacterSessionEndEvent.class).invoke(listener.getValue(), new CharacterSessionEndEvent(player, session, CharacterSessionEndEvent.Reason.SWITCH)); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java index b6a65a1..25cef62 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java @@ -199,8 +199,10 @@ void tabCompletionReflectsPermissionsAndMatchesTheLastArgument() { when(player.hasPermission("instruments.studio")).thenReturn(true); when(player.hasPermission("instruments.copy")).thenReturn(true); assertTrue(commands.onTabComplete(player, cmd, "music", new String[]{""}).containsAll(List.of("copy", "studio"))); - for (String action : List.of("record", "volume", "mute", "remove")) - assertEquals(List.of("1", "2", "3", "4"), commands.onTabComplete(player, cmd, "music", new String[]{action, ""})); + assertEquals(List.of("1", "2", "3", "4"), commands.onTabComplete(player, cmd, "music", new String[]{"record", ""})); + when(studio.completionTracks(player)).thenReturn(List.of("1", "2")); + for (String action : List.of("volume", "mute", "remove")) + assertEquals(List.of("1", "2"), commands.onTabComplete(player, cmd, "music", new String[]{action, ""})); assertEquals(List.of("take"), commands.onTabComplete(player, cmd, "music", new String[]{"preview", "T"})); assertEquals(List.of("on", "off"), commands.onTabComplete(player, cmd, "music", new String[]{"metronome", ""})); assertEquals(List.of("confirm"), commands.onTabComplete(player, cmd, "music", new String[]{"reset", ""})); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java new file mode 100644 index 0000000..e89f712 --- /dev/null +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java @@ -0,0 +1,138 @@ +package net.tfminecraft.musicalinstruments.studio; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.logging.Logger; +import net.tfminecraft.musicalinstruments.InstrumentPlugin; +import net.tfminecraft.musicalinstruments.managers.InstrumentManager; +import org.bukkit.Server; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.bukkit.scheduler.BukkitScheduler; +import org.bukkit.scheduler.BukkitTask; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class StudioCompletionTest { + @TempDir Path directory; + + @Test void completionUsesOwnerSnapshotsAndSavedTracksWithoutKeystrokeIo() throws Exception { + var plugin = mock(InstrumentPlugin.class); + var server = mock(Server.class); + var scheduler = mock(BukkitScheduler.class); + when(plugin.getServer()).thenReturn(server); + when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + when(server.getScheduler()).thenReturn(scheduler); + when(scheduler.runTaskTimer(any(), any(Runnable.class), anyLong(), anyLong())).thenReturn(mock(BukkitTask.class)); + var tasks = new ArrayList(); + when(scheduler.runTask(eq(plugin), any(Runnable.class))).thenAnswer(call -> { + tasks.add(call.getArgument(1)); return mock(BukkitTask.class); + }); + var player = mock(Player.class); + UUID account = UUID.randomUUID(); + when(player.getUniqueId()).thenReturn(account); + when(player.getName()).thenReturn("Artist"); + when(player.hasPermission("instruments.record")).thenReturn(true); + var identity = new TestIdentity(); identity.defaultHeirs = false; + var owner = new StudioIdentity.Owner(account, UUID.randomUUID()); + var foreign = new StudioIdentity.Owner(UUID.randomUUID(), UUID.randomUUID()); + identity.sessions.put(account, new StudioIdentity.Token(owner, 1)); + var store = spy(new StudioStore(directory)); + var track = new Track(3, 20, 1, false, List.of(new Note(0, "lute", "sample", 1, 1))); + var first = new Project(new Song(UUID.randomUUID(), account, "Author", "First", List.of(track)), null, 100, false); + var other = new Project(new Song(UUID.randomUUID(), foreign.account(), "Other", "Foreign", List.of()), null, 100, false); + store.saveOwned(owner, first); store.saveOwned(foreign, other); + var studio = new StudioService(plugin, mock(InstrumentManager.class), store, + new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 20, 0, 0), StudioStation.none(), mock(DiscItems.class), identity); + var commands = new StudioCommand(studio, mock(StudioMenu.class)); + try { + clearInvocations(store); + for (int i = 0; i < 4; i++) assertTrue(commands.onTabComplete(player, null, "music", new String[]{"open", ""}).isEmpty()); + verifyNoInteractions(store); + assertEquals(1, tasks.size()); + tasks.removeFirst().run(); + clearInvocations(store); + for (String alias : List.of("music", "recording")) { + assertEquals(List.of(first.song().id().toString()), commands.onTabComplete(player, null, alias, new String[]{"open", ""})); + assertEquals(List.of(first.song().id().toString()), commands.onTabComplete(player, null, alias, + new String[]{"OPEN", first.song().id().toString().substring(0, 8).toUpperCase()})); + assertTrue(commands.onTabComplete(player, null, alias, new String[]{"open", "missing"}).isEmpty()); + for (String action : List.of("volume", "mute", "remove")) { + assertEquals(List.of("3"), commands.onTabComplete(player, null, alias, new String[]{action, ""})); + assertTrue(commands.onTabComplete(player, null, alias, new String[]{action, "1"}).isEmpty()); + } + assertEquals(List.of("1", "2", "3", "4"), commands.onTabComplete(player, null, alias, new String[]{"record", ""})); + } + verifyNoInteractions(store); + studio.create(player, "New", false); + assertEquals(2, studio.completionProjectIds(player).size()); + assertTrue(studio.completionTracks(player).isEmpty()); + studio.deleteProject(player, first); + assertFalse(studio.completionProjectIds(player).contains(first.song().id().toString())); + identity.sessions.put(account, new StudioIdentity.Token(new StudioIdentity.Owner(account, UUID.randomUUID()), 2)); + assertTrue(studio.completionProjectIds(player).isEmpty()); + assertTrue(studio.completionTracks(player).isEmpty()); + tasks.removeFirst().run(); + assertTrue(studio.completionProjectIds(player).isEmpty()); + identity.sessions.put(account, null); + assertTrue(studio.completionProjectIds(player).isEmpty()); + assertTrue(studio.completionTracks(player).isEmpty()); + var console = mock(CommandSender.class); + when(console.hasPermission(anyString())).thenReturn(true); + for (String alias : List.of("music", "recording")) { + assertTrue(commands.onTabComplete(console, null, alias, new String[]{""}).isEmpty()); + assertTrue(commands.onTabComplete(console, null, alias, new String[]{"metronome", ""}).isEmpty()); + assertTrue(commands.onTabComplete(console, null, alias, new String[0]).isEmpty()); + } + } finally { studio.close(); } + } + @Test void unavailableDataAndChangedSessionsNeverLeakCachedProjectsOrRunStaleLoads() throws Exception { + var plugin = mock(InstrumentPlugin.class, RETURNS_DEEP_STUBS); + var scheduler = plugin.getServer().getScheduler(); + when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + var tasks = new ArrayList(); + when(scheduler.runTask(eq(plugin), any(Runnable.class))).thenAnswer(call -> { + tasks.add(call.getArgument(1)); return mock(BukkitTask.class); + }); + var identity = mock(StudioIdentity.class); + var player = mock(Player.class); + UUID account = UUID.randomUUID(); + when(player.getUniqueId()).thenReturn(account); + when(player.getName()).thenReturn("Artist"); + var token = new StudioIdentity.Token(new StudioIdentity.Owner(account, UUID.randomUUID()), 1); + when(identity.current(player)).thenReturn(java.util.Optional.of(token)); + var store = mock(StudioStore.class); + var studio = new StudioService(plugin, mock(InstrumentManager.class), store, + new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 20, 0, 0), StudioStation.none(), mock(DiscItems.class), identity); + try { + clearInvocations(store); + assertTrue(studio.completionProjectIds(player).isEmpty()); // Stale token cannot warm. + assertTrue(tasks.isEmpty()); + when(identity.current(token)).thenReturn(true); + assertTrue(studio.completionProjectIds(player).isEmpty()); + when(identity.current(token)).thenReturn(false); + tasks.removeFirst().run(); + verifyNoInteractions(store); + studio.transferred(token.owner().character()); + when(identity.current(token)).thenReturn(true); + assertTrue(studio.completionProjectIds(player).isEmpty()); + when(store.summaries(token.owner().character())).thenThrow(new java.io.IOException("unavailable")); + tasks.removeFirst().run(); + assertTrue(studio.completionProjectIds(player).isEmpty()); + verify(store, times(1)).summaries(token.owner().character()); + studio.transferred(token.owner().character()); + assertTrue(studio.completionProjectIds(player).isEmpty()); + var command = new StudioCommand(studio, mock(StudioMenu.class)); + assertTrue(command.onTabComplete(player, null, "music", new String[0]).isEmpty()); + studio.close(); + tasks.removeFirst().run(); + assertTrue(studio.completionProjectIds(player).isEmpty()); + } finally { studio.close(); } + } + +} From 227740d391b82e31db4f50b81a690a906721c0e0 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:29:07 +0000 Subject: [PATCH 2/3] fix(studio): warm completion data only on demand --- .../studio/CharacterStudioBridge.java | 6 -- .../studio/StudioBridgeTest.java | 5 +- .../studio/StudioCompletionTest.java | 68 +++++++++++++++++++ 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java index 54d1b18..7ecf9c6 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/CharacterStudioBridge.java @@ -10,7 +10,6 @@ import org.bukkit.plugin.Plugin; import net.tfminecraft.tlibs.character.CharacterSession; import net.tfminecraft.tlibs.character.CharacterSessionEndEvent; -import net.tfminecraft.tlibs.character.CharacterSessionStartEvent; import net.tfminecraft.tlibs.character.Characters; /** Loaded only after the optional TLibs character API has been probed. */ @@ -43,10 +42,6 @@ public Optional heir(UUID account) { public static void start(Plugin plugin, StudioService studio, StudioMenu menu) { plugin.getServer().getPluginManager().registerEvents(new Listener() { - @EventHandler(priority = EventPriority.MONITOR) - public void start(CharacterSessionStartEvent event) { - studio.completionProjectIds(event.getPlayer()); - } @EventHandler(priority = EventPriority.LOWEST) public void end(CharacterSessionEndEvent event) { // The event's session is already noncurrent; finish with its captured owner. @@ -54,6 +49,5 @@ public void end(CharacterSessionEndEvent event) { menu.end(event.getPlayer()); } }, plugin); - Bukkit.getOnlinePlayers().forEach(studio::completionProjectIds); } } diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java index c056f79..27d53a7 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioBridgeTest.java @@ -43,11 +43,8 @@ class StudioBridgeTest { StudioService studio = mock(StudioService.class); StudioMenu menu = mock(StudioMenu.class); bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.List.of(player)); CharacterStudioBridge.start(plugin, studio, menu); - verify(studio).completionProjectIds(player); + verifyNoInteractions(studio, menu); var listener = ArgumentCaptor.forClass(Listener.class); verify(manager).registerEvents(listener.capture(), eq(plugin)); - listener.getValue().getClass().getMethod("start", net.tfminecraft.tlibs.character.CharacterSessionStartEvent.class) - .invoke(listener.getValue(), new net.tfminecraft.tlibs.character.CharacterSessionStartEvent(player, session)); - verify(studio, times(2)).completionProjectIds(player); characters.when(() -> Characters.isCurrent(session)).thenReturn(false); listener.getValue().getClass().getMethod("end", CharacterSessionEndEvent.class).invoke(listener.getValue(), new CharacterSessionEndEvent(player, session, CharacterSessionEndEvent.Reason.SWITCH)); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java index e89f712..39ae069 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java @@ -10,6 +10,9 @@ import java.util.logging.Logger; import net.tfminecraft.musicalinstruments.InstrumentPlugin; import net.tfminecraft.musicalinstruments.managers.InstrumentManager; +import net.tfminecraft.tlibs.character.CharacterRef; +import net.tfminecraft.tlibs.character.CharacterSession; +import net.tfminecraft.tlibs.character.CharacterSessionStartEvent; import org.bukkit.Server; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -17,10 +20,75 @@ import org.bukkit.scheduler.BukkitTask; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockbukkit.mockbukkit.MockBukkit; class StudioCompletionTest { @TempDir Path directory; + @Test void startupAndSessionStartsLeaveProjectsColdUntilExplicitCompletion() throws Exception { + var eventServer = MockBukkit.mock(); + try { + var bridgePlugin = MockBukkit.createMockPlugin(); + var player = eventServer.addPlayer(); + player.addAttachment(bridgePlugin, "instruments.record", true); + var plugin = mock(InstrumentPlugin.class, RETURNS_DEEP_STUBS); + when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + var scheduler = plugin.getServer().getScheduler(); + var tasks = new ArrayList(); + when(scheduler.runTask(eq(plugin), any(Runnable.class))).thenAnswer(call -> { + tasks.add(call.getArgument(1)); return mock(BukkitTask.class); + }); + var identity = new TestIdentity(); identity.defaultHeirs = false; + var first = new StudioIdentity.Owner(player.getUniqueId(), UUID.randomUUID()); + var second = new StudioIdentity.Owner(player.getUniqueId(), UUID.randomUUID()); + identity.sessions.put(player.getUniqueId(), new StudioIdentity.Token(first, 1)); + var store = spy(new StudioStore(directory)); + var track = new Track(3, 20, 1, false, List.of(new Note(0, "lute", "sample", 1, 1))); + var saved = new Project(new Song(UUID.randomUUID(), second.account(), "Artist", "Saved", List.of(track)), null, 100, false); + store.saveOwned(first, new Project(new Song(UUID.randomUUID(), first.account(), "Artist", "Previous", List.of()), null, 100, false)); + store.saveOwned(second, saved); + var studio = new StudioService(plugin, mock(InstrumentManager.class), store, + new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 20, 0, 0), StudioStation.none(), mock(DiscItems.class), identity); + try { + clearInvocations(store); + CharacterStudioBridge.start(bridgePlugin, studio, mock(StudioMenu.class)); + assertTrue(tasks.isEmpty(), "Startup must not schedule completion loading for online players"); + verifyNoInteractions(store); + eventServer.getPluginManager().callEvent(new CharacterSessionStartEvent(player, + new CharacterSession(new CharacterRef(first.account(), first.character()), 1))); + identity.sessions.put(player.getUniqueId(), new StudioIdentity.Token(second, 2)); + eventServer.getPluginManager().callEvent(new CharacterSessionStartEvent(player, + new CharacterSession(new CharacterRef(second.account(), second.character()), 2))); + assertTrue(tasks.isEmpty(), "Login and character switches must not schedule completion loading"); + verifyNoInteractions(store); + + var commands = new StudioCommand(studio, mock(StudioMenu.class)); + for (String alias : List.of("music", "recording")) { + assertTrue(commands.onTabComplete(player, null, alias, new String[]{"open", ""}).isEmpty()); + for (String action : List.of("volume", "mute", "remove")) { + assertTrue(commands.onTabComplete(player, null, alias, new String[]{action, ""}).isEmpty()); + } + } + assertEquals(1, tasks.size(), "Cold completion requests must share one scheduled load"); + verifyNoInteractions(store); + tasks.removeFirst().run(); + verify(store).summaries(second.character()); + verify(store).project(second.character()); + verify(store, never()).summaries(first.character()); + verify(store, never()).project(first.character()); + clearInvocations(store); + for (String alias : List.of("music", "recording")) { + assertEquals(List.of(saved.song().id().toString()), commands.onTabComplete(player, null, alias, new String[]{"open", ""})); + for (String action : List.of("volume", "mute", "remove")) { + assertEquals(List.of("3"), commands.onTabComplete(player, null, alias, new String[]{action, ""})); + } + } + assertTrue(tasks.isEmpty(), "Warm completions must not schedule another load"); + verifyNoInteractions(store); + } finally { studio.close(); } + } finally { MockBukkit.unmock(); } + } + @Test void completionUsesOwnerSnapshotsAndSavedTracksWithoutKeystrokeIo() throws Exception { var plugin = mock(InstrumentPlugin.class); var server = mock(Server.class); From 3d85aa794fb11be9a7ec56b9f5c0094e49e45b7e Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:51:29 +0000 Subject: [PATCH 3/3] fix: retry failed studio completion loads after cooldown --- .../studio/StudioService.java | 10 +- .../studio/StudioCompletionTest.java | 120 ++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java index c6578b3..d872b5d 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java @@ -52,6 +52,7 @@ public final class StudioService { private final Map projects = new HashMap<>(); private final Map> completionIds = new HashMap<>(); private final Set completionLoaded = new HashSet<>(); + private final Map completionRetryAt = new HashMap<>(); private final Map recordings = new HashMap<>(); private final Map previews = new HashMap<>(); private final Map> privateSounds = new HashMap<>(); @@ -193,6 +194,7 @@ boolean quiescent(UUID character) { void transferred(UUID character) { projects.remove(character); owners.remove(character); completionIds.remove(character); completionLoaded.remove(character); + completionRetryAt.remove(character); } public record Activity(int track, int seconds, int notes, int countdown, boolean previewing) { @@ -304,18 +306,22 @@ private List listProjects(StudioIdentity.Token token) throws IOE } // Completion reads memory only. Coalesce a cold owner lookup into one later server task. + // Failed loads retry only on explicit requests after the same cooldown used for failed saves. private StudioIdentity.Token completionToken(Player player) { StudioIdentity.Token token = identity.current(player).orElse(null); if (closed || token == null || !current(token, player)) return null; UUID owner = token.owner().character(); - if (completionLoaded.add(owner)) { + if (clock >= completionRetryAt.getOrDefault(owner, 0L) && completionLoaded.add(owner)) { plugin.getServer().getScheduler().runTask(plugin, () -> { if (closed || !current(token, player)) return; try { listProjects(player); project(player); + completionRetryAt.remove(owner); } catch (IOException | IllegalArgumentException ex) { completionIds.remove(owner); + completionLoaded.remove(owner); + completionRetryAt.put(owner, clock + RETRY_TICKS); plugin.getLogger().log(Level.FINE, "Studio completion data unavailable", ex); } }); @@ -950,6 +956,7 @@ public void quit(Player player) { if (token == null) return; UUID owner = token.owner().character(); completionIds.remove(owner); completionLoaded.remove(owner); + completionRetryAt.remove(owner); if (pendingSaves.containsKey(owner)) return; // completeSave evicts the project once it is written. if (unsaved.contains(owner)) queueSave(owners.get(owner), projects.get(owner)); // Retry a write that failed. else projects.remove(owner); // Its files are current; the next visit reloads it. @@ -961,6 +968,7 @@ public void end(Player player, StudioIdentity.Token ending) { stopPreview(player); UUID character = ending.owner().character(); completionIds.remove(character); completionLoaded.remove(character); + completionRetryAt.remove(character); if (!pendingSaves.containsKey(character)) { if (unsaved.contains(character)) queueSave(ending.owner(), projects.get(character)); else projects.remove(character); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java index 39ae069..8fdbbf3 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCompletionTest.java @@ -20,6 +20,8 @@ import org.bukkit.scheduler.BukkitTask; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockbukkit.mockbukkit.MockBukkit; class StudioCompletionTest { @@ -203,4 +205,122 @@ class StudioCompletionTest { } finally { studio.close(); } } + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void failedCompletionLoadsRecoverInTheSameSessionWithoutKeystrokeRetries(boolean selectedProjectFails) throws Exception { + var plugin = mock(InstrumentPlugin.class, RETURNS_DEEP_STUBS); + when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + var scheduler = plugin.getServer().getScheduler(); + var ticks = new ArrayList(); + when(scheduler.runTaskTimer(eq(plugin), any(Runnable.class), eq(1L), eq(1L))).thenAnswer(call -> { + ticks.add(call.getArgument(1)); return mock(BukkitTask.class); + }); + var tasks = new ArrayList(); + when(scheduler.runTask(eq(plugin), any(Runnable.class))).thenAnswer(call -> { + tasks.add(call.getArgument(1)); return mock(BukkitTask.class); + }); + var player = mock(Player.class); + UUID account = UUID.randomUUID(); + when(player.getUniqueId()).thenReturn(account); + when(player.hasPermission("instruments.record")).thenReturn(true); + var identity = new TestIdentity(); identity.defaultHeirs = false; + var owner = new StudioIdentity.Owner(account, UUID.randomUUID()); + identity.sessions.put(account, new StudioIdentity.Token(owner, 1)); + var store = spy(new StudioStore(directory)); + var track = new Track(3, 20, 1, false, List.of(new Note(0, "lute", "sample", 1, 1))); + var saved = new Project(new Song(UUID.randomUUID(), account, "Artist", "Saved", List.of(track)), null, 100, false); + store.saveOwned(owner, saved); + var failure = new java.io.IOException("Storage temporarily unavailable"); + if (selectedProjectFails) doThrow(failure).doThrow(failure).doCallRealMethod().when(store).project(owner.character()); + else doThrow(failure).doThrow(failure).doCallRealMethod().when(store).summaries(owner.character()); + var studio = new StudioService(plugin, mock(InstrumentManager.class), store, + new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 20, 0, 0), StudioStation.none(), mock(DiscItems.class), identity); + var commands = new StudioCommand(studio, mock(StudioMenu.class)); + try { + clearInvocations(store); + assertTrue(commands.onTabComplete(player, null, "music", new String[]{"open", ""}).isEmpty()); + assertEquals(1, tasks.size()); + verifyNoInteractions(store); + for (int attempt = 0; attempt < 2; attempt++) { + tasks.removeFirst().run(); + verify(store).summaries(owner.character()); + if (selectedProjectFails) verify(store).project(owner.character()); + else verify(store, never()).project(owner.character()); + clearInvocations(store); + for (long elapsed = 0; elapsed < StudioService.RETRY_TICKS; elapsed++) { + for (String alias : List.of("music", "recording")) { + assertTrue(commands.onTabComplete(player, null, alias, new String[]{"open", ""}).isEmpty()); + assertTrue(commands.onTabComplete(player, null, alias, new String[]{"volume", ""}).isEmpty()); + } + assertTrue(tasks.isEmpty(), "Failed storage must not be retried during the cooldown"); + ticks.getFirst().run(); + } + assertTrue(tasks.isEmpty(), "Elapsed time alone must not trigger a disk load"); + verifyNoInteractions(store); + assertTrue(commands.onTabComplete(player, null, "music", new String[]{"open", ""}).isEmpty()); + assertTrue(commands.onTabComplete(player, null, "recording", new String[]{"remove", ""}).isEmpty()); + assertEquals(1, tasks.size(), "The next explicit requests must share one retry after the cooldown"); + verifyNoInteractions(store); + } + tasks.removeFirst().run(); + verify(store).summaries(owner.character()); + verify(store).project(owner.character()); + clearInvocations(store); + for (String alias : List.of("music", "recording")) { + assertEquals(List.of(saved.song().id().toString()), commands.onTabComplete(player, null, alias, new String[]{"open", ""})); + for (String action : List.of("volume", "mute", "remove")) { + assertEquals(List.of("3"), commands.onTabComplete(player, null, alias, new String[]{action, ""})); + } + } + assertTrue(tasks.isEmpty()); + verifyNoInteractions(store); + } finally { studio.close(); } + } + + @ParameterizedTest + @ValueSource(strings = {"quit", "end", "transferred"}) + void completionRetryCooldownIsInvalidatedWithTheOwnerSnapshot(String invalidation) throws Exception { + var plugin = mock(InstrumentPlugin.class, RETURNS_DEEP_STUBS); + when(plugin.getLogger()).thenReturn(Logger.getAnonymousLogger()); + var scheduler = plugin.getServer().getScheduler(); + var tasks = new ArrayList(); + when(scheduler.runTask(eq(plugin), any(Runnable.class))).thenAnswer(call -> { + tasks.add(call.getArgument(1)); return mock(BukkitTask.class); + }); + var player = mock(Player.class); + UUID account = UUID.randomUUID(); + when(player.getUniqueId()).thenReturn(account); + var identity = new TestIdentity(); identity.defaultHeirs = false; + var owner = new StudioIdentity.Owner(account, UUID.randomUUID()); + var token = new StudioIdentity.Token(owner, 1); + identity.sessions.put(account, token); + var store = spy(new StudioStore(directory)); + var saved = new Project(new Song(UUID.randomUUID(), account, "Artist", "Saved", List.of()), null, 100, false); + store.saveOwned(owner, saved); + doThrow(new IllegalArgumentException("Storage temporarily unavailable")).doCallRealMethod().when(store).summaries(owner.character()); + var studio = new StudioService(plugin, mock(InstrumentManager.class), store, + new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 20, 0, 0), StudioStation.none(), mock(DiscItems.class), identity); + try { + assertTrue(studio.completionProjectIds(player).isEmpty()); + tasks.removeFirst().run(); + assertTrue(studio.completionProjectIds(player).isEmpty()); + assertTrue(tasks.isEmpty()); + clearInvocations(store); + switch (invalidation) { + case "quit" -> studio.quit(player); + case "end" -> studio.end(player, token); + case "transferred" -> studio.transferred(owner.character()); + default -> fail("Unknown invalidation"); + } + identity.sessions.put(account, new StudioIdentity.Token(owner, 2)); + assertTrue(studio.completionProjectIds(player).isEmpty()); + assertTrue(studio.completionTracks(player).isEmpty()); + assertEquals(1, tasks.size(), "An invalidated snapshot must not retain its previous session's cooldown"); + verifyNoInteractions(store); + tasks.removeFirst().run(); + assertEquals(List.of(saved.song().id().toString()), studio.completionProjectIds(player)); + assertTrue(tasks.isEmpty()); + } finally { studio.close(); } + } + }