colin/sublime-music
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fixed ordering of albums out of the cache

Browse Source
This commit is contained in:
Sumner Evans
2020-05-15 21:31:52 -06:00
parent eb59fa4adf
commit 0817282628
11 changed files with 170 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
sublime/adapters/api_objects.py
View File

@@ -127,7 +127,7 @@ class PlayQueue(abc.ABC):
@lru_cache(maxsize=8192)
def similarity_ratio(query: str, string: Optional[str]) -> int:
def similarity_ratio(query: str, string: str) -> int:
"""
Return the :class:`fuzzywuzzy.fuzz.partial_ratio` between the ``query`` and
the given ``string``.
@@ -138,9 +138,7 @@ def similarity_ratio(query: str, string: Optional[str]) -> int:
:param query: the query string
:param string: the string to compare to the query string
"""
if not string:
return 0
return fuzz.partial_ratio(query.lower(), string.lower())
return fuzz.partial_ratio(query, string)
class SearchResult:
@@ -164,20 +162,29 @@ class SearchResult:
member = f"_{result_type}"
cast(Dict[str, Any], getattr(self, member)).update({r.id: r for r in results})
def update(self, search_result: "SearchResult"):
self._artists.update(search_result._artists)
self._albums.update(search_result._albums)
self._songs.update(search_result._songs)
self._playlists.update(search_result._playlists)
def update(self, other: "SearchResult"):
assert self.query == other.query
self._artists.update(other._artists)
self._albums.update(other._albums)
self._songs.update(other._songs)
self._playlists.update(other._playlists)
_S = TypeVar("_S")
def _to_result(
self, it: Dict[str, _S], transform: Callable[[_S], Tuple[Optional[str], ...]],
) -> List[_S]:
assert self.query
all_results = sorted(
(
(max(map(partial(similarity_ratio, self.query), transform(x))), x)
(
max(
partial(similarity_ratio, self.query.lower())(t.lower())
for t in transform(x)
if t is not None
),
x,
)
for x in it.values()
),
key=lambda rx: rx[0],

99
sublime/adapters/filesystem/adapter.py
View File

@@ -4,7 +4,7 @@ import shutil
import threading
from datetime import datetime
from pathlib import Path
from typing import Any, cast, Dict, Optional, Sequence, Set, Tuple, Union
from typing import Any, cast, Dict, Optional, Sequence, Set, Union
from peewee import fn
@@ -109,12 +109,8 @@ class FilesystemAdapter(CachingAdapter):
model: Any,
cache_key: CachingAdapter.CachedDataKey,
ignore_cache_miss: bool = False,
where_clause: Optional[Tuple[Any, ...]] = None,
) -> Sequence:
query = model.select()
if where_clause:
query = query.where(*where_clause)
result = list(query)
result = list(model.select())
if self.is_cache and not ignore_cache_miss:
# Determine if the adapter has ingested data for this key before, and if
# not, cache miss.
@@ -130,10 +126,10 @@ class FilesystemAdapter(CachingAdapter):
model: Any,
id: str,
cache_key: CachingAdapter.CachedDataKey,
where_clause: Tuple[Any, ...] = (),
cache_where_clause: Tuple[Any, ...] = (),
# where_clause: Tuple[Any, ...] = (),
# cache_where_clause: Tuple[Any, ...] = (),
) -> Any:
obj = model.get_or_none(model.id == id, *where_clause)
obj = model.get_or_none(model.id == id)
# Handle the case that this is the ground truth adapter.
if not self.is_cache:
@@ -147,7 +143,6 @@ class FilesystemAdapter(CachingAdapter):
models.CacheInfo.cache_key == cache_key,
models.CacheInfo.parameter == id,
models.CacheInfo.valid == True, # noqa: 712
*cache_where_clause,
)
if not cache_info:
raise CacheMissError(partial_data=obj)
@@ -186,12 +181,19 @@ class FilesystemAdapter(CachingAdapter):
return SongCacheStatus.NOT_CACHED
_playlists = None
def get_playlists(self, ignore_cache_miss: bool = False) -> Sequence[API.Playlist]:
return self._get_list(
if self._playlists is not None:
print('Serving out of RAM')
return self._playlists
self._playlists = self._get_list(
models.Playlist,
CachingAdapter.CachedDataKey.PLAYLISTS,
ignore_cache_miss=ignore_cache_miss,
)
return self._playlists
def get_playlist_details(self, playlist_id: str) -> API.PlaylistDetails:
return self._get_object_details(
@@ -237,7 +239,6 @@ class FilesystemAdapter(CachingAdapter):
)
def get_artists(self, ignore_cache_miss: bool = False) -> Sequence[API.Artist]:
# TODO order_by
return self._get_list(
models.Artist,
CachingAdapter.CachedDataKey.ARTISTS,
@@ -250,11 +251,27 @@ class FilesystemAdapter(CachingAdapter):
)
def get_albums(self, query: AlbumSearchQuery) -> Sequence[API.Album]:
# TODO: deal with ordering
# TODO: deal with paging
# TODO: deal with cache invalidation
strhash = query.strhash()
query_result = models.AlbumQueryResult.get_or_none(
models.AlbumQueryResult.query_hash == strhash
)
# If we've cached the query result, then just return it. If it's stale, then
# return the old value as a cache miss error.
if query_result and (
cache_info := models.CacheInfo.get_or_none(
models.CacheInfo.cache_key == CachingAdapter.CachedDataKey.ALBUMS,
models.CacheInfo.parameter == strhash,
)
):
if cache_info.valid:
return query_result.albums
else:
raise CacheMissError(partial_data=query_result.albums)
# If we haven't ever cached the query result, try to construct one, and return
# it as a CacheMissError result.
sql_query = models.Album.select()
# TODO use the new ``where_clause`` from get_list
Type = AlbumSearchQuery.Type
if query.type == Type.GENRE:
@@ -265,27 +282,20 @@ class FilesystemAdapter(CachingAdapter):
Type.RANDOM: sql_query.order_by(fn.Random()),
Type.NEWEST: sql_query.order_by(models.Album.created.desc()),
Type.FREQUENT: sql_query.order_by(models.Album.play_count.desc()),
Type.RECENT: sql_query, # TODO IMPLEMENT
Type.STARRED: sql_query.where(models.Album.starred.is_null(False)),
Type.STARRED: sql_query.where(models.Album.starred.is_null(False)).order_by(
models.Album.name
),
Type.ALPHABETICAL_BY_NAME: sql_query.order_by(models.Album.name),
Type.ALPHABETICAL_BY_ARTIST: sql_query.order_by(models.Album.artist.name),
Type.YEAR_RANGE: sql_query.where(
models.Album.year.between(*query.year_range)
).order_by(models.Album.year),
Type.GENRE: sql_query.where(models.Album.genre == genre_name),
}[query.type]
).order_by(models.Album.year, models.Album.name),
Type.GENRE: sql_query.where(models.Album.genre == genre_name).order_by(
models.Album.name
),
}.get(query.type)
if self.is_cache:
# Determine if the adapter has ingested data for this key before, and if
# not, cache miss.
if not models.CacheInfo.get_or_none(
models.CacheInfo.cache_key == CachingAdapter.CachedDataKey.ALBUMS,
models.CacheInfo.parameter == query.strhash(),
models.CacheInfo.valid == True, # noqa: 712
):
raise CacheMissError(partial_data=sql_query)
return sql_query
raise CacheMissError(partial_data=sql_query)
def get_all_albums(self) -> Sequence[API.Album]:
return self._get_list(
@@ -316,7 +326,7 @@ class FilesystemAdapter(CachingAdapter):
return self._get_list(models.Genre, CachingAdapter.CachedDataKey.GENRES)
def search(self, query: str) -> API.SearchResult:
search_result = API.SearchResult()
search_result = API.SearchResult(query)
search_result.add_results("albums", self.get_all_albums())
search_result.add_results("artists", self.get_artists(ignore_cache_miss=True))
search_result.add_results(
@@ -503,8 +513,6 @@ class FilesystemAdapter(CachingAdapter):
if api_artist.artist_image_url
else None,
}
# del artist_data["artist_image_url"]
# del artist_data["similar_artists"]
artist, created = models.Artist.get_or_create(
id=api_artist.id, defaults=artist_data
@@ -605,10 +613,18 @@ class FilesystemAdapter(CachingAdapter):
return_val = ingest_album_data(data)
elif data_key == KEYS.ALBUMS:
for a in data:
ingest_album_data(a)
# TODO deal with sorting here
# TODO need some other way of deleting stale albums
albums = [ingest_album_data(a) for a in data]
album_query_result, created = models.AlbumQueryResult.get_or_create(
query_hash=param, defaults={"query_hash": param, "albums": albums}
)
if not created:
album_query_result.albums = albums
try:
album_query_result.save()
except ValueError:
# No save necessary.
pass
elif data_key == KEYS.ARTIST:
return_val = ingest_artist_data(data)
@@ -649,6 +665,7 @@ class FilesystemAdapter(CachingAdapter):
return_val = ingest_playlist(data)
elif data_key == KEYS.PLAYLISTS:
self._playlists = None
for p in data:
ingest_playlist(p)
models.Playlist.delete().where(
@@ -720,10 +737,6 @@ class FilesystemAdapter(CachingAdapter):
cache_info.save()
# song = models.Song.get_by_id(params[0])
# song.file = cache_info
# song.save()
return return_val if return_val is not None else cache_info
def _do_invalidate_data(

7
sublime/adapters/filesystem/models.py
View File

@@ -108,6 +108,11 @@ class Album(BaseModel):
return None
class AlbumQueryResult(BaseModel):
query_hash = TextField(primary_key=True)
albums = SortedManyToManyField(Album)
class IgnoredArticle(BaseModel):
name = TextField(unique=True, primary_key=True)
@@ -213,6 +218,8 @@ class Version(BaseModel):
ALL_TABLES = (
Album,
AlbumQueryResult,
AlbumQueryResult.albums.get_through_model(),
Artist,
CacheInfo,
Directory,

32
sublime/adapters/manager.py
View File

@@ -148,7 +148,7 @@ class Result(Generic[T]):
class AdapterManager:
available_adapters: Set[Any] = {FilesystemAdapter, SubsonicAdapter}
current_download_uris: Set[str] = set()
current_download_ids: Set[str] = set()
download_set_lock = threading.Lock()
executor: ThreadPoolExecutor = ThreadPoolExecutor()
download_executor: ThreadPoolExecutor = ThreadPoolExecutor()
@@ -304,7 +304,7 @@ class AdapterManager:
return Result(future_fn)
@staticmethod
def _create_download_fn(uri: str) -> Callable[[], str]:
def _create_download_fn(uri: str, id: str) -> Callable[[], str]:
"""
Create a function to download the given URI to a temporary file, and return the
filename. The returned function will spin-loop if the resource is already being
@@ -319,9 +319,9 @@ class AdapterManager:
resource_downloading = False
with AdapterManager.download_set_lock:
if uri in AdapterManager.current_download_uris:
if id in AdapterManager.current_download_ids:
resource_downloading = True
AdapterManager.current_download_uris.add(uri)
AdapterManager.current_download_ids.add(id)
# TODO (#122): figure out how to retry if the other request failed.
if resource_downloading:
@@ -331,7 +331,7 @@ class AdapterManager:
# it has completed. Then, just return the path to the
# resource.
t = 0.0
while uri in AdapterManager.current_download_uris and t < 20:
while id in AdapterManager.current_download_ids and t < 20:
sleep(0.2)
t += 0.2
# TODO (#122): handle the timeout
@@ -351,7 +351,7 @@ class AdapterManager:
finally:
# Always release the download set lock, even if there's an error.
with AdapterManager.download_set_lock:
AdapterManager.current_download_uris.discard(uri)
AdapterManager.current_download_ids.discard(id)
logging.info(f"{uri} downloaded. Returning.")
return str(download_tmp_filename)
@@ -410,7 +410,7 @@ class AdapterManager:
@staticmethod
def _get_from_cache_or_ground_truth(
function_name: str,
param: Optional[str],
param: Optional[Union[str, AlbumSearchQuery]],
cache_key: CachingAdapter.CachedDataKey = None,
before_download: Callable[[], None] = None,
use_ground_truth_adapter: bool = False,
@@ -440,6 +440,8 @@ class AdapterManager:
assert (caching_adapter := AdapterManager._instance.caching_adapter)
try:
logging.info(f"END: {function_name}: serving from cache")
if param is None:
return Result(getattr(caching_adapter, function_name)(**kwargs))
return Result(getattr(caching_adapter, function_name)(param, **kwargs))
except CacheMissError as e:
partial_data = e.partial_data
@@ -447,12 +449,18 @@ class AdapterManager:
except Exception:
logging.exception(f"Error on {function_name} retrieving from cache.")
param_str = param.strhash() if isinstance(param, AlbumSearchQuery) else param
if (
cache_key
and AdapterManager._instance.caching_adapter
and use_ground_truth_adapter
):
AdapterManager._instance.caching_adapter.invalidate_data(cache_key, param)
AdapterManager._instance.caching_adapter.invalidate_data(
cache_key, param_str
)
# TODO If any of the following fails, do we want to return what the caching
# adapter has?
# TODO (#188): don't short circuit if not allow_download because it could be the
# filesystem adapter.
@@ -473,7 +481,7 @@ class AdapterManager:
if AdapterManager._instance.caching_adapter:
if cache_key:
result.add_done_callback(
AdapterManager._create_caching_done_callback(cache_key, param)
AdapterManager._create_caching_done_callback(cache_key, param_str)
)
if on_result_finished:
@@ -708,6 +716,7 @@ class AdapterManager:
AdapterManager._instance.ground_truth_adapter.get_cover_art_uri(
cover_art_id, AdapterManager._get_scheme()
),
cover_art_id,
),
is_download=True,
default_value=existing_cover_art_filename,
@@ -802,6 +811,7 @@ class AdapterManager:
AdapterManager._instance.ground_truth_adapter.get_song_uri(
song_id, AdapterManager._get_scheme()
),
song_id,
)()
AdapterManager._instance.caching_adapter.ingest_new_data(
CachingAdapter.CachedDataKey.SONG_FILE,
@@ -998,7 +1008,7 @@ class AdapterManager:
) -> Result[Sequence[Album]]:
return AdapterManager._get_from_cache_or_ground_truth(
"get_albums",
query.strhash(),
query,
cache_key=CachingAdapter.CachedDataKey.ALBUMS,
before_download=before_download,
use_ground_truth_adapter=force,
@@ -1162,7 +1172,7 @@ class AdapterManager:
if not AdapterManager._instance.caching_adapter:
return SongCacheStatus.NOT_CACHED
if song.id in AdapterManager.current_download_uris:
if song.id in AdapterManager.current_download_ids:
return SongCacheStatus.DOWNLOADING
return AdapterManager._instance.caching_adapter.get_cached_status(song)

18
sublime/app.py
View File

@@ -568,25 +568,21 @@ class SublimeMusicApp(Gtk.Application):
def on_shuffle_press(self, *args):
if self.app_config.state.shuffle_on:
# Revert to the old play queue.
old_play_queue_copy = self.app_config.state.old_play_queue.copy()
old_play_queue_copy = self.app_config.state.old_play_queue
self.app_config.state.current_song_index = old_play_queue_copy.index(
self.app_config.state.current_song.id
)
self.app_config.state.play_queue = old_play_queue_copy
else:
self.app_config.state.old_play_queue = (
self.app_config.state.play_queue.copy()
)
self.app_config.state.old_play_queue = self.app_config.state.play_queue
mutable_play_queue = list(self.app_config.state.play_queue)
# Remove the current song, then shuffle and put the song back.
song_id = self.app_config.state.current_song.id
del self.app_config.state.play_queue[
self.app_config.state.current_song_index
]
random.shuffle(self.app_config.state.play_queue)
self.app_config.state.play_queue = [
song_id
] + self.app_config.state.play_queue
del mutable_play_queue[self.app_config.state.current_song_index]
random.shuffle(mutable_play_queue)
self.app_config.state.play_queue = (song_id,) + tuple(mutable_play_queue)
self.app_config.state.current_song_index = 0
self.app_config.state.shuffle_on = not self.app_config.state.shuffle_on

2
sublime/dbus/manager.py
View File

@@ -182,7 +182,7 @@ class DBusManager:
if get_playlists_result.data_is_available:
playlist_count = len(get_playlists_result.result())
except Exception:
logging.exception("Couldn't get playlists")
pass
return {
"org.mpris.MediaPlayer2": {

2
sublime/ui/albums.py
View File

@@ -469,7 +469,7 @@ class AlbumsGrid(Gtk.Overlay):
)
self.error_dialog.format_secondary_markup(
# TODO make this error better
f"Getting albums by {self.current_query.type} failed due to the"
f"Getting albums by {self.current_query.type} failed due to the "
f"following error\n\n{e}"
)
self.error_dialog.run()

45
sublime/ui/browse.py
View File

@@ -102,8 +102,6 @@ class ListAndDrilldown(Gtk.Paned):
),
}
id_stack = None
def __init__(self):
Gtk.Paned.__init__(self, orientation=Gtk.Orientation.HORIZONTAL)
@@ -116,8 +114,8 @@ class ListAndDrilldown(Gtk.Paned):
)
self.pack1(self.list, False, False)
self.drilldown = Gtk.Box()
self.pack2(self.drilldown, True, False)
self.box = Gtk.Box()
self.pack2(self.box, True, False)
def update(
self,
@@ -125,8 +123,7 @@ class ListAndDrilldown(Gtk.Paned):
app_config: AppConfiguration,
force: bool = False,
):
*rest, dir_id = id_stack
child_id_stack = tuple(rest)
*child_id_stack, dir_id = id_stack
selected_id = child_id_stack[-1] if len(child_id_stack) > 0 else None
self.list.update(
@@ -136,26 +133,26 @@ class ListAndDrilldown(Gtk.Paned):
force=force,
)
if self.id_stack == id_stack:
# We always want to update, but in this case, we don't want to blow
# away the drilldown.
if isinstance(self.drilldown, ListAndDrilldown):
self.drilldown.update(child_id_stack, app_config, force=force)
return
self.id_stack = id_stack
children = self.box.get_children()
if len(child_id_stack) == 0:
if len(children) > 0:
self.box.remove(children[0])
else:
if len(children) == 0:
drilldown = ListAndDrilldown()
drilldown.connect(
"song-clicked", lambda _, *args: self.emit("song-clicked", *args),
)
drilldown.connect(
"refresh-window",
lambda _, *args: self.emit("refresh-window", *args),
)
self.box.add(drilldown)
self.box.show_all()
if len(child_id_stack) > 0:
self.remove(self.drilldown)
self.drilldown = ListAndDrilldown()
self.drilldown.connect(
"song-clicked", lambda _, *args: self.emit("song-clicked", *args),
self.box.get_children()[0].update(
tuple(child_id_stack), app_config, force=force
)
self.drilldown.connect(
"refresh-window", lambda _, *args: self.emit("refresh-window", *args),
)
self.drilldown.update(child_id_stack, app_config, force=force)
self.drilldown.show_all()
self.pack2(self.drilldown, True, False)
class MusicDirectoryList(Gtk.Box):

2
sublime/ui/player_controls.py
View File

@@ -181,6 +181,8 @@ class PlayerControls(Gtk.ActionBar):
f"<b>Play Queue:</b> {play_queue_len} {song_label}"
)
# TODO this is super freaking stupid inefficient.
# IDEAS: batch it, don't get the queue until requested
self.editing_play_queue_song_list = True
new_store = []

1
sublime/ui/util.py
View File

@@ -226,6 +226,7 @@ def show_song_popover(
download_sensitive, remove_download_sensitive = False, False
albums, artists, parents = set(), set(), set()
for song_id in song_ids:
# TODO lazy load these
details = AdapterManager.get_song_details(song_id).result()
status = AdapterManager.get_cached_status(details)
albums.add(album.id if (album := details.album) else None)

39
tests/adapter_tests/filesystem_adapter_tests.py
View File

@@ -885,6 +885,39 @@ def test_get_music_directory(cache_adapter: FilesystemAdapter):
assert dir_child.name == "Crash My Party"
def test_search(adapter: FilesystemAdapter):
# TODO
pass
def test_search(cache_adapter: FilesystemAdapter):
with pytest.raises(CacheMissError):
cache_adapter.get_artist("artist1")
with pytest.raises(CacheMissError):
cache_adapter.get_album("album1")
with pytest.raises(CacheMissError):
cache_adapter.get_song_details("s1")
search_result = SublimeAPI.SearchResult()
search_result.add_results(
"albums",
[
SubsonicAPI.Album("album1", "Foo", artist_id="artist1", cover_art="cal1"),
SubsonicAPI.Album("album2", "Boo", artist_id="artist1", cover_art="cal2"),
],
)
search_result.add_results(
"artists",
[
SubsonicAPI.ArtistAndArtistInfo("artist1", "foo", cover_art="car1"),
SubsonicAPI.ArtistAndArtistInfo("artist2", "better boo", cover_art="car2"),
],
)
search_result.add_results(
"songs",
[
SubsonicAPI.Song("s1", "amazing boo", cover_art="s1"),
SubsonicAPI.Song("s2", "foo of all foo", cover_art="s2"),
],
)
cache_adapter.ingest_new_data(KEYS.SEARCH_RESULTS, None, search_result)
search_result = cache_adapter.search("foo")
assert [s.title for s in search_result.songs] == ["foo of all foo", "amazing boo"]
assert [a.name for a in search_result.artists] == ["foo", "better boo"]
assert [a.name for a in search_result.albums] == ["Foo", "Boo"]