diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py index 386d19b4..765476f7 100644 --- a/mopidy/backends/base.py +++ b/mopidy/backends/base.py @@ -11,20 +11,36 @@ class Backend(object): audio = None #: The library provider. An instance of - # :class:`mopidy.backends.base.BaseLibraryProvider`. + #: :class:`mopidy.backends.base.BaseLibraryProvider`, or :class:`None` if + #: the backend doesn't provide a library. library = None #: The playback provider. An instance of - #: :class:`mopidy.backends.base.BasePlaybackProvider`. + #: :class:`mopidy.backends.base.BasePlaybackProvider`, or :class:`None` if + #: the backend doesn't provide playback. playback = None #: The stored playlists provider. An instance of - #: :class:`mopidy.backends.base.BaseStoredPlaylistsProvider`. + #: :class:`mopidy.backends.base.BaseStoredPlaylistsProvider`, or + #: class:`None` if the backend doesn't provide stored playlists. stored_playlists = None #: List of URI schemes this backend can handle. uri_schemes = [] + # Because the providers is marked as pykka_traversible, we can't get() them + # from another actor, and need helper methods to check if the providers are + # set or None. + + def has_library(self): + return self.library is not None + + def has_playback(self): + return self.playback is not None + + def has_stored_playlists(self): + return self.stored_playlists is not None + class BaseLibraryProvider(object): """ diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 7ced97ed..52027d96 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -60,10 +60,18 @@ class Backends(list): def __init__(self, backends): super(Backends, self).__init__(backends) + # These lists keeps the backends in the original order, but only + # includes those which implements the required backend provider. Since + # it is important to keep the order, we can't simply use .values() on + # the X_by_uri_scheme dicts below. + self.with_library = [b for b in backends if b.has_library().get()] + self.with_playback = [b for b in backends if b.has_playback().get()] + self.with_stored_playlists = [b for b in backends + if b.has_stored_playlists().get()] + self.by_uri_scheme = {} for backend in backends: - uri_schemes = backend.uri_schemes.get() - for uri_scheme in uri_schemes: + for uri_scheme in backend.uri_schemes.get(): assert uri_scheme not in self.by_uri_scheme, ( 'Cannot add URI scheme %s for %s, ' 'it is already handled by %s' @@ -71,3 +79,15 @@ class Backends(list): uri_scheme, backend.__class__.__name__, self.by_uri_scheme[uri_scheme].__class__.__name__) self.by_uri_scheme[uri_scheme] = backend + + self.with_library_by_uri_scheme = {} + self.with_playback_by_uri_scheme = {} + self.with_stored_playlists_by_uri_scheme = {} + + for uri_scheme, backend in self.by_uri_scheme.items(): + if backend.has_library().get(): + self.with_library_by_uri_scheme[uri_scheme] = backend + if backend.has_playback().get(): + self.with_playback_by_uri_scheme[uri_scheme] = backend + if backend.has_stored_playlists().get(): + self.with_stored_playlists_by_uri_scheme[uri_scheme] = backend diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 801ed983..58263fd1 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -17,7 +17,7 @@ class LibraryController(object): def _get_backend(self, uri): uri_scheme = urlparse.urlparse(uri).scheme - return self.backends.by_uri_scheme.get(uri_scheme, None) + return self.backends.with_library_by_uri_scheme.get(uri_scheme, None) def find_exact(self, **query): """ @@ -36,7 +36,8 @@ class LibraryController(object): :type query: dict :rtype: :class:`mopidy.models.Playlist` """ - futures = [b.library.find_exact(**query) for b in self.backends] + futures = [b.library.find_exact(**query) + for b in self.backends.with_library] results = pykka.get_all(futures) return Playlist(tracks=[ track for playlist in results for track in playlist.tracks]) @@ -67,7 +68,8 @@ class LibraryController(object): if backend: backend.library.refresh(uri).get() else: - futures = [b.library.refresh(uri) for b in self.backends] + futures = [b.library.refresh(uri) + for b in self.backends.with_library] pykka.get_all(futures) def search(self, **query): @@ -87,7 +89,8 @@ class LibraryController(object): :type query: dict :rtype: :class:`mopidy.models.Playlist` """ - futures = [b.library.search(**query) for b in self.backends] + futures = [b.library.search(**query) + for b in self.backends.with_library] results = pykka.get_all(futures) track_lists = [playlist.tracks for playlist in results] tracks = list(itertools.chain(*track_lists)) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 4d8c8699..5f517c66 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -92,7 +92,7 @@ class PlaybackController(object): return None uri = self.current_cp_track.track.uri uri_scheme = urlparse.urlparse(uri).scheme - return self.backends.by_uri_scheme[uri_scheme] + return self.backends.with_playback_by_uri_scheme.get(uri_scheme, None) def _get_cpid(self, cp_track): if cp_track is None: @@ -300,9 +300,10 @@ class PlaybackController(object): def time_position(self): """Time position in milliseconds.""" backend = self._get_backend() - if backend is None: + if backend: + return backend.playback.get_time_position().get() + else: return 0 - return backend.playback.get_time_position().get() @property def volume(self): @@ -389,7 +390,7 @@ class PlaybackController(object): def pause(self): """Pause playback.""" backend = self._get_backend() - if backend is None or backend.playback.pause().get(): + if not backend or backend.playback.pause().get(): self.state = PlaybackState.PAUSED self._trigger_track_playback_paused() @@ -421,7 +422,8 @@ class PlaybackController(object): if cp_track is not None: self.current_cp_track = cp_track self.state = PlaybackState.PLAYING - if not self._get_backend().playback.play(cp_track.track).get(): + backend = self._get_backend() + if not backend or not backend.playback.play(cp_track.track).get(): # Track is not playable if self.random and self._shuffled: self._shuffled.remove(cp_track) @@ -447,8 +449,10 @@ class PlaybackController(object): def resume(self): """If paused, resume playing the current track.""" - if (self.state == PlaybackState.PAUSED and - self._get_backend().playback.resume().get()): + if self.state != PlaybackState.PAUSED: + return + backend = self._get_backend() + if backend and backend.playback.resume().get(): self.state = PlaybackState.PLAYING self._trigger_track_playback_resumed() @@ -474,7 +478,11 @@ class PlaybackController(object): self.next() return True - success = self._get_backend().playback.seek(time_position).get() + backend = self._get_backend() + if not backend: + return False + + success = backend.playback.seek(time_position).get() if success: self._trigger_seeked(time_position) return success @@ -488,7 +496,8 @@ class PlaybackController(object): :type clear_current_track: boolean """ if self.state != PlaybackState.STOPPED: - if self._get_backend().playback.stop().get(): + backend = self._get_backend() + if not backend or backend.playback.stop().get(): self._trigger_track_playback_ended() self.state = PlaybackState.STOPPED if clear_current_track: diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py index 2c5d2c54..cae39ca9 100644 --- a/mopidy/core/stored_playlists.py +++ b/mopidy/core/stored_playlists.py @@ -20,7 +20,8 @@ class StoredPlaylistsController(object): Read-only. List of :class:`mopidy.models.Playlist`. """ - futures = [b.stored_playlists.playlists for b in self.backends] + futures = [b.stored_playlists.playlists + for b in self.backends.with_stored_playlists] results = pykka.get_all(futures) return list(itertools.chain(*results)) @@ -42,10 +43,10 @@ class StoredPlaylistsController(object): :type uri_scheme: string :rtype: :class:`mopidy.models.Playlist` """ - if uri_scheme in self.backends.by_uri_scheme: + if uri_scheme in self.backends.with_stored_playlists_by_uri_scheme: backend = self.backends.by_uri_scheme[uri_scheme] else: - backend = self.backends[0] + backend = self.backends.with_stored_playlists[0] return backend.stored_playlists.create(name).get() def delete(self, uri): @@ -59,8 +60,9 @@ class StoredPlaylistsController(object): :type uri: string """ uri_scheme = urlparse.urlparse(uri).scheme - if uri_scheme in self.backends.by_uri_scheme: - backend = self.backends.by_uri_scheme[uri_scheme] + backend = self.backends.with_stored_playlists_by_uri_scheme.get( + uri_scheme, None) + if backend: backend.stored_playlists.delete(uri).get() def get(self, **criteria): @@ -103,7 +105,8 @@ class StoredPlaylistsController(object): :rtype: :class:`mopidy.models.Playlist` or :class:`None` """ uri_scheme = urlparse.urlparse(uri).scheme - backend = self.backends.by_uri_scheme.get(uri_scheme, None) + backend = self.backends.with_stored_playlists_by_uri_scheme.get( + uri_scheme, None) if backend: return backend.stored_playlists.lookup(uri).get() else: @@ -122,11 +125,13 @@ class StoredPlaylistsController(object): :type uri_scheme: string """ if uri_scheme is None: - futures = [b.stored_playlists.refresh() for b in self.backends] + futures = [b.stored_playlists.refresh() + for b in self.backends.with_stored_playlists] pykka.get_all(futures) else: - if uri_scheme in self.backends.by_uri_scheme: - backend = self.backends.by_uri_scheme[uri_scheme] + backend = self.backends.with_stored_playlists_by_uri_scheme.get( + uri_scheme, None) + if backend: backend.stored_playlists.refresh().get() def save(self, playlist): @@ -154,7 +159,7 @@ class StoredPlaylistsController(object): if playlist.uri is None: return uri_scheme = urlparse.urlparse(playlist.uri).scheme - if uri_scheme not in self.backends.by_uri_scheme: - return - backend = self.backends.by_uri_scheme[uri_scheme] - return backend.stored_playlists.save(playlist).get() + backend = self.backends.with_stored_playlists_by_uri_scheme.get( + uri_scheme, None) + if backend: + return backend.stored_playlists.save(playlist).get() diff --git a/tests/core/library_test.py b/tests/core/library_test.py index cb590428..7886b85c 100644 --- a/tests/core/library_test.py +++ b/tests/core/library_test.py @@ -21,7 +21,13 @@ class CoreLibraryTest(unittest.TestCase): self.library2 = mock.Mock(spec=base.BaseLibraryProvider) self.backend2.library = self.library2 - self.core = Core(audio=None, backends=[self.backend1, self.backend2]) + # A backend without the optional library provider + self.backend3 = mock.Mock() + self.backend3.uri_schemes.get.return_value = ['dummy3'] + self.backend3.has_library().get.return_value = False + + self.core = Core(audio=None, backends=[ + self.backend1, self.backend2, self.backend3]) def test_lookup_selects_dummy1_backend(self): self.core.library.lookup('dummy1:a') @@ -35,6 +41,13 @@ class CoreLibraryTest(unittest.TestCase): self.assertFalse(self.library1.lookup.called) self.library2.lookup.assert_called_once_with('dummy2:a') + def test_lookup_fails_for_dummy3_track(self): + result = self.core.library.lookup('dummy3:a') + + self.assertIsNone(result) + self.assertFalse(self.library1.lookup.called) + self.assertFalse(self.library2.lookup.called) + def test_refresh_with_uri_selects_dummy1_backend(self): self.core.library.refresh('dummy1:a') @@ -47,6 +60,12 @@ class CoreLibraryTest(unittest.TestCase): self.assertFalse(self.library1.refresh.called) self.library2.refresh.assert_called_once_with('dummy2:a') + def test_refresh_with_uri_fails_silently_for_dummy3_uri(self): + self.core.library.refresh('dummy3:a') + + self.assertFalse(self.library1.refresh.called) + self.assertFalse(self.library2.refresh.called) + def test_refresh_without_uri_calls_all_backends(self): self.core.library.refresh() diff --git a/tests/core/playback_test.py b/tests/core/playback_test.py index db39e716..2dc9bf10 100644 --- a/tests/core/playback_test.py +++ b/tests/core/playback_test.py @@ -3,7 +3,7 @@ from __future__ import unicode_literals import mock from mopidy.backends import base -from mopidy.core import Core +from mopidy.core import Core, PlaybackState from mopidy.models import Track from tests import unittest @@ -21,17 +21,24 @@ class CorePlaybackTest(unittest.TestCase): self.playback2 = mock.Mock(spec=base.BasePlaybackProvider) self.backend2.playback = self.playback2 + # A backend without the optional playback provider + self.backend3 = mock.Mock() + self.backend3.uri_schemes.get.return_value = ['dummy3'] + self.backend3.has_playback().get.return_value = False + self.tracks = [ - Track(uri='dummy1://foo', length=40000), - Track(uri='dummy1://bar', length=40000), - Track(uri='dummy2://foo', length=40000), - Track(uri='dummy2://bar', length=40000), + Track(uri='dummy1:a', length=40000), + Track(uri='dummy2:a', length=40000), + Track(uri='dummy3:a', length=40000), # Unplayable + Track(uri='dummy1:b', length=40000), ] - self.core = Core(audio=None, backends=[self.backend1, self.backend2]) + self.core = Core(audio=None, backends=[ + self.backend1, self.backend2, self.backend3]) self.core.current_playlist.append(self.tracks) self.cp_tracks = self.core.current_playlist.cp_tracks + self.unplayable_cp_track = self.cp_tracks[2] def test_play_selects_dummy1_backend(self): self.core.playback.play(self.cp_tracks[0]) @@ -40,10 +47,19 @@ class CorePlaybackTest(unittest.TestCase): self.assertFalse(self.playback2.play.called) def test_play_selects_dummy2_backend(self): - self.core.playback.play(self.cp_tracks[2]) + self.core.playback.play(self.cp_tracks[1]) self.assertFalse(self.playback1.play.called) - self.playback2.play.assert_called_once_with(self.tracks[2]) + self.playback2.play.assert_called_once_with(self.tracks[1]) + + def test_play_skips_to_next_on_unplayable_track(self): + self.core.playback.play(self.unplayable_cp_track) + + self.playback1.play.assert_called_once_with(self.tracks[3]) + self.assertFalse(self.playback2.play.called) + + self.assertEqual(self.core.playback.current_cp_track, + self.cp_tracks[3]) def test_pause_selects_dummy1_backend(self): self.core.playback.play(self.cp_tracks[0]) @@ -53,12 +69,20 @@ class CorePlaybackTest(unittest.TestCase): self.assertFalse(self.playback2.pause.called) def test_pause_selects_dummy2_backend(self): - self.core.playback.play(self.cp_tracks[2]) + self.core.playback.play(self.cp_tracks[1]) self.core.playback.pause() self.assertFalse(self.playback1.pause.called) self.playback2.pause.assert_called_once_with() + def test_pause_changes_state_even_if_track_is_unplayable(self): + self.core.playback.current_cp_track = self.unplayable_cp_track + self.core.playback.pause() + + self.assertEqual(self.core.playback.state, PlaybackState.PAUSED) + self.assertFalse(self.playback1.pause.called) + self.assertFalse(self.playback2.pause.called) + def test_resume_selects_dummy1_backend(self): self.core.playback.play(self.cp_tracks[0]) self.core.playback.pause() @@ -68,13 +92,22 @@ class CorePlaybackTest(unittest.TestCase): self.assertFalse(self.playback2.resume.called) def test_resume_selects_dummy2_backend(self): - self.core.playback.play(self.cp_tracks[2]) + self.core.playback.play(self.cp_tracks[1]) self.core.playback.pause() self.core.playback.resume() self.assertFalse(self.playback1.resume.called) self.playback2.resume.assert_called_once_with() + def test_resume_does_nothing_if_track_is_unplayable(self): + self.core.playback.current_cp_track = self.unplayable_cp_track + self.core.playback.state = PlaybackState.PAUSED + self.core.playback.resume() + + self.assertEqual(self.core.playback.state, PlaybackState.PAUSED) + self.assertFalse(self.playback1.resume.called) + self.assertFalse(self.playback2.resume.called) + def test_stop_selects_dummy1_backend(self): self.core.playback.play(self.cp_tracks[0]) self.core.playback.stop() @@ -83,12 +116,21 @@ class CorePlaybackTest(unittest.TestCase): self.assertFalse(self.playback2.stop.called) def test_stop_selects_dummy2_backend(self): - self.core.playback.play(self.cp_tracks[2]) + self.core.playback.play(self.cp_tracks[1]) self.core.playback.stop() self.assertFalse(self.playback1.stop.called) self.playback2.stop.assert_called_once_with() + def test_stop_changes_state_even_if_track_is_unplayable(self): + self.core.playback.current_cp_track = self.unplayable_cp_track + self.core.playback.state = PlaybackState.PAUSED + self.core.playback.stop() + + self.assertEqual(self.core.playback.state, PlaybackState.STOPPED) + self.assertFalse(self.playback1.stop.called) + self.assertFalse(self.playback2.stop.called) + def test_seek_selects_dummy1_backend(self): self.core.playback.play(self.cp_tracks[0]) self.core.playback.seek(10000) @@ -97,12 +139,21 @@ class CorePlaybackTest(unittest.TestCase): self.assertFalse(self.playback2.seek.called) def test_seek_selects_dummy2_backend(self): - self.core.playback.play(self.cp_tracks[2]) + self.core.playback.play(self.cp_tracks[1]) self.core.playback.seek(10000) self.assertFalse(self.playback1.seek.called) self.playback2.seek.assert_called_once_with(10000) + def test_seek_fails_for_unplayable_track(self): + self.core.playback.current_cp_track = self.unplayable_cp_track + self.core.playback.state = PlaybackState.PLAYING + success = self.core.playback.seek(1000) + + self.assertFalse(success) + self.assertFalse(self.playback1.seek.called) + self.assertFalse(self.playback2.seek.called) + def test_time_position_selects_dummy1_backend(self): self.core.playback.play(self.cp_tracks[0]) self.core.playback.seek(10000) @@ -112,9 +163,18 @@ class CorePlaybackTest(unittest.TestCase): self.assertFalse(self.playback2.get_time_position.called) def test_time_position_selects_dummy2_backend(self): - self.core.playback.play(self.cp_tracks[2]) + self.core.playback.play(self.cp_tracks[1]) self.core.playback.seek(10000) self.core.playback.time_position self.assertFalse(self.playback1.get_time_position.called) self.playback2.get_time_position.assert_called_once_with() + + def test_time_position_returns_0_if_track_is_unplayable(self): + self.core.playback.current_cp_track = self.unplayable_cp_track + + result = self.core.playback.time_position + + self.assertEqual(result, 0) + self.assertFalse(self.playback1.get_time_position.called) + self.assertFalse(self.playback2.get_time_position.called) diff --git a/tests/core/stored_playlists_test.py b/tests/core/stored_playlists_test.py index 4efb9acf..79b7d012 100644 --- a/tests/core/stored_playlists_test.py +++ b/tests/core/stored_playlists_test.py @@ -21,6 +21,12 @@ class StoredPlaylistsTest(unittest.TestCase): self.sp2 = mock.Mock(spec=base.BaseStoredPlaylistsProvider) self.backend2.stored_playlists = self.sp2 + # A backend without the optional stored playlists provider + self.backend3 = mock.Mock() + self.backend3.uri_schemes.get.return_value = ['dummy3'] + self.backend3.has_stored_playlists().get.return_value = False + self.backend3.stored_playlists = None + self.pl1a = Playlist(tracks=[Track(uri='dummy1:a')]) self.pl1b = Playlist(tracks=[Track(uri='dummy1:b')]) self.sp1.playlists.get.return_value = [self.pl1a, self.pl1b] @@ -29,7 +35,8 @@ class StoredPlaylistsTest(unittest.TestCase): self.pl2b = Playlist(tracks=[Track(uri='dummy2:b')]) self.sp2.playlists.get.return_value = [self.pl2a, self.pl2b] - self.core = Core(audio=None, backends=[self.backend1, self.backend2]) + self.core = Core(audio=None, backends=[ + self.backend3, self.backend1, self.backend2]) def test_get_playlists_combines_result_from_backends(self): result = self.core.stored_playlists.playlists @@ -61,6 +68,17 @@ class StoredPlaylistsTest(unittest.TestCase): self.assertFalse(self.sp1.create.called) self.sp2.create.assert_called_once_with('foo') + def test_create_with_unsupported_uri_scheme_uses_first_backend(self): + playlist = Playlist() + self.sp1.create().get.return_value = playlist + self.sp1.reset_mock() + + result = self.core.stored_playlists.create('foo', uri_scheme='dummy3') + + self.assertEqual(playlist, result) + self.sp1.create.assert_called_once_with('foo') + self.assertFalse(self.sp2.create.called) + def test_delete_selects_the_dummy1_backend(self): self.core.stored_playlists.delete('dummy1:a') @@ -79,6 +97,12 @@ class StoredPlaylistsTest(unittest.TestCase): self.assertFalse(self.sp1.delete.called) self.assertFalse(self.sp2.delete.called) + def test_delete_ignores_backend_without_playlist_support(self): + self.core.stored_playlists.delete('dummy3:a') + + self.assertFalse(self.sp1.delete.called) + self.assertFalse(self.sp2.delete.called) + def test_lookup_selects_the_dummy1_backend(self): self.core.stored_playlists.lookup('dummy1:a') @@ -91,6 +115,13 @@ class StoredPlaylistsTest(unittest.TestCase): self.assertFalse(self.sp1.lookup.called) self.sp2.lookup.assert_called_once_with('dummy2:a') + def test_lookup_track_in_backend_without_playlists_fails(self): + result = self.core.stored_playlists.lookup('dummy3:a') + + self.assertIsNone(result) + self.assertFalse(self.sp1.lookup.called) + self.assertFalse(self.sp2.lookup.called) + def test_refresh_without_uri_scheme_refreshes_all_backends(self): self.core.stored_playlists.refresh() @@ -109,6 +140,12 @@ class StoredPlaylistsTest(unittest.TestCase): self.assertFalse(self.sp1.refresh.called) self.assertFalse(self.sp2.refresh.called) + def test_refresh_ignores_backend_without_playlist_support(self): + self.core.stored_playlists.refresh(uri_scheme='dummy3') + + self.assertFalse(self.sp1.refresh.called) + self.assertFalse(self.sp2.refresh.called) + def test_save_selects_the_dummy1_backend(self): playlist = Playlist(uri='dummy1:a') self.sp1.save().get.return_value = playlist @@ -144,3 +181,10 @@ class StoredPlaylistsTest(unittest.TestCase): self.assertIsNone(result) self.assertFalse(self.sp1.save.called) self.assertFalse(self.sp2.save.called) + + def test_save_ignores_backend_without_playlist_support(self): + result = self.core.stored_playlists.save(Playlist(uri='dummy3:a')) + + self.assertIsNone(result) + self.assertFalse(self.sp1.save.called) + self.assertFalse(self.sp2.save.called)