#266 Refactor song loader logic and enhance playlist handling

Reorganized song loader methods for better readability, updated playlist comparison logic, and added detailed logging. Introduced tests for playlist change detection and improved error-handling with custom exceptions. These changes ensure more accurate playlist updates and improved maintainability.

modules/servant/servant.py

@@ -17,7 +17,7 @@
 from modules.servant.tag_manager.tag_manager import TagManager
 from utils.cmd_line_parser import parse_args
 from utils.exceptions import RocksnifferConnectionError, RSPLNotLoggedInError, \
-    RSPLPlaylistIsNotEnabledError, ConfigError
+    RSPLPlaylistIsNotEnabledError, ConfigError, SongLoaderError
 from utils.project_dir_setter import set_project_directory
 from utils.rocksniffer import Rocksniffer
 
@@ -132,7 +132,7 @@ def manage_songs(self, db_file):
         except Exception as e:
             log.error("Exception in manage_songs: %s", e)
             self.fatal_error_event.set()  # Signal a fatal error to stop the program
-            sys.exit(1)  # Exit the application with an error code
+            raise SongLoaderError("A fatal error occurred in the manage_songs method.") from e
 
     def update_game_info_and_setlist(self):
         while not self.fatal_error_event.is_set():  # Periodically check if stop is requested

modules/servant/song_loader/song_loader.py

@@ -9,15 +9,17 @@
 from modules.servant.database.db_manager import DBManager
 from modules.servant.song_loader.rs_playlist_data import RsPlaylist
 from modules.servant.song_loader.song_data import SongData, ArtistTitle
-from modules.servant.song_loader.song_loader_helper import playlist_does_not_changed, check_cdlc_archive_dir, \
-    check_rocksmith_cdlc_dir, update_tags_in_song_data, is_official, log_new_songs_found
+from modules.servant.song_loader.song_loader_helper import check_cdlc_archive_dir, \
+    check_rocksmith_cdlc_dir, update_tags_in_song_data, is_official, log_new_songs_found, playlist_does_not_changed
 from modules.servant.tag_manager import tag_utils
 from utils import file_utils, psarc_reader
 from utils.collection_utils import is_collection_not_empty, is_collection_empty, repr_in_multi_line
 from utils.exceptions import BadDirectoryError
 from utils.exceptions import RSPLNotLoggedInError
-from utils.rs_playlist_util import get_playlist, user_is_not_logged_in, set_tag_to_download, set_tag_loaded, \
-    unset_user_tags
+from utils.rs_playlist_util import (get_playlist,
+                                    user_is_not_logged_in,
+                                    set_tag_to_download,
+                                    unset_user_tags, set_tag_loaded)
 from utils.string_utils import time_float_to_string
 
 HEARTBEAT = 5
@@ -38,6 +40,8 @@ def __init__(self, config_data: ConfigData, songs):
             # TODO Shouldn't we activate the tag manager here and load it? Or just set it to none?
             self.rspl_tags = config_data.song_loader.rspl_tags
 
+            self.rsplaylist_request_strings = set()
+
             self.cdlc_archive_dir = check_cdlc_archive_dir(config_data.song_loader.cdlc_archive_dir)
             self.destination_dir = config_data.song_loader.destination_dir
             self.download_dirs = config_data.file_manager.download_dirs
@@ -87,9 +91,9 @@ def run(self):
 
             if time_waited >= HEARTBEAT:
 
-                self.__update_songs_from_rs_dir()
+                updated_songs_count = self.__update_songs_from_rs_dir()
 
-                if self.__playlist_has_been_changed():
+                if self.__update_playlist_and_get_playlist_has_been_changed() or updated_songs_count > 0:
                     log.info("Playlist has been changed, update songs!")
 
                     self.__find_existing_song_filenames_from_db_according_to_the_requests()
@@ -108,36 +112,46 @@ def run(self):
     #     for source_dir in self.download_dirs:
     #         self.__store_and_return_all_the_new_song_datas(source_dir, self.songs.songs_in_tmp)
 
-    def __update_songs_from_rs_dir(self):
-        self.__store_and_return_all_the_new_song_datas(self.rocksmith_cdlc_dir, self.songs.songs_in_rs)
+    def __update_songs_from_rs_dir(self) -> int:
+        updated_songs_count = self.__store_and_return_all_the_new_song_datas(
+            self.rocksmith_cdlc_dir, self.songs.songs_in_rs)
+        if updated_songs_count > 0:
+            log.debug("Count of songs updated from RS dir: %s", updated_songs_count)
+        return updated_songs_count
 
     # def __update_songs_from_import_dir(self):
     #     self.__store_and_return_all_the_new_song_datas(self.destination_dir, self.songs.songs_in_import)
 
     # def __update_songs_from_tmp_dir(self):
     #     self.__store_and_return_all_the_new_song_datas(self.destination_dir, self.songs.songs_in_tmp)
 
-    def __playlist_has_been_changed(self):
+    def __update_playlist_and_get_playlist_has_been_changed(self):
+        is_initial_load = self.rsplaylist_json is None
         new_playlist = get_playlist(self.twitch_channel, self.phpsessid)
 
+        self.__update_rsplaylist_with(new_playlist)
         self.__stop_if_user_is_not_logged_in_on_rspl_page(new_playlist)
 
-        if self.rsplaylist_json is None:
-            self.__update_playlist_with(new_playlist)
-            log.info("Initial load of the playlist is done...")
-            tag_utils.validate_and_log_rspl_tags(self.rsplaylist.channel_tags, self.rspl_tags)
+        if is_initial_load:
+            self.__handle_initial_playlist_load()
             return True
 
-        if playlist_does_not_changed(self.rsplaylist_json, new_playlist):
+        new_rsplaylist_request_strings = self.__extract_request_strings()
+        if playlist_does_not_changed(self.rsplaylist_request_strings, new_rsplaylist_request_strings):
             return False
 
         log.info("Playlist has been changed, lets update!")
+        self.rsplaylist_request_strings = self.__extract_request_strings()
+        return True
 
-        self.__update_playlist_with(new_playlist)
+    def __extract_request_strings(self):
+        return {item.string for item in self.rsplaylist.playlist}
 
-        return True
+    def __handle_initial_playlist_load(self):
+        log.info("Initial load of the playlist is done...")
+        tag_utils.validate_and_log_rspl_tags(self.rsplaylist.channel_tags, self.rspl_tags)
 
-    def __update_playlist_with(self, new_playlist):
+    def __update_rsplaylist_with(self, new_playlist):
         self.rsplaylist_json = new_playlist
         self.rsplaylist = from_dict(data_class=RsPlaylist, data=new_playlist)
 
@@ -210,7 +224,7 @@ def __remove_duplicates(self, duplicates, filenames_from_archive_dir):
     def __store_and_return_all_the_new_song_datas(self, directory, songs_to_update: dict):
         log.info('Loading songs from directory: %s', directory)
 
-        counter_new_songs = 0
+        new_songs_count = 0
         songs = {}
 
         filenames = file_utils.get_file_names_from(directory)
@@ -228,7 +242,7 @@ def __store_and_return_all_the_new_song_datas(self, directory, songs_to_update:
                 if song_data is None:
                     song_data = self.__extract_song_information(directory, filename)
                     self.db_manager.insert_song(song_data)
-                    counter_new_songs += 1
+                    new_songs_count += 1
                     log.info('New song added to DB: %s', song_data)
 
                 songs.update({song_data.song_filename: song_data})
@@ -237,7 +251,12 @@ def __store_and_return_all_the_new_song_datas(self, directory, songs_to_update:
 
         songs_to_update.update(songs)
 
-        log.info("---- Loaded %s songs, and from this, %s new song(s) stored in DB", len(songs), counter_new_songs)
+        if new_songs_count > 0:
+            log.info("---- Loaded %s songs, and from this, %s new song(s) stored in DB", len(songs), new_songs_count)
+            return new_songs_count
+
+        log.debug("---- No new songs were loaded!")
+        return 0
 
     @staticmethod
     def __remove_missing_songs_from(songs_to_update: dict[str, SongData](), filenames):
@@ -317,44 +336,42 @@ def __has_no_tag_loaded(self, song_data):
     def __has_no_tag_to_download(self, song_data):
         return self.rspl_tags.tag_to_download not in song_data.tags
 
-    def __do_not_has_the_tag_to_download(self, playlist_item):
-        return self.rspl_tags.tag_to_download not in playlist_item.tags
+    @staticmethod
+    def __no_file_found_in_db_for_this_song(songs_in_the_db):
+        return is_collection_empty(songs_in_the_db)
 
     def __find_existing_song_filenames_from_db_according_to_the_requests(self):
         playlist = self.rsplaylist.playlist
         for playlist_item in playlist:
+            dlc_items_count = len(playlist_item.dlc_set)
+            officials_count = 0
+            to_download_count = 0
+            loaded_count = 0
+            log.debug("Number of items in dlc_set for playlist_item position=%s id=%s dlc_items_count=%s",
+                      playlist_item.position, playlist_item.id, dlc_items_count)
+
             for dlc_set_item in playlist_item.dlc_set:
                 cdlc_id = dlc_set_item.cdlc_id
                 artist = dlc_set_item.artist
                 title = dlc_set_item.title
+                log.debug("Searching for --> cdlc_id=%s | %s - %s", cdlc_id, artist, title)
 
                 official = dlc_set_item.official
                 if is_official(official):
-                    log.info("Skipping ODLC request with cdlc_id=%s - %s - %s", cdlc_id, artist, title)
-                    unset_user_tags(self.twitch_channel,
-                                    self.phpsessid,
-                                    playlist_item.id,
-                                    self.rspl_tags,
-                                    playlist_item.tags)
+                    officials_count += 1
+                    log.debug("Skipping ODLC request with cdlc_id=%s - %s - %s", cdlc_id, artist, title)
                     continue
 
                 songs_in_the_db = self.db_manager.search_song_by_artist_and_title(artist, title)
 
                 rspl_position = str(playlist_item.position)
 
-                if is_collection_empty(songs_in_the_db):
-                    if self.__do_not_has_the_tag_to_download(playlist_item):
-                        log.warning("User must download the song: rspl_position=%s cdlc_id=%s - %s - %s",
-                                    rspl_position, cdlc_id, artist, title)
-                        set_tag_to_download(self.twitch_channel,
-                                            self.phpsessid,
-                                            playlist_item.id,
-                                            self.rspl_tags)
-
-                        log.info("Tag is set! rspl_position=%s cdlc_id=%s - %s - %s",
-                                 rspl_position, cdlc_id, artist, title)
+                if self.__no_file_found_in_db_for_this_song(songs_in_the_db):
+                    to_download_count += 1
+                    log.debug("This must be downloaded --> cdlc_id=%s - %s - %s", cdlc_id, artist, title)
                     continue
 
+                loaded_count += 1
                 for song_filename in songs_in_the_db:
                     song_data = SongData(song_filename=song_filename,
                                          artist_title=ArtistTitle(artist, title),
@@ -370,6 +387,31 @@ def __find_existing_song_filenames_from_db_according_to_the_requests(self):
 
                     log.info("Request found in DB: %s", song_data)
 
+            # if any tag has to set
+            if officials_count > 0 or to_download_count > 0 or loaded_count > 0:
+                log.debug(
+                    "Unset tags, as tags may be actualised! officials_count=%s to_download_count=%s loaded_count=%s ",
+                    officials_count, to_download_count, loaded_count)
+                unset_user_tags(self.twitch_channel,
+                                self.phpsessid,
+                                playlist_item.id,
+                                self.rspl_tags,
+                                playlist_item.tags)
+
+            if to_download_count > 0:
+                log.debug("To-download count: %s", to_download_count)
+                set_tag_to_download(self.twitch_channel,
+                                    self.phpsessid,
+                                    playlist_item.id,
+                                    self.rspl_tags)
+
+            if loaded_count > 0:
+                log.debug("Loaded count: %s", to_download_count)
+                set_tag_loaded(self.twitch_channel,
+                               self.phpsessid,
+                               playlist_item.id,
+                               self.rspl_tags)
+
         if is_collection_not_empty(self.songs.requested_songs_found_in_db):
             log.info("Existing songs found: %s", repr_in_multi_line(self.songs.requested_songs_found_in_db))
 

modules/servant/song_loader/song_loader_helper.py

@@ -20,7 +20,19 @@
 log = logging.getLogger()
 
 
-def playlist_does_not_changed(old_playlist, new_playlist):
+def playlist_does_not_changed(rsplaylist_request_strings: set[str], new_rsplaylist_request_strings: set[str]) -> bool:
+    if rsplaylist_request_strings == new_rsplaylist_request_strings:
+        log.info("Playlist does not changed!")
+        return True
+
+    removed = rsplaylist_request_strings - new_rsplaylist_request_strings
+    added = new_rsplaylist_request_strings - rsplaylist_request_strings
+    log.warning("Playlist has been changed! Removed: %s, Added: %s", removed, added)
+
+    return rsplaylist_request_strings == new_rsplaylist_request_strings
+
+
+def rsplaylist_does_not_changed(old_playlist, new_playlist):
     diff = DeepDiff(old_playlist, new_playlist, exclude_regex_paths="\\['inactive_time'\\]|\\['tags'\\]")
     if str(diff) == "{}":
         log.debug("Playlist does not changed!")

modules/servant/tag_manager/tag_manager.py

@@ -20,8 +20,6 @@ def __init__(self, config_data: ConfigData, song_loader: SongLoader):
         self.user_tags = self.__fetch_user_tags(rsplaylist)
         self.server_tags = self.__fetch_server_tags(rsplaylist)
 
-        pass
-
     @staticmethod
     def get_rsplaylist(config_data):
         loader = config_data.song_loader

tests/test_song_loader_helper.py

@@ -0,0 +1,51 @@
+# tests/test_song_loader_helper.py
+
+from modules.servant.song_loader.song_loader_helper import playlist_does_not_changed
+
+
+def test_playlist_does_not_change_identical_sets():
+    rsplaylist_request_strings = {"song1", "song2", "song3"}
+    new_rsplaylist_request_strings = {"song1", "song2", "song3"}
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is True
+
+
+def test_playlist_does_not_change_identical_sets_different_order():
+    rsplaylist_request_strings = {"song1", "song2", "song3"}
+    new_rsplaylist_request_strings = {"song2", "song1", "song3"}
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is True
+
+
+def test_playlist_changes_differing_sets():
+    rsplaylist_request_strings = {"song1", "song2", "song3"}
+    new_rsplaylist_request_strings = {"song1", "song2", "song4"}
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is False
+
+
+def test_playlist_changes_differing_sets_less_new():
+    rsplaylist_request_strings = {"song1", "song2", "song3"}
+    new_rsplaylist_request_strings = {"song1", "song2"}
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is False
+
+
+def test_playlist_changes_differing_sets_more_new():
+    rsplaylist_request_strings = {"song1", "song2"}
+    new_rsplaylist_request_strings = {"song1", "song2", "song3"}
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is False
+
+
+def test_playlist_changes_empty_vs_non_empty():
+    rsplaylist_request_strings = set()
+    new_rsplaylist_request_strings = {"song1", "song2", "song3"}
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is False
+
+
+def test_playlist_changes_non_empty_vs_empty():
+    rsplaylist_request_strings = {"song1", "song2", "song3"}
+    new_rsplaylist_request_strings = set()
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is False
+
+
+def test_playlist_does_not_change_both_empty():
+    rsplaylist_request_strings = set()
+    new_rsplaylist_request_strings = set()
+    assert playlist_does_not_changed(rsplaylist_request_strings, new_rsplaylist_request_strings) is True

utils/rs_playlist_util.py

@@ -56,12 +56,10 @@ def unset_user_tags(channel, php_session_id, request_id, tags, item_tags):
 
 
 def set_tag_loaded(channel, php_session_id, request_id, tags):
-    unset_user_tags(channel, php_session_id, request_id, tags, [tags.tag_to_download])
     __set_tag(channel, php_session_id, request_id, tags.tag_loaded)
 
 
 def set_tag_to_download(channel, php_session_id, request_id, tags):
-    unset_user_tags(channel, php_session_id, request_id, tags, [tags.tag_loaded])
     __set_tag(channel, php_session_id, request_id, tags.tag_to_download)