From 9143c6029ab383e26307b39e3dfd63d49e301929 Mon Sep 17 00:00:00 2001 From: Marcin Macierzanka Date: Fri, 11 Sep 2026 01:29:31 +0200 Subject: [PATCH] fix: prevent overwriting saved queue with empty track lists in repository and music player --- tests/test_music_player.py | 30 ++++++++++++++++++++++++++++ tests/test_queue_repository.py | 16 +++++++++++++++ zyrarbot/music/music_player.py | 16 +++++++-------- zyrarbot/storage/queue_repository.py | 2 ++ 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/tests/test_music_player.py b/tests/test_music_player.py index fa71a18..8cd0da2 100644 --- a/tests/test_music_player.py +++ b/tests/test_music_player.py @@ -296,6 +296,36 @@ class MusicPlayerQueueTest(unittest.IsolatedAsyncioTestCase): with self.assertRaisesRegex(ValueError, "not connected"): await player.restore_queue(GUILD) + def test_save_queue_does_not_overwrite_with_empty_queue(self): + with tempfile.TemporaryDirectory() as directory: + player, _ = build_player() + player._queues = QueueRepository(directory) + saved = [{"id": "saved", "title": "Saved", "plays_left": 2}] + player._queues.save(GUILD, saved) + + # Kolejka w stanie odtwarzacza jest pusta + player.state(GUILD).queue = [] + player.save_queue(GUILD) + + # Zapisana wcześniej kolejka powinna pozostać nienaruszona + self.assertEqual(player._queues.load(GUILD), saved) + + def test_save_queue_does_not_save_when_all_plays_consumed(self): + with tempfile.TemporaryDirectory() as directory: + player, _ = build_player() + player._queues = QueueRepository(directory) + saved = [{"id": "saved", "title": "Saved", "plays_left": 2}] + player._queues.save(GUILD, saved) + + # Wszystkie utwory w kolejce mają plays_left = 0 + track = FakeTrack("consumed") + track.plays_left = 0 + player.state(GUILD).queue = [track] + player.save_queue(GUILD) + + # Zapisana wcześniej kolejka powinna pozostać nienaruszona + self.assertEqual(player._queues.load(GUILD), saved) + class LoopModeTest(unittest.TestCase): def test_cycle_order(self): diff --git a/tests/test_queue_repository.py b/tests/test_queue_repository.py index 2fa3227..f1a36a1 100644 --- a/tests/test_queue_repository.py +++ b/tests/test_queue_repository.py @@ -20,6 +20,22 @@ class QueueRepositoryTest(unittest.TestCase): self.assertTrue((Path(directory) / "10.json").exists()) self.assertTrue((Path(directory) / "20.json").exists()) + def test_save_empty_tracks_does_nothing(self): + with tempfile.TemporaryDirectory() as directory: + repository = QueueRepository(directory) + tracks = [{"id": "first", "title": "First", "plays_left": 2}] + + repository.save(10, tracks) + self.assertEqual(repository.load(10), tracks) + + # Próba zapisania pustej listy nie powinna nadpisać poprzedniej + repository.save(10, []) + self.assertEqual(repository.load(10), tracks) + + # Zapis pustej kolejki dla nowego serwera nie powinien tworzyć pliku + repository.save(20, []) + self.assertFalse(repository.exists(20)) + if __name__ == "__main__": unittest.main() diff --git a/zyrarbot/music/music_player.py b/zyrarbot/music/music_player.py index 319e025..4f1bbf2 100644 --- a/zyrarbot/music/music_player.py +++ b/zyrarbot/music/music_player.py @@ -111,14 +111,14 @@ class MusicPlayer: if not self._queues: return state = self.state(guild_id) - self._queues.save( - guild_id, - [ - self._track_snapshot(track) - for track in state.queue - if self.plays_left(track, guild_id) > 0 - ], - ) + tracks = [ + self._track_snapshot(track) + for track in state.queue + if self.plays_left(track, guild_id) > 0 + ] + if not tracks: + return + self._queues.save(guild_id, tracks) def finish_current(self, guild_id: int) -> None: """Consume one play when playback ends outside the regular callback.""" diff --git a/zyrarbot/storage/queue_repository.py b/zyrarbot/storage/queue_repository.py index d51592b..84067ee 100644 --- a/zyrarbot/storage/queue_repository.py +++ b/zyrarbot/storage/queue_repository.py @@ -21,6 +21,8 @@ class QueueRepository: return self._stores[guild_id] def save(self, guild_id: int, tracks: list[dict[str, Any]]) -> None: + if not tracks: + return self._store(guild_id).set("tracks", tracks) def load(self, guild_id: int) -> list[dict[str, Any]]: