Cache search results

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"

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):
+        def setattrs(obj: Any, data: Dict[str, 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]):
             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]
+            ingest_playlist(data)
-            playlist_data = {**asdict(data), "songs": song_objects}
-            playlist, playlist_created = models.Playlist.get_or_create(
-                id=playlist_data["id"], defaults=playlist_data
-            )
 
-            # Update the values if the playlist already existed.
+        elif data_key == CachingAdapter.CachedDataKey.PLAYLISTS:
-            if not playlist_created:
+            for p in data:
-                for k, v in playlist_data.items():
+                ingest_playlist(p)
-                    setattr(playlist, k, v)
+            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,11 +539,10 @@ 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 artist := models.Artist.get_or_none(models.Artist.id == params[0]):
-            if not artist:
+                self._do_invalidate_data(
-                return
+                    cover_art_cache_key, (artist.artist_image_url,)
-
+                )
-            self._do_invalidate_data(cover_art_cache_key, (artist.artist_image_url,))
                 for album in artist.albums or []:
                     self._do_invalidate_data(
                         CachingAdapter.CachedDataKey.ALBUM, (album.id,)
@@ -519,53 +550,39 @@ class FilesystemAdapter(CachingAdapter):
 
         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 := models.Playlist.get_or_none(models.Playlist.id == params[0]):
-            if playlist:
                 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 := models.Song.get_or_none(models.Song.id == params[0]):
-            if song:
                 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.
+        # Invalidate it.
-        models.CacheInfo.delete().where(
+        self._do_invalidate_data(data_key, params)
-            models.CacheInfo.cache_key == data_key,
+        cover_art_cache_key = CachingAdapter.CachedDataKey.COVER_ART_FILE
-            models.CacheInfo.params_hash == util.params_hash(*params),
-        ).execute()
 
-        def delete_cover_art(cover_art_id: str):
+        if data_key == CachingAdapter.CachedDataKey.COVER_ART_FILE:
-            cover_art_params_hash = util.params_hash(cover_art_id)
+            cover_art_file = self.cover_art_dir.joinpath(util.params_hash(*params))
-            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.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 playlist := models.Playlist.get_or_none(models.Playlist.id == params[0]):
-            if not playlist:
+                if cover_art := playlist.cover_art:
-                return
+                    self._do_delete_data(cover_art_cache_key, (cover_art,))
-
-            if playlist.cover_art:
-                delete_cover_art(playlist.cover_art)
 
                 playlist.delete_instance()
 
         elif data_key == CachingAdapter.CachedDataKey.SONG_FILE:
-            song = models.Song.get_or_none(models.Song.id == params[0])
+            if song := models.Song.get_or_none(models.Song.id == params[0]):
-            if not song:
-                return
-
                 # 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:
+                if cover_art := song.cover_art:
-                delete_cover_art(song.cover_art)
+                    self._do_delete_data(cover_art_cache_key, (cover_art,))

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(
+                ground_truth_search_results = AdapterManager._instance.ground_truth_adapter.search(  # noqa: E501
-                    AdapterManager._instance.ground_truth_adapter.search(query)
+                    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),

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.adapters import AdapterManager, api_objects as API, Result
-from sublime.cache_manager import CacheManager
 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):
+    class _AlbumModel(GObject.Object):
-        def __init__(self, album: Album):
+        def __init__(self, album: API.Album):
-            self.album: Album = 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

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

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):