diff --git a/src/main/java/net/tfminecraft/musicalinstruments/InstrumentPlugin.java b/src/main/java/net/tfminecraft/musicalinstruments/InstrumentPlugin.java index 863c5ab..e9106ed 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/InstrumentPlugin.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/InstrumentPlugin.java @@ -69,7 +69,7 @@ public void onEnable() { getCommand("music").setExecutor(musicCommand); getCommand("music").setTabCompleter(musicCommand); getServer().getPluginManager().registerEvents(studioMenu, this); - getServer().getPluginManager().registerEvents(new StudioListener(studio, studioMenu), this); + getServer().getPluginManager().registerEvents(new StudioListener(this, studio, studioMenu), this); InstrumentCommand commandHandler = new InstrumentCommand(this, manager); getCommand("instruments").setExecutor(commandHandler); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/Note.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/Note.java index 02a6b5b..1783f08 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/Note.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/Note.java @@ -1,10 +1,14 @@ package net.tfminecraft.musicalinstruments.studio; +import java.util.regex.Pattern; + /** A resolved sample trigger, captured independently of subsequent config changes. */ public record Note(int tick, String instrument, String sound, float volume, float pitch) { + private static final Pattern SOUND = Pattern.compile("[a-z0-9_.-]+(:[a-z0-9_./-]+)?"); + public Note { if (tick < 0 || tick >= Song.MAX_TICKS || instrument == null || instrument.length() > 100 - || sound == null || sound.length() > 200 || !sound.matches("[a-z0-9_.-]+(:[a-z0-9_./-]+)?") + || sound == null || sound.length() > 200 || !SOUND.matcher(sound).matches() || !Float.isFinite(volume) || volume < 0 || volume > 16 || !Float.isFinite(pitch) || pitch < 0.5f || pitch > 2) { throw new IllegalArgumentException("Invalid recorded note"); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/Project.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/Project.java index a7d8baf..f8973f7 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/Project.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/Project.java @@ -6,6 +6,10 @@ public record Project(Song song, Track pending, int bpm, boolean metronome) { throw new IllegalArgumentException("Tempo must be between 40 and 240 BPM"); } if (pending != null) { + // A take uses its track's current mix. Without a saved track, as after removing it, the + // take is kept at full volume like one recorded on an empty track. + Track saved = song.track(pending.slot()); + pending = saved != null ? pending.mix(saved.gain(), saved.muted()) : pending.mix(1, false); song.replace(pending); // Validate the prospective mix, including its total note bound. } } diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/ProjectSummary.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/ProjectSummary.java new file mode 100644 index 0000000..c922c71 --- /dev/null +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/ProjectSummary.java @@ -0,0 +1,22 @@ +package net.tfminecraft.musicalinstruments.studio; + +import java.util.Comparator; +import java.util.Objects; +import java.util.UUID; + +/** What the project library shows, without loading a project's notes. */ +public record ProjectSummary(UUID id, String title, int tracks, int lengthTicks, int pendingSlot) { + public static final Comparator ORDER = Comparator.comparing(ProjectSummary::title, + String.CASE_INSENSITIVE_ORDER).thenComparing(ProjectSummary::id); + + public ProjectSummary { + Objects.requireNonNull(id); + Objects.requireNonNull(title); + } + + public static ProjectSummary of(Project project) { + Song song = project.song(); + return new ProjectSummary(song.id(), song.title(), song.tracks().size(), song.lengthTicks(), + project.pending() == null ? 0 : project.pending().slot()); + } +} diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/Song.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/Song.java index 71e1477..4aaea08 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/Song.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/Song.java @@ -41,6 +41,23 @@ public Track track(int slot) { return tracks.stream().filter(track -> track.slot() == slot).findFirst().orElse(null); } + /** Whether playback emits any sound; matches the notes {@link Timeline} schedules. */ + public boolean audible() { + return tracks.stream().anyMatch(track -> !track.muted() && track.gain() > 0 + && track.notes().stream().anyMatch(note -> note.volume() > 0)); + } + + /** The same song with only what playback emits: muted, silent and empty tracks and silent notes are left out. */ + public Song audibleMix() { + List audible = new ArrayList<>(); + for (Track track : tracks) { + if (track.muted() || track.gain() <= 0) continue; + List notes = track.notes().stream().filter(note -> note.volume() > 0).toList(); + if (!notes.isEmpty()) audible.add(new Track(track.slot(), track.lengthTicks(), track.gain(), false, notes)); + } + return new Song(id, owner, author, title, audible); + } + public Song replace(Track track) { List result = new ArrayList<>(tracks); result.removeIf(existing -> existing.slot() == track.slot()); @@ -52,8 +69,4 @@ public Song replace(Track track) { public Song remove(int slot) { return new Song(id, owner, author, title, tracks.stream().filter(t -> t.slot() != slot).toList()); } - - public Song edition() { - return new Song(UUID.randomUUID(), owner, author, title, tracks); - } } diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java index ab2dfa6..3ca005d 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioCommand.java @@ -56,7 +56,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St case "projects" -> { var projects = studio.listProjects(player); if (projects.isEmpty()) player.sendMessage("No projects yet. Open /music studio to create a song."); - for (Project project : projects) player.sendMessage(project.song().title() + " | " + project.song().id()); + for (ProjectSummary project : projects) player.sendMessage(project.title() + " | " + project.id()); } case "open" -> { need(args, 2, "open "); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java index 3a75756..bfc3470 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java @@ -11,6 +11,7 @@ import org.bukkit.event.block.Action; import org.bukkit.event.block.BlockBreakEvent; import org.bukkit.event.inventory.InventoryMoveItemEvent; +import org.bukkit.event.inventory.InventoryType; import org.bukkit.event.player.PlayerInteractEntityEvent; import org.bukkit.event.player.PlayerInteractEvent; import org.bukkit.event.player.PlayerQuitEvent; @@ -18,14 +19,17 @@ import org.bukkit.event.world.WorldUnloadEvent; import org.bukkit.inventory.EquipmentSlot; import org.bukkit.inventory.ItemStack; +import org.bukkit.plugin.Plugin; import java.io.IOException; public final class StudioListener implements Listener { + private final Plugin plugin; private final StudioService studio; private final StudioMenu menu; - public StudioListener(StudioService studio, StudioMenu menu) { + public StudioListener(Plugin plugin, StudioService studio, StudioMenu menu) { + this.plugin = plugin; this.studio = studio; this.menu = menu; } @@ -34,6 +38,8 @@ public StudioListener(StudioService studio, StudioMenu menu) { public void interact(PlayerInteractEvent event) { if (event.getAction() != Action.RIGHT_CLICK_BLOCK || event.getClickedBlock() == null || event.useInteractedBlock() == Event.Result.DENY + // Check the type first: getState() copies block entities such as chest inventories. + || event.getClickedBlock().getType() != Material.JUKEBOX || !(event.getClickedBlock().getState() instanceof Jukebox box)) { return; } @@ -128,6 +134,25 @@ public void hopper(InventoryMoveItemEvent event) { } } + @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) + public void extract(InventoryMoveItemEvent event) { + // A rejected transfer restores the source disc after the event, so check on the next tick. + // Hoppers move items constantly, so the cheap item and inventory type checks come first. + if (studio.discs().custom(event.getItem()) && event.getSource().getType() == InventoryType.JUKEBOX + && event.getSource().getLocation() != null) { + Block block = event.getSource().getLocation().getBlock(); + plugin.getServer().getScheduler().runTask(plugin, () -> { + // Do not reload a chunk that unloaded while the transfer completed. + if (block.getWorld().isChunkLoaded(block.getX() >> 4, block.getZ() >> 4) + && block.getType() == Material.JUKEBOX + && block.getState() instanceof Jukebox box + && !studio.discs().custom(box.getRecord())) { + studio.stop(block); + } + }); + } + } + @EventHandler public void quit(PlayerQuitEvent event) { studio.quit(event.getPlayer()); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioMenu.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioMenu.java index 3b64cbc..d818d9b 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioMenu.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioMenu.java @@ -88,16 +88,16 @@ private void render(Player player, Holder holder) throws IOException { } private void renderLibrary(Player player, Holder holder) throws IOException { - List projects = studio.listProjects(player); + List projects = studio.listProjects(player); inventoryItem(holder, 4, Material.BOOKSHELF, "Your songs", NamedTextColor.GOLD, projects.size() + "/36 projects", "Left-click to open a song.", "Right-click to delete a song after confirmation."); for (int index = 0; index < Math.min(projects.size(), StudioService.MAX_PROJECTS); index++) { - Project project = projects.get(index); + ProjectSummary project = projects.get(index); int slot = 9 + index; - holder.choices.put(slot, project.song().id()); - inventoryItem(holder, slot, Material.MUSIC_DISC_13, project.song().title(), NamedTextColor.YELLOW, - project.song().tracks().size() + " tracks | " + time(project.song().lengthTicks()), - project.pending() == null ? "No pending take" : "Pending take on track " + project.pending().slot(), + holder.choices.put(slot, project.id()); + inventoryItem(holder, slot, Material.MUSIC_DISC_13, project.title(), NamedTextColor.YELLOW, + project.tracks() + " tracks | " + time(project.lengthTicks()), + project.pendingSlot() == 0 ? "No pending take" : "Pending take on track " + project.pendingSlot(), "Left-click: open this project.", "Right-click: delete with confirmation."); } if (projects.isEmpty()) inventoryItem(holder, 22, Material.WRITABLE_BOOK, "Your studio is empty", NamedTextColor.GOLD, diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java index 698366d..787ab53 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java @@ -21,20 +21,25 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; /** All capture/playback uses one main-thread clock, including overdub monitoring. */ public final class StudioService { public static final int MAX_PROJECTS = 36; + static final long RETRY_TICKS = 1200; + static final long SHUTDOWN_WAIT_SECONDS = 30; private final InstrumentPlugin plugin; private final InstrumentManager manager; private final StudioStore store; @@ -44,6 +49,17 @@ public final class StudioService { private final Map projects = new HashMap<>(); private final Map recordings = new HashMap<>(); private final Map previews = new HashMap<>(); + private final Map> privateSounds = new HashMap<>(); + // Each owner's latest save. An in-flight snapshot is immutable; pending snapshots of the same + // owner share a future and are replaced with the newest project instead of filling the executor. + private final Map> pendingSaves = new HashMap<>(); + // Accessed by the main thread and the writer only while holding this map's monitor. + private final Map queuedSaves = new HashMap<>(); + // Owners whose cached project is newer than its files: a write is still queued, or the last one failed. + // Other cached projects match the disk, so disconnects and shutdown need not write them again. + private final Set unsaved = new HashSet<>(); + // Clock tick of each player's last publication, kept only while the cooldown applies. + private final Map lastPublished = new HashMap<>(); private final Map jukeboxes = new HashMap<>(); private final Map editions = new LinkedHashMap<>(16, 0.75f, true) { @Override @@ -52,16 +68,17 @@ protected boolean removeEldestEntry(Map.Entry eldest) { } }; private final BukkitTask task; - private final ExecutorService checkpointWriter = Executors.newSingleThreadExecutor( - Thread.ofPlatform().daemon(true).name("musical-instruments-checkpoints").factory()); + private final ExecutorService checkpointWriter; private long clock; + private boolean closed; + private volatile long shutdownDeadline; public StudioService(InstrumentPlugin plugin, InstrumentManager manager) throws IOException { this(plugin, manager, loadSettings(plugin)); } private StudioService(InstrumentPlugin plugin, InstrumentManager manager, YamlConfiguration yaml) throws IOException { - this(plugin, manager, new StudioStore(plugin.getDataFolder().toPath().resolve("studio")), StudioSettings.read(yaml), + this(plugin, manager, new StudioStore(plugin.getDataFolder().toPath().resolve("studio"), plugin.getLogger()), StudioSettings.read(yaml), StudioStation.parse(yaml.getString("station", ""), plugin.getLogger()), new DiscItems(plugin)); } @@ -80,12 +97,19 @@ private static YamlConfiguration loadSettings(InstrumentPlugin plugin) throws IO StudioService(InstrumentPlugin plugin, InstrumentManager manager, StudioStore store, StudioSettings settings, StudioStation station, DiscItems discs) { + this(plugin, manager, store, settings, station, discs, Executors.newSingleThreadExecutor( + Thread.ofPlatform().daemon(true).name("musical-instruments-checkpoints").factory())); + } + + StudioService(InstrumentPlugin plugin, InstrumentManager manager, StudioStore store, StudioSettings settings, + StudioStation station, DiscItems discs, ExecutorService checkpointWriter) { this.plugin = plugin; this.manager = manager; this.settings = settings; this.station = station; this.store = store; this.discs = discs; + this.checkpointWriter = checkpointWriter; task = plugin.getServer().getScheduler().runTaskTimer(plugin, this::tick, 1L, 1L); } @@ -127,14 +151,15 @@ public Project requireProject(Player player) throws IOException { } public void create(Player player, String title, boolean reset) throws IOException { - requireIdle(player); - if (!reset) requireProjectSpace(player); + requireSaved(player); Project previous = project(player); - if (previous != null) save(player, previous); // Also preserve a take retained after a failed write. + // Without an active project to replace, a reset creates another project. + if (!reset || previous == null) requireProjectSpace(player); + persist(player); // A project whose last write failed is written before another replaces it. UUID id = reset && previous != null ? previous.song().id() : UUID.randomUUID(); save(player, new Project(new Song(id, player.getUniqueId(), player.getName(), title, List.of()), null, 100, true)); - previews.remove(player.getUniqueId()); + stopPreview(player); } public void requireProjectSpace(Player player) throws IOException { @@ -143,61 +168,99 @@ public void requireProjectSpace(Player player) throws IOException { } } + /** Loads only the requested project; a cached version may contain changes not yet written. */ public Project project(Player player, UUID id) throws IOException { - Project current = project(player); - return current != null && current.song().id().equals(id) ? current : store.project(player.getUniqueId(), id); + Project current = projects.get(player.getUniqueId()); + if (current != null && current.song().id().equals(id)) return current; + return store.project(player.getUniqueId(), id); } + /** Deletes the project only if it still matches what the player confirmed. */ public void deleteProject(Player player, Project expected) throws IOException { - requireIdle(player); - if (!expected.song().owner().equals(player.getUniqueId())) throw new IllegalArgumentException("This is not your project"); + requireSaved(player); // A queued write would bring the deleted project back. + requireOwner(player, expected); UUID id = expected.song().id(); - if (!project(player, id).equals(expected)) throw new IllegalArgumentException("The project changed. Open the delete confirmation again"); - Project current = project(player); + // Summaries omit notes, mix and settings, so they cannot protect a stale confirmation. + if (!project(player, id).equals(expected)) { + throw new IllegalArgumentException("The project changed. Open the delete confirmation again"); + } store.delete(player.getUniqueId(), id); + Project current = projects.get(player.getUniqueId()); if (current != null && current.song().id().equals(id)) { projects.remove(player.getUniqueId()); - previews.remove(player.getUniqueId()); + unsaved.remove(player.getUniqueId()); + stopPreview(player); } player.sendMessage("Deleted '" + expected.song().title() + "'. Published discs keep their music."); } - public List listProjects(Player player) throws IOException { - Map all = new LinkedHashMap<>(); - for (Project project : store.projects(player.getUniqueId())) all.put(project.song().id(), project); - Project active = project(player); - if (active != null) all.put(active.song().id(), active); - return all.values().stream().sorted(java.util.Comparator.comparing((Project project) -> project.song().title(), - String.CASE_INSENSITIVE_ORDER).thenComparing(project -> project.song().id())).toList(); + public List listProjects(Player player) throws IOException { + Map all = new LinkedHashMap<>(); + for (ProjectSummary summary : store.summaries(player.getUniqueId())) all.put(summary.id(), summary); + // An uncached project matches its files; a cached one may be newer than them. + Project active = projects.get(player.getUniqueId()); + if (active != null) all.put(active.song().id(), ProjectSummary.of(active)); + return all.values().stream().sorted(ProjectSummary.ORDER).toList(); } public void selectProject(Player player, UUID id) throws IOException { Project current = project(player); if (current != null && current.song().id().equals(id)) return; - requireIdle(player); + requireSaved(player); // The next project is read from its file, which a queued write could still change. Project next = store.project(player.getUniqueId(), id); - if (current != null) save(player, current); - save(player, next); - previews.remove(player.getUniqueId()); + persist(player); + store.select(player.getUniqueId(), id); // Both projects already match their files. + projects.put(player.getUniqueId(), next); + stopPreview(player); } + /** Writes the project before it becomes the active one. Used when creating or replacing a project. */ public void save(Player player, Project project) throws IOException { - if (!project.song().owner().equals(player.getUniqueId())) { - throw new IllegalArgumentException("This is not your project"); - } + requireOwner(player, project); + requireSaved(player); store.save(project); // Persist successfully before replacing the in-memory version. projects.put(player.getUniqueId(), project); + unsaved.remove(player.getUniqueId()); } - public void edit(Player player, Project project) throws IOException { + /** + * Applies a change to the active project at once and writes it in the background, after any + * earlier write. If that write fails, the change stays in memory and is written again. + */ + public void edit(Player player, Project project) { + requireOwner(player, project); requireIdle(player); - save(player, project); + projects.put(player.getUniqueId(), project); + unsaved.add(player.getUniqueId()); + queueSave(project); + } + + private static void requireOwner(Player player, Project project) { + if (!project.song().owner().equals(player.getUniqueId())) { + throw new IllegalArgumentException("This is not your project"); + } + } + + // Retries a write that failed. A project with no failed write already matches its files. + private void persist(Player player) throws IOException { + Project current = projects.get(player.getUniqueId()); + if (unsaved.contains(player.getUniqueId())) save(player, current); } public void requireIdle(Player player) { if (recordings.containsKey(player.getUniqueId())) { throw new IllegalArgumentException("Stop the recording with /music stop first"); } + CompletableFuture pending = pendingSaves.get(player.getUniqueId()); + if (pending != null && pending.isDone()) completeSave(player.getUniqueId(), pending); + } + + /** Also requires the project's files to be current, for changes that read or remove them. */ + public void requireSaved(Player player) { + requireIdle(player); + if (pendingSaves.containsKey(player.getUniqueId())) { + throw new IllegalArgumentException("Your project is still saving. Try again shortly"); + } } public void record(Player player, int slot) throws IOException { @@ -214,7 +277,7 @@ public void record(Player player, int slot) throws IOException { && manager.getInstrument(player.getInventory().getItemInMainHand()) == null) { throw new IllegalArgumentException("Hold an instrument in your hand or off-hand"); } - previews.remove(player.getUniqueId()); + stopPreview(player); capacity(); recordings.put(player.getUniqueId(), new Capture(project, slot, clock + settings.countInTicks())); player.sendMessage("Recording starts in " + settings.countInTicks() / 20 + " seconds. Play with the hotbar, " @@ -250,7 +313,7 @@ public void capture(Player player, String instrument, String sound, float volume } public void stop(Player player) { - previews.remove(player.getUniqueId()); + stopPreview(player); Capture capture = recordings.get(player.getUniqueId()); if (capture != null) { finish(player, capture, "Recording stopped"); @@ -264,14 +327,19 @@ public void preview(Player player, boolean pending) throws IOException { Project project = requireProject(player); Song song = project.song(); if (pending) { - if (project.pending() == null) { + Track take = project.pending(); + if (take == null) { throw new IllegalArgumentException("No take to preview"); } - song = song.replace(project.pending()); + // The take carries its track's mix, but reviewing it must be audible even on a muted or silent track. + song = song.replace(take.mix(take.gain() > 0 ? take.gain() : 1, false)); } if (song.tracks().isEmpty()) { throw new IllegalArgumentException("Record and keep a track first"); } + if (!song.audible()) { + throw new IllegalArgumentException("Every saved track is muted or silent. Unmute or raise a track first"); + } previewSong(player, song); player.sendMessage(pending ? "Previewing the new take with the other tracks." : "Previewing the saved mix."); } @@ -282,16 +350,23 @@ public void previewTrack(Player player, int slot) throws IOException { Track track = song.track(slot); if (track == null) throw new IllegalArgumentException("Record and keep this track first"); if (track.muted()) throw new IllegalArgumentException("Unmute this track to listen to it"); - previewSong(player, new Song(song.id(), song.owner(), song.author(), song.title(), List.of(track))); + Song single = new Song(song.id(), song.owner(), song.author(), song.title(), List.of(track)); + if (!single.audible()) throw new IllegalArgumentException("Raise this track's volume to listen to it"); + previewSong(player, single); player.sendMessage("Listening to saved track " + slot + "."); } private void previewSong(Player player, Song song) { - previews.remove(player.getUniqueId()); + stopPreview(player); capacity(); previews.put(player.getUniqueId(), new Preview(song, clock + 1)); } + private void stopPreview(Player player) { + previews.remove(player.getUniqueId()); + stopPrivateSounds(player); + } + public void makeBlank(Player player) { ItemStack held = player.getInventory().getItemInMainHand(); if (!ordinaryDisc(held) || held.getAmount() != 1) { @@ -332,11 +407,19 @@ private Song publishEdition(Player player) throws IOException { if (project.pending() != null) { throw new IllegalArgumentException("Keep or discard your pending take first"); } - if (new Timeline(project.song(), -1).due(Song.MAX_TICKS).isEmpty()) { + if (!project.song().audible()) { throw new IllegalArgumentException("The mix needs at least one audible note"); } - Song edition = project.song().edition(); - store.publish(edition); // No item is consumed when publication fails. + // Each publication of changed music adds a file that discs may reference forever. + lastPublished.values().removeIf(at -> clock - at >= settings.publishCooldownTicks()); + Long last = lastPublished.get(player.getUniqueId()); + if (last != null) { + long seconds = (last + settings.publishCooldownTicks() - clock + 19) / 20; + throw new IllegalArgumentException("Wait " + seconds + " seconds before publishing again"); + } + // No item is consumed when publication fails. + Song edition = store.publishEdition(project.song(), settings.editionsPerDay()); + lastPublished.put(player.getUniqueId(), clock); editions.put(edition.id(), edition); return edition; } @@ -482,6 +565,8 @@ private void capacity() { private void tick() { clock++; + completeSaves(); + if (clock % RETRY_TICKS == 0) retryFailedSaves(); for (var entry : new ArrayList<>(recordings.entrySet())) { Player player = plugin.getServer().getPlayer(entry.getKey()); if (player == null) { @@ -520,14 +605,7 @@ private void tick() { if (capture.checkpoint.isCompletedExceptionally()) { finish(player, capture, "Checkpoint failed; recording stopped to protect your take"); } else if (capture.checkpoint.isDone()) { - Project snapshot = capture.snapshot((int) elapsed); - capture.checkpoint = CompletableFuture.runAsync(() -> { - try { - store.save(snapshot); - } catch (IOException ex) { - throw new CompletionException(ex); - } - }, checkpointWriter); + capture.checkpoint = write(capture.snapshot((int) elapsed)); } } } @@ -538,6 +616,8 @@ private void tick() { int elapsed = (int) (clock - preview.start); if (player == null || elapsed >= preview.song.lengthTicks() + settings.tailTicks()) { it.remove(); + if (player != null) stopPrivateSounds(player); + else privateSounds.remove(entry.getKey()); } else if (elapsed >= 0) { emit(player, preview.timeline.due(elapsed)); } @@ -553,8 +633,9 @@ private void tick() { } Block block = world.getBlockAt(key.x, key.y, key.z); int elapsed = (int) (clock - playback.start); - if (!(block.getState() instanceof Jukebox box) - || !playback.song.id().equals(discs.songId(box.getRecord())) + // The block type is cheap to read every tick. Reading the record copies the block state and + // item, so it is checked once a second; ejection and breaking stop playback immediately. + if (block.getType() != Material.JUKEBOX || elapsed % 20 == 0 && !holds(block, playback.song) || elapsed >= playback.song.lengthTicks() + settings.tailTicks()) { it.remove(); continue; @@ -566,7 +647,7 @@ private void tick() { for (Player listener : world.getPlayers()) { if (listener.getLocation().distanceSquared(origin) <= settings.radius() * settings.radius()) { for (Timeline.Trigger trigger : due) { - sample(listener, origin, trigger); + sample(listener, origin, trigger, SoundCategory.RECORDS); } } } @@ -575,82 +656,171 @@ private void tick() { } } + private boolean holds(Block block, Song song) { + return block.getState() instanceof Jukebox box && song.id().equals(discs.songId(box.getRecord())); + } + private void emit(Player player, List due) { for (Timeline.Trigger trigger : due) { - sample(player, player.getLocation(), trigger); + privateSounds.computeIfAbsent(player.getUniqueId(), key -> new HashSet<>()).add(trigger.note().sound()); + sample(player, player.getLocation(), trigger, SoundCategory.PLAYERS); } } - private void sample(Player player, Location origin, Timeline.Trigger trigger) { + private void sample(Player player, Location origin, Timeline.Trigger trigger, SoundCategory category) { Note note = trigger.note(); - player.playSound(origin, note.sound(), SoundCategory.RECORDS, + player.playSound(origin, note.sound(), category, Math.min(16, note.volume() * trigger.gain()), note.pitch()); } private void click(Player player) { - player.playSound(player.getLocation(), "block.note_block.hat", SoundCategory.RECORDS, 0.35f, 1); + privateSounds.computeIfAbsent(player.getUniqueId(), key -> new HashSet<>()).add("block.note_block.hat"); + player.playSound(player.getLocation(), "block.note_block.hat", SoundCategory.PLAYERS, 0.35f, 1); } private void finish(Player player, Capture capture, String reason) { recordings.remove(player.getUniqueId()); + stopPreview(player); player.sendActionBar(Component.empty()); if (capture.notes.isEmpty()) { player.sendMessage(reason + ". No notes captured; the previous track was preserved."); return; } Project result = capture.snapshot((int) Math.max(1, Math.min(settings.durationTicks(), clock - capture.start + 1))); - // Retain the take in memory even if the disk fails, so /music keep can retry. + // The take is kept in memory even if the disk fails; it can be reviewed while it is written. projects.put(player.getUniqueId(), result); - // An older asynchronous checkpoint must finish before this final synchronous write. - try { - capture.checkpoint.join(); - } catch (CompletionException ex) { - plugin.getLogger().log(Level.WARNING, "Recording checkpoint failed; retrying final save", ex.getCause()); + unsaved.add(player.getUniqueId()); + queueSave(result); + player.sendMessage(reason + ". Review the take: /music preview take, then /music keep or /music discard."); + } + + private void queueSave(Project project) { + pendingSaves.put(project.song().owner(), write(project)); + } + + // One queued snapshot per owner, plus the single snapshot currently being written. Changes + // arriving during a write go to the tail, letting other owners save before this owner runs again. + private CompletableFuture write(Project project) { + UUID owner = project.song().owner(); + synchronized (queuedSaves) { + QueuedSave queued = queuedSaves.get(owner); + if (queued != null) { + queued.project = project; + return queued.future; + } + QueuedSave next = new QueuedSave(project); + queuedSaves.put(owner, next); + CompletableFuture.runAsync(() -> { + Project snapshot; + synchronized (queuedSaves) { + queuedSaves.remove(owner); + snapshot = next.project; + } + saveSnapshot(snapshot); + }, checkpointWriter).whenComplete((ignored, error) -> { + if (error == null) next.future.complete(null); + else next.future.completeExceptionally(error); + }); + return next.future; } + } + + private void saveSnapshot(Project project) { try { - store.save(result); - player.sendMessage(reason + ". Take saved: /music preview take, /music keep, or /music discard."); + try { + store.save(project); + } catch (IOException ex) { + // A failure can appear only after close has checked the pending saves. Retry it + // once on this same worker, even after shutdown(), while the close budget remains. + long deadline = shutdownDeadline; + if (deadline == 0 || System.nanoTime() - deadline >= 0) throw ex; + store.save(project); + } } catch (IOException ex) { - failure(player, ex); + plugin.getLogger().log(Level.SEVERE, "Could not save studio project for " + project.song().owner(), ex); + throw new CompletionException(ex); + } + } + + private void completeSave(UUID owner, CompletableFuture pending) { + pendingSaves.remove(owner); + Player player = plugin.getServer().getPlayer(owner); + if (pending.isCompletedExceptionally()) { + // Retried by the next change, every minute, on disconnect and on shutdown. + if (player != null) player.sendMessage("Could not save your project. Your changes are kept and will be saved again; " + + "contact a server administrator if this continues."); + return; + } + // The latest write holds the cached project, so it now matches its files. + unsaved.remove(owner); + if (player == null) projects.remove(owner); + } + + private void completeSaves() { + for (var entry : new ArrayList<>(pendingSaves.entrySet())) { + // Only completed writes: this never waits on disk from the musical clock. + if (entry.getValue().isDone()) completeSave(entry.getKey(), entry.getValue()); + } + } + + // Queues another write for every cached project whose last write failed. A recording is skipped: + // its checkpoints and final take include the cached project, which still lacks the take and would erase it. + private void retryFailedSaves() { + for (UUID owner : unsaved) { + if (!pendingSaves.containsKey(owner) && !recordings.containsKey(owner)) queueSave(projects.get(owner)); } } public void quit(Player player) { Capture capture = recordings.get(player.getUniqueId()); if (capture != null) { - finish(player, capture, "Recording saved on disconnect"); - } - previews.remove(player.getUniqueId()); - Project project = projects.get(player.getUniqueId()); - if (project != null) { - try { - store.save(project); - projects.remove(player.getUniqueId()); - } catch (IOException ex) { - failure(player, ex); - } + finish(player, capture, "Recording stopped on disconnect"); } + stopPreview(player); + UUID owner = player.getUniqueId(); + if (pendingSaves.containsKey(owner)) return; // completeSave evicts the project once it is written. + if (unsaved.contains(owner)) queueSave(projects.get(owner)); // Retry a write that failed. + else projects.remove(owner); // Its files are current; the next visit reloads it. } public void close() { + if (closed) return; + shutdownDeadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(SHUTDOWN_WAIT_SECONDS); + closed = true; task.cancel(); + // Observe completed failures before deciding which cached takes need another save. + completeSaves(); for (var entry : new ArrayList<>(recordings.entrySet())) { Player player = plugin.getServer().getPlayer(entry.getKey()); if (player != null) { - finish(player, entry.getValue(), "Recording saved on shutdown"); + finish(player, entry.getValue(), "Recording stopped on shutdown"); } } - for (Project project : projects.values()) { - try { - store.save(project); - } catch (IOException ex) { - plugin.getLogger().log(Level.SEVERE, "Could not save studio project on shutdown", ex); - } + retryFailedSaves(); + for (UUID owner : new ArrayList<>(privateSounds.keySet())) { + Player player = plugin.getServer().getPlayer(owner); + if (player != null) stopPreview(player); } jukeboxes.clear(); previews.clear(); recordings.clear(); + privateSounds.clear(); checkpointWriter.shutdown(); + try { + // Losing a take is worse than a slower shutdown, but a stuck disk must not hang it forever. + if (!checkpointWriter.awaitTermination(SHUTDOWN_WAIT_SECONDS, TimeUnit.SECONDS)) { + plugin.getLogger().severe("Studio saves are still pending after the shutdown timeout"); + } + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + plugin.getLogger().log(Level.SEVERE, "Interrupted while saving studio projects on shutdown", ex); + } + completeSaves(); + } + + private void stopPrivateSounds(Player player) { + Set sounds = privateSounds.remove(player.getUniqueId()); + if (sounds != null) for (String sound : sounds) player.stopSound(sound, SoundCategory.PLAYERS); } public void failure(Player player, IOException ex) { @@ -658,6 +828,15 @@ public void failure(Player player, IOException ex) { player.sendMessage("Could not read/write the recording. Your disc was preserved; contact a server administrator."); } + private static final class QueuedSave { + private Project project; + private final CompletableFuture future = new CompletableFuture<>(); + + private QueuedSave(Project project) { + this.project = project; + } + } + private static final class Capture { private final Project project; private final int slot; diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioSettings.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioSettings.java index dc6e839..4a82804 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioSettings.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioSettings.java @@ -3,7 +3,8 @@ import org.bukkit.configuration.file.YamlConfiguration; public record StudioSettings(int tracks, int durationTicks, int notesPerTrack, int notesPerSong, - int sessions, int radius, int countInTicks, int tailTicks, int checkpointTicks) { + int sessions, int radius, int countInTicks, int tailTicks, int checkpointTicks, + int publishCooldownTicks, int editionsPerDay) { public static StudioSettings read(YamlConfiguration config) { return new StudioSettings(bound(config, "max-tracks", 4, 1, Song.MAX_TRACKS), bound(config, "max-duration-seconds", 180, 1, Song.MAX_TICKS / 20) * 20, @@ -13,7 +14,9 @@ public static StudioSettings read(YamlConfiguration config) { bound(config, "jukebox-radius", 32, 1, 128), bound(config, "count-in-seconds", 3, 1, 10) * 20, bound(config, "sample-tail-seconds", 3, 0, 10) * 20, - bound(config, "checkpoint-ticks", 200, 20, 1200)); + bound(config, "checkpoint-ticks", 200, 20, 1200), + bound(config, "publish-cooldown-seconds", 30, 0, 3600) * 20, + bound(config, "max-editions-per-day", 20, 0, 1000)); } private static int bound(YamlConfiguration config, String key, int fallback, int min, int max) { diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java index 34b062d..f391866 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java @@ -9,7 +9,11 @@ import org.bukkit.plugin.Plugin; import org.bukkit.util.BoundingBox; +import java.lang.reflect.Method; import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; /** @@ -20,6 +24,7 @@ public final class StudioStation { /** Squared reach within which a player can keep using the station's controls. */ private static final double REACH_SQUARED = 36; + private static final Map> METHODS = new ConcurrentHashMap<>(); private final Material material; private final String itemsAdderId; @@ -91,6 +96,10 @@ public boolean matches(Block block) { if (material != null) { return block.getType() == material; } + // This runs on every right-click on a block; without ItemsAdder each lookup would fail slowly. + if (!pluginEnabled("ItemsAdder")) { + return false; + } if (itemsAdderId.equals(namespacedId("dev.lone.itemsadder.api.CustomBlock", "byAlreadyPlaced", Block.class, block))) { return true; } @@ -107,7 +116,7 @@ public boolean matches(Block block) { } public boolean matches(Entity entity) { - return itemsAdderId != null && entity != null + return itemsAdderId != null && entity != null && pluginEnabled("ItemsAdder") && itemsAdderId.equals(namespacedId("dev.lone.itemsadder.api.CustomFurniture", "byAlreadySpawned", Entity.class, entity)); } @@ -135,13 +144,34 @@ public Block require(Player player, Location location) { // CustomBlock.byAlreadyPlaced(block) / CustomFurniture.byAlreadySpawned(entity), then getNamespacedID(). private static String namespacedId(String type, String lookup, Class parameter, Object target) { try { - Object custom = Class.forName(type).getMethod(lookup, parameter).invoke(null, target); - return custom == null ? null : (String) custom.getClass().getMethod("getNamespacedID").invoke(custom); + Object custom = method(type + "#" + lookup + "#" + parameter.getName(), + () -> Class.forName(type).getMethod(lookup, parameter)).invoke(null, target); + return custom == null ? null : (String) method(custom.getClass().getName() + "#getNamespacedID", + () -> custom.getClass().getMethod("getNamespacedID")).invoke(custom); } catch (ReflectiveOperationException | RuntimeException ex) { return null; } } + @FunctionalInterface + private interface Lookup { Method find() throws ReflectiveOperationException; } + + // matches(Block) runs on every right-click on a block and every studio menu refresh, so each + // reflective lookup is resolved once. Lookups run only while ItemsAdder is enabled, so a method + // missing from that ItemsAdder version stays missing and is remembered too. + private static Method method(String key, Lookup lookup) throws ReflectiveOperationException { + Optional method = METHODS.get(key); + if (method == null) { + try { + method = Optional.of(lookup.find()); + } catch (NoSuchMethodException ex) { + method = Optional.empty(); + } + METHODS.put(key, method); + } + return method.orElseThrow(() -> new NoSuchMethodException(key)); + } + private static boolean pluginEnabled(String name) { Plugin plugin = Bukkit.getPluginManager().getPlugin(name); return plugin != null && plugin.isEnabled(); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java index 5eed80b..9821dfa 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java @@ -4,81 +4,252 @@ import org.bukkit.configuration.file.YamlConfiguration; import java.io.IOException; +import java.nio.charset.CharacterCodingException; import java.nio.charset.StandardCharsets; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.time.Clock; import java.util.ArrayList; -import java.util.Comparator; +import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.TreeMap; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; /** Versioned, bounded data files. A failed write never truncates the last saved project. */ public final class StudioStore { + // Enough for a full 12,000-note song plus a replacement take, even with a distinct long name per note. + static final int MAX_FILE_BYTES = 32_000_000; + // Format 1, written by 3.3.0 with a YAML list per note, is not read. + static final int FORMAT = 2; + // Neither is a .yml file, so project listings skip them. + static final String INDEX = "library.index"; + // Holds the selected project's ID, so the selection is not a second copy of the project. + static final String SELECTED = "selected"; + // Stands in for the edition ID while hashing, so the ID depends only on the music and its labels. + private static final UUID CONTENT_ID = new UUID(0, 0); + private static final long DAY_MILLIS = TimeUnit.DAYS.toMillis(1); private final Path root; + private final Logger logger; + private final Clock clock; + // Unreadable files already logged, with the size and time they had then, so each is reported once. + private final Map reported = new ConcurrentHashMap<>(); + + /** A file whose contents cannot be decoded, unlike a failure to reach the disk. */ + static final class DamagedFileException extends IOException { + DamagedFileException(String message, Throwable cause) { + super(message, cause); + } + } public StudioStore(Path root) throws IOException { + this(root, Logger.getLogger(StudioStore.class.getName())); + } + + public StudioStore(Path root, Logger logger) throws IOException { + this(root, logger, Clock.systemUTC()); + } + + StudioStore(Path root, Logger logger, Clock clock) throws IOException { this.root = root; + this.logger = logger; + this.clock = clock; Files.createDirectories(root.resolve("projects")); Files.createDirectories(root.resolve("songs")); + // A crash between writing and moving leaves a temporary file; it never holds the only copy. + try (var files = Files.walk(root, 3)) { + for (Path file : files.filter(StudioStore::temporary).toList()) Files.deleteIfExists(file); + } + } + + private static boolean temporary(Path file) { + String name = file.getFileName().toString(); + return name.startsWith("studio-") && name.endsWith(".tmp"); } public Optional project(UUID owner) throws IOException { - Path file = root.resolve("projects").resolve(owner + ".yml"); - if (!Files.exists(file)) { + UUID id = selected(owner); + Path file = id == null ? null : projectFile(owner, id); + if (file == null || !Files.exists(file)) return Optional.empty(); + try { + return Optional.of(decodeProject(file, owner, id)); + } catch (DamagedFileException ex) { + // Without an active project the player can still create or open another one. + report(file, ex.getMessage()); return Optional.empty(); } - Project selected = decodeProject(file, owner, null); - Path current = projectFile(owner, selected.song().id()); - return Optional.of(Files.exists(current) && Files.getLastModifiedTime(current).compareTo(Files.getLastModifiedTime(file)) > 0 - ? decodeProject(current, owner, selected.song().id()) : selected); } public Project project(UUID owner, UUID id) throws IOException { Path file = projectFile(owner, id); - if (Files.exists(file)) return decodeProject(file, owner, id); - return project(owner).filter(project -> project.song().id().equals(id)) - .orElseThrow(() -> new IllegalArgumentException("That project is no longer available")); + if (!Files.exists(file)) throw new IllegalArgumentException("That project is no longer available"); + return decodeProject(file, owner, id); } - public List projects(UUID owner) throws IOException { - Map result = new LinkedHashMap<>(); - project(owner).ifPresent(project -> result.put(project.song().id(), project)); - Path directory = root.resolve("projects").resolve(owner.toString()); + /** The owner's projects for the library, sorted by title, without loading their notes. */ + public List summaries(UUID owner) throws IOException { + return index(owner).values().stream().sorted(ProjectSummary.ORDER).toList(); + } + + private Path directory(UUID owner) { + return root.resolve("projects").resolve(owner.toString()); + } + + private Path projectFile(UUID owner, UUID id) { + return directory(owner).resolve(id + ".yml"); + } + + private UUID selected(UUID owner) throws IOException { + Path file = directory(owner).resolve(SELECTED); + if (!Files.exists(file)) return null; + try { + // Invalid UTF-8 becomes replacement characters, which no project ID contains. + return UUID.fromString(Files.size(file) > 64 ? "" : new String(Files.readAllBytes(file), StandardCharsets.UTF_8).strip()); + } catch (IllegalArgumentException ex) { + report(file, "it does not hold a project ID; the next save replaces it"); + return null; + } + } + + private void report(Path file, String problem) throws IOException { + String stamp = stamp(file); + if (!stamp.equals(reported.put(file, stamp))) { + logger.warning("Skipping unreadable studio file " + file + ": " + problem + + ". The player's other projects remain available; repair or remove the file."); + } + } + + /** Makes an existing project the owner's active one, without rewriting it. */ + public void select(UUID owner, UUID id) throws IOException { + if (!Files.exists(projectFile(owner, id))) throw new IllegalArgumentException("That project is no longer available"); + point(owner, id); + } + + private void point(UUID owner, UUID id) throws IOException { + if (!id.equals(selected(owner))) write(directory(owner).resolve(SELECTED), id.toString().getBytes(StandardCharsets.UTF_8), true); + } + + public void delete(UUID owner, UUID id) throws IOException { + // Validates identity and ownership of every project file. + if (!index(owner).containsKey(id.toString())) { + throw new IllegalArgumentException("That project is no longer available"); + } + if (id.equals(selected(owner))) Files.delete(directory(owner).resolve(SELECTED)); + Files.deleteIfExists(projectFile(owner, id)); + } + + // Library summaries are cached in one small file per owner, keyed by each project file's size and + // modification time, so listing parses only files written outside save(). The cache is rebuilt when + // missing, stale or unreadable; concurrent updates may lose an entry, which costs one reparse later. + private Map index(UUID owner) throws IOException { + Path directory = directory(owner); + Map> cached = readIndex(directory); + Map> fresh = new TreeMap<>(); + Map result = new LinkedHashMap<>(); if (Files.isDirectory(directory)) { try (var files = Files.list(directory)) { for (Path file : files.filter(path -> path.getFileName().toString().endsWith(".yml")).toList()) { + String name = file.getFileName().toString(); UUID id; try { - String name = file.getFileName().toString(); id = UUID.fromString(name.substring(0, name.length() - 4)); } catch (IllegalArgumentException ex) { - throw new IOException("Invalid project filename: " + file.getFileName(), ex); + report(file, "its name is not a project ID"); + continue; } - Project project = decodeProject(file, owner, id); - result.putIfAbsent(id, project); + summarize(id.toString(), file, owner, id, cached, fresh, result); } } } - return result.values().stream().sorted(Comparator.comparing((Project project) -> project.song().title(), - String.CASE_INSENSITIVE_ORDER).thenComparing(project -> project.song().id())).toList(); + if (!fresh.equals(cached)) writeIndex(directory, fresh); + return result; } - private Path projectFile(UUID owner, UUID id) { - return root.resolve("projects").resolve(owner.toString()).resolve(id + ".yml"); + // One unreadable project must not hide the others, nor stop the player from creating or deleting + // projects. It is left on disk, logged and skipped; the index remembers it until the file changes. + private void summarize(String key, Path file, UUID owner, UUID id, Map> cached, + Map> fresh, Map result) throws IOException { + String stamp = stamp(file); + List entry = cached.get(key); + List damaged = List.of(stamp, "unreadable"); // Not a project ID, so never a valid summary. + if (damaged.equals(entry)) { + report(file, "it could not be read when the library was last listed"); + fresh.put(key, damaged); + return; + } + ProjectSummary summary = decodeSummary(entry, stamp, id); + if (summary == null) { + try { + summary = ProjectSummary.of(decodeProject(file, owner, id)); + } catch (DamagedFileException ex) { + report(file, ex.getMessage()); + fresh.put(key, damaged); + return; + } + } + fresh.put(key, encodeSummary(stamp, summary)); + result.put(key, summary); } - public void delete(UUID owner, UUID id) throws IOException { - project(owner, id); // Validate identity and ownership before removing any file. - Optional selected = project(owner); - if (selected.isPresent() && selected.get().song().id().equals(id)) { - Files.deleteIfExists(root.resolve("projects").resolve(owner + ".yml")); + // Records a saved project's summary so the next listing need not parse the file just written. + private void remember(Project project, Path file) throws IOException { + Map> entries = readIndex(file.getParent()); + ProjectSummary summary = ProjectSummary.of(project); + entries.put(summary.id().toString(), encodeSummary(stamp(file), summary)); + writeIndex(file.getParent(), entries); + } + + private static String stamp(Path file) throws IOException { + return Files.size(file) + ":" + Files.getLastModifiedTime(file).to(TimeUnit.MICROSECONDS); + } + + private static List encodeSummary(String stamp, ProjectSummary summary) { + return List.of(stamp, summary.id().toString(), summary.title(), summary.tracks(), summary.lengthTicks(), + summary.pendingSlot()); + } + + private ProjectSummary decodeSummary(List entry, String stamp, UUID id) { + if (entry == null) return null; + try { + if (!stamp.equals(entry.getFirst())) return null; + ProjectSummary summary = new ProjectSummary(UUID.fromString((String) entry.get(1)), (String) entry.get(2), + integer(entry.get(3)), integer(entry.get(4)), integer(entry.get(5))); + return summary.id().equals(id) ? summary : null; + } catch (RuntimeException ex) { + return null; // A malformed entry is rebuilt from its project file. + } + } + + private Map> readIndex(Path directory) { + Map> entries = new TreeMap<>(); + try { + YamlConfiguration yaml = read(directory.resolve(INDEX)); + for (String key : yaml.getKeys(false)) { + if (yaml.get(key) instanceof List entry) entries.put(key, entry); + } + } catch (IOException ex) { + // A missing or unreadable cache is rebuilt from the project files. + } + return entries; + } + + private void writeIndex(Path directory, Map> entries) { + YamlConfiguration yaml = new YamlConfiguration(); + yaml.set("format", FORMAT); + entries.forEach(yaml::set); + try { + Files.createDirectories(directory); + write(directory.resolve(INDEX), yaml, true); + } catch (IOException ex) { + // The cache is optional: listing still works and retries the write next time. } - Files.deleteIfExists(projectFile(owner, id)); } private Project decodeProject(Path file, UUID owner, UUID id) throws IOException { @@ -88,11 +259,11 @@ private Project decodeProject(Path file, UUID owner, UUID id) throws IOException if (!song.owner().equals(owner)) { throw new IllegalArgumentException("Project owner mismatch"); } - if (id != null && !song.id().equals(id)) throw new IllegalArgumentException("Project ID mismatch"); + if (!song.id().equals(id)) throw new IllegalArgumentException("Project ID mismatch"); Track pending = yaml.isSet("pending") ? decodeTrack(map(yaml.get("pending"))) : null; return new Project(song, pending, yaml.getInt("bpm"), yaml.getBoolean("metronome")); } catch (RuntimeException ex) { - throw new IOException("Invalid project " + owner, ex); + throw new DamagedFileException("Invalid project " + owner, ex); } } @@ -105,23 +276,17 @@ public Song song(UUID id) throws IOException { } return song; } catch (RuntimeException ex) { - throw new IOException("Invalid song " + id, ex); + throw new DamagedFileException("Invalid song " + id, ex); } } public void save(Project project) throws IOException { UUID owner = project.song().owner(); - Path selectedFile = root.resolve("projects").resolve(owner + ".yml"); - Files.createDirectories(projectFile(owner, project.song().id()).getParent()); - // Archive the original single-project file before selecting a different song. - if (Files.exists(selectedFile)) { - Project previous = decodeProject(selectedFile, owner, null); - Path previousFile = projectFile(owner, previous.song().id()); - if (!Files.exists(previousFile)) write(previousFile, encodeProject(previous), true); - } - YamlConfiguration yaml = encodeProject(project); - write(projectFile(owner, project.song().id()), yaml, true); - write(selectedFile, yaml, true); + Path file = projectFile(owner, project.song().id()); + Files.createDirectories(file.getParent()); + write(file, encodeProject(project), true); + point(owner, project.song().id()); + remember(project, file); } private YamlConfiguration encodeProject(Project project) { @@ -138,29 +303,94 @@ public void publish(Song song) throws IOException { write(root.resolve("songs").resolve(song.id() + ".yml"), encodeSong(song), false); } + /** + * Publishes an immutable edition of the song. Editions are named after their content, so + * republishing an unchanged mix returns the earlier edition instead of adding another file. + */ + public Song publishEdition(Song song, int perDay) throws IOException { + // Muted and silent parts are never played, so they are not kept forever either. + List tracks = song.audibleMix().tracks(); + String content = encodeSong(new Song(CONTENT_ID, song.owner(), song.author(), song.title(), tracks)).saveToString(); + UUID id = UUID.nameUUIDFromBytes(content.getBytes(StandardCharsets.UTF_8)); + Song edition = new Song(id, song.owner(), song.author(), song.title(), tracks); + Path file = root.resolve("songs").resolve(id + ".yml"); + if (Files.exists(file)) { + // The same bytes are the same edition; comparing them is cheaper than parsing it again. + byte[] contents = encodeSong(edition).saveToString().getBytes(StandardCharsets.UTF_8); + if (Files.size(file) != contents.length || !Arrays.equals(Files.readAllBytes(file), contents)) { + throw new IOException("Published edition does not match its content: " + id); + } + return edition; + } + long now = clock.millis(); + List recent = recentEditions(song.owner(), now); + if (perDay > 0 && recent.size() >= perDay) { + long minutes = (recent.getFirst() + DAY_MILLIS - now + 59_999) / 60_000; + throw new IllegalArgumentException("You can publish " + perDay + " new song editions a day. Try again in " + + (minutes > 90 ? (minutes + 59) / 60 + " hours" : minutes + " minutes")); + } + publish(edition); + recent.add(now); + Path log = editionLog(song.owner()); + Files.createDirectories(log.getParent()); + write(log, String.join("\n", recent.stream().map(String::valueOf).toList()).getBytes(StandardCharsets.UTF_8), true); + return edition; + } + + private Path editionLog(UUID owner) { + return root.resolve("editions").resolve(owner + ".log"); + } + + // When each of the owner's editions of the last 24 hours was published, oldest first. + private List recentEditions(UUID owner, long now) throws IOException { + Path file = editionLog(owner); + List recent = new ArrayList<>(); + if (!Files.exists(file)) return recent; + // Invalid UTF-8 becomes replacement characters, so only those lines are skipped below. + for (String line : new String(Files.readAllBytes(file), StandardCharsets.UTF_8).split("\n")) { + try { + long at = Long.parseLong(line.strip()); + if (at > now - DAY_MILLIS) recent.add(at); + } catch (NumberFormatException ex) { + // A damaged line no longer counts; the next publication rewrites the log. + } + } + recent.sort(null); + return recent; + } + private YamlConfiguration read(Path file) throws IOException { - if (Files.size(file) > 8_000_000) { - throw new IOException("Studio file exceeds the size limit: " + file.getFileName()); + if (Files.size(file) > MAX_FILE_BYTES) { + throw new DamagedFileException("Studio file exceeds the size limit: " + file.getFileName(), null); } YamlConfiguration yaml = new YamlConfiguration(); try { yaml.loadFromString(Files.readString(file, StandardCharsets.UTF_8)); + } catch (CharacterCodingException ex) { + throw new DamagedFileException("Invalid UTF-8 in studio file: " + file.getFileName(), ex); } catch (InvalidConfigurationException ex) { - throw new IOException("Invalid studio YAML: " + file.getFileName(), ex); + throw new DamagedFileException("Invalid studio YAML: " + file.getFileName(), ex); } - if (yaml.getInt("format") != 1) { - throw new IOException("Unsupported studio format: " + file.getFileName()); + if (yaml.getInt("format") != FORMAT) { + throw new DamagedFileException("Unsupported studio format: " + file.getFileName(), null); } return yaml; } private void write(Path file, YamlConfiguration yaml, boolean replace) throws IOException { + write(file, yaml.saveToString().getBytes(StandardCharsets.UTF_8), replace); + } + + private void write(Path file, byte[] contents, boolean replace) throws IOException { if (!replace && Files.exists(file)) { throw new IOException("Published editions cannot be overwritten"); } + if (contents.length > MAX_FILE_BYTES) { + throw new IOException("Studio file exceeds the size limit: " + file.getFileName()); + } Path temp = Files.createTempFile(file.getParent(), "studio-", ".tmp"); try { - Files.writeString(temp, yaml.saveToString(), StandardCharsets.UTF_8); + Files.write(temp, contents); try { if (replace) { Files.move(temp, file, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); @@ -181,7 +411,7 @@ private void write(Path file, YamlConfiguration yaml, boolean replace) throws IO private YamlConfiguration encodeSong(Song song) { YamlConfiguration yaml = new YamlConfiguration(); - yaml.set("format", 1); + yaml.set("format", FORMAT); yaml.set("id", song.id().toString()); yaml.set("owner", song.owner().toString()); yaml.set("author", song.author()); @@ -196,8 +426,7 @@ private Map encodeTrack(Track track) { result.put("length", track.lengthTicks()); result.put("gain", track.gain()); result.put("muted", track.muted()); - result.put("notes", track.notes().stream().map(note -> List.of( - note.tick(), note.instrument(), note.sound(), note.volume(), note.pitch())).toList()); + encodeNotes(track, result); return result; } @@ -212,21 +441,56 @@ private Song decodeSong(YamlConfiguration yaml) { } private Track decodeTrack(Map raw) { - List entries = list(raw.get("notes")); - if (entries.size() > Song.MAX_NOTES) { - throw new IllegalArgumentException("Too many notes"); + return new Track(integer(raw.get("slot")), integer(raw.get("length")), + number(raw.get("gain")).floatValue(), (Boolean) raw.get("muted"), compactNotes(raw)); + } + + // Each track lists its instrument and sample names once. Each note is one line of + // " ", the names given + // as positions in that list. One text value per track also parses much faster than a YAML list per note. + private void encodeNotes(Track track, Map result) { + Map names = new LinkedHashMap<>(); + StringBuilder lines = new StringBuilder(track.notes().size() * 16); + int previous = 0; + for (Note note : track.notes()) { + if (!lines.isEmpty()) lines.append('\n'); + lines.append(note.tick() - previous).append(' ') + .append(names.computeIfAbsent(note.instrument(), name -> names.size())).append(' ') + .append(names.computeIfAbsent(note.sound(), name -> names.size())).append(' ') + .append(note.volume()).append(' ').append(note.pitch()); + previous = note.tick(); + } + result.put("names", List.copyOf(names.keySet())); + result.put("notes", lines.toString()); + } + + private List compactNotes(Map raw) { + List entries = list(raw.get("names")); + if (entries.size() > 2 * Song.MAX_NOTES) { + throw new IllegalArgumentException("Too many names"); + } + String[] names = entries.stream().map(this::text).toArray(String[]::new); + if (!(raw.get("notes") instanceof String lines)) { + throw new IllegalArgumentException("Expected note lines"); } List notes = new ArrayList<>(); - for (Object value : entries) { - List note = list(value); - if (note.size() != 5) { + int tick = 0; + for (int start = 0; start < lines.length(); ) { + if (notes.size() == Song.MAX_NOTES) { + throw new IllegalArgumentException("Too many notes"); + } + int end = lines.indexOf('\n', start); + if (end < 0) end = lines.length(); + String[] fields = lines.substring(start, end).split(" "); + if (fields.length != 5) { throw new IllegalArgumentException("Invalid note fields"); } - notes.add(new Note(integer(note.get(0)), (String) note.get(1), (String) note.get(2), - number(note.get(3)).floatValue(), number(note.get(4)).floatValue())); + tick = Math.addExact(tick, Integer.parseInt(fields[0])); + notes.add(new Note(tick, names[Integer.parseInt(fields[1])], names[Integer.parseInt(fields[2])], + Float.parseFloat(fields[3]), Float.parseFloat(fields[4]))); + start = end + 1; } - return new Track(integer(raw.get("slot")), integer(raw.get("length")), - number(raw.get("gain")).floatValue(), (Boolean) raw.get("muted"), notes); + return notes; } private Map map(Object value) { @@ -241,6 +505,14 @@ private List list(Object value) { return (List) value; } + private String text(Object value) { + if (!(value instanceof String text)) { + throw new IllegalArgumentException("Expected text"); + } + // Songs repeat a few instrument and sample names; share one copy instead of one per note. + return text.intern(); + } + private Number number(Object value) { if (!(value instanceof Number number)) { throw new IllegalArgumentException("Expected a number"); diff --git a/src/main/resources/studio.yml b/src/main/resources/studio.yml index b227655..c4b0481 100644 --- a/src/main/resources/studio.yml +++ b/src/main/resources/studio.yml @@ -11,6 +11,12 @@ sample-tail-seconds: 3 # Background checkpoints run at this interval. A hard crash loses work since # the last completed checkpoint; stop/disconnect/shutdown save the complete take. checkpoint-ticks: 200 +# Minimum wait between a player's publications. Published editions are kept forever +# because discs reference them; republishing unchanged music reuses its edition. 0 disables. +publish-cooldown-seconds: 30 +# New editions each player can publish in any 24 hours, which bounds how fast published +# songs grow on disk. Republishing music that is already published does not count. 0 disables. +max-editions-per-day: 20 # Right-clicking this block or furniture opens the studio (permission instruments.record). # v. vanilla block, e.g. v.loom (a jukebox cannot be used: it plays recorded discs) # ia. ItemsAdder custom block or furniture, e.g. ia.tfmc:music_station diff --git a/src/test/java/net/tfminecraft/musicalinstruments/InstrumentPluginTest.java b/src/test/java/net/tfminecraft/musicalinstruments/InstrumentPluginTest.java index 8bc6432..2f64f41 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/InstrumentPluginTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/InstrumentPluginTest.java @@ -164,7 +164,7 @@ void recordsPlayedNotesIntoAStudioTake() { } assertTrue(player.performCommand("music stop")); - assertEquals("Recording stopped. Take saved: /music preview take, /music keep, or /music discard.", player.nextMessage()); + assertEquals("Recording stopped. Review the take: /music preview take, then /music keep or /music discard.", player.nextMessage()); } @Test diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java index dedb7b9..b6a65a1 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioCommandWorkflowTest.java @@ -25,7 +25,7 @@ class StudioCommandWorkflowTest { @BeforeEach void setup() throws Exception { when(player.hasPermission("instruments.record")).thenReturn(true); - when(studio.settings()).thenReturn(new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 20)); + when(studio.settings()).thenReturn(new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 20, 0, 0)); var first = new Track(1, 20, 1, false, List.of(new Note(0, "lute", "sample", 1, 1))); var second = new Track(2, 20, 1, true, first.notes()); project = new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Author", "Old", List.of(first, second)), @@ -67,7 +67,7 @@ void creatingRenamingAndOpeningProjectsPreservesTheSongData() throws Exception { verify(studio).selectProject(player, project.song().id()); run("projects"); verify(player).sendMessage("No projects yet. Open /music studio to create a song."); - when(studio.listProjects(player)).thenReturn(List.of(project)); + when(studio.listProjects(player)).thenReturn(List.of(ProjectSummary.of(project))); run("projects"); verify(player).sendMessage("Old | " + project.song().id()); } diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java index bf3ac36..c5f11f9 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java @@ -23,6 +23,8 @@ import static org.mockito.Mockito.*; class StudioListenerTest { + private org.bukkit.plugin.Plugin plugin; + private org.bukkit.scheduler.BukkitScheduler scheduler; private StudioService studio; private StudioListener listener; private StudioMenu menu; @@ -38,7 +40,12 @@ void setup() { studio = mock(StudioService.class); discs = mock(DiscItems.class); menu = mock(StudioMenu.class); - listener = new StudioListener(studio, menu); + plugin = mock(org.bukkit.plugin.Plugin.class); + var server = mock(org.bukkit.Server.class); + scheduler = mock(org.bukkit.scheduler.BukkitScheduler.class); + when(plugin.getServer()).thenReturn(server); + when(server.getScheduler()).thenReturn(scheduler); + listener = new StudioListener(plugin, studio, menu); player = mock(Player.class); inventory = mock(PlayerInventory.class); block = mock(Block.class); @@ -47,6 +54,7 @@ void setup() { when(studio.discs()).thenReturn(discs); when(studio.station()).thenReturn(StudioStation.none()); when(block.getState()).thenReturn(box); + when(block.getType()).thenReturn(Material.JUKEBOX); when(player.getInventory()).thenReturn(inventory); when(player.hasPermission("instruments.play")).thenReturn(true); when(discs.custom(disc)).thenReturn(true); @@ -164,6 +172,16 @@ void rightClickingTheStationOpensTheStudioOnceAndKeepsTheKeyboardClosed() throws assertEquals(Event.Result.DENY, off.useItemInHand()); } + @Test + void clicksOnOtherBlocksDoNotCopyTheirBlockState() throws Exception { + when(block.getType()).thenReturn(Material.CHEST); + PlayerInteractEvent click = event(EquipmentSlot.HAND); + listener.interact(click); + verify(block, never()).getState(); + verify(studio, never()).play(any(), any()); + assertNotEquals(Event.Result.DENY, click.useItemInHand()); + } + @Test void otherBlocksClicksAndSneakPlacementKeepVanillaBehaviour() throws Exception { when(studio.station()).thenReturn(StudioStation.vanilla(Material.LOOM)); @@ -292,4 +310,78 @@ void worldLifecycleAndHoppersRespectCustomRecordingPlayback() { listener.hopper(ordinary); assertFalse(ordinary.isCancelled()); } + @Test + void aHopperTakingARecordingDiscStopsItsJukeboxAfterTheTransfer() { + var jukebox = mock(org.bukkit.inventory.Inventory.class); + var hopper = mock(org.bukkit.inventory.Inventory.class); + var location = mock(org.bukkit.Location.class); + when(location.getBlock()).thenReturn(block); + when(jukebox.getType()).thenReturn(org.bukkit.event.inventory.InventoryType.JUKEBOX); + when(hopper.getType()).thenReturn(org.bukkit.event.inventory.InventoryType.HOPPER); + when(discs.custom(disc)).thenReturn(true); + listener.extract(new org.bukkit.event.inventory.InventoryMoveItemEvent(hopper, disc, jukebox, true)); + listener.extract(new org.bukkit.event.inventory.InventoryMoveItemEvent(jukebox, disc, hopper, true)); // No location. + verify(studio, never()).stop(any(Block.class)); + when(jukebox.getLocation()).thenReturn(location); + listener.extract(new org.bukkit.event.inventory.InventoryMoveItemEvent(jukebox, disc, hopper, true)); + verify(studio, never()).stop(any(Block.class)); + var callback = org.mockito.ArgumentCaptor.forClass(Runnable.class); + verify(scheduler).runTask(eq(plugin), callback.capture()); + var world = mock(org.bukkit.World.class); + when(block.getWorld()).thenReturn(world); + when(world.isChunkLoaded(0, 0)).thenReturn(true); + callback.getValue().run(); // Transfer completed: the jukebox no longer holds the recording. + verify(studio).stop(block); + when(discs.custom(disc)).thenReturn(false); // Vanilla discs keep vanilla behaviour. + listener.extract(new org.bukkit.event.inventory.InventoryMoveItemEvent(jukebox, disc, hopper, true)); + verify(studio, times(1)).stop(block); + verify(scheduler, times(1)).runTask(eq(plugin), any(Runnable.class)); + } + + @Test + void aRejectedHopperTransferKeepsTheRestoredDiscPlaying() { + var source = mock(org.bukkit.inventory.Inventory.class); + var hopper = mock(org.bukkit.inventory.Inventory.class); + var location = mock(Location.class); + var world = mock(org.bukkit.World.class); + when(source.getType()).thenReturn(org.bukkit.event.inventory.InventoryType.JUKEBOX); + when(source.getLocation()).thenReturn(location); + when(location.getBlock()).thenReturn(block); + when(block.getWorld()).thenReturn(world); + when(world.isChunkLoaded(0, 0)).thenReturn(true); + listener.extract(new org.bukkit.event.inventory.InventoryMoveItemEvent(source, disc, hopper, true)); + verify(studio, never()).stop(any(Block.class)); + var callback = org.mockito.ArgumentCaptor.forClass(Runnable.class); + verify(scheduler).runTask(eq(plugin), callback.capture()); + // The destination rejects the transfer and restores the disc after the event. + when(box.getRecord()).thenReturn(disc); + callback.getValue().run(); + verify(studio, never()).stop(any(Block.class)); + } + + @Test + void deferredHopperCheckSkipsUnloadedAndReplacedJukeboxes() { + var source = mock(org.bukkit.inventory.Inventory.class); + var hopper = mock(org.bukkit.inventory.Inventory.class); + var location = mock(Location.class); + var world = mock(org.bukkit.World.class); + when(source.getType()).thenReturn(org.bukkit.event.inventory.InventoryType.JUKEBOX); + when(source.getLocation()).thenReturn(location); + when(location.getBlock()).thenReturn(block); + when(block.getWorld()).thenReturn(world); + listener.extract(new org.bukkit.event.inventory.InventoryMoveItemEvent(source, disc, hopper, true)); + var callback = org.mockito.ArgumentCaptor.forClass(Runnable.class); + verify(scheduler).runTask(eq(plugin), callback.capture()); + callback.getValue().run(); // Chunk unloaded; do not access or reload the block. + verify(block, never()).getType(); + verify(block, never()).getState(); + when(world.isChunkLoaded(0, 0)).thenReturn(true); + when(block.getType()).thenReturn(Material.STONE); + callback.getValue().run(); + when(block.getType()).thenReturn(Material.JUKEBOX); + when(block.getState()).thenReturn(mock(org.bukkit.block.BlockState.class)); + callback.getValue().run(); + verify(studio, never()).stop(any(Block.class)); + } + } diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioMenuTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioMenuTest.java index 374b5a0..006379d 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioMenuTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioMenuTest.java @@ -29,6 +29,39 @@ import static org.mockito.Mockito.*; class StudioMenuTest { + @Test + void keepingTakePreservesLatestTrackMix() throws Exception { + Track saved = project.song().track(1); + project = new Project(project.song(), saved, 100, false); + when(studio.requireProject(player)).thenReturn(project); + click(12, ClickType.LEFT); + executeClick(); + var edited = ArgumentCaptor.forClass(Project.class); + verify(studio).edit(eq(player), edited.capture()); + Project mixed = edited.getValue(); + assertTrue(mixed.song().track(1).muted()); + assertTrue(mixed.accept().song().track(1).muted(), "Keeping the take should not undo the mute button"); + } + + @Test + void takePreviewAndKeepUseNewVolumeAndCanUnmuteAPreviouslyMutedTrack() throws Exception { + Track saved = project.song().track(1).mix(.5f, true); + project = new Project(project.song().replace(saved), saved, 100, false); + when(studio.requireProject(player)).thenReturn(project); + click(12, ClickType.LEFT); executeClick(); + var edits = ArgumentCaptor.forClass(Project.class); + verify(studio).edit(eq(player), edits.capture()); + Project unmuted = edits.getValue(); + assertFalse(unmuted.pending().muted()); + when(studio.requireProject(player)).thenReturn(unmuted); + click(11, ClickType.LEFT); executeClick(); + verify(studio, times(2)).edit(eq(player), edits.capture()); + Project adjusted = edits.getAllValues().getLast(); + assertEquals(.75f, adjusted.pending().gain()); + assertEquals(.75f, adjusted.accept().song().track(1).gain()); + assertFalse(adjusted.accept().song().track(1).muted()); + } + private final InstrumentPlugin plugin = mock(InstrumentPlugin.class); private final StudioService studio = mock(StudioService.class); private final StudioDialogs dialogs = mock(StudioDialogs.class); @@ -78,8 +111,8 @@ void setup() throws Exception { project = new Project(new Song(UUID.randomUUID(), player.getUniqueId(), "Player", "Song", List.of(saved)), pending, 100, false); when(studio.project(player)).thenReturn(project); when(studio.requireProject(player)).thenReturn(project); - when(studio.listProjects(player)).thenReturn(List.of(project)); - when(studio.settings()).thenReturn(new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 2)); + when(studio.listProjects(player)).thenReturn(List.of(ProjectSummary.of(project))); + when(studio.settings()).thenReturn(new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 2, 0, 0)); when(studio.activity(player)).thenReturn(new StudioService.Activity(0, 0, 0, 0, false)); StudioIcons icons = mock(StudioIcons.class); when(icons.item(any(), any(), anyList())).thenReturn(mock(ItemStack.class)); @@ -268,7 +301,7 @@ void switchingTheActiveProjectRejectsAnAlreadyQueuedTrackClick() throws Exceptio @Test void tracksFiveToEightHaveTheirOwnPageAndPreserveTrackNumbers() throws Exception { - when(studio.settings()).thenReturn(new StudioSettings(8, 40, 20, 60, 8, 32, 2, 0, 2)); + when(studio.settings()).thenReturn(new StudioSettings(8, 40, 20, 60, 8, 32, 2, 0, 2, 0, 0)); menu.openProject(player, station); click(53, ClickType.LEFT); executeClick(); @@ -279,17 +312,18 @@ void tracksFiveToEightHaveTheirOwnPageAndPreserveTrackNumbers() throws Exception @Test void libraryFillsAllThirtySixSlotsAndRightClickRequestsDeletion() throws Exception { - List library = java.util.stream.IntStream.range(0, 36).mapToObj(index -> + List projects = java.util.stream.IntStream.range(0, 36).mapToObj(index -> new Project(new Song(UUID.randomUUID(), player.getUniqueId(), "Player", "Song " + index, List.of()), null, 100, true)).toList(); + List library = projects.stream().map(ProjectSummary::of).toList(); when(studio.listProjects(player)).thenReturn(library); menu.open(player, station); click(9, ClickType.RIGHT); executeClick(); - verify(dialogs).delete(player, new Location(world, 0, 0, 0), library.getFirst().song().id()); - when(studio.requireProject(player)).thenReturn(library.getLast()); + verify(dialogs).delete(player, new Location(world, 0, 0, 0), library.getFirst().id()); + when(studio.requireProject(player)).thenReturn(projects.getLast()); click(44, ClickType.LEFT); executeClick(); - verify(studio).selectProject(player, library.getLast().song().id()); + verify(studio).selectProject(player, library.getLast().id()); } @Test @@ -398,7 +432,7 @@ void renderingShowsRecordingListeningMutedTracksAndAnEmptyLibrary() throws Excep Track muted = project.song().track(1).mix(.5f, true); project = new Project(project.song().replace(muted), null, 100, true); when(studio.requireProject(player)).thenReturn(project); - when(studio.settings()).thenReturn(new StudioSettings(2, 40, 20, 60, 8, 32, 2, 0, 2)); + when(studio.settings()).thenReturn(new StudioSettings(2, 40, 20, 60, 8, 32, 2, 0, 2, 0, 0)); when(studio.activity(player)).thenReturn(new StudioService.Activity(1, 2, 1, 0, false)); menu.openProject(player, station); when(studio.activity(player)).thenReturn(new StudioService.Activity(0, 0, 0, 0, true)); @@ -440,7 +474,7 @@ void navigatingBackClosingAndUnassignedControlsHaveNoTrackEffects() throws Excep click(9, ClickType.LEFT); executeClick(); click(0, ClickType.LEFT); executeClick(); for (int raw : new int[]{9, 17, 46, 45, 53, 49}) { click(raw, ClickType.LEFT); executeClick(); } - when(studio.settings()).thenReturn(new StudioSettings(2, 40, 20, 60, 8, 32, 2, 0, 2)); + when(studio.settings()).thenReturn(new StudioSettings(2, 40, 20, 60, 8, 32, 2, 0, 2, 0, 0)); click(28, ClickType.LEFT); executeClick(); verify(studio, never()).record(any(), anyInt()); } @@ -467,7 +501,7 @@ void volumeResetMissingSavedTracksDiscardAndTempoStepControlsAreValidated() thro @Test void trackPagingReturnsToThePreviousFourTracks() throws Exception { - when(studio.settings()).thenReturn(new StudioSettings(8, 40, 20, 60, 8, 32, 2, 0, 2)); + when(studio.settings()).thenReturn(new StudioSettings(8, 40, 20, 60, 8, 32, 2, 0, 2, 0, 0)); menu.openProject(player, station); click(53, ClickType.LEFT); executeClick(); click(45, ClickType.LEFT); executeClick(); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioModelTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioModelTest.java index ba098fe..835eff2 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioModelTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioModelTest.java @@ -41,19 +41,38 @@ void backingExcludesTheTrackBeingReplacedAndMutedTracks() { } @Test - void acceptingOrDiscardingATakePreservesPublishedEditions() { + void audibilityIgnoresMutedSilentAndZeroVolumeTracks() { + Track muted = new Track(1, 10, 1, false, List.of(note(0, "a"))).mix(1, true); + Track silent = new Track(2, 10, 0, false, List.of(note(0, "b"))); + Track quiet = new Track(3, 10, 1, false, List.of(new Note(0, "lute", "c", 0, 1))); + assertFalse(song(muted, silent, quiet).audible()); + assertFalse(song().audible()); + assertTrue(song(muted, new Track(4, 10, 1, false, List.of(new Note(0, "lute", "d", 0, 1), note(1, "e")))).audible()); + } + + @Test + void acceptingOrDiscardingATakeLeavesTheOriginalSongUnchanged() { Song original = song(new Track(1, 100, 1, false, List.of(note(0, "original")))); - Song edition = original.edition(); Track replacement = new Track(1, 50, 0.5f, false, List.of(note(20, "replacement"))); Project project = new Project(original, replacement, 100, true); assertEquals("original", project.discard().song().track(1).notes().getFirst().sound()); assertEquals("replacement", project.accept().song().track(1).notes().getFirst().sound()); assertEquals(50, project.accept().song().lengthTicks()); - assertEquals("original", edition.track(1).notes().getFirst().sound()); - assertNotEquals(original.id(), edition.id()); + assertEquals("original", original.track(1).notes().getFirst().sound()); assertNull(project.accept().pending()); } + @Test + void aTakeWhoseSavedTrackWasRemovedIsKeptAtFullVolume() { + Song original = song(new Track(1, 100, 0.5f, true, List.of(note(0, "original")))); + Project project = new Project(original, new Track(1, 50, 1, false, List.of(note(20, "replacement"))), 100, true); + assertEquals(0.5f, project.pending().gain()); + assertTrue(project.pending().muted()); + Track kept = new Project(original.remove(1), project.pending(), 100, true).accept().song().track(1); + assertEquals(1, kept.gain()); + assertFalse(kept.muted()); + } + @Test void dataSnapshotsDoNotShareMutableNoteLists() { var notes = new ArrayList<>(List.of(note(0, "sample"))); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioSaveQueueTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioSaveQueueTest.java new file mode 100644 index 0000000..e954cf7 --- /dev/null +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioSaveQueueTest.java @@ -0,0 +1,341 @@ +package net.tfminecraft.musicalinstruments.studio; + +import net.tfminecraft.musicalinstruments.InstrumentPlugin; +import net.tfminecraft.musicalinstruments.managers.InstrumentManager; +import org.bukkit.Server; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.command.Command; +import org.bukkit.entity.Player; +import org.bukkit.inventory.PlayerInventory; +import org.bukkit.scheduler.BukkitScheduler; +import org.bukkit.scheduler.BukkitTask; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Logger; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +/** Regressions for slow storage, coalesced saves, and shutdown failures. */ +class StudioSaveQueueTest { + @TempDir Path directory; + + @Test + void inFlightSnapshotRemainsImmutableAndNewEditsYieldToAnotherOwner() throws Exception { + var worker = new ThreadPoolExecutor(1, 1, 0, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>()); + var fixture = new Fixture(directory, worker); + Player first = fixture.player(); + Player second = fixture.player(); + Project original = fixture.project(first); + Project other = fixture.project(second); + fixture.studio.save(first, original); + fixture.studio.save(second, other); + var writes = java.util.Collections.synchronizedList(new ArrayList()); + var started = new CountDownLatch(1); + var release = new CountDownLatch(1); + doAnswer(invocation -> { + Project snapshot = invocation.getArgument(0); + writes.add(snapshot); + if (writes.size() == 1) { + started.countDown(); + if (!release.await(5, TimeUnit.SECONDS)) throw new IOException("In-flight save timed out"); + } + return invocation.callRealMethod(); + }).when(fixture.store).save(any()); + Project firstEdit = new Project(original.song(), null, 80, false); + Project finalEdit = new Project(original.song(), null, 180, true); + try { + fixture.studio.edit(first, firstEdit); + assertTrue(started.await(2, TimeUnit.SECONDS)); + fixture.studio.edit(second, other); + for (int i = 0; i < 1000; i++) fixture.studio.edit(first, finalEdit); + assertEquals(2, worker.getQueue().size()); // One per queued owner, besides the running write. + release.countDown(); + worker.submit(() -> {}).get(5, TimeUnit.SECONDS); + assertEquals(List.of(firstEdit, other, finalEdit), writes); + assertEquals(finalEdit, new StudioStore(directory).project(first.getUniqueId()).orElseThrow()); + assertEquals(other, new StudioStore(directory).project(second.getUniqueId()).orElseThrow()); + } finally { + release.countDown(); + fixture.studio.close(); + } + } + + @Test + void persistentStorageFailureDuringShutdownGetsOnlyOneFinalRetry() throws Exception { + var worker = Executors.newSingleThreadExecutor(); + var fixture = new Fixture(directory, worker); + Player player = fixture.player(); + fixture.studio.save(player, fixture.project(player)); + clearInvocations(fixture.store); + doThrow(new IOException("Storage stays unavailable")).when(fixture.store).save(any()); + try { + fixture.take(player, "retained_take"); + worker.submit(() -> {}).get(3, TimeUnit.SECONDS); // Failure completes before close. + fixture.studio.close(); + verify(fixture.store, times(3)).save(any()); // Initial attempt, closing attempt, one closing retry. + assertNotNull(fixture.studio.requireProject(player).pending()); + assertNull(new StudioStore(directory).project(player.getUniqueId()).orElseThrow().pending()); + } finally { + fixture.studio.close(); + } + } + + @Test + void failureAfterTheShutdownDeadlineDoesNotStartAnotherWrite() throws Exception { + var worker = Executors.newSingleThreadExecutor(); + var fixture = new Fixture(directory, worker); + Player player = fixture.player(); + fixture.studio.save(player, fixture.project(player)); + var started = new CountDownLatch(1); + var release = new CountDownLatch(1); + doAnswer(invocation -> { + started.countDown(); + if (!release.await(5, TimeUnit.SECONDS)) throw new IOException("Late save timed out"); + throw new IOException("The write failed after the shutdown deadline"); + }).when(fixture.store).save(any()); + try { + fixture.take(player, "take_to_retry_when_storage_recovers"); + assertTrue(started.await(2, TimeUnit.SECONDS)); + var deadline = StudioService.class.getDeclaredField("shutdownDeadline"); + deadline.setAccessible(true); + deadline.setLong(fixture.studio, System.nanoTime() - 1); + release.countDown(); + worker.submit(() -> {}).get(3, TimeUnit.SECONDS); + verify(fixture.store, times(2)).save(any()); // Original project and one failed take write. + assertNotNull(fixture.studio.requireProject(player).pending()); + } finally { + release.countDown(); + doCallRealMethod().when(fixture.store).save(any()); + fixture.studio.close(); + } + } + + @Test + void anotherMusiciansFinalTakeMustNotWaitBehindEveryObsoleteEdit() throws Exception { + var worker = new ThreadPoolExecutor(1, 1, 0, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>()); + var fixture = new Fixture(directory, worker); + Player first = fixture.player(); + Player second = fixture.player(); + Project project = fixture.project(first); + fixture.studio.save(first, project); + fixture.studio.save(second, fixture.project(second)); + var writes = java.util.Collections.synchronizedList(new ArrayList()); + doAnswer(invocation -> { + writes.add(((Project) invocation.getArgument(0)).song().owner()); + return invocation.callRealMethod(); + }).when(fixture.store).save(any()); + var started = new CountDownLatch(1); + var release = new CountDownLatch(1); + worker.execute(() -> { + started.countDown(); + try { release.await(5, TimeUnit.SECONDS); } + catch (InterruptedException ex) { Thread.currentThread().interrupt(); } + }); + assertTrue(started.await(2, TimeUnit.SECONDS)); + try { + for (int i = 0; i < 100; i++) fixture.studio.edit(first, + new Project(project.song(), null, 40 + i, false)); + fixture.take(second, "second_musicians_new_take"); + assertNotNull(fixture.studio.requireProject(second).pending()); + release.countDown(); + worker.submit(() -> {}).get(10, TimeUnit.SECONDS); + int olderWrites = writes.indexOf(second.getUniqueId()); + System.out.println("READINESS_FAIRNESS obsolete_edits_before_other_take=" + olderWrites); + assertTrue(olderWrites <= 2, + "Other musicians' final takes should not wait behind " + olderWrites + " obsolete edits"); + } finally { + release.countDown(); + fixture.studio.close(); + } + } + + @Test + void transientFailureCompletingDuringShutdownMustNotLoseTheOnlyNewTake() throws Exception { + ExecutorService writer = spy(Executors.newSingleThreadExecutor( + Thread.ofPlatform().daemon(true).name("readiness-shutdown-writer").factory())); + var fixture = new Fixture(directory, writer); + Player player = fixture.player(); + fixture.studio.save(player, fixture.project(player)); + var started = new CountDownLatch(1); + var release = new CountDownLatch(1); + var failedOnce = new AtomicBoolean(); + doAnswer(invocation -> { + if (Thread.currentThread().getName().equals("readiness-shutdown-writer") + && failedOnce.compareAndSet(false, true)) { + started.countDown(); + if (!release.await(5, TimeUnit.SECONDS)) throw new IOException("Review latch timed out"); + throw new IOException("One transient final-save failure; subsequent saves would succeed"); + } + return invocation.callRealMethod(); + }).when(fixture.store).save(any()); + // The failure is discovered only after close has checked which writes need retrying. + doAnswer(invocation -> { + release.countDown(); + return invocation.callRealMethod(); + }).when(writer).awaitTermination(eq(StudioService.SHUTDOWN_WAIT_SECONDS), eq(TimeUnit.SECONDS)); + try { + fixture.take(player, "new_take_that_must_survive_restart"); + assertTrue(started.await(2, TimeUnit.SECONDS)); + assertNotNull(fixture.studio.requireProject(player).pending()); + fixture.studio.close(); + Project persisted = new StudioStore(directory).project(player.getUniqueId()).orElseThrow(); + System.out.println("READINESS_SHUTDOWN take_in_memory=" + + (fixture.studio.requireProject(player).pending() != null) + + " take_on_disk=" + (persisted.pending() != null)); + assertNotNull(persisted.pending(), + "A one-off failure discovered during shutdown must be retried before discarding the only take"); + } finally { + release.countDown(); + fixture.studio.close(); + writer.shutdown(); + assertTrue(writer.awaitTermination(3, TimeUnit.SECONDS)); + } + } + + @Test + void retryingAFailedEditDoesNotEraseTheCheckpointOfARecordingInProgress() throws Exception { + var worker = Executors.newSingleThreadExecutor(); + var fixture = new Fixture(directory, worker); + Player player = fixture.player(); + Project original = fixture.project(player); + fixture.studio.save(player, original); + Project edited = new Project(original.song(), null, 120, false); + doThrow(new IOException("One transient failure before the recording")).when(fixture.store).save(edited); + try { + fixture.studio.edit(player, edited); + worker.submit(() -> {}).get(3, TimeUnit.SECONDS); + fixture.tick.run(); // Clock 1 observes the failure; the edit stays cached and unsaved. + doCallRealMethod().when(fixture.store).save(any()); + fixture.studio.record(player, 2); // The take starts at clock 3. + fixture.tick.run(); fixture.tick.run(); + fixture.studio.capture(player, "lute", "new_take", 1, 1); + for (int clock = 4; clock < StudioService.RETRY_TICKS; clock++) fixture.tick.run(); + worker.submit(() -> {}).get(3, TimeUnit.SECONDS); + Project checkpoint = new StudioStore(directory).project(player.getUniqueId()).orElseThrow(); + assertEquals("new_take", checkpoint.pending().notes().getFirst().sound()); + assertEquals(120, checkpoint.bpm()); // Checkpoints include the failed edit. + fixture.tick.run(); // The periodic retry skips the recording musician. + worker.submit(() -> {}).get(3, TimeUnit.SECONDS); + assertTrue(fixture.studio.activity(player).recording()); + assertEquals(checkpoint, new StudioStore(directory).project(player.getUniqueId()).orElseThrow()); + fixture.studio.stop(player); + worker.submit(() -> {}).get(3, TimeUnit.SECONDS); + fixture.tick.run(); + assertEquals(fixture.studio.requireProject(player), new StudioStore(directory).project(player.getUniqueId()).orElseThrow()); + } finally { + fixture.studio.close(); + } + } + + private static final class Fixture { + private final Server server = mock(Server.class); + private final StudioStore store; + private final StudioService studio; + private final World world = mock(World.class); + private Runnable tick; + + private Fixture(Path directory, ExecutorService worker) throws Exception { + var plugin = mock(InstrumentPlugin.class); + var scheduler = mock(BukkitScheduler.class); + var manager = mock(InstrumentManager.class); + when(plugin.getServer()).thenReturn(server); + when(plugin.getLogger()).thenReturn(Logger.getLogger("readiness-review")); + when(server.getScheduler()).thenReturn(scheduler); + when(manager.getInstrument(any())).thenReturn("lute"); + when(scheduler.runTaskTimer(eq(plugin), any(Runnable.class), eq(1L), eq(1L))) + .thenAnswer(invocation -> { tick = invocation.getArgument(1); return mock(BukkitTask.class); }); + store = spy(new StudioStore(directory)); + studio = new StudioService(plugin, manager, store, + new StudioSettings(4, 3600, 1500, 6000, 32, 32, 2, 60, 200, 600, 20), + StudioStation.none(), mock(DiscItems.class), worker); + } + + private Player player() { + var player = mock(Player.class); + UUID owner = UUID.randomUUID(); + when(player.getUniqueId()).thenReturn(owner); + when(player.getName()).thenReturn("Musician"); + when(player.getInventory()).thenReturn(mock(PlayerInventory.class)); + when(player.getLocation()).thenReturn(new Location(world, 0, 64, 0)); + when(server.getPlayer(owner)).thenReturn(player); + return player; + } + + private Project project(Player player) { + return new Project(new Song(UUID.randomUUID(), player.getUniqueId(), "Musician", "Song", + List.of(new Track(1, 20, 1, false, List.of(new Note(0, "lute", "original_take", 1, 1))))), + null, 100, false); + } + + private void take(Player player, String sound) throws Exception { + studio.record(player, 1); + tick.run(); tick.run(); + studio.capture(player, "lute", sound, 1, 1); + studio.stop(player); + } + } + + @Test + void repeatedEditsMustNotCreateAnUnboundedSaveBacklogBehindASlowDisk() 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.getLogger("readiness-review")); + when(server.getScheduler()).thenReturn(scheduler); + when(scheduler.runTaskTimer(eq(plugin), any(Runnable.class), anyLong(), anyLong())) + .thenReturn(mock(BukkitTask.class)); + var player = mock(Player.class); + UUID owner = UUID.randomUUID(); + when(player.getUniqueId()).thenReturn(owner); + when(player.hasPermission("instruments.record")).thenReturn(true); + when(server.getPlayer(owner)).thenReturn(player); + var store = mock(StudioStore.class); + var worker = new ThreadPoolExecutor(1, 1, 0, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>()); + var studio = new StudioService(plugin, mock(InstrumentManager.class), store, + new StudioSettings(4, 3600, 1500, 6000, 32, 32, 60, 60, 200, 600, 20), + StudioStation.none(), mock(DiscItems.class), worker); + var song = new Song(UUID.randomUUID(), owner, "Musician", "Song", + List.of(new Track(1, 20, 1, false, List.of(new Note(0, "lute", "note", 1, 1))))); + studio.save(player, new Project(song, null, 100, false)); + var started = new CountDownLatch(1); + var release = new CountDownLatch(1); + worker.execute(() -> { + started.countDown(); + try { release.await(5, TimeUnit.SECONDS); } + catch (InterruptedException ex) { Thread.currentThread().interrupt(); } + }); + assertTrue(started.await(2, TimeUnit.SECONDS)); + try { + // Go through the public command handler using the normal recording permission. + var commands = new StudioCommand(studio, mock(StudioMenu.class)); + var command = mock(Command.class); + for (int i = 0; i < 1000; i++) commands.onCommand(player, command, "music", + new String[] { "bpm", String.valueOf(40 + i % 201) }); + int queued = worker.getQueue().size(); + System.out.println("READINESS_QUEUE edits=1000 queued=" + queued + " owners=1"); + assertTrue(queued <= 2, + "A slow write should retain a bounded number of snapshots per owner, but queued " + queued); + } finally { + release.countDown(); + studio.close(); + assertTrue(worker.awaitTermination(3, TimeUnit.SECONDS)); + } + } +} diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java index 5d86224..1231615 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java @@ -19,6 +19,8 @@ import org.junit.jupiter.api.AfterEach; 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.mockito.ArgumentCaptor; import java.io.IOException; @@ -35,6 +37,289 @@ import static org.mockito.Mockito.*; class StudioServiceTest { + @Test + void stoppingPreviewStopsAlreadySentSamples() throws Exception { + studio.save(player, new Project(song(track(1, "long_sample")), null, 100, false)); + studio.preview(player, false); + tick.run(); + verify(player).playSound(any(Location.class), eq("long_sample"), eq(SoundCategory.PLAYERS), anyFloat(), anyFloat()); + studio.stop(player); awaitSaves(); + verify(player).stopSound("long_sample", SoundCategory.PLAYERS); + verify(player, never()).stopSound(anyString(), eq(SoundCategory.RECORDS)); + } + + @Test + void slowCheckpointDoesNotBlockStopCaller() throws Exception { + studio.create(player, "Song", false); + CountDownLatch started = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + doAnswer(invocation -> { + if (Thread.currentThread().getName().equals("musical-instruments-checkpoints")) { + started.countDown(); + if (!release.await(5, TimeUnit.SECONDS)) throw new IOException("Checkpoint timed out"); + } + return invocation.callRealMethod(); + }).when(store).save(any()); + studio.record(player, 1); + tick.run(); + tick.run(); + studio.capture(player, "lute", "first", 4, 1); + tick.run(); + tick.run(); + assertTrue(started.await(3, TimeUnit.SECONDS)); + CompletableFuture stopped = CompletableFuture.runAsync(() -> studio.stop(player)); + try { + assertDoesNotThrow(() -> stopped.get(200, TimeUnit.MILLISECONDS), + "The same call runs on the server thread when Stop is clicked"); + } finally { + release.countDown(); + stopped.get(3, TimeUnit.SECONDS); + awaitSaves(); + } + } + + @Test + void editsDuringAPendingTakeSaveApplyAtOnceAndAreWrittenAfterIt() throws Exception { + studio.create(player, "Song", false); + CountDownLatch started = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + doAnswer(invocation -> { + if (Thread.currentThread().getName().equals("musical-instruments-checkpoints")) { + started.countDown(); + if (!release.await(3, TimeUnit.SECONDS)) throw new IOException("Save timed out"); + } + return invocation.callRealMethod(); + }).when(store).save(any()); + studio.record(player, 1); + tick.run(); tick.run(); + studio.capture(player, "lute", "final", 1, 1); + studio.stop(player); + assertTrue(started.await(3, TimeUnit.SECONDS)); + Project kept = studio.requireProject(player).accept(); + try { + studio.edit(player, kept); // Does not wait for the take to be written. + assertEquals(kept, studio.requireProject(player)); + // Changes that read or remove project files wait until they are current. + assertThrows(IllegalArgumentException.class, () -> studio.create(player, "Other", false)); + assertThrows(IllegalArgumentException.class, () -> studio.deleteProject(player, kept)); + tick.run(); // An incomplete save never waits on the server clock. + studio.quit(player); + when(server.getPlayer(owner)).thenReturn(null); + } finally { + release.countDown(); + awaitSaves(); + } + assertEquals(kept, new StudioStore(directory).project(owner).orElseThrow()); + when(server.getPlayer(owner)).thenReturn(player); + assertEquals(kept, studio.requireProject(player)); // Evicted after the write, then read back. + verify(player, never()).sendMessage(startsWith("Could not save")); + } + + @Test + void completedSaveAllowsImmediateEditsBeforeTheNextClockTick() throws Exception { + studio.create(player, "Song", false); + studio.record(player, 1); + tick.run(); tick.run(); + studio.capture(player, "lute", "note", 1, 1); + studio.stop(player); + awaitSaveFutures(); + studio.create(player, "Next", false); + assertEquals("Next", new StudioStore(directory).project(owner).orElseThrow().song().title()); + } + + @Test + void previewRestartAndRecordingStopClearPrivateAudioWithoutStoppingJukeboxes() throws Exception { + Song song = song(track(1, "shared_sample")); + ItemStack disc = mock(ItemStack.class); + Block box = jukebox(song, disc); + studio.save(player, new Project(song, null, 100, false)); + studio.play(box, disc); + studio.preview(player, false); + tick.run(); + verify(player).playSound(any(Location.class), eq("shared_sample"), eq(SoundCategory.RECORDS), anyFloat(), anyFloat()); + verify(player).playSound(any(Location.class), eq("shared_sample"), eq(SoundCategory.PLAYERS), anyFloat(), anyFloat()); + studio.preview(player, false); + verify(player).stopSound("shared_sample", SoundCategory.PLAYERS); + tick.run(); + studio.record(player, 2); + verify(player, times(2)).stopSound("shared_sample", SoundCategory.PLAYERS); + tick.run(); tick.run(); + studio.stop(player); + verify(player, times(3)).stopSound("shared_sample", SoundCategory.PLAYERS); + verify(player, never()).stopSound(anyString(), eq(SoundCategory.RECORDS)); + } + + @Test + void aFailedSaveAfterDisconnectRetainsTheTakeForRetryOnRejoin() throws Exception { + studio.create(player, "Song", false); + studio.record(player, 1); + tick.run(); tick.run(); + studio.capture(player, "lute", "retained", 1, 1); + doThrow(new IOException("Disk full")).when(store).save(any()); + studio.quit(player); + when(server.getPlayer(owner)).thenReturn(null); + awaitSaves(); + when(server.getPlayer(owner)).thenReturn(player); + Project retained = studio.requireProject(player); + assertEquals("retained", retained.pending().notes().getFirst().sound()); + doCallRealMethod().when(store).save(any()); + studio.edit(player, retained.accept()); + awaitSaves(); + assertEquals("retained", new StudioStore(directory).project(owner).orElseThrow() + .song().track(1).notes().getFirst().sound()); + } + + @Test + void shutdownDiscardsDisconnectedPreviewSoundTracking() throws Exception { + studio.save(player, new Project(song(track(1, "sample")), null, 100, false)); + studio.preview(player, false); tick.run(); + when(server.getPlayer(owner)).thenReturn(null); + studio.close(); + assertEquals("sample", new StudioStore(directory).project(owner).orElseThrow() + .song().track(1).notes().getFirst().sound()); + verify(player, never()).stopSound(anyString(), any(SoundCategory.class)); + } + + @Test + void shutdownRetriesAFailedFinalSaveWhenStorageHasRecovered() throws Exception { + studio.create(player, "Song", false); + studio.record(player, 1); tick.run(); tick.run(); + studio.capture(player, "lute", "recovered", 1, 1); + doThrow(new IOException("Temporary disk failure")).when(store).save(any()); + studio.stop(player); + awaitSaveFutures(); // The failure has not yet been observed by the musical clock. + doCallRealMethod().when(store).save(any()); + studio.close(); + assertEquals("recovered", new StudioStore(directory).project(owner).orElseThrow() + .pending().notes().getFirst().sound()); + verify(player, never()).sendMessage(startsWith("Take saved")); + } + + @Test + void disconnectAndShutdownDoNotRewriteProjectsAlreadyOnDisk() throws Exception { + studio.save(player, new Project(song(track(1, "cached")), null, 100, false)); + clearInvocations(store); + studio.quit(player); + studio.close(); + verify(store, never()).save(any()); + verify(player, never()).sendMessage(startsWith("Take saved")); + } + + @Test + void disconnectEvictsACurrentProjectSoTheNextVisitReadsItsFiles() throws Exception { + studio.save(player, new Project(song(track(1, "before")), null, 100, false)); + studio.quit(player); + new StudioStore(directory).save(new Project(song(track(1, "after")), null, 100, false)); + assertEquals("after", studio.requireProject(player).song().track(1).notes().getFirst().sound()); + } + + @Test + void disconnectRetriesATakeWhoseSaveFailedWhileOnline() throws Exception { + studio.create(player, "Song", false); + studio.record(player, 1); tick.run(); tick.run(); + studio.capture(player, "lute", "retry", 1, 1); + doThrow(new IOException("Temporary disk failure")).when(store).save(any()); + studio.stop(player); + awaitSaves(); + doCallRealMethod().when(store).save(any()); + studio.quit(player); + when(server.getPlayer(owner)).thenReturn(null); + awaitSaves(); + assertEquals("retry", new StudioStore(directory).project(owner).orElseThrow() + .pending().notes().getFirst().sound()); + } + + @Test + void failedEditsAreKeptAndRetriedEveryMinuteUntilTheyAreWritten() throws Exception { + Project project = new Project(song(track(1, "note")), null, 100, false); + studio.save(player, project); + Project renamed = new Project(new Song(project.song().id(), owner, "Musician", "Renamed", project.song().tracks()), null, 100, false); + doThrow(new IOException("Disk full")).when(store).save(any()); + studio.edit(player, renamed); + awaitSaves(); + verify(player).sendMessage(startsWith("Could not save your project")); + assertEquals(renamed, studio.requireProject(player)); + assertEquals(project, new StudioStore(directory).project(owner).orElseThrow()); + doCallRealMethod().when(store).save(any()); + clock(StudioService.RETRY_TICKS - 2); + tick.run(); + assertFalse(awaitSaveFutures()); // Not retried before the minute. + tick.run(); + awaitSaves(); + assertEquals(renamed, new StudioStore(directory).project(owner).orElseThrow()); + clearInvocations(store); + clock(2 * StudioService.RETRY_TICKS - 1); + tick.run(); + verify(store, never()).save(any()); // Nothing left to retry. + } + + @Test + void switchingProjectsOnlyRewritesTheSelectionUnlessAWriteFailed() throws Exception { + Project first = new Project(song(track(1, "first")), null, 100, false); + Project second = new Project(song(track(1, "second")), null, 100, false); + studio.save(player, first); + studio.save(player, second); + clearInvocations(store); + studio.selectProject(player, first.song().id()); + verify(store, never()).save(any()); + assertEquals(first, new StudioStore(directory).project(owner).orElseThrow()); + Project renamed = new Project(new Song(first.song().id(), owner, "Musician", "Renamed", first.song().tracks()), null, 100, false); + doThrow(new IOException("Disk full")).when(store).save(any()); + studio.edit(player, renamed); + awaitSaves(); + doCallRealMethod().when(store).save(any()); + studio.selectProject(player, second.song().id()); + assertEquals(renamed, new StudioStore(directory).project(owner, first.song().id())); + assertEquals(second, new StudioStore(directory).project(owner).orElseThrow()); + doReturn(second).when(store).project(owner, first.song().id()); // The file disappears after it was read. + java.nio.file.Files.delete(directory.resolve("projects").resolve(owner.toString()).resolve(first.song().id() + ".yml")); + assertThrows(IllegalArgumentException.class, () -> studio.selectProject(player, first.song().id())); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void shutdownWaitIsBoundedAndInterruptionPreservesQueuedSaves(boolean interrupted) throws Exception { + studio.close(); + var executor = java.util.concurrent.Executors.newSingleThreadExecutor( + Thread.ofPlatform().daemon(true).name("musical-instruments-checkpoints").factory()); + var worker = spy(executor); + if (interrupted) doThrow(new InterruptedException("Shutdown interrupted")).when(worker).awaitTermination(StudioService.SHUTDOWN_WAIT_SECONDS, TimeUnit.SECONDS); + else doReturn(false).when(worker).awaitTermination(StudioService.SHUTDOWN_WAIT_SECONDS, TimeUnit.SECONDS); + studio = new StudioService(plugin, manager, store, new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 2, 0, 0), + StudioStation.none(), discs, worker); + var callbacks = ArgumentCaptor.forClass(Runnable.class); + verify(server.getScheduler(), times(2)).runTaskTimer(eq(plugin), callbacks.capture(), eq(1L), eq(1L)); + tick = callbacks.getAllValues().getLast(); + studio.create(player, "Song", false); + CountDownLatch started = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + doAnswer(invocation -> { + if (Thread.currentThread().getName().equals("musical-instruments-checkpoints")) { + started.countDown(); + if (!release.await(3, TimeUnit.SECONDS)) throw new IOException("Save timed out"); + } + return invocation.callRealMethod(); + }).when(store).save(any()); + studio.record(player, 1); tick.run(); tick.run(); + studio.capture(player, "lute", "final", 1, 1); + studio.stop(player); + assertTrue(started.await(3, TimeUnit.SECONDS)); + try { + studio.close(); + assertEquals(interrupted, Thread.currentThread().isInterrupted()); + verify(worker).shutdown(); + verify(worker).awaitTermination(StudioService.SHUTDOWN_WAIT_SECONDS, TimeUnit.SECONDS); + assertNotNull(studio.requireProject(player).pending()); + } finally { + Thread.interrupted(); + release.countDown(); + awaitSaves(); + assertTrue(executor.awaitTermination(3, TimeUnit.SECONDS)); + } + assertEquals("final", new StudioStore(directory).project(owner).orElseThrow() + .pending().notes().getFirst().sound()); + } + @TempDir Path directory; private InstrumentPlugin plugin; private Server server; @@ -78,7 +363,7 @@ void setup() throws Exception { when(world.isChunkLoaded(anyInt(), anyInt())).thenReturn(true); when(world.getPlayers()).thenReturn(List.of(player)); when(scheduler.runTaskTimer(eq(plugin), any(Runnable.class), eq(1L), eq(1L))).thenReturn(task); - studio = new StudioService(plugin, manager, store, new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 2), + studio = new StudioService(plugin, manager, store, new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 2, 0, 0), StudioStation.none(), discs); var callback = ArgumentCaptor.forClass(Runnable.class); verify(scheduler).runTaskTimer(eq(plugin), callback.capture(), eq(1L), eq(1L)); @@ -90,6 +375,25 @@ void close() { studio.close(); } + private void awaitSaves() throws Exception { + if (awaitSaveFutures()) tick.run(); + } + + private boolean awaitSaveFutures() throws Exception { + var field = StudioService.class.getDeclaredField("pendingSaves"); + field.setAccessible(true); + var pending = (java.util.Map) field.get(studio); + if (pending.isEmpty()) return false; + for (Object save : List.copyOf(pending.values())) { + try { + ((CompletableFuture) save).get(3, TimeUnit.SECONDS); + } catch (java.util.concurrent.ExecutionException expectedFailure) { + // The musical clock reports failures and retains the take for retry. + } + } + return true; + } + private Song song(Track... tracks) { return new Song(UUID.randomUUID(), owner, "Musician", "Song", List.of(tracks)); } @@ -107,10 +411,10 @@ void countInIgnoresNotesAndOverdubRetainsThePreviousTrackUntilKept() throws Exce tick.run(); studio.capture(player, "flute", "early", 4, 1); tick.run(); - verify(player).playSound(any(Location.class), eq("backing"), eq(SoundCategory.RECORDS), eq(4f), eq(1f)); + verify(player).playSound(any(Location.class), eq("backing"), eq(SoundCategory.PLAYERS), eq(4f), eq(1f)); verify(player, never()).playSound(any(Location.class), eq("old_take"), any(SoundCategory.class), anyFloat(), anyFloat()); studio.capture(player, "flute", "new_take", 2, 1.5f); - studio.stop(player); + studio.stop(player); awaitSaves(); Project pending = studio.requireProject(player); assertEquals(original, pending.song()); assertEquals(1, pending.pending().notes().size()); @@ -124,8 +428,8 @@ void previewsPlaySimultaneousTracksWithoutEmittingLivePerformanceEvents() throws studio.save(player, new Project(song(track(1, "lute"), track(2, "flute")), null, 100, false)); studio.preview(player, false); tick.run(); - verify(player).playSound(any(Location.class), eq("lute"), eq(SoundCategory.RECORDS), eq(4f), eq(1f)); - verify(player).playSound(any(Location.class), eq("flute"), eq(SoundCategory.RECORDS), eq(4f), eq(1f)); + verify(player).playSound(any(Location.class), eq("lute"), eq(SoundCategory.PLAYERS), eq(4f), eq(1f)); + verify(player).playSound(any(Location.class), eq("flute"), eq(SoundCategory.PLAYERS), eq(4f), eq(1f)); verifyNoInteractions(pluginManager); } @@ -153,7 +457,7 @@ void changingProjectsIsBlockedDuringCaptureAndIsSafeAfterFinishing() throws Exce studio.record(player, 1); assertThrows(IllegalArgumentException.class, () -> studio.selectProject(player, first)); assertThrows(IllegalArgumentException.class, () -> studio.create(player, "Third", false)); - studio.stop(player); + studio.stop(player); awaitSaves(); studio.selectProject(player, first); assertEquals(first, studio.requireProject(player).song().id()); } @@ -163,25 +467,28 @@ void listeningToOneTrackDoesNotEmitTheRestOfTheMix() throws Exception { studio.save(player, new Project(song(track(1, "solo"), track(2, "other")), null, 100, false)); studio.previewTrack(player, 1); tick.run(); - verify(player).playSound(any(Location.class), eq("solo"), eq(SoundCategory.RECORDS), anyFloat(), anyFloat()); + verify(player).playSound(any(Location.class), eq("solo"), eq(SoundCategory.PLAYERS), anyFloat(), anyFloat()); verify(player, never()).playSound(any(Location.class), eq("other"), any(SoundCategory.class), anyFloat(), anyFloat()); } @Test void checkpointsAndShutdownRecoverAnUnacceptedTake() throws Exception { studio.create(player, "Song", false); + // Wait for the checkpoint to finish rather than polling its file: on Windows, reading a + // file can make the writer's replacement of it fail. + CountDownLatch checkpointed = new CountDownLatch(1); + doAnswer(invocation -> { + Object result = invocation.callRealMethod(); + checkpointed.countDown(); + return result; + }).when(store).save(any()); studio.record(player, 1); tick.run(); tick.run(); studio.capture(player, "lute", "first", 4, 1); tick.run(); tick.run(); - verify(store, timeout(3000).times(2)).save(any()); - // Mockito observes the call at entry; await the atomically installed file contents. - long deadline = System.nanoTime() + 3_000_000_000L; - while (store.project(owner).orElseThrow().pending() == null && System.nanoTime() < deadline) { - Thread.onSpinWait(); - } + assertTrue(checkpointed.await(3, TimeUnit.SECONDS)); assertEquals("first", new StudioStore(directory).project(owner).orElseThrow().pending().notes().getFirst().sound()); studio.capture(player, "lute", "second", 4, 1); studio.close(); @@ -196,7 +503,7 @@ void emptyTakeDoesNotReplaceASavedTrack() throws Exception { Song original = song(track(1, "saved")); studio.save(player, new Project(original, null, 100, false)); studio.record(player, 1); - studio.stop(player); + studio.stop(player); awaitSaves(); assertEquals(original, studio.requireProject(player).song()); assertNull(studio.requireProject(player).pending()); } @@ -226,6 +533,7 @@ void anOlderBackgroundCheckpointCannotOverwriteTheFinalTake() throws Exception { CompletableFuture stopped = CompletableFuture.runAsync(() -> studio.stop(player)); release.countDown(); stopped.get(3, TimeUnit.SECONDS); + awaitSaves(); Project saved = new StudioStore(directory).project(owner).orElseThrow(); assertEquals(List.of("first", "second"), saved.pending().notes().stream().map(Note::sound).toList()); } @@ -243,7 +551,7 @@ void noteLimitFinishesTheTakeWithoutOverwritingTheOldTrack() throws Exception { Project project = studio.requireProject(player); assertEquals(original, project.song()); assertEquals(20, project.pending().notes().size()); - studio.requireIdle(player); + awaitSaves(); studio.requireIdle(player); } @Test @@ -258,7 +566,7 @@ void maximumDurationFinishesAndPersistsTheTake() throws Exception { } Project project = studio.requireProject(player); assertEquals(40, project.pending().lengthTicks()); - studio.requireIdle(player); + awaitSaves(); studio.requireIdle(player); assertEquals(project, store.project(owner).orElseThrow()); } @@ -271,10 +579,44 @@ void failedPublicationPreservesThePhysicalBlankDisc() throws Exception { when(discs.blank(blank)).thenReturn(true); when(discs.disc(any())).thenReturn(mock(ItemStack.class)); doThrow(new IOException("Disk full")).when(store).publish(any()); - assertThrows(IOException.class, () -> studio.publish(player)); + IOException failure = assertThrows(IOException.class, () -> studio.publish(player)); + studio.failure(player, failure); + verify(player).sendMessage(contains("Your disc was preserved")); verify(inventory, never()).setItemInMainHand(any()); } + @Test + void publicationCooldownLimitsHowOftenEditionsCanBeWritten() throws Exception { + configure(new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 2, 40, 0)); + studio.save(player, new Project(song(track(1, "note")), null, 100, false)); + ItemStack disc = mock(ItemStack.class); + when(discs.musicDisc(disc)).thenReturn(true); + when(disc.getAmount()).thenReturn(1); + when(discs.disc(any(Song.class), eq(disc))).thenReturn(disc); + studio.publishOnDisc(player, disc); + var wait = assertThrows(IllegalArgumentException.class, () -> studio.publishOnDisc(player, disc)); + assertEquals("Wait 2 seconds before publishing again", wait.getMessage()); + for (int elapsed = 0; elapsed < 40; elapsed++) tick.run(); + studio.publishOnDisc(player, disc); + verify(store, times(1)).publish(any()); // Unchanged music reuses the first edition. + } + + @Test + void theDailyEditionLimitKeepsTheDiscWhenReached() throws Exception { + configure(new StudioSettings(4, 40, 20, 60, 8, 32, 2, 0, 2, 0, 1)); + Project project = new Project(song(track(1, "note")), null, 100, false); + studio.save(player, project); + ItemStack disc = mock(ItemStack.class); + when(discs.musicDisc(disc)).thenReturn(true); + when(disc.getAmount()).thenReturn(1); + when(discs.disc(any(Song.class), eq(disc))).thenReturn(disc); + studio.publishOnDisc(player, disc); + studio.save(player, new Project(project.song().replace(track(2, "other")), null, 100, false)); + var limit = assertThrows(IllegalArgumentException.class, () -> studio.publishOnDisc(player, disc)); + assertTrue(limit.getMessage().startsWith("You can publish 1 new song editions a day")); + verify(discs, times(1)).disc(any(Song.class), eq(disc)); + } + private ItemStack inventoryItem(Material type, int amount, boolean blank) { ItemStack item = mock(ItemStack.class); Material material = mock(Material.class); @@ -383,6 +725,22 @@ private Block jukebox(Song song, ItemStack disc) throws IOException { return block; } + @Test + void jukeboxRecordIsCheckedOnceASecondAndARemovedJukeboxStopsAtOnce() throws Exception { + Song song = song(new Track(1, 60, 1, false, List.of(new Note(0, "lute", "first", 4, 1), + new Note(5, "lute", "second", 4, 1), new Note(25, "lute", "third", 4, 1)))); + ItemStack disc = mock(ItemStack.class); + Block block = jukebox(song, disc); + studio.play(block, disc); + clearInvocations(block); + for (int elapsed = 0; elapsed <= 5; elapsed++) tick.run(); + verify(block, times(1)).getState(); + verify(player).playSound(any(Location.class), eq("second"), eq(SoundCategory.RECORDS), anyFloat(), anyFloat()); + when(block.getType()).thenReturn(Material.AIR); + for (int elapsed = 6; elapsed <= 25; elapsed++) tick.run(); + verify(player, never()).playSound(any(Location.class), eq("third"), any(SoundCategory.class), anyFloat(), anyFloat()); + } + @Test void jukeboxAppliesItsSnapshotAndPlaysEveryTrackOnlyToNearbyListeners() throws Exception { Song song = song(track(1, "lute"), track(2, "flute")); @@ -435,10 +793,16 @@ void fullLibraryRejectsCreationUntilAProjectIsDeletedAndResetRemainsAllowed() th assertEquals(active, studio.requireProject(player)); studio.create(player, "Reset", true); assertEquals(36, studio.listProjects(player).size()); - Project inactive = studio.listProjects(player).stream().filter(p -> !p.song().id().equals(active.song().id())).findFirst().orElseThrow(); - studio.deleteProject(player, inactive); + ProjectSummary inactive = studio.listProjects(player).stream().filter(p -> !p.id().equals(active.song().id())).findFirst().orElseThrow(); + studio.deleteProject(player, studio.project(player, inactive.id())); studio.create(player, "Replacement", false); assertEquals(36, studio.listProjects(player).size()); + // Without an active project, a reset would add a 37th project. + studio.quit(player); + java.nio.file.Files.writeString(directory.resolve("projects").resolve(owner.toString()).resolve(StudioStore.SELECTED), "garbage"); + assertNull(studio.project(player)); + assertThrows(IllegalArgumentException.class, () -> studio.create(player, "Untitled", true)); + assertEquals(36, studio.listProjects(player).size()); } @Test @@ -447,13 +811,12 @@ void deletingActiveProjectDoesNotResurrectItAndKeepsOtherProjectsAndEditions() t Project other = studio.requireProject(player); Project active = new Project(song(track(1, "note")), null, 100, false); studio.save(player, active); - Song edition = active.song().edition(); - store.publish(edition); + Song edition = store.publishEdition(active.song(), 0); studio.deleteProject(player, active); assertNull(studio.project(player)); - studio.quit(player); + studio.quit(player); awaitSaves(); assertTrue(store.project(owner).isEmpty()); - assertEquals(List.of(other), store.projects(owner)); + assertEquals(List.of(ProjectSummary.of(other)), store.summaries(owner)); assertEquals(edition, store.song(edition.id())); } @@ -461,8 +824,9 @@ void deletingActiveProjectDoesNotResurrectItAndKeepsOtherProjectsAndEditions() t void deletionRejectsStaleConfirmationForeignOwnerAndActiveRecording() throws Exception { Project before = new Project(song(track(1, "note")), null, 100, false); studio.save(player, before); - Project changed = new Project(before.song(), null, 105, false); - studio.edit(player, changed); + Song song = before.song(); + Project changed = new Project(new Song(song.id(), owner, song.author(), "Renamed", song.tracks()), null, 100, false); + studio.edit(player, changed); awaitSaves(); assertThrows(IllegalArgumentException.class, () -> studio.deleteProject(player, before)); Project foreign = new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Other", "Other", List.of()), null, 100, false); assertThrows(IllegalArgumentException.class, () -> studio.deleteProject(player, foreign)); @@ -471,6 +835,91 @@ void deletionRejectsStaleConfirmationForeignOwnerAndActiveRecording() throws Exc assertEquals(changed, store.project(owner).orElseThrow()); } + @Test + void deletionMustRejectAReplacementTakeOfTheSameDuration() throws Exception { + Project original = new Project(song(new Track(1, 20, 1, false, + List.of(new Note(0, "lute", "original_take", 1, 1)))), null, 100, false); + studio.save(player, original); + Project confirmation = studio.project(player, original.song().id()); + studio.record(player, 1); + tick.run(); tick.run(); + studio.capture(player, "lute", "replacement_take", 1, 1); + for (int index = 0; index < 19; index++) tick.run(); + studio.stop(player); + Project kept = studio.requireProject(player).accept(); + studio.edit(player, kept); + Project changed = new Project(kept.song().replace(kept.song().track(1).mix(.5f, true)), + null, 120, true); + studio.edit(player, changed); awaitSaves(); + assertEquals(ProjectSummary.of(confirmation), ProjectSummary.of(changed)); + assertThrows(IllegalArgumentException.class, () -> studio.deleteProject(player, confirmation)); + assertEquals(changed, store.project(owner, original.song().id())); + assertEquals(changed, studio.requireProject(player)); + } + + @Test + void deletionRejectsEditsToAnInactiveProjectAndLoadsOnlyThatProject() throws Exception { + Project original = new Project(song(track(1, "original")), null, 100, false); + store.save(original); + Project active = new Project(song(track(1, "other_song")), null, 100, false); + studio.save(player, active); + Project confirmation = studio.project(player, original.song().id()); + Project changed = new Project(original.song().replace(track(1, "replacement")), null, 100, false); + store.save(changed); + store.select(owner, active.song().id()); + assertEquals(ProjectSummary.of(original), ProjectSummary.of(changed)); + assertThrows(IllegalArgumentException.class, () -> studio.deleteProject(player, confirmation)); + verify(store, times(2)).project(owner, original.song().id()); + verify(store, never()).project(owner, active.song().id()); + verify(store, never()).summaries(owner); + assertEquals(changed, store.project(owner, original.song().id())); + assertEquals(active, studio.requireProject(player)); + } + + @ParameterizedTest + @ValueSource(strings = {"sound", "instrument", "timing", "volume", "pitch", "gain", "mute", "bpm", "metronome", "pending"}) + void deletionRejectsContentChangesThatLeaveTheLibrarySummaryUnchanged(String change) throws Exception { + Track track = track(1, "original"); + Track pending = track(1, "pending"); + Project original = new Project(song(track), pending, 100, false); + studio.save(player, original); + Project confirmation = studio.project(player, original.song().id()); + Note note = switch (change) { + case "sound" -> new Note(0, "lute", "replacement", 4, 1); + case "instrument" -> new Note(0, "flute", "original", 4, 1); + case "timing" -> new Note(1, "lute", "original", 4, 1); + case "volume" -> new Note(0, "lute", "original", 2, 1); + case "pitch" -> new Note(0, "lute", "original", 4, .5f); + default -> track.notes().getFirst(); + }; + Track replacement = new Track(1, track.lengthTicks(), change.equals("gain") ? .5f : 1, + change.equals("mute"), List.of(note)); + Project changed = new Project(original.song().replace(replacement), + change.equals("pending") ? track(1, "new_pending") : pending, + change.equals("bpm") ? 120 : 100, change.equals("metronome")); + studio.edit(player, changed); awaitSaves(); + assertEquals(ProjectSummary.of(confirmation), ProjectSummary.of(changed)); + assertThrows(IllegalArgumentException.class, () -> studio.deleteProject(player, confirmation)); + assertEquals(changed, store.project(owner, original.song().id())); + assertEquals(changed, studio.requireProject(player)); + } + + @Test + void deletionChecksTheNewerCachedContentAfterItsWriteFails() throws Exception { + Project original = new Project(song(track(1, "original")), null, 100, false); + studio.save(player, original); + Project confirmation = studio.project(player, original.song().id()); + Project changed = new Project(original.song().replace(track(1, "replacement")), null, 100, false); + doThrow(new IOException("Write failed")).when(store).save(changed); + studio.edit(player, changed); awaitSaves(); + assertEquals(original, store.project(owner, original.song().id())); + assertEquals(changed, studio.project(player, original.song().id())); + assertThrows(IllegalArgumentException.class, () -> studio.deleteProject(player, confirmation)); + assertEquals(original, store.project(owner, original.song().id())); + assertEquals(changed, studio.requireProject(player)); + doCallRealMethod().when(store).save(changed); + } + @Test void publishingOnProvidedDiscKeepsEditionsSeparateAndRejectsStacks() throws Exception { studio.save(player, new Project(song(track(1, "note")), null, 100, false)); @@ -577,12 +1026,12 @@ void projectSelectionReloadsAfterQuitAndResetWorksWithoutAnExistingProject() thr studio.create(player, "Reset", true); Project project = studio.requireProject(player); studio.selectProject(player, project.song().id()); - studio.quit(player); + studio.quit(player); awaitSaves(); assertEquals(project, studio.requireProject(player)); Project foreign = new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Other", "Foreign", List.of()), null, 100, false); assertThrows(IllegalArgumentException.class, () -> studio.save(player, foreign)); - studio.quit(player); - studio.quit(player); + studio.quit(player); awaitSaves(); + studio.quit(player); awaitSaves(); } @Test @@ -602,7 +1051,7 @@ void previewAndRecordingValidatePendingTakesSlotsAndAudibility() throws Exceptio assertTrue(studio.activity(player).previewing()); tick.run(); verify(player).playSound(any(Location.class), eq("pending"), any(SoundCategory.class), anyFloat(), anyFloat()); - studio.stop(player); + studio.stop(player); awaitSaves(); assertFalse(studio.activity(player).previewing()); studio.edit(player, project.discard()); ItemStack disc = mock(ItemStack.class); @@ -612,11 +1061,32 @@ void previewAndRecordingValidatePendingTakesSlotsAndAudibility() throws Exceptio studio.record(player, 1); tick.run(); tick.run(); studio.capture(player, "lute", "replacement", 1, 1); - studio.stop(player); + studio.stop(player); awaitSaves(); assertTrue(studio.requireProject(player).pending().muted()); assertEquals(.5f, studio.requireProject(player).pending().gain()); } + @Test + void takesOnMutedOrSilentTracksStayAudibleWhileReviewing() throws Exception { + Track muted = new Track(1, 10, .5f, true, track(1, "muted").notes()); + Track silent = new Track(2, 10, 0, false, track(2, "silent").notes()); + studio.save(player, new Project(song(muted, silent), null, 100, false)); + assertThrows(IllegalArgumentException.class, () -> studio.preview(player, false)); + assertThrows(IllegalArgumentException.class, () -> studio.previewTrack(player, 2)); + assertFalse(studio.activity(player).previewing()); + for (int slot : new int[] {1, 2}) { + Project project = studio.requireProject(player); + studio.save(player, new Project(project.song(), track(slot, "take" + slot), 100, false)); + studio.preview(player, true); + tick.run(); + verify(player).playSound(any(Location.class), eq("take" + slot), eq(SoundCategory.PLAYERS), eq(slot == 1 ? 2f : 4f), anyFloat()); + verify(player, never()).playSound(any(Location.class), eq("muted"), any(SoundCategory.class), anyFloat(), anyFloat()); + verify(player, never()).playSound(any(Location.class), eq("silent"), any(SoundCategory.class), anyFloat(), anyFloat()); + studio.stop(player); + studio.save(player, studio.requireProject(player).discard()); + } + } + @Test void invalidSamplesStopCaptureAndDisconnectedPlayersDoNotReceivePlayback() throws Exception { studio.capture(player, "lute", "ignored", 1, 1); @@ -627,6 +1097,7 @@ void invalidSamplesStopCaptureAndDisconnectedPlayersDoNotReceivePlayback() throw studio.capture(player, "lute", "invalid sound", 1, 1); assertFalse(studio.activity(player).recording()); assertEquals(1, studio.requireProject(player).pending().notes().size()); + awaitSaves(); studio.edit(player, studio.requireProject(player).accept()); studio.preview(player, false); when(server.getPlayer(owner)).thenReturn(null); @@ -666,10 +1137,10 @@ void failedCheckpointAndFinalSaveRetainTheTakeInMemoryForRetry() throws Exceptio tick.run(); tick.run(); assertFalse(studio.activity(player).recording()); assertEquals("saved", studio.requireProject(player).pending().notes().getFirst().sound()); - studio.quit(player); + studio.quit(player); awaitSaves(); assertNotNull(studio.requireProject(player).pending()); studio.close(); - verify(player, atLeastOnce()).sendMessage(contains("Could not read/write")); + verify(player, atLeastOnce()).sendMessage(startsWith("Could not save your project")); doCallRealMethod().when(store).save(any()); } @@ -749,6 +1220,9 @@ void newSessionsCanSelectStoredProjectsAndDeleteAnUnselectedProject() throws Exc studio.deleteProject(player, first); assertEquals(second, studio.requireProject(player)); studio.quit(player); + when(server.getPlayer(owner)).thenReturn(null); + awaitSaves(); + when(server.getPlayer(owner)).thenReturn(player); doReturn(java.util.Optional.empty()).when(store).project(owner); studio.selectProject(player, second.song().id()); assertEquals(second, studio.requireProject(player)); @@ -805,14 +1279,14 @@ private void clock(long value) throws Exception { @Test void simultaneousNoteFloodsAndSongLimitsBoundCapture() throws Exception { - configure(new StudioSettings(4, 40, 50, 60, 8, 32, 2, 0, 2)); + configure(new StudioSettings(4, 40, 50, 60, 8, 32, 2, 0, 2, 0, 0)); studio.create(player, "Song", false); studio.record(player, 1); tick.run(); tick.run(); for (int index = 0; index < 34; index++) studio.capture(player, "lute", "note", 1, 1); assertEquals(32, studio.activity(player).notes()); clock(42); studio.capture(player, "lute", "late", 1, 1); assertEquals(32, studio.activity(player).notes()); - studio.stop(player); studio.edit(player, studio.requireProject(player).accept()); + studio.stop(player); awaitSaves(); studio.edit(player, studio.requireProject(player).accept()); studio.record(player, 2); tick.run(); tick.run(); for (int index = 0; index < 29; index++) studio.capture(player, "lute", "note", 1, 1); assertFalse(studio.activity(player).recording()); @@ -821,7 +1295,7 @@ void simultaneousNoteFloodsAndSongLimitsBoundCapture() throws Exception { @Test void sessionCapacityDoesNotInterruptAnExistingPreviewAndPlaybackEndsCleanly() throws Exception { - configure(new StudioSettings(4, 40, 20, 60, 1, 32, 2, 0, 2)); + configure(new StudioSettings(4, 40, 20, 60, 1, 32, 2, 0, 2, 0, 0)); studio.save(player, new Project(song(track(1, "note")), null, 100, false)); studio.preview(player, false); Player second = mock(Player.class); @@ -834,7 +1308,7 @@ void sessionCapacityDoesNotInterruptAnExistingPreviewAndPlaybackEndsCleanly() th for (int index = 0; index < 12; index++) tick.run(); assertFalse(studio.activity(player).previewing()); studio.record(player, 1); - studio.quit(player); + studio.quit(player); awaitSaves(); assertFalse(studio.activity(player).recording()); } @@ -858,6 +1332,7 @@ void archivedProjectCanBeDeletedWhenTheSelectedFileIsMissing() throws Exception store.save(project); doReturn(java.util.Optional.empty()).when(store).project(owner); assertEquals(project, studio.project(player, project.song().id())); + assertThrows(IllegalArgumentException.class, () -> studio.project(player, UUID.randomUUID())); studio.deleteProject(player, project); assertNull(studio.project(player)); } @@ -867,7 +1342,7 @@ void anEmptyCaptureDoesNotQueueCheckpointsAndInvalidDiscsCannotPublish() throws studio.create(player, "Empty", false); studio.record(player, 1); for (int index = 0; index < 6; index++) tick.run(); verify(store, times(1)).save(any()); - studio.stop(player); + studio.stop(player); awaitSaves(); assertThrows(IllegalArgumentException.class, () -> studio.publishOnDisc(player, mock(ItemStack.class))); } diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStationTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStationTest.java index 0911e2f..070a3ba 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStationTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStationTest.java @@ -117,8 +117,43 @@ void vanillaStationsMatchTheirBlockOnly() { assertFalse(StudioStation.none().matches(furniture)); } + @Test + void itemsAdderStationsMatchNothingWhileItemsAdderIsNotEnabled() { + StudioStation station = StudioStation.itemsAdder("tfmc:music_station"); + CustomBlock.lookup = target -> new CustomBlock("tfmc:music_station"); + CustomFurniture.lookup = target -> new CustomFurniture("tfmc:music_station"); + assertFalse(station.matches(block)); + assertFalse(station.matches(furniture)); + verify(world, never()).getNearbyEntities(any(BoundingBox.class)); + var itemsAdder = MockBukkit.createMockPlugin("ItemsAdder"); + assertTrue(station.matches(block)); + assertTrue(station.matches(furniture)); + MockBukkit.getMock().getPluginManager().disablePlugin(itemsAdder); + assertFalse(station.matches(block)); + } + + @Test + void lookupsMissingFromTheInstalledItemsAdderAreRememberedAsMissing() throws Exception { + var method = StudioStation.class.getDeclaredMethod("method", String.class, + Class.forName(StudioStation.class.getName() + "$Lookup")); + method.setAccessible(true); + int[] lookups = {0}; + Object missing = java.lang.reflect.Proxy.newProxyInstance(getClass().getClassLoader(), + new Class[] {Class.forName(StudioStation.class.getName() + "$Lookup")}, (proxy, called, args) -> { + lookups[0]++; + throw new NoSuchMethodException("byAlreadySpawned(Block)"); + }); + for (int attempt = 0; attempt < 2; attempt++) { + var failure = assertThrows(java.lang.reflect.InvocationTargetException.class, + () -> method.invoke(null, "test#missing", missing)); + assertInstanceOf(NoSuchMethodException.class, failure.getCause()); + } + assertEquals(1, lookups[0]); + } + @Test void itemsAdderStationsMatchCustomBlocksAndFurniture() { + MockBukkit.createMockPlugin("ItemsAdder"); StudioStation station = StudioStation.itemsAdder("tfmc:music_station"); assertFalse(station.matches(block)); assertFalse(station.matches((Entity) null)); @@ -172,6 +207,7 @@ void playersMustStandNearAStation() { @Test void furnitureHitboxesResolveWithoutAnIntersectingEntityAndFallBackWhenUnavailable() { + MockBukkit.createMockPlugin("ItemsAdder"); StudioStation station = StudioStation.itemsAdder("tfmc:music_station"); CustomFurniture.blockLookup = target -> new CustomFurniture("tfmc:music_station"); assertTrue(station.matches(block)); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreRecoveryTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreRecoveryTest.java index 84f9420..cbcbcdc 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreRecoveryTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreRecoveryTest.java @@ -16,31 +16,39 @@ class StudioStoreRecoveryTest { @TempDir Path directory; + @Test + void oversizedSerializationCannotReplaceTheLastSavedFile() throws Exception { + StudioStore store = new StudioStore(directory); + Project project = project(); store.save(project); + Path selected = directory.resolve("projects/" + project.song().owner() + "/" + project.song().id() + ".yml"); + byte[] original = Files.readAllBytes(selected); + YamlConfiguration oversized = mock(YamlConfiguration.class); + when(oversized.saveToString()).thenReturn("x".repeat(StudioStore.MAX_FILE_BYTES + 1)); + var write = StudioStore.class.getDeclaredMethod("write", Path.class, YamlConfiguration.class, boolean.class); + write.setAccessible(true); + var failure = assertThrows(java.lang.reflect.InvocationTargetException.class, + () -> write.invoke(store, selected, oversized, true)); + assertInstanceOf(IOException.class, failure.getCause()); + assertArrayEquals(original, Files.readAllBytes(selected)); + assertEquals(project, store.project(project.song().owner()).orElseThrow()); + } + private Project project() { return new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Author", "Song", List.of(new Track(1, 10, 1, false, List.of(new Note(0, "lute", "note", 1, 1))))), null, 100, false); } @Test - void newerArchiveRecoversAnInterruptedSelectionWriteAndLegacySelectionRemainsUsable() throws Exception { + void removedProjectFilesAreNoLongerAvailable() throws Exception { StudioStore store = new StudioStore(directory); Project original = project(); store.save(original); - Path selected = directory.resolve("projects/" + original.song().owner() + ".yml"); - Path archive = directory.resolve("projects/" + original.song().owner() + "/" + original.song().id() + ".yml"); - Song song = original.song(); - Project renamed = new Project(new Song(song.id(), song.owner(), song.author(), "Recovered", song.tracks()), null, 100, false); - store.save(renamed); - Files.copy(archive, selected, StandardCopyOption.REPLACE_EXISTING); - YamlConfiguration yaml = YamlConfiguration.loadConfiguration(selected.toFile()); - yaml.set("title", "Original"); yaml.save(selected.toFile()); - Files.setLastModifiedTime(selected, FileTime.fromMillis(1)); - assertEquals(renamed, store.project(original.song().owner()).orElseThrow()); - Files.delete(archive); - assertEquals("Original", store.project(original.song().owner(), original.song().id()).song().title()); - assertThrows(IllegalArgumentException.class, () -> store.project(original.song().owner(), UUID.randomUUID())); - store.delete(original.song().owner(), original.song().id()); - assertTrue(store.projects(original.song().owner()).isEmpty()); + UUID owner = original.song().owner(); + assertThrows(IllegalArgumentException.class, () -> store.project(owner, UUID.randomUUID())); + Files.delete(directory.resolve("projects/" + owner + "/" + original.song().id() + ".yml")); + assertThrows(IllegalArgumentException.class, () -> store.project(owner, original.song().id())); + assertTrue(store.project(owner).isEmpty()); + assertTrue(store.summaries(owner).isEmpty()); } @Test @@ -49,9 +57,9 @@ void malformedArchiveNamesAndMismatchedIdsAreRejected() throws Exception { Project project = project(); store.save(project); Path folder = directory.resolve("projects/" + project.song().owner()); Files.writeString(folder.resolve("ignore.tmp"), "ignored"); - assertEquals(List.of(project), store.projects(project.song().owner())); + assertEquals(List.of(ProjectSummary.of(project)), store.summaries(project.song().owner())); Files.writeString(folder.resolve("invalid.yml"), "invalid"); - assertThrows(IOException.class, () -> store.projects(project.song().owner())); + assertEquals(List.of(ProjectSummary.of(project)), store.summaries(project.song().owner())); Files.delete(folder.resolve("invalid.yml")); UUID wrong = UUID.randomUUID(); Files.copy(folder.resolve(project.song().id() + ".yml"), folder.resolve(wrong + ".yml")); @@ -65,12 +73,12 @@ void deletionOfAnUnselectedProjectKeepsSelectionAndSortsEqualTitlesById() throws Song song = first.song(); Project second = new Project(new Song(UUID.randomUUID(), song.owner(), song.author(), song.title(), song.tracks()), null, 100, false); store.save(first); store.save(second); - assertEquals(2, store.projects(song.owner()).size()); + assertEquals(2, store.summaries(song.owner()).size()); store.delete(song.owner(), first.song().id()); assertEquals(second, store.project(song.owner()).orElseThrow()); - Files.delete(directory.resolve("projects/" + song.owner() + ".yml")); + Files.delete(directory.resolve("projects/" + song.owner() + "/" + StudioStore.SELECTED)); store.delete(song.owner(), second.song().id()); - assertTrue(store.projects(song.owner()).isEmpty()); + assertTrue(store.summaries(song.owner()).isEmpty()); } @Test @@ -85,17 +93,36 @@ void malformedNumericFieldsAndOversizedSongStructuresCannotLoad() throws Excepti track.put("slot", slot); yaml.set("tracks", List.of(track)); yaml.save(file.toFile()); assertThrows(IOException.class, () -> store.song(project.song().id())); } - for (int mode = 0; mode < 3; mode++) { + for (Object format : List.of(1, 3)) { // Format 1 was written by 3.3.0 and is not read. + YamlConfiguration yaml = new YamlConfiguration(); yaml.loadFromString(valid); + yaml.set("format", format); + yaml.save(file.toFile()); assertThrows(IOException.class, () -> store.song(project.song().id())); + } + YamlConfiguration nine = new YamlConfiguration(); nine.loadFromString(valid); + nine.set("tracks", Collections.nCopies(9, nine.getMapList("tracks").getFirst())); + nine.save(file.toFile()); assertThrows(IOException.class, () -> store.song(project.song().id())); + String tooMany = String.join("\n", Collections.nCopies(Song.MAX_NOTES + 1, "0 0 1 1.0 1.0")); + List compact = List.of( + new Object[] {List.of("lute", "note"), "0 0 1 1.0"}, // Missing field. + new Object[] {List.of("lute", "note"), "0 0 2 1.0 1.0"}, // Unknown name. + new Object[] {List.of("lute", "note"), "x 0 1 1.0 1.0"}, + new Object[] {List.of("lute", "note"), "0 0 1 1.0 1.0\n-1 0 1 1.0 1.0"}, // Earlier than the previous note. + new Object[] {List.of("lute", "note"), "0 0 1 1.0 1.0\n\n0 0 1 1.0 1.0"}, + new Object[] {List.of("lute", "note"), "2147483647 0 1 1.0 1.0\n1 0 1 1.0 1.0"}, + new Object[] {List.of("lute", "note"), "0 0 1 NaN 1.0"}, + new Object[] {List.of("lute", "note"), List.of("0 0 1 1.0 1.0")}, + new Object[] {List.of("lute", 7), "0 0 1 1.0 1.0"}, + new Object[] {Collections.nCopies(2 * Song.MAX_NOTES + 1, "lute"), "0 0 1 1.0 1.0"}, + new Object[] {List.of("lute", "note"), tooMany}); + for (Object[] invalid : compact) { YamlConfiguration yaml = new YamlConfiguration(); yaml.loadFromString(valid); Map track = new LinkedHashMap<>((Map) yaml.getMapList("tracks").getFirst()); - if (mode == 0) yaml.set("tracks", Collections.nCopies(9, track)); - else { - track.put("notes", mode == 1 ? Collections.nCopies(Song.MAX_NOTES + 1, List.of(0, "lute", "note", 1, 1)) : List.of(List.of(0, "lute"))); - yaml.set("tracks", List.of(track)); - } + track.put("names", invalid[0]); + track.put("notes", invalid[1]); + yaml.set("tracks", List.of(track)); yaml.save(file.toFile()); assertThrows(IOException.class, () -> store.song(project.song().id())); } - Files.writeString(file, " ".repeat(8_000_001)); + Files.writeString(file, " ".repeat(StudioStore.MAX_FILE_BYTES + 1)); assertThrows(IOException.class, () -> store.song(project.song().id())); } diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java index 0c51ff3..99b4c79 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java @@ -10,8 +10,43 @@ import java.util.UUID; import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; class StudioStoreTest { + @Test + void maximumSizeProjectAndReplacementTakeStayCompactAndReadable() throws Exception { + String instrument = "i".repeat(100); + String sound = "s".repeat(200); + var notes = java.util.stream.IntStream.range(0, Song.MAX_NOTES) + .mapToObj(tick -> new Note(tick, instrument, sound, 4, 1)).toList(); + Track saved = new Track(1, Song.MAX_TICKS, 1, false, notes); + Project project = new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Player", "Song", List.of(saved)), + saved, 100, false); + StudioStore store = new StudioStore(directory); + store.save(project); + long bytes = Files.size(directory.resolve("projects").resolve(project.song().owner().toString()).resolve(project.song().id() + ".yml")); + assertTrue(bytes < 600_000, "A YAML list per note needed over 8 MB for this project: " + bytes); + assertEquals(project, store.project(project.song().owner()).orElseThrow()); + } + + @Test + void compactNotesStoreEachNameOnceAndTimeSinceThePreviousNote() throws Exception { + var notes = List.of(new Note(5, "lute", "tfmc:lute.c4", 1, 1), new Note(5, "lute", "tfmc:lute.e4", 1, 1.25f), + new Note(12, "flute", "tfmc:lute.c4", .5f, 2)); + Song song = new Song(UUID.randomUUID(), UUID.randomUUID(), "Musician", "Song", + List.of(new Track(1, 20, 1, false, notes), new Track(2, 1, 1, false, List.of()))); + StudioStore store = new StudioStore(directory); + store.publish(song); + var yaml = org.bukkit.configuration.file.YamlConfiguration.loadConfiguration( + directory.resolve("songs").resolve(song.id() + ".yml").toFile()); + var track = yaml.getMapList("tracks").getFirst(); + assertEquals(List.of("lute", "tfmc:lute.c4", "tfmc:lute.e4", "flute"), track.get("names")); + assertEquals("5 0 1 1.0 1.0\n0 0 2 1.0 1.25\n7 3 1 0.5 2.0", track.get("notes")); + assertEquals("", yaml.getMapList("tracks").getLast().get("notes")); + assertEquals(song, store.song(song.id())); + } + @TempDir Path directory; private Project project() { @@ -20,6 +55,48 @@ private Project project() { return new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Musician", "Música", List.of(old)), pending, 120, false); } + @Test + void savesWriteOneProjectCopyAndASmallSelection() throws Exception { + StudioStore store = new StudioStore(directory); + Project first = project(); + Song song = first.song(); + Project second = new Project(new Song(UUID.randomUUID(), song.owner(), song.author(), "Second", List.of()), null, 100, false); + store.save(first); + store.save(first.accept()); + store.save(second); + Path folder = directory.resolve("projects").resolve(song.owner().toString()); + try (var files = Files.list(directory.resolve("projects"))) { + assertEquals(List.of(folder), files.toList()); + } + try (var files = Files.list(folder)) { + assertEquals(java.util.Set.of(first.song().id() + ".yml", second.song().id() + ".yml", StudioStore.SELECTED, StudioStore.INDEX), + files.map(path -> path.getFileName().toString()).collect(java.util.stream.Collectors.toSet())); + } + assertEquals(second.song().id().toString(), Files.readString(folder.resolve(StudioStore.SELECTED))); + assertEquals(second, new StudioStore(directory).project(song.owner()).orElseThrow()); + Files.delete(folder.resolve(second.song().id() + ".yml")); + assertTrue(new StudioStore(directory).project(song.owner()).isEmpty()); + for (String invalid : List.of("not a project", "x".repeat(65))) { + Files.writeString(folder.resolve(StudioStore.SELECTED), invalid); + assertTrue(new StudioStore(directory).project(song.owner()).isEmpty()); + } + } + + @Test + void startupRemovesTemporaryFilesLeftByAnInterruptedWrite() throws Exception { + Project project = project(); + new StudioStore(directory).save(project); + Path folder = directory.resolve("projects").resolve(project.song().owner().toString()); + Path leftover = Files.writeString(folder.resolve("studio-123.tmp"), "partial"); + Path unrelated = Files.writeString(folder.resolve("notes.tmp"), "kept"); + Path similar = Files.writeString(directory.resolve("songs").resolve("studio-readme.txt"), "kept"); + StudioStore reopened = new StudioStore(directory); + assertFalse(Files.exists(leftover)); + assertTrue(Files.exists(unrelated)); + assertTrue(Files.exists(similar)); + assertEquals(project, reopened.project(project.song().owner()).orElseThrow()); + } + @Test void projectsAndPendingTakesSurviveANewStoreInstance() throws Exception { Project project = project(); @@ -31,14 +108,90 @@ void projectsAndPendingTakesSurviveANewStoreInstance() throws Exception { void publishedEditionRemainsImmutableAfterProjectChanges() throws Exception { StudioStore store = new StudioStore(directory); Project project = project(); - Song edition = project.song().edition(); - store.publish(edition); + Song edition = store.publishEdition(project.song(), 0); + assertNotEquals(project.song().id(), edition.id()); store.save(project.accept()); assertEquals(edition, new StudioStore(directory).song(edition.id())); assertThrows(IOException.class, () -> store.publish(edition)); assertEquals(edition, store.song(edition.id())); } + @Test + void republishingUnchangedMusicReusesItsEditionWithoutAnotherFile() throws Exception { + StudioStore store = new StudioStore(directory); + Project project = project(); + Song edition = store.publishEdition(project.song(), 0); + Song renamed = new Song(project.song().id(), project.song().owner(), "Musician", "Renamed", project.song().tracks()); + assertEquals(edition, store.publishEdition(project.song(), 0)); + assertNotEquals(edition.id(), store.publishEdition(renamed, 0).id()); + try (var files = Files.list(directory.resolve("songs"))) { + assertEquals(2, files.count()); + } + Path file = directory.resolve("songs").resolve(edition.id() + ".yml"); + var yaml = org.bukkit.configuration.file.YamlConfiguration.loadConfiguration(file.toFile()); + yaml.set("title", "Tampered"); + yaml.save(file.toFile()); + assertThrows(IOException.class, () -> store.publishEdition(project.song(), 0)); + } + + @Test + void editionsKeepOnlyWhatPlaybackEmits() throws Exception { + var audible = new Note(0, "lute", "tfmc:lute", 2, 1); + var silent = new Note(1, "lute", "tfmc:lute", 0, 1); + Song song = new Song(UUID.randomUUID(), UUID.randomUUID(), "Musician", "Song", List.of( + new Track(1, 30, 1, false, List.of(audible, silent)), new Track(2, 40, 1, true, List.of(audible)), + new Track(3, 50, 0, false, List.of(audible)), new Track(4, 60, .5f, false, List.of(silent)))); + StudioStore store = new StudioStore(directory); + Song edition = store.publishEdition(song, 0); + assertEquals(List.of(new Track(1, 30, 1, false, List.of(audible))), edition.tracks()); + assertEquals(edition, store.song(edition.id())); + // Muting a track that is already silent publishes the same edition. + Song muted = song.replace(song.track(4).mix(.5f, true)); + assertEquals(edition.id(), store.publishEdition(muted, 0).id()); + } + + @Test + void eachPlayerCanPublishALimitedNumberOfNewEditionsADay() throws Exception { + java.time.Clock clock = mock(java.time.Clock.class); + long start = 1_000_000_000_000L; + when(clock.millis()).thenReturn(start); + StudioStore store = new StudioStore(directory, java.util.logging.Logger.getAnonymousLogger(), clock); + Project project = project(); + Song song = project.song().replace(new Track(2, 10, 1, false, List.of(new Note(0, "lute", "tfmc:lute", 1, 1)))); + Song first = store.publishEdition(song, 2); + when(clock.millis()).thenReturn(start + java.util.concurrent.TimeUnit.HOURS.toMillis(23)); + store.publishEdition(retitled(song, "Second"), 2); + assertEquals(first, store.publishEdition(song, 2)); // Republishing does not count. + var limit = assertThrows(IllegalArgumentException.class, () -> store.publishEdition(retitled(song, "Third"), 2)); + assertTrue(limit.getMessage().endsWith("Try again in 60 minutes"), limit.getMessage()); + when(clock.millis()).thenReturn(start + java.util.concurrent.TimeUnit.HOURS.toMillis(24)); + store.publishEdition(retitled(song, "Third"), 2); // The first publication left the window. + limit = assertThrows(IllegalArgumentException.class, () -> store.publishEdition(retitled(song, "Fourth"), 2)); + assertTrue(limit.getMessage().endsWith("Try again in 23 hours"), limit.getMessage()); + Path log = directory.resolve("editions").resolve(song.owner() + ".log"); + assertEquals(2, Files.readAllLines(log).size()); // Older entries are dropped. + Files.writeString(log, "damaged\n" + Files.readString(log)); + assertThrows(IllegalArgumentException.class, () -> store.publishEdition(retitled(song, "Fourth"), 2)); + store.publishEdition(retitled(song, "Fourth"), 0); // 0 disables the limit. + try (var files = Files.list(directory.resolve("songs"))) { + assertEquals(4, files.count()); + } + } + + private Song retitled(Song song, String title) { + return new Song(song.id(), song.owner(), song.author(), title, song.tracks()); + } + + @Test + void anEditionFileWithTheSameSizeButOtherContentIsRejected() throws Exception { + StudioStore store = new StudioStore(directory); + Song song = project().song().replace(new Track(2, 10, 1, false, List.of(new Note(0, "lute", "tfmc:lute", 1, 1)))); + Song edition = store.publishEdition(song, 0); + Path file = directory.resolve("songs").resolve(edition.id() + ".yml"); + Files.writeString(file, Files.readString(file).replace("tfmc:lute", "tfmc:lyre")); + assertThrows(IOException.class, () -> store.publishEdition(song, 0)); + } + @Test void atomicReplacementLeavesNoTemporaryFiles() throws Exception { StudioStore store = new StudioStore(directory); @@ -52,15 +205,74 @@ void atomicReplacementLeavesNoTemporaryFiles() throws Exception { } @Test - void corruptionAndUnknownFormatsAreReportedWithoutOverwritingData() throws Exception { - StudioStore store = new StudioStore(directory); - UUID owner = UUID.randomUUID(); - Path file = directory.resolve("projects").resolve(owner + ".yml"); - for (String invalid : List.of("format: 2\n", "format: 1\ntracks: nonsense\n", "[broken yaml")) { - Files.writeString(file, invalid); - assertThrows(IOException.class, () -> store.project(owner)); - assertEquals(invalid, Files.readString(file)); + void unreadableProjectsAreSkippedAndReportedOnceWhileTheRestOfTheLibraryWorks() throws Exception { + List warnings = new java.util.ArrayList<>(); + java.util.logging.Logger logger = java.util.logging.Logger.getAnonymousLogger(); + logger.setUseParentHandlers(false); + logger.addHandler(new java.util.logging.Handler() { + @Override public void publish(java.util.logging.LogRecord record) { warnings.add(record.getMessage()); } + @Override public void flush() {} + @Override public void close() {} + }); + StudioStore store = new StudioStore(directory, logger); + Project damaged = project(); + UUID owner = damaged.song().owner(); + Project healthy = new Project(new Song(UUID.randomUUID(), owner, "Musician", "Healthy", List.of()), null, 100, false); + store.save(damaged); + store.save(healthy); + Path folder = directory.resolve("projects").resolve(owner.toString()); + Path broken = folder.resolve(damaged.song().id() + ".yml"); + Files.writeString(broken, "format: 2\nid: [broken"); + Files.writeString(folder.resolve("notes.yml"), "not a project"); + for (int listing = 0; listing < 3; listing++) { + StudioStore reader = listing == 2 ? new StudioStore(directory, logger) : store; // The index remembers the damage. + assertEquals(List.of(ProjectSummary.of(healthy)), reader.summaries(owner)); } + assertEquals(4, warnings.size(), String.join("\n", warnings)); // Once per file and store. + assertTrue(warnings.stream().anyMatch(message -> message.contains(broken.toString()))); + assertThrows(IllegalArgumentException.class, () -> store.delete(owner, damaged.song().id())); + assertThrows(IOException.class, () -> store.project(owner, damaged.song().id())); + // A damaged active project leaves the player without one, so they can create or open another. + Files.writeString(folder.resolve(StudioStore.SELECTED), damaged.song().id().toString()); + assertTrue(store.project(owner).isEmpty()); + assertEquals(4, warnings.size()); // That file was already reported. + store.delete(owner, healthy.song().id()); + assertTrue(store.summaries(owner).isEmpty()); + assertEquals("format: 2\nid: [broken", Files.readString(broken)); + // A selection that is not a project ID is replaced by the next save. + Files.writeString(folder.resolve(StudioStore.SELECTED), "garbage"); + assertTrue(store.project(owner).isEmpty()); + assertTrue(store.project(owner).isEmpty()); + assertEquals(5, warnings.size()); + store.save(healthy); + assertEquals(healthy, store.project(owner).orElseThrow()); + } + + @Test + void invalidUtf8IsDamagedContentAndLeavesTheLibrarySelectionAndPublishingUsable() throws Exception { + byte[] invalid = { (byte) 0xc3, 0x28 }; + StudioStore store = new StudioStore(directory); + Project damaged = project(); + UUID owner = damaged.song().owner(); + Project healthy = new Project(new Song(UUID.randomUUID(), owner, "Musician", "Healthy", List.of()), null, 100, false); + store.save(healthy); + store.save(damaged); + Path folder = directory.resolve("projects").resolve(owner.toString()); + Files.write(folder.resolve(damaged.song().id() + ".yml"), invalid); + assertEquals(List.of(ProjectSummary.of(healthy)), store.summaries(owner)); + assertTrue(store.project(owner).isEmpty()); // The damaged project was the active one. + assertThrows(IOException.class, () -> store.project(owner, damaged.song().id())); + // A damaged selection is ignored and replaced by the next save. + Files.write(folder.resolve(StudioStore.SELECTED), invalid); + assertTrue(store.project(owner).isEmpty()); + store.save(healthy); + assertEquals(healthy, store.project(owner).orElseThrow()); + // A damaged publication log loses only its unreadable lines. + Path log = directory.resolve("editions").resolve(owner + ".log"); + Files.createDirectories(log.getParent()); + Files.write(log, invalid); + store.publishEdition(healthy.song().replace(new Track(1, 10, 1, false, List.of(new Note(0, "lute", "tfmc:lute", 1, 1)))), 1); + assertEquals(1, Files.readAllLines(log).size()); } @Test @@ -69,11 +281,14 @@ void filenameIdentityMustMatchTheStoredOwnerOrEdition() throws Exception { Project project = project(); store.save(project); UUID anotherOwner = UUID.randomUUID(); - Files.copy(directory.resolve("projects").resolve(project.song().owner() + ".yml"), - directory.resolve("projects").resolve(anotherOwner + ".yml")); - assertThrows(IOException.class, () -> store.project(anotherOwner)); - Song edition = project.song().edition(); - store.publish(edition); + Path foreign = directory.resolve("projects").resolve(anotherOwner.toString()); + Files.createDirectories(foreign); + Files.copy(directory.resolve("projects").resolve(project.song().owner().toString()).resolve(project.song().id() + ".yml"), + foreign.resolve(project.song().id() + ".yml")); + Files.writeString(foreign.resolve(StudioStore.SELECTED), project.song().id().toString()); + assertTrue(store.project(anotherOwner).isEmpty()); + assertTrue(store.summaries(anotherOwner).isEmpty()); + Song edition = store.publishEdition(project.song(), 0); UUID anotherId = UUID.randomUUID(); Files.copy(directory.resolve("songs").resolve(edition.id() + ".yml"), directory.resolve("songs").resolve(anotherId + ".yml")); @@ -81,21 +296,76 @@ void filenameIdentityMustMatchTheStoredOwnerOrEdition() throws Exception { } @Test - void switchingProjectsMigratesTheLegacySongWithoutLosingItsPendingTake() throws Exception { + void decodedNotesShareRepeatedInstrumentAndSampleNames() throws Exception { + var notes = List.of(new Note(0, "lute", "tfmc:lute.c4", 1, 1), new Note(1, "lute", "tfmc:lute.c4", 1, 1)); + Project project = new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Musician", "Song", + List.of(new Track(1, 10, 1, false, notes))), null, 100, false); + new StudioStore(directory).save(project); + Song song = new StudioStore(directory).project(project.song().owner()).orElseThrow().song(); + Note first = song.track(1).notes().get(0); + Note second = song.track(1).notes().get(1); + assertSame(first.instrument(), second.instrument()); + assertSame(first.sound(), second.sound()); + } + + @Test + void libraryListingUsesSavedSummariesWithoutReparsingProjects() throws Exception { StudioStore store = new StudioStore(directory); - Project legacy = project(); - store.save(legacy); - Path archived = directory.resolve("projects").resolve(legacy.song().owner().toString()).resolve(legacy.song().id() + ".yml"); - Files.delete(archived); // Simulate the original single-project layout. - Project second = new Project(new Song(UUID.randomUUID(), legacy.song().owner(), "Musician", "Second", List.of()), null, 100, true); - store.save(second); - StudioStore reopened = new StudioStore(directory); - assertEquals(2, reopened.projects(legacy.song().owner()).size()); - assertEquals(legacy, reopened.project(legacy.song().owner(), legacy.song().id())); - assertEquals(second, reopened.project(legacy.song().owner()).orElseThrow()); - reopened.save(legacy); - assertEquals(legacy, new StudioStore(directory).project(legacy.song().owner()).orElseThrow()); - assertEquals(second, reopened.project(legacy.song().owner(), second.song().id())); + Project project = project(); + store.save(project); + Path file = directory.resolve("projects").resolve(project.song().owner().toString()).resolve(project.song().id() + ".yml"); + var modified = Files.getLastModifiedTime(file); + Files.writeString(file, "x".repeat((int) Files.size(file))); // Same size and time: the index entry still applies. + Files.setLastModifiedTime(file, modified); + assertEquals(List.of(ProjectSummary.of(project)), new StudioStore(directory).summaries(project.song().owner())); + } + + @Test + void filesChangedOutsideSaveAndDamagedIndexEntriesAreReparsed() throws Exception { + StudioStore store = new StudioStore(directory); + Project project = project(); + UUID owner = project.song().owner(); + store.save(project); + Path folder = directory.resolve("projects").resolve(owner.toString()); + Path file = folder.resolve(project.song().id() + ".yml"); + var yaml = org.bukkit.configuration.file.YamlConfiguration.loadConfiguration(file.toFile()); + yaml.set("title", "Edited outside the studio"); + yaml.save(file.toFile()); + assertEquals("Edited outside the studio", store.summaries(owner).getFirst().title()); + Path index = folder.resolve(StudioStore.INDEX); + String key = project.song().id().toString(); + for (int damage = 0; damage < 2; damage++) { + var cache = org.bukkit.configuration.file.YamlConfiguration.loadConfiguration(index.toFile()); + var entry = new java.util.ArrayList(cache.getList(key)); + if (damage == 0) entry.set(1, UUID.randomUUID().toString()); // Belongs to another file. + else entry.subList(1, entry.size()).clear(); // Truncated. + cache.set(key, entry); + cache.save(index.toFile()); + assertEquals(project.song().id(), store.summaries(owner).getFirst().id()); + } + } + + @Test + void anUnwritableIndexLeavesTheLibraryUsable() throws Exception { + StudioStore store = new StudioStore(directory); + Project project = project(); + Path index = directory.resolve("projects").resolve(project.song().owner().toString()).resolve(StudioStore.INDEX); + Files.createDirectories(index.resolve("blocked")); // A non-empty directory cannot be replaced by the cache file. + store.save(project); + assertEquals(List.of(ProjectSummary.of(project)), store.summaries(project.song().owner())); + } + + @Test + void deletionRemovesTheSelectionAndRejectsUnknownProjects() throws Exception { + Project project = project(); + UUID owner = project.song().owner(); + StudioStore store = new StudioStore(directory); + store.save(project); + assertThrows(IllegalArgumentException.class, () -> store.delete(owner, UUID.randomUUID())); + store.delete(owner, project.song().id()); + assertFalse(Files.exists(directory.resolve("projects").resolve(owner.toString()).resolve(StudioStore.SELECTED))); + assertTrue(store.project(owner).isEmpty()); + assertTrue(store.summaries(owner).isEmpty()); } @Test @@ -105,10 +375,11 @@ void projectLibrariesAndDirectSelectionCannotCrossOwners() throws Exception { Project other = project(); store.save(first); store.save(other); - assertEquals(List.of(first), store.projects(first.song().owner())); + assertEquals(List.of(ProjectSummary.of(first)), store.summaries(first.song().owner())); assertThrows(IllegalArgumentException.class, () -> store.project(first.song().owner(), other.song().id())); Path copied = directory.resolve("projects").resolve(first.song().owner().toString()).resolve(other.song().id() + ".yml"); Files.copy(directory.resolve("projects").resolve(other.song().owner().toString()).resolve(other.song().id() + ".yml"), copied); - assertThrows(IOException.class, () -> store.projects(first.song().owner())); + assertEquals(List.of(ProjectSummary.of(first)), store.summaries(first.song().owner())); + assertThrows(IOException.class, () -> store.project(first.song().owner(), other.song().id())); } }