Verified Commit 916e190b authored by Sumner Evans's avatar Sumner Evans 💬
Browse files

Added order token infrastructure and fixed most issues with late-arriving results

parent 227309d6
Pipeline #119754392 passed with stages
in 8 minutes and 23 seconds
......@@ -798,6 +798,8 @@ class SublimeMusicApp(Gtk.Application):
play_queue_future.add_done_callback(
lambda f: GLib.idle_add(do_update, f))
song_playing_order_token = 0
def play_song(
self,
song_index: int,
......@@ -845,17 +847,31 @@ class SublimeMusicApp(Gtk.Application):
)
song_notification.show()
def on_cover_art_download_complete(cover_art_filename):
def on_cover_art_download_complete(
cover_art_filename,
order_token,
):
if order_token != self.song_playing_order_token:
return
# Add the image to the notification, and re-draw the
# notification.
song_notification.set_image_from_pixbuf(
GdkPixbuf.Pixbuf.new_from_file(cover_art_filename))
song_notification.show()
cover_art_future = CacheManager.get_cover_art_filename(
song.coverArt, size=70)
def get_cover_art_filename(order_token):
cover_art_future = CacheManager.get_cover_art_filename(
song.coverArt, size=70)
return cover_art_future.result(), order_token
self.song_playing_order_token += 1
cover_art_future = CacheManager.create_future(
get_cover_art_filename,
self.song_playing_order_token,
)
cover_art_future.add_done_callback(
lambda f: on_cover_art_download_complete(f.result()))
lambda f: on_cover_art_download_complete(*f.result()))
except Exception:
logging.warning(
'Unable to display notification. Is a notification '
......
......@@ -446,7 +446,11 @@ class CacheManager(metaclass=Singleton):
logging.info(f'{abs_path} not found. Downloading...')
os.makedirs(download_path.parent, exist_ok=True)
self.save_file(download_path, download_fn())
try:
self.save_file(download_path, download_fn())
except requests.exceptions.ConnectionError:
with self.download_set_lock:
self.current_downloads.discard(abs_path)
# Move the file to its cache download location.
os.makedirs(abs_path.parent, exist_ok=True)
......
......@@ -62,12 +62,12 @@ class Server:
the module's user to query the \\*sonic server via the API.
"""
def __init__(
self,
name: str,
hostname: str,
username: str,
password: str,
disable_cert_verify: bool,
self,
name: str,
hostname: str,
username: str,
password: str,
disable_cert_verify: bool,
):
self.name: str = name
self.hostname: str = hostname
......@@ -98,9 +98,8 @@ class Server:
if os.environ.get('SUBLIME_MUSIC_DEBUG_DELAY'):
logging.info(
"SUBLIME_MUSIC_DEBUG_DELAY enabled. Pausing for",
f"{os.environ['SUBLIME_MUSIC_DEBUG_DELAY']} seconds.",
)
"SUBLIME_MUSIC_DEBUG_DELAY enabled. Pausing for "
f"{os.environ['SUBLIME_MUSIC_DEBUG_DELAY']} seconds.")
sleep(int(os.environ['SUBLIME_MUSIC_DEBUG_DELAY']))
# Deal with datetime parameters (convert to milliseconds since 1970)
......@@ -122,9 +121,9 @@ class Server:
return result
def _get_json(
self,
url: str,
**params: Union[None, str, datetime, int, List[int]],
self,
url: str,
**params: Union[None, str, datetime, int, List[int]],
) -> Response:
"""
Make a get request to a *Sonic REST API. Handle all types of errors
......@@ -714,10 +713,10 @@ class Server:
return result.playlist
def create_playlist(
self,
playlist_id: int = None,
name: str = None,
song_id: Union[int, List[int]] = None,
self,
playlist_id: int = None,
name: str = None,
song_id: Union[int, List[int]] = None,
) -> Union[PlaylistWithSongs, Response]:
"""
Creates (or updates) a playlist.
......@@ -780,14 +779,14 @@ class Server:
return self._get_json(self._make_url('deletePlaylist'), id=id)
def get_stream_url(
self,
id: str,
max_bit_rate: int = None,
format: str = None,
time_offset: int = None,
size: int = None,
estimate_content_length: bool = False,
converted: bool = False,
self,
id: str,
max_bit_rate: int = None,
format: str = None,
time_offset: int = None,
size: int = None,
estimate_content_length: bool = False,
converted: bool = False,
):
"""
Gets the URL to streams a given file.
......
......@@ -122,7 +122,7 @@ class ArtistList(Gtk.Box):
before_download=lambda self: self.loading_indicator.show_all(),
on_failure=lambda self, e: self.loading_indicator.hide(),
)
def update(self, artists, state: ApplicationState):
def update(self, artists, state: ApplicationState, **kwargs):
new_store = []
selected_idx = None
for i, artist in enumerate(artists):
......@@ -157,6 +157,8 @@ class ArtistDetailPanel(Gtk.Box):
),
}
update_order_token = 0
def __init__(self, *args, **kwargs):
super().__init__(
*args,
......@@ -264,9 +266,9 @@ class ArtistDetailPanel(Gtk.Box):
self.add(artist_info_box)
def update(self, state: ApplicationState):
self.artist_id = state.selected_artist_id
if state.selected_artist_id is None:
self.artist_action_buttons.hide()
self.artist_id = None
self.artist_indicator.set_text('')
self.artist_name.set_markup('')
self.artist_stats.set_markup('')
......@@ -280,8 +282,13 @@ class ArtistDetailPanel(Gtk.Box):
self.albums = cast(List[Child], [])
self.albums_list.update(None)
else:
self.update_order_token += 1
self.artist_action_buttons.show()
self.update_artist_view(state.selected_artist_id, state=state)
self.update_artist_view(
state.selected_artist_id,
state=state,
order_token=self.update_order_token,
)
# TODO need to handle when this is force updated. Need to delete a bunch of
# stuff and un-cache things.
......@@ -295,14 +302,17 @@ class ArtistDetailPanel(Gtk.Box):
artist: ArtistWithAlbumsID3,
state: ApplicationState,
force=False,
order_token=None,
):
self.artist_id = artist.id
if order_token != self.update_order_token:
return
self.artist_indicator.set_text('ARTIST')
self.artist_name.set_markup(util.esc(f'<b>{artist.name}</b>'))
self.artist_stats.set_markup(self.format_stats(artist))
self.update_artist_info(artist.id)
self.update_artist_artwork(artist)
self.update_artist_info(artist.id, order_token=order_token)
self.update_artist_artwork(artist, order_token=order_token)
self.albums = artist.get('album', artist.get('child', []))
self.albums_list.update(artist)
......@@ -314,7 +324,12 @@ class ArtistDetailPanel(Gtk.Box):
self,
artist_info: ArtistInfo2,
state: ApplicationState,
force=False,
order_token=None,
):
if order_token != self.update_order_token:
return
self.artist_bio.set_markup(util.esc(''.join(artist_info.biography)))
self.play_shuffle_buttons.show_all()
......@@ -344,21 +359,35 @@ class ArtistDetailPanel(Gtk.Box):
self,
cover_art_filename,
state: ApplicationState,
force=False,
order_token=None,
):
if order_token != self.update_order_token:
return
self.artist_artwork.set_from_file(cover_art_filename)
self.artist_artwork.set_loading(False)
# Event Handlers
# =========================================================================
def on_view_refresh_click(self, *args):
self.update_artist_view(self.artist_id, force=True)
self.update_artist_view(
self.artist_id,
force=True,
update_order_token=self.update_order_token,
)
def on_download_all_click(self, btn):
CacheManager.batch_download_songs(
self.get_artist_songs(),
before_download=lambda: self.update_artist_view(self.artist_id),
before_download=lambda: self.update_artist_view(
self.artist_id,
update_order_token=self.update_order_token,
),
on_song_download_complete=lambda i: self.update_artist_view(
self.artist_id),
self.artist_id,
update_order_token=self.update_order_token,
),
)
def on_play_all_clicked(self, btn):
......
......@@ -30,6 +30,7 @@ class BrowsePanel(Gtk.Overlay):
}
id_stack = None
update_order_token = 0
def __init__(self):
super().__init__()
......@@ -60,15 +61,20 @@ class BrowsePanel(Gtk.Overlay):
if not CacheManager.ready:
return
def do_update(id_stack):
self.update_order_token += 1
def do_update(id_stack, update_order_token):
if self.update_order_token != update_order_token:
return
self.root_directory_listing.update(
id_stack.result(),
id_stack,
state=state,
force=force,
)
self.spinner.hide()
def calculate_path():
def calculate_path(update_order_token):
if state.selected_browse_element_id is None:
return []
......@@ -83,10 +89,14 @@ class BrowsePanel(Gtk.Overlay):
id_stack.append(directory.id)
current_dir_id = directory.parent
return id_stack
return id_stack, update_order_token
path_fut = CacheManager.create_future(calculate_path)
path_fut.add_done_callback(lambda f: GLib.idle_add(do_update, f))
path_fut = CacheManager.create_future(
calculate_path,
self.update_order_token,
)
path_fut.add_done_callback(
lambda f: GLib.idle_add(do_update, *f.result()))
class ListAndDrilldown(Gtk.Paned):
......@@ -384,6 +394,8 @@ class DrilldownList(Gtk.Box):
class IndexList(DrilldownList):
update_order_token = 0
def update(
self,
selected_id,
......@@ -391,8 +403,13 @@ class IndexList(DrilldownList):
force=False,
**kwargs,
):
self.update_order_token += 1
self.selected_id = selected_id
self.update_store(force=force, state=state)
self.update_store(
force=force,
state=state,
order_token=self.update_order_token,
)
def on_refresh_clicked(self, _):
self.update(self.selected_id, force=True)
......@@ -407,7 +424,11 @@ class IndexList(DrilldownList):
artists,
state: ApplicationState = None,
force=False,
order_token=None,
):
if order_token != self.update_order_token:
return
self.do_update_store(artists)
def on_download_state_change(self, song_id=None):
......@@ -415,6 +436,8 @@ class IndexList(DrilldownList):
class MusicDirectoryList(DrilldownList):
update_order_token = 0
def update(
self,
selected_id,
......@@ -424,7 +447,12 @@ class MusicDirectoryList(DrilldownList):
):
self.directory_id = directory_id
self.selected_id = selected_id
self.update_store(directory_id, force=force, state=state)
self.update_store(
directory_id,
force=force,
state=state,
order_token=self.update_order_token,
)
def on_refresh_clicked(self, _):
self.update(
......@@ -440,7 +468,11 @@ class MusicDirectoryList(DrilldownList):
directory,
state: ApplicationState = None,
force=False,
order_token=None,
):
if order_token != self.update_order_token:
return
self.do_update_store(directory.child)
def on_download_state_change(self, song_id=None):
......
......@@ -279,9 +279,11 @@ class AlbumWithSongs(Gtk.Box):
on_failure=lambda self, e: self.set_loading(False),
)
def update_album_songs(
self,
album: Union[AlbumWithSongsID3, Child, Directory],
state: ApplicationState,
self,
album: Union[AlbumWithSongsID3, Child, Directory],
state: ApplicationState,
force=False,
order_token=None,
):
new_store = [
[
......
......@@ -57,6 +57,7 @@ class PlayerControls(Gtk.ActionBar):
current_song = None
current_device = None
chromecasts: List[ChromecastPlayer] = []
cover_art_update_order_token = 0
def __init__(self):
Gtk.ActionBar.__init__(self)
......@@ -130,7 +131,12 @@ class PlayerControls(Gtk.ActionBar):
# Update the current song information.
# TODO add popup of bigger cover art photo here
if state.current_song is not None:
self.update_cover_art(state.current_song.coverArt, size='70')
self.cover_art_update_order_token += 1
self.update_cover_art(
state.current_song.coverArt,
size='70',
order_token=self.cover_art_update_order_token,
)
self.song_title.set_markup(util.esc(state.current_song.title))
self.album_name.set_markup(util.esc(state.current_song.album))
......@@ -245,7 +251,16 @@ class PlayerControls(Gtk.ActionBar):
before_download=lambda self: self.album_art.set_loading(True),
on_failure=lambda self, e: self.album_art.set_loading(False),
)
def update_cover_art(self, cover_art_filename: str, state):
def update_cover_art(
self,
cover_art_filename: str,
state,
force=False,
order_token=None,
):
if order_token != self.cover_art_update_order_token:
return
self.album_art.set_from_file(cover_art_filename)
self.album_art.set_loading(False)
......
......@@ -177,9 +177,11 @@ class PlaylistList(Gtk.Box):
on_failure=lambda self, e: self.loading_indicator.hide(),
)
def update_list(
self,
playlists: List[PlaylistWithSongs],
state: ApplicationState,
self,
playlists: List[PlaylistWithSongs],
state: ApplicationState,
force=False,
order_token=None,
):
new_store = []
selected_idx = None
......@@ -429,6 +431,8 @@ class PlaylistDetailPanel(Gtk.Overlay):
self.playlist_view_loading_box.add(playlist_view_spinner)
self.add_overlay(self.playlist_view_loading_box)
update_playlist_view_order_token = 0
def update(self, state: ApplicationState, force=False):
if state.selected_playlist_id is None:
self.playlist_artwork.set_from_file(None)
......@@ -441,10 +445,12 @@ class PlaylistDetailPanel(Gtk.Overlay):
self.playlist_view_loading_box.hide()
self.playlist_artwork.set_loading(False)
else:
self.update_playlist_view_order_token += 1
self.update_playlist_view(
state.selected_playlist_id,
state=state,
force=force,
order_token=self.update_playlist_view_order_token,
)
@util.async_callback(
......@@ -453,11 +459,17 @@ class PlaylistDetailPanel(Gtk.Overlay):
on_failure=lambda self, e: self.playlist_view_loading_box.hide(),
)
def update_playlist_view(
self,
playlist,
state: ApplicationState = None,
force=False,
self,
playlist,
state: ApplicationState = None,
force=False,
order_token=None,
):
if self.update_playlist_view_order_token != order_token:
return
# If the selected playlist has changed, then clear the selections in
# the song list.
if self.playlist_id != playlist.id:
self.playlist_songs.get_selection().unselect_all()
......@@ -474,7 +486,10 @@ class PlaylistDetailPanel(Gtk.Overlay):
self.playlist_stats.set_markup(self.format_stats(playlist))
# Update the artwork.
self.update_playlist_artwork(playlist.coverArt)
self.update_playlist_artwork(
playlist.coverArt,
order_token=order_token,
)
# Update the song list model. This requires some fancy diffing to
# update the list.
......@@ -506,17 +521,26 @@ class PlaylistDetailPanel(Gtk.Overlay):
on_failure=lambda self, e: self.playlist_artwork.set_loading(False),
)
def update_playlist_artwork(
self,
cover_art_filename,
state: ApplicationState,
self,
cover_art_filename,
state: ApplicationState,
force=False,
order_token=None,
):
if self.update_playlist_view_order_token != order_token:
return
self.playlist_artwork.set_from_file(cover_art_filename)
self.playlist_artwork.set_loading(False)
# Event Handlers
# =========================================================================
def on_view_refresh_click(self, button):
self.update_playlist_view(self.playlist_id, force=True)
self.update_playlist_view(
self.playlist_id,
force=True,
order_token=self.update_playlist_view_order_token,
)
def on_playlist_edit_button_click(self, button):
dialog = EditPlaylistDialog(
......@@ -555,7 +579,11 @@ class PlaylistDetailPanel(Gtk.Overlay):
def on_playlist_list_download_all_button_click(self, button):
def download_state_change(*args):
GLib.idle_add(self.update_playlist_view, self.playlist_id)
GLib.idle_add(
lambda: self.update_playlist_view(
self.playlist_id,
order_token=self.update_playlist_view_order_token,
))
song_ids = [s[-1] for s in self.playlist_song_store]
CacheManager.batch_download_songs(
......@@ -608,7 +636,11 @@ class PlaylistDetailPanel(Gtk.Overlay):
allow_deselect = False
def on_download_state_change(song_id=None):
GLib.idle_add(self.update_playlist_view, self.playlist_id)
GLib.idle_add(
lambda: self.update_playlist_view(
self.playlist_id,
order_token=self.update_playlist_view_order_token,
))
# Use the new selection instead of the old one for calculating what
# to do the right click on.
......@@ -629,7 +661,11 @@ class PlaylistDetailPanel(Gtk.Overlay):
playlist_id=self.playlist_id,
song_index_to_remove=[p.get_indices()[0] for p in paths],
)
self.update_playlist_view(self.playlist_id, force=True)
self.update_playlist_view(
self.playlist_id,
force=True,
order_token=self.update_playlist_view_order_token,
)
remove_text = (
'Remove ' + util.pluralize('song', len(song_ids))
......@@ -678,7 +714,7 @@ class PlaylistDetailPanel(Gtk.Overlay):
)
@util.async_callback(lambda *a, **k: CacheManager.get_playlist(*a, **k))
def update_playlist_order(self, playlist, state: ApplicationState):
def update_playlist_order(self, playlist, state, **kwargs):
self.playlist_view_loading_box.show_all()
update_playlist_future = CacheManager.update_playlist(
playlist_id=playlist.id,
......@@ -688,7 +724,11 @@ class PlaylistDetailPanel(Gtk.Overlay):
update_playlist_future.add_done_callback(
lambda f: GLib.idle_add(
lambda: self.update_playlist_view(playlist.id, force=True)))
lambda: self.update_playlist_view(
playlist.id,
force=True,
order_token=self.update_playlist_view_order_token,
)))
def format_stats(self, playlist):
created_date = playlist.created.strftime('%B %d, %Y')
......
......@@ -319,7 +319,14 @@ def async_callback(
"""
def decorator(callback_fn):
@functools.wraps(callback_fn)
def wrapper(self, *args, state=None, **kwargs):
def wrapper(
self,
*args,
state=None,
order_token=None,
force=False,
**kwargs,
):
if before_download:
on_before_download = (
lambda: GLib.idle_add(before_download, self))
......@@ -334,11 +341,19 @@ def async_callback(
on_failure(self, e)
return
return GLib.idle_add(callback_fn, self, result, state)
return GLib.idle_add(
lambda: callback_fn(