diff --git a/sublime/adapters/adapter_base.py b/sublime/adapters/adapter_base.py index 1dbc00f..9cccc8b 100644 --- a/sublime/adapters/adapter_base.py +++ b/sublime/adapters/adapter_base.py @@ -613,8 +613,9 @@ class CachingAdapter(Adapter): COVER_ART_FILE = "cover_art_file" GENRES = "genres" IGNORED_ARTICLES = "ignored_articles" - PLAYLISTS = "get_playlists" PLAYLIST_DETAILS = "get_playlist_details" + PLAYLISTS = "get_playlists" + SEARCH_RESULTS = "search_results" SONG_DETAILS = "song_details" SONG_FILE = "song_file" SONG_FILE_PERMANENT = "song_file_permanent" diff --git a/sublime/adapters/filesystem/adapter.py b/sublime/adapters/filesystem/adapter.py index 4268011..0c6ed8f 100644 --- a/sublime/adapters/filesystem/adapter.py +++ b/sublime/adapters/filesystem/adapter.py @@ -4,9 +4,8 @@ import threading from dataclasses import asdict from datetime import datetime from pathlib import Path -from typing import Any, Dict, Optional, Sequence, Set, Tuple +from typing import Any, cast, Dict, Optional, Sequence, Set, Tuple, Union -import sublime from sublime import util from sublime.adapters import api_objects as API @@ -299,6 +298,12 @@ class FilesystemAdapter(CachingAdapter): params: Tuple[Any, ...], data: Any, ): + # TODO: this entire function is not exactly efficient due to the nested + # dependencies and everything. I'm not sure how to improve it, and I'm not sure + # if it needs improving at this point. + + # TODO refactor to to be a recursive function like invalidate_data? + # TODO may need to remove reliance on asdict in order to support more backends. params_hash = util.params_hash(*params) models.CacheInfo.insert( @@ -307,13 +312,7 @@ class FilesystemAdapter(CachingAdapter): last_ingestion_time=datetime.now(), ).on_conflict_replace().execute() - def ingest_list(model: Any, data: Any, id_property: Any): - model.insert_many(map(asdict, data)).on_conflict_replace().execute() - model.delete().where( - id_property.not_in([getattr(p, id_property.name) for p in data]) - ).execute() - - def set_attrs(obj: Any, data: Dict[str, Any]): + def setattrs(obj: Any, data: Dict[str, Any]): for k, v in data.items(): if v: setattr(obj, k, v) @@ -325,7 +324,7 @@ class FilesystemAdapter(CachingAdapter): ) if not created: - set_attrs(directory, directory_data) + setattrs(directory, directory_data) directory.save() return directory @@ -333,11 +332,11 @@ class FilesystemAdapter(CachingAdapter): def ingest_genre_data(api_genre: API.Genre) -> models.Genre: genre_data = asdict(api_genre) genre, created = models.Genre.get_or_create( - name=api_genre.name, defaults=asdict(api_genre) + name=api_genre.name, defaults=genre_data ) if not created: - set_attrs(genre, genre_data) + setattrs(genre, genre_data) genre.save() return genre @@ -362,7 +361,7 @@ class FilesystemAdapter(CachingAdapter): ) if not created: - set_attrs(album, album_data) + setattrs(album, album_data) album.save() return album @@ -396,7 +395,7 @@ class FilesystemAdapter(CachingAdapter): ) if not created: - set_attrs(artist, artist_data) + setattrs(artist, artist_data) artist.save() return artist @@ -423,11 +422,36 @@ class FilesystemAdapter(CachingAdapter): ) if not created: - set_attrs(song, song_data) + setattrs(song, song_data) song.save() return song + def ingest_playlist( + api_playlist: Union[API.Playlist, API.PlaylistDetails] + ) -> models.Playlist: + playlist_data = { + **asdict(api_playlist), + "songs": [ + ingest_song_data(s) + for s in ( + api_playlist.songs + if isinstance(api_playlist, API.PlaylistDetails) + else () + ) + ], + } + playlist, playlist_created = models.Playlist.get_or_create( + id=playlist_data["id"], defaults=playlist_data + ) + + # Update the values if the playlist already existed. + if not playlist_created: + setattrs(playlist, playlist_data) + playlist.save() + + return playlist + if data_key == CachingAdapter.CachedDataKey.ALBUM: ingest_album_data(data) @@ -451,7 +475,8 @@ class FilesystemAdapter(CachingAdapter): shutil.copy(str(data), str(self.cover_art_dir.joinpath(params_hash))) elif data_key == CachingAdapter.CachedDataKey.GENRES: - ingest_list(models.Genre, data, models.Genre.name) + for g in data: + ingest_genre_data(g) elif data_key == CachingAdapter.CachedDataKey.IGNORED_ARTICLES: models.IgnoredArticle.insert_many( @@ -461,22 +486,29 @@ class FilesystemAdapter(CachingAdapter): models.IgnoredArticle.name.not_in(data) ).execute() - elif data_key == CachingAdapter.CachedDataKey.PLAYLISTS: - ingest_list(models.Playlist, data, models.Playlist.id) - elif data_key == CachingAdapter.CachedDataKey.PLAYLIST_DETAILS: - song_objects = [ingest_song_data(s) for s in data.songs] - playlist_data = {**asdict(data), "songs": song_objects} - playlist, playlist_created = models.Playlist.get_or_create( - id=playlist_data["id"], defaults=playlist_data - ) + ingest_playlist(data) - # Update the values if the playlist already existed. - if not playlist_created: - for k, v in playlist_data.items(): - setattr(playlist, k, v) + elif data_key == CachingAdapter.CachedDataKey.PLAYLISTS: + for p in data: + ingest_playlist(p) + models.Playlist.delete().where( + models.Playlist.id.not_in([p.id for p in data]) + ).execute() - playlist.save() + elif data_key == CachingAdapter.CachedDataKey.SEARCH_RESULTS: + data = cast(API.SearchResult, data) + for a in data._artists.values(): + ingest_artist_data(a) + + for a in data._albums.values(): + ingest_album_data(a) + + for s in data._songs.values(): + ingest_song_data(s) + + for p in data._playlists.values(): + ingest_song_data(p) elif data_key == CachingAdapter.CachedDataKey.SONG_DETAILS: ingest_song_data(data) @@ -507,65 +539,50 @@ class FilesystemAdapter(CachingAdapter): elif data_key == CachingAdapter.CachedDataKey.ARTIST: # Invalidate the corresponding cover art. - artist = models.Artist.get_or_none(models.Artist.id == params[0]) - if not artist: - return - - self._do_invalidate_data(cover_art_cache_key, (artist.artist_image_url,)) - for album in artist.albums or []: + if artist := models.Artist.get_or_none(models.Artist.id == params[0]): self._do_invalidate_data( - CachingAdapter.CachedDataKey.ALBUM, (album.id,) + cover_art_cache_key, (artist.artist_image_url,) ) + for album in artist.albums or []: + self._do_invalidate_data( + CachingAdapter.CachedDataKey.ALBUM, (album.id,) + ) elif data_key == CachingAdapter.CachedDataKey.PLAYLIST_DETAILS: # Invalidate the corresponding cover art. - playlist = models.Playlist.get_or_none(models.Playlist.id == params[0]) - if playlist: + if playlist := models.Playlist.get_or_none(models.Playlist.id == params[0]): self._do_invalidate_data(cover_art_cache_key, (playlist.cover_art,)) elif data_key == CachingAdapter.CachedDataKey.SONG_FILE: # Invalidate the corresponding cover art. - song = models.Song.get_or_none(models.Song.id == params[0]) - if song: + if song := models.Song.get_or_none(models.Song.id == params[0]): self._do_invalidate_data(cover_art_cache_key, (song.cover_art,)) def _do_delete_data( self, data_key: CachingAdapter.CachedDataKey, params: Tuple[Any, ...], ): - # Delete it from the cache info. - models.CacheInfo.delete().where( - models.CacheInfo.cache_key == data_key, - models.CacheInfo.params_hash == util.params_hash(*params), - ).execute() + # Invalidate it. + self._do_invalidate_data(data_key, params) + cover_art_cache_key = CachingAdapter.CachedDataKey.COVER_ART_FILE - def delete_cover_art(cover_art_id: str): - cover_art_params_hash = util.params_hash(cover_art_id) - if cover_art_file := self.cover_art_dir.joinpath(cover_art_params_hash): - cover_art_file.unlink(missing_ok=True) - self._do_invalidate_data( - CachingAdapter.CachedDataKey.COVER_ART_FILE, (cover_art_id,) - ) + if data_key == CachingAdapter.CachedDataKey.COVER_ART_FILE: + cover_art_file = self.cover_art_dir.joinpath(util.params_hash(*params)) + cover_art_file.unlink(missing_ok=True) - if data_key == CachingAdapter.CachedDataKey.PLAYLIST_DETAILS: + elif data_key == CachingAdapter.CachedDataKey.PLAYLIST_DETAILS: # Delete the playlist and corresponding cover art. - playlist = models.Playlist.get_or_none(models.Playlist.id == params[0]) - if not playlist: - return + if playlist := models.Playlist.get_or_none(models.Playlist.id == params[0]): + if cover_art := playlist.cover_art: + self._do_delete_data(cover_art_cache_key, (cover_art,)) - if playlist.cover_art: - delete_cover_art(playlist.cover_art) - - playlist.delete_instance() + playlist.delete_instance() elif data_key == CachingAdapter.CachedDataKey.SONG_FILE: - song = models.Song.get_or_none(models.Song.id == params[0]) - if not song: - return + if song := models.Song.get_or_none(models.Song.id == params[0]): + # Delete the song + music_filename = self.music_dir.joinpath(song.path) + music_filename.unlink(missing_ok=True) - # Delete the song - music_filename = self.music_dir.joinpath(song.path) - music_filename.unlink(missing_ok=True) - - # Delete the corresponding cover art. - if song.cover_art: - delete_cover_art(song.cover_art) + # Delete the corresponding cover art. + if cover_art := song.cover_art: + self._do_delete_data(cover_art_cache_key, (cover_art,)) diff --git a/sublime/adapters/manager.py b/sublime/adapters/manager.py index 8962f94..f5082c9 100644 --- a/sublime/adapters/manager.py +++ b/sublime/adapters/manager.py @@ -86,8 +86,13 @@ class Result(Generic[T]): self._on_cancel = on_cancel def _on_future_complete(self, future: Future): - if not future.cancelled() and not future.exception(): + try: self._data = future.result() + except Exception as e: + if self._default_value: + self._data = self._default_value + else: + raise e def result(self) -> T: """ @@ -103,7 +108,7 @@ class Result(Generic[T]): assert 0, "AdapterManager.Result had neither _data nor _future member!" except Exception as e: if self._default_value: - return self._default_value + self._data = self._default_value raise e def add_done_callback(self, fn: Callable, *args): @@ -924,9 +929,7 @@ class AdapterManager: # Albums @staticmethod def get_albums( - album_id: str, - before_download: Callable[[], None] = lambda: None, - force: bool = False, + before_download: Callable[[], None] = lambda: None, force: bool = False, ) -> Result[Sequence[Album]]: return AdapterManager._get_from_cache_or_ground_truth( "get_albums", @@ -1042,15 +1045,23 @@ class AdapterManager: return False try: - search_result.update( - AdapterManager._instance.ground_truth_adapter.search(query) + ground_truth_search_results = AdapterManager._instance.ground_truth_adapter.search( # noqa: E501 + query ) + search_result.update(ground_truth_search_results) search_callback(search_result) except Exception: logging.exception( "Failed getting search results from server for query '{query}'" ) + if AdapterManager._instance.caching_adapter: + AdapterManager._instance.caching_adapter.ingest_new_data( + CachingAdapter.CachedDataKey.SEARCH_RESULTS, + (), + ground_truth_search_results, + ) + return True # When the future is cancelled (this will happen if a new search is created), diff --git a/sublime/ui/albums.py b/sublime/ui/albums.py index 73413de..0cf69f3 100644 --- a/sublime/ui/albums.py +++ b/sublime/ui/albums.py @@ -1,17 +1,13 @@ import datetime -from typing import Any, Callable, Iterable, Optional, Tuple, Union +from typing import Any, Callable, Iterable, Optional, Tuple from gi.repository import Gdk, Gio, GLib, GObject, Gtk, Pango -from sublime.adapters import AdapterManager, Result -from sublime.cache_manager import CacheManager +from sublime.adapters import AdapterManager, api_objects as API, Result from sublime.config import AppConfiguration -from sublime.server.api_objects import AlbumWithSongsID3, Child from sublime.ui import util from sublime.ui.common import AlbumWithSongs, IconButton, SpinnerImage -Album = Union[Child, AlbumWithSongsID3] - class AlbumsPanel(Gtk.Box): __gsignals__ = { @@ -253,9 +249,9 @@ class AlbumsGrid(Gtk.Overlay): current_min_size_request = 30 server_hash = None - class AlbumModel(GObject.Object): - def __init__(self, album: Album): - self.album: Album = album + class _AlbumModel(GObject.Object): + def __init__(self, album: API.Album): + self.album = album super().__init__() @property @@ -384,9 +380,9 @@ class AlbumsGrid(Gtk.Overlay): error_dialog = None def update_grid( - self, order_token: int, force: bool = False, selected_id: str = None, + self, order_token: int, force: bool = False, selected_id: str = None ): - if not CacheManager.ready(): + if not AdapterManager.can_get_artists(): self.spinner.hide() return @@ -395,7 +391,7 @@ class AlbumsGrid(Gtk.Overlay): if self.type_ == "alphabetical": type_ += {"name": "ByName", "artist": "ByArtist"}[self.alphabetical_type] - def do_update(f: CacheManager.Result): + def do_update(f: Result[Iterable[API.Album]]): try: albums = f.result() except Exception as e: @@ -431,7 +427,7 @@ class AlbumsGrid(Gtk.Overlay): selected_index = None for i, album in enumerate(albums): - model = AlbumsGrid.AlbumModel(album) + model = AlbumsGrid._AlbumModel(album) if model.id == selected_id: selected_index = i @@ -446,7 +442,7 @@ class AlbumsGrid(Gtk.Overlay): ) self.spinner.hide() - future = CacheManager.get_album_list( + future = AdapterManager.get_albums( type_=type_, from_year=self.from_year, to_year=self.to_year, @@ -484,7 +480,7 @@ class AlbumsGrid(Gtk.Overlay): # Helper Methods # ========================================================================= - def create_widget(self, item: "AlbumsGrid.AlbumModel") -> Gtk.Box: + def create_widget(self, item: _AlbumModel) -> Gtk.Box: widget_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL) # Cover art image diff --git a/tests/adapter_tests/adapter_manager_tests.py b/tests/adapter_tests/adapter_manager_tests.py index ea654d4..58d93f5 100644 --- a/tests/adapter_tests/adapter_manager_tests.py +++ b/tests/adapter_tests/adapter_manager_tests.py @@ -56,3 +56,14 @@ def test_result_future_callback(): assert t < 2 t += 0.1 sleep(0.1) + + +def test_default_value(): + def resolve_fail() -> int: + sleep(1) + raise Exception() + + result = Result(resolve_fail, default_value=42) + assert not result.data_is_available + assert result.result() == 42 + assert result.data_is_available diff --git a/tests/adapter_tests/filesystem_adapter_tests.py b/tests/adapter_tests/filesystem_adapter_tests.py index c0ef66e..a22b410 100644 --- a/tests/adapter_tests/filesystem_adapter_tests.py +++ b/tests/adapter_tests/filesystem_adapter_tests.py @@ -478,7 +478,7 @@ def test_caching_get_genres(cache_adapter: FilesystemAdapter): SubsonicAPI.Genre("Foo", 10, 20), ], ) - assert [g.name for g in cache_adapter.get_genres()] == ["Bar", "Baz", "Foo"] + assert {g.name for g in cache_adapter.get_genres()} == {"Bar", "Baz", "Foo"} def test_caching_get_song_details(cache_adapter: FilesystemAdapter):