diff --git a/sublime/adapters/adapter_base.py b/sublime/adapters/adapter_base.py index eca6c58..e97b10d 100644 --- a/sublime/adapters/adapter_base.py +++ b/sublime/adapters/adapter_base.py @@ -448,16 +448,16 @@ class Adapter(abc.ABC): return False @property - def can_stream(self) -> bool: + def can_get_song_file_uri(self) -> bool: """ - Whether or not the adapter can provide a stream URI. + Whether or not the adapter supports :class:`get_song_file_uri`. """ return False @property - def can_get_song_uri(self) -> bool: + def can_get_song_stream_uri(self) -> bool: """ - Whether or not the adapter supports :class:`get_song_uri`. + Whether or not the adapter supports :class:`get_song_stream_uri`. """ return False @@ -645,19 +645,26 @@ class Adapter(abc.ABC): """ raise self._check_can_error("get_cover_art_uri") - def get_song_uri(self, song_id: str, scheme: str, stream: bool = False) -> str: + def get_song_file_uri(self, song_id: str, schemes: Iterable[str]) -> str: """ - Get a URI for a given song. + Get a URI for a given song. This URI must give the full file. :param song_id: The ID of the song to get a URI for. - :param scheme: The URI scheme that should be returned. It is guaranteed that - ``scheme`` will be one of the schemes returned by + :param schemes: A set of URI schemes that can be returned. It is guaranteed that + all of the items in ``schemes`` will be one of the schemes returned by :class:`supported_schemes`. - :param stream: Whether or not the URI returned should be a stream URI. This will - only be ``True`` if :class:`supports_streaming` returns ``True``. :returns: The URI for the given song. """ - raise self._check_can_error("get_song_uri") + raise self._check_can_error("get_song_file_uri") + + def get_song_stream_uri(self, song_id: str) -> str: + """ + Get a URI for streaming the given song. + + :param song_id: The ID of the song to get the stream URI for. + :returns: the stream URI for the given song. + """ + raise self._check_can_error("get_song_stream_uri") def get_song_details(self, song_id: str) -> Song: """ diff --git a/sublime/adapters/filesystem/adapter.py b/sublime/adapters/filesystem/adapter.py index fc595e0..473f18c 100644 --- a/sublime/adapters/filesystem/adapter.py +++ b/sublime/adapters/filesystem/adapter.py @@ -101,7 +101,7 @@ class FilesystemAdapter(CachingAdapter): # TODO (#200) make these dependent on cache state. Need to do this kinda efficiently can_get_cover_art_uri = True - can_get_song_uri = True + can_get_song_file_uri = True can_get_song_details = True can_get_artist = True can_get_albums = True @@ -286,7 +286,7 @@ class FilesystemAdapter(CachingAdapter): raise CacheMissError() - def get_song_uri(self, song_id: str, scheme: str, stream: bool = False) -> str: + def get_song_file_uri(self, song_id: str, schemes: Iterable[str]) -> str: song = models.Song.get_or_none(models.Song.id == song_id) if not song: if self.is_cache: diff --git a/sublime/adapters/manager.py b/sublime/adapters/manager.py index b32761b..466ab5f 100644 --- a/sublime/adapters/manager.py +++ b/sublime/adapters/manager.py @@ -655,13 +655,17 @@ class AdapterManager: return AdapterManager._ground_truth_can_do("delete_playlist") @staticmethod - def can_get_song_filename_or_stream() -> bool: - return AdapterManager._ground_truth_can_do("get_song_uri") + def can_get_song_file_uri() -> bool: + return AdapterManager._ground_truth_can_do("get_song_file_uri") + + @staticmethod + def can_get_song_stream_uri() -> bool: + return AdapterManager._ground_truth_can_do("get_song_stream_uri") @staticmethod def can_batch_download_songs() -> bool: # We can only download from the ground truth adapter. - return AdapterManager._ground_truth_can_do("get_song_uri") + return AdapterManager._ground_truth_can_do("get_song_file_uri") @staticmethod def can_get_genres() -> bool: @@ -848,10 +852,10 @@ class AdapterManager: except CacheMissError as e: if e.partial_data is not None: existing_filename = cast(str, e.partial_data) - logging.info(f'Cache Miss on {"get_cover_art_uri"}.') + logging.info("Cache Miss on get_cover_art_uri.") except Exception: logging.exception( - f'Error on {"get_cover_art_uri"} retrieving from cache.' + "Error on get_cover_art_uri retrieving from cache." ) # If we are forcing, invalidate the existing cached data. @@ -887,40 +891,40 @@ class AdapterManager: return Result("") - # TODO (#189): allow this to take a set of schemes @staticmethod - def get_song_filename_or_stream( - song: Song, format: str = None, force_stream: bool = False - ) -> str: + def get_song_file_uri(song: Song) -> str: assert AdapterManager._instance cached_song_filename = None - if AdapterManager._can_use_cache(force_stream, "get_song_uri"): - assert AdapterManager._instance.caching_adapter + if AdapterManager._can_use_cache(False, "get_song_file_uri"): + assert (caching_adapter := AdapterManager._instance.caching_adapter) try: - return AdapterManager._instance.caching_adapter.get_song_uri( - song.id, "file" - ) + if "file" not in caching_adapter.supported_schemes: + raise Exception("file not a supported scheme") + + return caching_adapter.get_song_file_uri(song.id, "file") except CacheMissError as e: if e.partial_data is not None: cached_song_filename = cast(str, e.partial_data) - logging.info(f'Cache Miss on {"get_song_filename_or_stream"}.') + logging.info("Cache Miss on get_song_file_uri.") except Exception: - logging.exception( - f'Error on {"get_song_filename_or_stream"} retrieving from cache.' - ) + logging.exception("Error on get_song_file_uri retrieving from cache.") + ground_truth_adapter = AdapterManager._instance.ground_truth_adapter if ( - not AdapterManager._ground_truth_can_do("stream") - or not AdapterManager._ground_truth_can_do("get_song_uri") - or ( - AdapterManager._instance.ground_truth_adapter.is_networked - and AdapterManager._offline_mode - ) + not AdapterManager._ground_truth_can_do("get_song_file_uri") + or (ground_truth_adapter.is_networked and AdapterManager._offline_mode) + or ("file" not in ground_truth_adapter.supported_schemes) ): raise CacheMissError(partial_data=cached_song_filename) - return AdapterManager._instance.ground_truth_adapter.get_song_uri( - song.id, AdapterManager._get_networked_scheme(), stream=True, + return ground_truth_adapter.get_song_file_uri(song.id, "file") + + @staticmethod + def get_song_stream_uri(song: Song) -> str: + assert AdapterManager._instance + # TODO + return AdapterManager._instance.ground_truth_adapter.get_song_stream_uri( + song.id ) @staticmethod @@ -968,7 +972,9 @@ class AdapterManager: # Download the actual song file. try: # If the song file is already cached, just indicate done immediately. - AdapterManager._instance.caching_adapter.get_song_uri(song_id, "file") + AdapterManager._instance.caching_adapter.get_song_file_uri( + song_id, "file" + ) AdapterManager._instance.download_limiter_semaphore.release() AdapterManager._instance.song_download_progress( song_id, DownloadProgress(DownloadProgress.Type.DONE), @@ -984,7 +990,7 @@ class AdapterManager: song_tmp_filename_result: Result[ str ] = AdapterManager._create_download_result( - AdapterManager._instance.ground_truth_adapter.get_song_uri( + AdapterManager._instance.ground_truth_adapter.get_song_file_uri( song_id, AdapterManager._get_networked_scheme() ), song_id, diff --git a/sublime/adapters/subsonic/adapter.py b/sublime/adapters/subsonic/adapter.py index c8b4158..de7133a 100644 --- a/sublime/adapters/subsonic/adapter.py +++ b/sublime/adapters/subsonic/adapter.py @@ -281,7 +281,8 @@ class SubsonicAdapter(Adapter): can_get_playlist_details = True can_get_playlists = True can_get_song_details = True - can_get_song_uri = True + can_get_song_file_uri = True + can_get_song_stream_uri = True can_scrobble_song = True can_search = True can_stream = True @@ -537,10 +538,14 @@ class SubsonicAdapter(Adapter): params = {"id": cover_art_id, "size": size, **self._get_params()} return self._make_url("getCoverArt") + "?" + urlencode(params) - def get_song_uri(self, song_id: str, scheme: str, stream: bool = False) -> str: + def get_song_file_uri(self, song_id: str, schemes: Iterable[str]) -> str: + assert any(s in schemes for s in self.supported_schemes) params = {"id": song_id, **self._get_params()} - endpoint = "stream" if stream else "download" - return self._make_url(endpoint) + "?" + urlencode(params) + return self._make_url("download") + "?" + urlencode(params) + + def get_song_stream_uri(self, song_id: str) -> str: + params = {"id": song_id, **self._get_params()} + return self._make_url("stream") + "?" + urlencode(params) def get_song_details(self, song_id: str) -> API.Song: song = self._get_json(self._make_url("getSong"), id=song_id).song diff --git a/sublime/app.py b/sublime/app.py index 3084e27..cb906ca 100644 --- a/sublime/app.py +++ b/sublime/app.py @@ -7,6 +7,7 @@ from datetime import timedelta from functools import partial from pathlib import Path from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple +from urllib.parse import urlparse try: import osxmmkeys @@ -35,6 +36,7 @@ except Exception: from .adapters import ( AdapterManager, AlbumSearchQuery, + CacheMissError, DownloadProgress, Result, SongCacheStatus, @@ -1112,9 +1114,27 @@ class SublimeMusicApp(Gtk.Application): if order_token != self.song_playing_order_token: return - # TODO (#189): make this actually use the player's allowed list of schemes - # to play. - uri = AdapterManager.get_song_filename_or_stream(song) + uri = None + try: + if "file" in self.player_manager.supported_schemes: + uri = AdapterManager.get_song_file_uri(song) + except CacheMissError: + logging.debug("Couldn't find the file, will attempt to stream.") + + if not uri: + try: + uri = AdapterManager.get_song_stream_uri(song) + except Exception: + pass + if ( + not uri + or urlparse(uri).scheme not in self.player_manager.supported_schemes + ): + self.app_config.state.current_notification = UIState.UINotification( + markup=f"Unable to play {song.title}.", + icon="dialog-error", + ) + return # Prevent it from doing the thing where it continually loads # songs when it has to download. @@ -1190,7 +1210,7 @@ class SublimeMusicApp(Gtk.Application): os.system(f"osascript -e '{' '.join(osascript_command)}'") except Exception: - logging.exception( + logging.warning( "Unable to display notification. Is a notification daemon running?" # noqa: E501 ) @@ -1215,7 +1235,7 @@ class SublimeMusicApp(Gtk.Application): assert self.player_manager if self.player_manager.can_start_playing_with_no_latency: self.player_manager.play_media( - AdapterManager.get_song_filename_or_stream(song), + AdapterManager.get_song_file_uri(song), self.app_config.state.song_progress, song, ) diff --git a/sublime/players/chromecast.py b/sublime/players/chromecast.py index f193185..6fdfe02 100644 --- a/sublime/players/chromecast.py +++ b/sublime/players/chromecast.py @@ -237,8 +237,7 @@ class ChromecastPlayer(Player): song = AdapterManager.get_song_details( self._serving_song_id.value.decode() ).result() - filename = AdapterManager.get_song_filename_or_stream(song) - assert filename.startswith("file://") + filename = AdapterManager.get_song_file_uri(song) with open(filename[7:], "rb") as fin: song_buffer = io.BytesIO(fin.read()) diff --git a/sublime/players/manager.py b/sublime/players/manager.py index 86f21c7..6c840ec 100644 --- a/sublime/players/manager.py +++ b/sublime/players/manager.py @@ -1,6 +1,6 @@ import logging from datetime import timedelta -from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union +from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Type, Union from sublime.adapters.api_objects import Song @@ -89,6 +89,12 @@ class PlayerManager: if current_player_type := self._get_current_player_type(): return self.players.get(current_player_type) + @property + def supported_schemes(self) -> Set[str]: + if cp := self._get_current_player(): + return cp.supported_schemes + return set() + @property def can_start_playing_with_no_latency(self) -> bool: if self._current_device_id: diff --git a/tests/adapter_tests/filesystem_adapter_tests.py b/tests/adapter_tests/filesystem_adapter_tests.py index 5afe4b8..8dd7007 100644 --- a/tests/adapter_tests/filesystem_adapter_tests.py +++ b/tests/adapter_tests/filesystem_adapter_tests.py @@ -371,13 +371,13 @@ def test_invalidate_song_file(cache_adapter: FilesystemAdapter): cache_adapter.invalidate_data(KEYS.COVER_ART_FILE, "s1") with pytest.raises(CacheMissError): - cache_adapter.get_song_uri("1", "file") + cache_adapter.get_song_file_uri("1", "file") with pytest.raises(CacheMissError): cache_adapter.get_cover_art_uri("s1", "file", size=300) # Make sure it didn't delete the other song. - assert cache_adapter.get_song_uri("2", "file").endswith("song2.mp3") + assert cache_adapter.get_song_file_uri("2", "file").endswith("song2.mp3") def test_malformed_song_path(cache_adapter: FilesystemAdapter): @@ -390,10 +390,10 @@ def test_malformed_song_path(cache_adapter: FilesystemAdapter): KEYS.SONG_FILE, "2", ("fine/path/song2.mp3", MOCK_SONG_FILE2, None) ) - song_uri = cache_adapter.get_song_uri("1", "file") + song_uri = cache_adapter.get_song_file_uri("1", "file") assert song_uri.endswith(f"/music/{MOCK_SONG_FILE_HASH}") - song_uri2 = cache_adapter.get_song_uri("2", "file") + song_uri2 = cache_adapter.get_song_file_uri("2", "file") assert song_uri2.endswith("fine/path/song2.mp3") @@ -467,7 +467,7 @@ def test_delete_song_data(cache_adapter: FilesystemAdapter): KEYS.COVER_ART_FILE, "s1", MOCK_ALBUM_ART, ) - music_file_path = cache_adapter.get_song_uri("1", "file") + music_file_path = cache_adapter.get_song_file_uri("1", "file") cover_art_path = cache_adapter.get_cover_art_uri("s1", "file", size=300) cache_adapter.delete_data(KEYS.SONG_FILE, "1") @@ -477,7 +477,7 @@ def test_delete_song_data(cache_adapter: FilesystemAdapter): assert not Path(cover_art_path).exists() try: - cache_adapter.get_song_uri("1", "file") + cache_adapter.get_song_file_uri("1", "file") assert 0, "DID NOT raise CacheMissError" except CacheMissError as e: assert e.partial_data is None