From 7b9c682e951fcd41c10fb633a929ab9ca8c311e5 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 28 Sep 2012 02:24:58 +0200 Subject: [PATCH 01/21] Make core.uri_schemes include URI schemes from all backends --- mopidy/core/actor.py | 5 ++++- tests/core/actor_test.py | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 tests/core/actor_test.py diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index ea360055..f5de038d 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -43,7 +43,10 @@ class Core(pykka.ThreadingActor, AudioListener): @property def uri_schemes(self): """List of URI schemes we can handle""" - return self._backends[0].uri_schemes.get() + futures = [backend.uri_schemes for backend in self._backends] + results = pykka.get_all(futures) + schemes = [uri_scheme for result in results for uri_scheme in result] + return sorted(schemes) def reached_end_of_stream(self): self.playback.on_end_of_track() diff --git a/tests/core/actor_test.py b/tests/core/actor_test.py new file mode 100644 index 00000000..95639cf8 --- /dev/null +++ b/tests/core/actor_test.py @@ -0,0 +1,26 @@ +import mock +import pykka + +from mopidy.core import Core + +from tests import unittest + + +class CoreActorTest(unittest.TestCase): + def setUp(self): + self.backend1 = mock.Mock() + self.backend1.uri_schemes.get.return_value = ['dummy1'] + + self.backend2 = mock.Mock() + self.backend2.uri_schemes.get.return_value = ['dummy2'] + + self.core = Core(audio=None, backends=[self.backend1, self.backend2]) + + def tearDown(self): + pykka.ActorRegistry.stop_all() + + def test_uri_schemes_has_uris_from_all_backends(self): + result = self.core.uri_schemes + + self.assertIn('dummy1', result) + self.assertIn('dummy2', result) From c47cec9e654a055459a8059d96359b32bcfde5af Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 27 Oct 2012 11:27:54 +0200 Subject: [PATCH 02/21] Make core.playback select backend based on track URI --- mopidy/core/playback.py | 28 +++++++-- tests/core/playback_test.py | 118 ++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 tests/core/playback_test.py diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 85faaa13..4cef8db6 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -78,6 +78,11 @@ class PlaybackController(object): self.audio = audio self.backends = backends + uri_schemes_by_backend = {backend: backend.uri_schemes.get() + for backend in backends} + self.backends_by_uri_scheme = {uri_scheme: backend + for backend, uri_schemes in uri_schemes_by_backend.items() + for uri_scheme in uri_schemes} self.core = core @@ -86,6 +91,13 @@ class PlaybackController(object): self._first_shuffle = True self._volume = None + def _get_backend(self): + if self.current_cp_track is None: + return None + track = self.current_cp_track.track + uri_scheme = track.uri.split(':', 1)[0] + return self.backends_by_uri_scheme[uri_scheme] + def _get_cpid(self, cp_track): if cp_track is None: return None @@ -291,7 +303,10 @@ class PlaybackController(object): @property def time_position(self): """Time position in milliseconds.""" - return self.backends[0].playback.get_time_position().get() + backend = self._get_backend() + if backend is None: + return 0 + return backend.playback.get_time_position().get() @property def volume(self): @@ -377,7 +392,8 @@ class PlaybackController(object): def pause(self): """Pause playback.""" - if self.backends[0].playback.pause().get(): + backend = self._get_backend() + if backend is None or backend.playback.pause().get(): self.state = PlaybackState.PAUSED self._trigger_track_playback_paused() @@ -409,7 +425,7 @@ class PlaybackController(object): if cp_track is not None: self.current_cp_track = cp_track self.state = PlaybackState.PLAYING - if not self.backends[0].playback.play(cp_track.track).get(): + if not self._get_backend().playback.play(cp_track.track).get(): # Track is not playable if self.random and self._shuffled: self._shuffled.remove(cp_track) @@ -436,7 +452,7 @@ class PlaybackController(object): def resume(self): """If paused, resume playing the current track.""" if (self.state == PlaybackState.PAUSED and - self.backends[0].playback.resume().get()): + self._get_backend().playback.resume().get()): self.state = PlaybackState.PLAYING self._trigger_track_playback_resumed() @@ -462,7 +478,7 @@ class PlaybackController(object): self.next() return True - success = self.backends[0].playback.seek(time_position).get() + success = self._get_backend().playback.seek(time_position).get() if success: self._trigger_seeked(time_position) return success @@ -476,7 +492,7 @@ class PlaybackController(object): :type clear_current_track: boolean """ if self.state != PlaybackState.STOPPED: - if self.backends[0].playback.stop().get(): + if self._get_backend().playback.stop().get(): self._trigger_track_playback_ended() self.state = PlaybackState.STOPPED if clear_current_track: diff --git a/tests/core/playback_test.py b/tests/core/playback_test.py new file mode 100644 index 00000000..b3a75773 --- /dev/null +++ b/tests/core/playback_test.py @@ -0,0 +1,118 @@ +import mock + +from mopidy.backends import base +from mopidy.core import Core +from mopidy.models import Track + +from tests import unittest + + +class CorePlaybackTest(unittest.TestCase): + def setUp(self): + self.backend1 = mock.Mock() + self.backend1.uri_schemes.get.return_value = ['dummy1'] + self.playback1 = mock.Mock(spec=base.BasePlaybackProvider) + self.backend1.playback = self.playback1 + + self.backend2 = mock.Mock() + self.backend2.uri_schemes.get.return_value = ['dummy2'] + self.playback2 = mock.Mock(spec=base.BasePlaybackProvider) + self.backend2.playback = self.playback2 + + 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), + ] + + self.core = Core(audio=None, backends=[self.backend1, self.backend2]) + self.core.current_playlist.append(self.tracks) + + self.cp_tracks = self.core.current_playlist.cp_tracks + + def test_play_selects_dummy1_backend(self): + self.core.playback.play(self.cp_tracks[0]) + + self.playback1.play.assert_called_once_with(self.tracks[0]) + self.assertFalse(self.playback2.play.called) + + def test_play_selects_dummy2_backend(self): + self.core.playback.play(self.cp_tracks[2]) + + self.assertFalse(self.playback1.play.called) + self.playback2.play.assert_called_once_with(self.tracks[2]) + + def test_pause_selects_dummy1_backend(self): + self.core.playback.play(self.cp_tracks[0]) + self.core.playback.pause() + + self.playback1.pause.assert_called_once_with() + self.assertFalse(self.playback2.pause.called) + + def test_pause_selects_dummy2_backend(self): + self.core.playback.play(self.cp_tracks[2]) + self.core.playback.pause() + + self.assertFalse(self.playback1.pause.called) + self.playback2.pause.assert_called_once_with() + + def test_resume_selects_dummy1_backend(self): + self.core.playback.play(self.cp_tracks[0]) + self.core.playback.pause() + self.core.playback.resume() + + self.playback1.resume.assert_called_once_with() + self.assertFalse(self.playback2.resume.called) + + def test_resume_selects_dummy2_backend(self): + self.core.playback.play(self.cp_tracks[2]) + self.core.playback.pause() + self.core.playback.resume() + + self.assertFalse(self.playback1.resume.called) + self.playback2.resume.assert_called_once_with() + + def test_stop_selects_dummy1_backend(self): + self.core.playback.play(self.cp_tracks[0]) + self.core.playback.stop() + + self.playback1.stop.assert_called_once_with() + self.assertFalse(self.playback2.stop.called) + + def test_stop_selects_dummy2_backend(self): + self.core.playback.play(self.cp_tracks[2]) + self.core.playback.stop() + + self.assertFalse(self.playback1.stop.called) + self.playback2.stop.assert_called_once_with() + + def test_seek_selects_dummy1_backend(self): + self.core.playback.play(self.cp_tracks[0]) + self.core.playback.seek(10000) + + self.playback1.seek.assert_called_once_with(10000) + self.assertFalse(self.playback2.seek.called) + + def test_seek_selects_dummy2_backend(self): + self.core.playback.play(self.cp_tracks[2]) + self.core.playback.seek(10000) + + self.assertFalse(self.playback1.seek.called) + self.playback2.seek.assert_called_once_with(10000) + + def test_time_position_selects_dummy1_backend(self): + self.core.playback.play(self.cp_tracks[0]) + self.core.playback.seek(10000) + self.core.playback.time_position + + self.playback1.get_time_position.assert_called_once_with() + 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.seek(10000) + self.core.playback.time_position + + self.assertFalse(self.playback1.get_time_position.called) + self.playback2.get_time_position.assert_called_once_with() From a35deec0507497c7b4d68869d0ae4e34f36f1ff8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 27 Oct 2012 14:47:56 +0200 Subject: [PATCH 03/21] Make core.library support multiple backends --- mopidy/core/library.py | 43 ++++++++++++++++++-- tests/core/library_test.py | 82 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 tests/core/library_test.py diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 469b6160..80d9cbe5 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -1,10 +1,25 @@ +import pykka + +from mopidy.models import Playlist + + class LibraryController(object): pykka_traversable = True def __init__(self, backends, core): self.backends = backends + uri_schemes_by_backend = {backend: backend.uri_schemes.get() + for backend in backends} + self.backends_by_uri_scheme = {uri_scheme: backend + for backend, uri_schemes in uri_schemes_by_backend.items() + for uri_scheme in uri_schemes} + self.core = core + def _get_backend(self, uri): + uri_scheme = uri.split(':', 1)[0] + return self.backends_by_uri_scheme.get(uri_scheme) + def find_exact(self, **query): """ Search the library for tracks where ``field`` is ``values``. @@ -22,7 +37,12 @@ class LibraryController(object): :type query: dict :rtype: :class:`mopidy.models.Playlist` """ - return self.backends[0].library.find_exact(**query).get() + futures = [] + for backend in self.backends: + futures.append(backend.library.find_exact(**query)) + results = pykka.get_all(futures) + return Playlist(tracks=[ + track for playlist in results for track in playlist.tracks]) def lookup(self, uri): """ @@ -32,7 +52,9 @@ class LibraryController(object): :type uri: string :rtype: :class:`mopidy.models.Track` or :class:`None` """ - return self.backends[0].library.lookup(uri).get() + backend = self._get_backend(uri) + if backend: + return backend.library.lookup(uri).get() def refresh(self, uri=None): """ @@ -41,7 +63,15 @@ class LibraryController(object): :param uri: directory or track URI :type uri: string """ - return self.backends[0].library.refresh(uri).get() + if uri is not None: + backend = self._get_backend(uri) + if backend: + return backend.library.refresh(uri).get() + else: + futures = [] + for backend in self.backends: + futures.append(backend.library.refresh(uri)) + return pykka.get_all(futures) def search(self, **query): """ @@ -60,4 +90,9 @@ class LibraryController(object): :type query: dict :rtype: :class:`mopidy.models.Playlist` """ - return self.backends[0].library.search(**query).get() + futures = [] + for backend in self.backends: + futures.append(backend.library.search(**query)) + results = pykka.get_all(futures) + return Playlist(tracks=[ + track for playlist in results for track in playlist.tracks]) diff --git a/tests/core/library_test.py b/tests/core/library_test.py new file mode 100644 index 00000000..04f19909 --- /dev/null +++ b/tests/core/library_test.py @@ -0,0 +1,82 @@ +import mock + +from mopidy.backends import base +from mopidy.core import Core +from mopidy.models import Playlist, Track + +from tests import unittest + + +class CoreLibraryTest(unittest.TestCase): + def setUp(self): + self.backend1 = mock.Mock() + self.backend1.uri_schemes.get.return_value = ['dummy1'] + self.library1 = mock.Mock(spec=base.BaseLibraryProvider) + self.backend1.library = self.library1 + + self.backend2 = mock.Mock() + self.backend2.uri_schemes.get.return_value = ['dummy2'] + self.library2 = mock.Mock(spec=base.BaseLibraryProvider) + self.backend2.library = self.library2 + + self.core = Core(audio=None, backends=[self.backend1, self.backend2]) + + def test_lookup_selects_dummy1_backend(self): + self.core.library.lookup('dummy1:a') + + self.library1.lookup.assert_called_once_with('dummy1:a') + self.assertFalse(self.library2.lookup.called) + + def test_lookup_selects_dummy2_backend(self): + self.core.library.lookup('dummy2:a') + + self.assertFalse(self.library1.lookup.called) + self.library2.lookup.assert_called_once_with('dummy2:a') + + def test_refresh_with_uri_selects_dummy1_backend(self): + self.core.library.refresh('dummy1:a') + + self.library1.refresh.assert_called_once_with('dummy1:a') + self.assertFalse(self.library2.refresh.called) + + def test_refresh_with_uri_selects_dummy2_backend(self): + self.core.library.refresh('dummy2:a') + + self.assertFalse(self.library1.refresh.called) + self.library2.refresh.assert_called_once_with('dummy2:a') + + def test_refresh_without_uri_calls_all_backends(self): + self.core.library.refresh() + + self.library1.refresh.assert_called_once_with(None) + self.library2.refresh.assert_called_once_with(None) + + def test_find_exact_combines_results_from_all_backends(self): + track1 = Track(uri='dummy1:a') + track2 = Track(uri='dummy2:a') + self.library1.find_exact().get.return_value = Playlist(tracks=[track1]) + self.library1.find_exact.reset_mock() + self.library2.find_exact().get.return_value = Playlist(tracks=[track2]) + self.library2.find_exact.reset_mock() + + result = self.core.library.find_exact(any=['a']) + + self.assertIn(track1, result.tracks) + self.assertIn(track2, result.tracks) + self.library1.find_exact.assert_called_once_with(any=['a']) + self.library2.find_exact.assert_called_once_with(any=['a']) + + def test_search_combines_results_from_all_backends(self): + track1 = Track(uri='dummy1:a') + track2 = Track(uri='dummy2:a') + self.library1.search().get.return_value = Playlist(tracks=[track1]) + self.library1.search.reset_mock() + self.library2.search().get.return_value = Playlist(tracks=[track2]) + self.library2.search.reset_mock() + + result = self.core.library.search(any=['a']) + + self.assertIn(track1, result.tracks) + self.assertIn(track2, result.tracks) + self.library1.search.assert_called_once_with(any=['a']) + self.library2.search.assert_called_once_with(any=['a']) From 0641d2d2074cc4e7da80e13410ada5387ded7421 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 27 Oct 2012 23:07:19 +0200 Subject: [PATCH 04/21] Make core.stored_playlists.playlists support multiple backends --- mopidy/core/stored_playlists.py | 15 ++++++++++- tests/core/stored_playlists_test.py | 41 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 tests/core/stored_playlists_test.py diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py index d7bcbd0c..4a8f5463 100644 --- a/mopidy/core/stored_playlists.py +++ b/mopidy/core/stored_playlists.py @@ -1,3 +1,6 @@ +import pykka + + class StoredPlaylistsController(object): pykka_traversable = True @@ -12,10 +15,14 @@ class StoredPlaylistsController(object): Read/write. List of :class:`mopidy.models.Playlist`. """ - return self.backends[0].stored_playlists.playlists.get() + futures = [backend.stored_playlists.playlists + for backend in self.backends] + results = pykka.get_all(futures) + return [playlist for result in results for playlist in result] @playlists.setter # noqa def playlists(self, playlists): + # TODO Support multiple backends self.backends[0].stored_playlists.playlists = playlists def create(self, name): @@ -26,6 +33,7 @@ class StoredPlaylistsController(object): :type name: string :rtype: :class:`mopidy.models.Playlist` """ + # TODO Support multiple backends return self.backends[0].stored_playlists.create(name).get() def delete(self, playlist): @@ -35,6 +43,7 @@ class StoredPlaylistsController(object): :param playlist: the playlist to delete :type playlist: :class:`mopidy.models.Playlist` """ + # TODO Support multiple backends return self.backends[0].stored_playlists.delete(playlist).get() def get(self, **criteria): @@ -76,12 +85,14 @@ class StoredPlaylistsController(object): :type uri: string :rtype: :class:`mopidy.models.Playlist` """ + # TODO Support multiple backends return self.backends[0].stored_playlists.lookup(uri).get() def refresh(self): """ Refresh the stored playlists in :attr:`playlists`. """ + # TODO Support multiple backends return self.backends[0].stored_playlists.refresh().get() def rename(self, playlist, new_name): @@ -93,6 +104,7 @@ class StoredPlaylistsController(object): :param new_name: the new name :type new_name: string """ + # TODO Support multiple backends return self.backends[0].stored_playlists.rename( playlist, new_name).get() @@ -103,4 +115,5 @@ class StoredPlaylistsController(object): :param playlist: the playlist :type playlist: :class:`mopidy.models.Playlist` """ + # TODO Support multiple backends return self.backends[0].stored_playlists.save(playlist).get() diff --git a/tests/core/stored_playlists_test.py b/tests/core/stored_playlists_test.py new file mode 100644 index 00000000..d92b89c0 --- /dev/null +++ b/tests/core/stored_playlists_test.py @@ -0,0 +1,41 @@ +import mock + +from mopidy.backends import base +from mopidy.core import Core +from mopidy.models import Playlist, Track + +from tests import unittest + + +class StoredPlaylistsTest(unittest.TestCase): + def setUp(self): + self.backend1 = mock.Mock() + self.backend1.uri_schemes.get.return_value = ['dummy1'] + self.sp1 = mock.Mock(spec=base.BaseStoredPlaylistsProvider) + self.backend1.stored_playlists = self.sp1 + + self.backend2 = mock.Mock() + self.backend2.uri_schemes.get.return_value = ['dummy2'] + self.sp2 = mock.Mock(spec=base.BaseStoredPlaylistsProvider) + self.backend2.stored_playlists = self.sp2 + + 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] + + self.pl2a = Playlist(tracks=[Track(uri='dummy2:a')]) + 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]) + + def test_get_playlists_combines_result_from_backends(self): + result = self.core.stored_playlists.playlists + + self.assertIn(self.pl1a, result) + self.assertIn(self.pl1b, result) + self.assertIn(self.pl2a, result) + self.assertIn(self.pl2b, result) + + # TODO The rest of the stored playlists API is pending redesign before + # we'll update it to support multiple backends. From d450e5a238795e194fed2c68fb116596fcef9e8a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 28 Oct 2012 11:11:05 +0100 Subject: [PATCH 05/21] Turn both local and Spotify backend on by default --- mopidy/settings.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/mopidy/settings.py b/mopidy/settings.py index 31de4a6e..c1f35887 100644 --- a/mopidy/settings.py +++ b/mopidy/settings.py @@ -10,17 +10,17 @@ All available settings and their default values. #: List of playback backends to use. See :ref:`backend-implementations` for all #: available backends. #: +#: When results from multiple backends are combined, they are combined in the +#: order the backends are listed here. +#: #: Default:: #: -#: BACKENDS = (u'mopidy.backends.spotify.SpotifyBackend',) -#: -#: Other typical values:: -#: -#: BACKENDS = (u'mopidy.backends.local.LocalBackend',) -#: -#: .. note:: -#: Currently only the first backend in the list is used. +#: BACKENDS = ( +#: u'mopidy.backends.local.LocalBackend', +#: u'mopidy.backends.spotify.SpotifyBackend', +#: ) BACKENDS = ( + u'mopidy.backends.local.LocalBackend', u'mopidy.backends.spotify.SpotifyBackend', ) From 9a617b180372972add84f6dc41e2c10bb93c86e5 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 28 Oct 2012 20:58:51 +0100 Subject: [PATCH 06/21] Improvements after code review --- mopidy/core/actor.py | 6 ++-- mopidy/core/library.py | 28 +++++++++---------- mopidy/core/playback.py | 11 +++++--- tests/frontends/mpd/protocol/playback_test.py | 8 +++--- 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index f5de038d..05c085fd 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -1,3 +1,5 @@ +import itertools + import pykka from mopidy.audio import AudioListener @@ -45,8 +47,8 @@ class Core(pykka.ThreadingActor, AudioListener): """List of URI schemes we can handle""" futures = [backend.uri_schemes for backend in self._backends] results = pykka.get_all(futures) - schemes = [uri_scheme for result in results for uri_scheme in result] - return sorted(schemes) + uri_schemes = itertools.chain(*results) + return sorted(uri_schemes) def reached_end_of_stream(self): self.playback.on_end_of_track() diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 80d9cbe5..37c8c522 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -1,3 +1,5 @@ +import urlparse + import pykka from mopidy.models import Playlist @@ -8,16 +10,18 @@ class LibraryController(object): def __init__(self, backends, core): self.backends = backends - uri_schemes_by_backend = {backend: backend.uri_schemes.get() + uri_schemes_by_backend = { + backend: backend.uri_schemes.get() for backend in backends} - self.backends_by_uri_scheme = {uri_scheme: backend + self.backends_by_uri_scheme = { + uri_scheme: backend for backend, uri_schemes in uri_schemes_by_backend.items() for uri_scheme in uri_schemes} self.core = core def _get_backend(self, uri): - uri_scheme = uri.split(':', 1)[0] + uri_scheme = urlparse.urlparse(uri).scheme return self.backends_by_uri_scheme.get(uri_scheme) def find_exact(self, **query): @@ -37,9 +41,7 @@ class LibraryController(object): :type query: dict :rtype: :class:`mopidy.models.Playlist` """ - futures = [] - for backend in self.backends: - futures.append(backend.library.find_exact(**query)) + futures = [b.library.find_exact(**query) for b in self.backends] results = pykka.get_all(futures) return Playlist(tracks=[ track for playlist in results for track in playlist.tracks]) @@ -55,6 +57,8 @@ class LibraryController(object): backend = self._get_backend(uri) if backend: return backend.library.lookup(uri).get() + else: + return None def refresh(self, uri=None): """ @@ -66,12 +70,10 @@ class LibraryController(object): if uri is not None: backend = self._get_backend(uri) if backend: - return backend.library.refresh(uri).get() + backend.library.refresh(uri).get() else: - futures = [] - for backend in self.backends: - futures.append(backend.library.refresh(uri)) - return pykka.get_all(futures) + futures = [b.library.refresh(uri) for b in self.backends] + pykka.get_all(futures) def search(self, **query): """ @@ -90,9 +92,7 @@ class LibraryController(object): :type query: dict :rtype: :class:`mopidy.models.Playlist` """ - futures = [] - for backend in self.backends: - futures.append(backend.library.search(**query)) + futures = [b.library.search(**query) for b in self.backends] results = pykka.get_all(futures) return Playlist(tracks=[ track for playlist in results for track in playlist.tracks]) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 4cef8db6..721bc2a8 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -1,5 +1,6 @@ import logging import random +import urlparse from . import listener @@ -78,9 +79,11 @@ class PlaybackController(object): self.audio = audio self.backends = backends - uri_schemes_by_backend = {backend: backend.uri_schemes.get() + uri_schemes_by_backend = { + backend: backend.uri_schemes.get() for backend in backends} - self.backends_by_uri_scheme = {uri_scheme: backend + self.backends_by_uri_scheme = { + uri_scheme: backend for backend, uri_schemes in uri_schemes_by_backend.items() for uri_scheme in uri_schemes} @@ -94,8 +97,8 @@ class PlaybackController(object): def _get_backend(self): if self.current_cp_track is None: return None - track = self.current_cp_track.track - uri_scheme = track.uri.split(':', 1)[0] + uri = self.current_cp_track.track.uri + uri_scheme = urlparse.urlparse(uri).scheme return self.backends_by_uri_scheme[uri_scheme] def _get_cpid(self, cp_track): diff --git a/tests/frontends/mpd/protocol/playback_test.py b/tests/frontends/mpd/protocol/playback_test.py index ab254bdf..202ac649 100644 --- a/tests/frontends/mpd/protocol/playback_test.py +++ b/tests/frontends/mpd/protocol/playback_test.py @@ -392,9 +392,9 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse(u'OK') def test_seek_with_songpos(self): - seek_track = Track(uri='dummy:2', length=40000) + seek_track = Track(uri='dummy:b', length=40000) self.core.current_playlist.append( - [Track(uri='dummy:1', length=40000), seek_track]) + [Track(uri='dummy:a', length=40000), seek_track]) self.sendRequest(u'seek "1" "30"') self.assertEqual(self.core.playback.current_track.get(), seek_track) @@ -417,9 +417,9 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse(u'OK') def test_seekid_with_cpid(self): - seek_track = Track(uri='dummy:2', length=40000) + seek_track = Track(uri='dummy:b', length=40000) self.core.current_playlist.append( - [Track(uri='dummy:1', length=40000), seek_track]) + [Track(uri='dummy:a', length=40000), seek_track]) self.sendRequest(u'seekid "1" "30"') self.assertEqual(1, self.core.playback.current_cpid.get()) From a6200415842b2bd1cf06856e0f157afed6ee1d07 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 28 Oct 2012 21:37:37 +0100 Subject: [PATCH 07/21] More improvements after code review --- mopidy/core/stored_playlists.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py index 4a8f5463..9de1545f 100644 --- a/mopidy/core/stored_playlists.py +++ b/mopidy/core/stored_playlists.py @@ -1,3 +1,5 @@ +import itertools + import pykka @@ -15,10 +17,9 @@ class StoredPlaylistsController(object): Read/write. List of :class:`mopidy.models.Playlist`. """ - futures = [backend.stored_playlists.playlists - for backend in self.backends] + futures = [b.stored_playlists.playlists for b in self.backends] results = pykka.get_all(futures) - return [playlist for result in results for playlist in result] + return list(itertools.chain(*results)) @playlists.setter # noqa def playlists(self, playlists): From c2cde5267ab35a91f7ce048907f703e750a1722e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 28 Oct 2012 21:49:29 +0100 Subject: [PATCH 08/21] Update changelog with multi-backend changes --- docs/changes.rst | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index c68db685..5a17c810 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -12,6 +12,54 @@ v0.9.0 (in development) - Pykka >= 1.0 is now required. +**Multiple backends support** + +Support for using the local and Spotify backends simultaneously have for a very +long time been our most requested feature. Finally, it's here! + +- Both the local backend and the Spotify backend are now turned on by default. + The local backend is listed first in the :attr:`mopidy.settings.BACKENDS` + setting, and are thus given the highest priority in e.g. search results, + meaning that we're listing search hits from the local backend first. If you + want to prioritize the backends in another way, simply set ``BACKENDS`` in + your own settings file and reorder the backends. + + There are no other setting changes related to the local and Spotify backends. + As always, see :mod:`mopidy.settings` for the full list of available + settings. + +Internally, Mopidy have seen a lot of changes to pave the way for multiple +backends: + +- A new layer and actor, "core", have been added to our stack, inbetween the + frontends and the backends. The responsibility of this layer and actor is to + take requests from the frontends, pass them on to one or more backends, and + combining the response from the backends into a single response to the + requesting frontend. + + The frontends no longer know anything about the backends. They just use the + :ref:`core-api`. + +- The base playback provider have gotten sane default behavior instead of the + old empty functions. By default, the playback provider now lets GStreamer + keep track of the current track's time position. The local backend simply + uses the base playback provider without any changes. The same applies to any + future backend that just needs GStreamer to play an URI for it. + +- The dependency graph between the core controllers and the backend providers + have been straightened out, so that we don't have any circular dependencies + or similar. The frontend, core, backend, and audio layers are now strictly + separate. The frontend layer calls on the core layer, and the core layer + calls on the backend layer. Both the core layer and the backends are allowed + to call on the audio layer. Any data flow in the opposite direction is done + by broadcasting of events to listeners, through e.g. + :class:`mopidy.core.CoreListener` and :class:`mopidy.audio.AudioListener`. + +- All dependencies are now explicitly passed to the constructors of the + frontends, core, and the backends. This makes testing each layer with + dummy/mocked lower layers easier than with the old variant, where + dependencies where looked up in Pykka's actor registry. + **Bug fixes** - :issue:`213`: Fix "streaming task paused, reason not-negotiated" errors From 86ca1bf3c9e637713f687966d2f6e2c9a1843b29 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 28 Oct 2012 21:58:07 +0100 Subject: [PATCH 09/21] Update README with multi-backend, MPRIS and DLNA possibilities --- README.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index e7ecd614..a7df7692 100644 --- a/README.rst +++ b/README.rst @@ -4,11 +4,17 @@ Mopidy .. image:: https://secure.travis-ci.org/mopidy/mopidy.png?branch=develop -Mopidy is a music server which can play music from `Spotify -`_ or from your local hard drive. To search for music -in Spotify's vast archive, manage playlists, and play music, you can use any -`MPD client `_. MPD clients are available for most -platforms, including Windows, Mac OS X, Linux, Android and iOS. +Mopidy is a music server which can play music both from your local hard drive +and from `Spotify `_. Searches returns results from +both your local hard drive and from Spotify, and you can mix tracks from both +sources in your play queue. Your Spotify playlists are also available for use, +though we don't support modifying them yet. + +To control your music server, you can use the Ubuntu Sound Menu on the machine +running Mopidy, any device on the same network which supports the DLNA media +controller spec (with the help of Rygel in addition to Mopidy), or any `MPD +client `_. MPD clients are available for most platforms, +including Windows, Mac OS X, Linux, Android and iOS. To install Mopidy, check out `the installation docs `_. From 17b0a2ccc3413c0ce939c2df21434194f526aa6c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 28 Oct 2012 22:00:55 +0100 Subject: [PATCH 10/21] Update local backend settings guide --- docs/settings.rst | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index a79dfd78..88004e11 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -43,20 +43,10 @@ Music from local storage ======================== If you want use Mopidy to play music you have locally at your machine instead -of using Spotify, you need to change the backend from the default to -:mod:`mopidy.backends.local` by adding the following line to your settings -file:: - - BACKENDS = (u'mopidy.backends.local.LocalBackend',) - -You may also want to change some of the ``LOCAL_*`` settings. See -:mod:`mopidy.settings`, for a full list of available settings. - -.. note:: - - Currently, Mopidy supports using Spotify *or* local storage as a music - source. We're working on using both sources simultaneously, and will - have support for this in a future release. +of or in addition to using Spotify, you need to review and maybe change some of +the ``LOCAL_*`` settings. See :mod:`mopidy.settings`, for a full list of +available settings. Then you need to generate a tag cache for your local +music... .. _generating_a_tag_cache: @@ -66,7 +56,7 @@ Generating a tag cache Before Mopidy 0.3 the local storage backend relied purely on ``tag_cache`` files generated by the original MPD server. To remedy this the command -:command:`mopidy-scan` has been created. The program will scan your current +:command:`mopidy-scan` was created. The program will scan your current :attr:`mopidy.settings.LOCAL_MUSIC_PATH` and build a MPD compatible ``tag_cache``. From 519fdb9326c7d6748f622a2a1853193eaa886b98 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 01:09:36 +0100 Subject: [PATCH 11/21] Update concepts description and graphs --- docs/api/concepts.rst | 117 ++++++++++++++++++++++++++++++++--------- docs/api/frontends.rst | 2 + 2 files changed, 95 insertions(+), 24 deletions(-) diff --git a/docs/api/concepts.rst b/docs/api/concepts.rst index ae959237..5eca2349 100644 --- a/docs/api/concepts.rst +++ b/docs/api/concepts.rst @@ -1,29 +1,98 @@ .. _concepts: -********************************************** -The backend, controller, and provider concepts -********************************************** +************************* +Architecture and concepts +************************* -Backend: - The backend is mostly for convenience. It is a container that holds - references to all the controllers. -Controllers: - Each controller has responsibility for a given part of the backend - functionality. Most, but not all, controllers delegates some work to one or - more providers. The controllers are responsible for choosing the right - provider for any given task based upon i.e. the track's URI. See - :ref:`core-api` for more details. -Providers: - Anything specific to i.e. Spotify integration or local storage is contained - in the providers. To integrate with new music sources, you just add new - providers. See :ref:`backend-api` for more details. +The overall architecture of Mopidy is organized around multiple frontends and +backends. The frontends use the core API. The core actor makes multiple backends +work as one. The backends connect to various music sources. Both the core actor +and the backends use the audio actor to play audio and control audio volume. -.. digraph:: backend_relations +.. digraph:: overall_architecture - Backend -> "Current\nplaylist\ncontroller" - Backend -> "Library\ncontroller" - "Library\ncontroller" -> "Library\nproviders" - Backend -> "Playback\ncontroller" - "Playback\ncontroller" -> "Playback\nproviders" - Backend -> "Stored\nplaylists\ncontroller" - "Stored\nplaylists\ncontroller" -> "Stored\nplaylist\nproviders" + "Multiple frontends" -> Core + Core -> "Multiple backends" + Core -> Audio + "Multiple backends" -> Audio + + +Frontends +========= + +Frontends expose Mopidy to the external world. They can implement servers for +protocols like MPD and MPRIS, and they can be used to update other services +when something happens in Mopidy, like the Last.fm scrobbler frontend does. See +:ref:`frontend-api` for more details. + +.. digraph:: frontend_architecture + + "MPD\nfrontend" -> Core + "MPRIS\nfrontend" -> Core + "Last.fm\nfrontend" -> Core + + +Core +==== + +The core is organized as a set of controllers with responsiblity for separate +sets of functionality. + +The core is the single actor that the frontends send their requests to. For +every request from a frontend it calls out to one or more backends which does +the real work, and when the backends respond, the core actor is responsible for +combining the responses into a single response to the requesting frontend. + +The core actor also keeps track of the current playlist, since it doesn't +belong to a specific backend. + +See :ref:`core-api` for more details. + +.. digraph:: core_architecture + + Core -> "Current\nplaylist\ncontroller" + Core -> "Library\ncontroller" + Core -> "Playback\ncontroller" + Core -> "Stored\nplaylists\ncontroller" + + "Library\ncontroller" -> "Local backend" + "Library\ncontroller" -> "Spotify backend" + + "Playback\ncontroller" -> "Local backend" + "Playback\ncontroller" -> "Spotify backend" + "Playback\ncontroller" -> Audio + + "Stored\nplaylists\ncontroller" -> "Local backend" + "Stored\nplaylists\ncontroller" -> "Spotify backend" + +Backends +======== + +The backends are organized as a set of providers with responsiblity forseparate +sets of functionality, similar to the core actor. + +Anything specific to i.e. Spotify integration or local storage is contained in +the backends. To integrate with new music sources, you just add a new backend. +See :ref:`backend-api` for more details. + +.. digraph:: backend_architecture + + "Local backend" -> "Local\nlibrary\nprovider" -> "Local disk" + "Local backend" -> "Local\nplayback\nprovider" -> "Local disk" + "Local backend" -> "Local\nstored\nplaylists\nprovider" -> "Local disk" + "Local\nplayback\nprovider" -> Audio + + "Spotify backend" -> "Spotify\nlibrary\nprovider" -> "Spotify service" + "Spotify backend" -> "Spotify\nplayback\nprovider" -> "Spotify service" + "Spotify backend" -> "Spotify\nstored\nplaylists\nprovider" -> "Spotify service" + "Spotify\nplayback\nprovider" -> Audio + + +Audio +===== + +The audio actor is a thin wrapper around the parts of the GStreamer library we +use. In addition to playback, it's responsible for volume control through both +GStreamer's own volume mixers, and mixers we've created ourselves. If you +implement an advanced backend, you may need to implement your own playback +provider using the :ref:`audio-api`. diff --git a/docs/api/frontends.rst b/docs/api/frontends.rst index fc54a8a2..2237b4e7 100644 --- a/docs/api/frontends.rst +++ b/docs/api/frontends.rst @@ -1,3 +1,5 @@ +.. _frontend-api: + ************ Frontend API ************ From 6427f7e6bcdb89d53e62e5eefb8a4cab85ae7a06 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 08:30:28 +0100 Subject: [PATCH 12/21] Split up two-level list comprehension --- mopidy/core/library.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 37c8c522..e0df8928 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -1,3 +1,4 @@ +import itertools import urlparse import pykka @@ -94,5 +95,6 @@ class LibraryController(object): """ futures = [b.library.search(**query) for b in self.backends] results = pykka.get_all(futures) - return Playlist(tracks=[ - track for playlist in results for track in playlist.tracks]) + track_lists = [playlist.tracks for playlist in results] + tracks = list(itertools.chain(*track_lists)) + return Playlist(tracks=tracks) From ea912620f3c3a1251b3c7346994d3da6a0461a8d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 09:20:43 +0100 Subject: [PATCH 13/21] Formatting --- docs/api/concepts.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/api/concepts.rst b/docs/api/concepts.rst index 5eca2349..c3696179 100644 --- a/docs/api/concepts.rst +++ b/docs/api/concepts.rst @@ -65,6 +65,7 @@ See :ref:`core-api` for more details. "Stored\nplaylists\ncontroller" -> "Local backend" "Stored\nplaylists\ncontroller" -> "Spotify backend" + Backends ======== From 6a39516d05a40e5ec95fbc71c9e5e848ead57380 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 09:22:41 +0100 Subject: [PATCH 14/21] Fix typo --- docs/api/concepts.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/concepts.rst b/docs/api/concepts.rst index c3696179..203418de 100644 --- a/docs/api/concepts.rst +++ b/docs/api/concepts.rst @@ -69,8 +69,8 @@ See :ref:`core-api` for more details. Backends ======== -The backends are organized as a set of providers with responsiblity forseparate -sets of functionality, similar to the core actor. +The backends are organized as a set of providers with responsiblity for +separate sets of functionality, similar to the core actor. Anything specific to i.e. Spotify integration or local storage is contained in the backends. To integrate with new music sources, you just add a new backend. From c17f07e14bceb73496ed8f2e44926f22f05f4f89 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 09:25:48 +0100 Subject: [PATCH 15/21] Code review improvements --- docs/changes.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 5a17c810..8df19842 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -31,13 +31,13 @@ long time been our most requested feature. Finally, it's here! Internally, Mopidy have seen a lot of changes to pave the way for multiple backends: -- A new layer and actor, "core", have been added to our stack, inbetween the - frontends and the backends. The responsibility of this layer and actor is to - take requests from the frontends, pass them on to one or more backends, and - combining the response from the backends into a single response to the +- A new layer and actor, "core", has been added to our stack, inbetween the + frontends and the backends. The responsibility of the core layer and actor is + to take requests from the frontends, pass them on to one or more backends, + and combining the response from the backends into a single response to the requesting frontend. - The frontends no longer know anything about the backends. They just use the + Frontends no longer know anything about the backends. They just use the :ref:`core-api`. - The base playback provider have gotten sane default behavior instead of the From b352a6ed4f9aa234dac4cfb4ba123c2c6ac7e66d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 09:38:32 +0100 Subject: [PATCH 16/21] Code review improvements --- docs/changes.rst | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 8df19842..9129584c 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -40,19 +40,19 @@ backends: Frontends no longer know anything about the backends. They just use the :ref:`core-api`. -- The base playback provider have gotten sane default behavior instead of the - old empty functions. By default, the playback provider now lets GStreamer - keep track of the current track's time position. The local backend simply - uses the base playback provider without any changes. The same applies to any - future backend that just needs GStreamer to play an URI for it. +- The base playback provider has been updated with sane default behavior + instead of empty functions. By default, the playback provider now lets + GStreamer keep track of the current track's time position. The local backend + simply uses the base playback provider without any changes. The same applies + to any future backend that just needs GStreamer to play an URI for it. - The dependency graph between the core controllers and the backend providers - have been straightened out, so that we don't have any circular dependencies - or similar. The frontend, core, backend, and audio layers are now strictly - separate. The frontend layer calls on the core layer, and the core layer - calls on the backend layer. Both the core layer and the backends are allowed - to call on the audio layer. Any data flow in the opposite direction is done - by broadcasting of events to listeners, through e.g. + have been straightened out, so that we don't have any circular dependencies. + The frontend, core, backend, and audio layers are now strictly separate. The + frontend layer calls on the core layer, and the core layer calls on the + backend layer. Both the core layer and the backends are allowed to call on + the audio layer. Any data flow in the opposite direction is done by + broadcasting of events to listeners, through e.g. :class:`mopidy.core.CoreListener` and :class:`mopidy.audio.AudioListener`. - All dependencies are now explicitly passed to the constructors of the From 2e6e53b14dd96e060c5187474de64f91d36dacec Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 09:48:53 +0100 Subject: [PATCH 17/21] Remove code duplication --- mopidy/core/actor.py | 29 ++++++++++++++++++++++++----- mopidy/core/library.py | 10 +--------- mopidy/core/playback.py | 11 +---------- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 05c085fd..0af8c3b2 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -30,25 +30,44 @@ class Core(pykka.ThreadingActor, AudioListener): def __init__(self, audio=None, backends=None): super(Core, self).__init__() - self._backends = backends + self.backends = Backends(backends) self.current_playlist = CurrentPlaylistController(core=self) - self.library = LibraryController(backends=backends, core=self) + self.library = LibraryController(backends=self.backends, core=self) self.playback = PlaybackController( - audio=audio, backends=backends, core=self) + audio=audio, backends=self.backends, core=self) self.stored_playlists = StoredPlaylistsController( - backends=backends, core=self) + backends=self.backends, core=self) @property def uri_schemes(self): """List of URI schemes we can handle""" - futures = [backend.uri_schemes for backend in self._backends] + futures = [b.uri_schemes for b in self.backends] results = pykka.get_all(futures) uri_schemes = itertools.chain(*results) return sorted(uri_schemes) def reached_end_of_stream(self): self.playback.on_end_of_track() + + +class Backends(object): + def __init__(self, backends): + self._backends = backends + + uri_schemes_by_backend = { + backend: backend.uri_schemes.get() + for backend in backends} + self.by_uri_scheme = { + uri_scheme: backend + for backend, uri_schemes in uri_schemes_by_backend.items() + for uri_scheme in uri_schemes} + + def __len__(self): + return len(self._backends) + + def __getitem__(self, key): + return self._backends[key] diff --git a/mopidy/core/library.py b/mopidy/core/library.py index e0df8928..bf14f5d3 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -11,19 +11,11 @@ class LibraryController(object): def __init__(self, backends, core): self.backends = backends - uri_schemes_by_backend = { - backend: backend.uri_schemes.get() - for backend in backends} - self.backends_by_uri_scheme = { - uri_scheme: backend - for backend, uri_schemes in uri_schemes_by_backend.items() - for uri_scheme in uri_schemes} - self.core = core def _get_backend(self, uri): uri_scheme = urlparse.urlparse(uri).scheme - return self.backends_by_uri_scheme.get(uri_scheme) + return self.backends.by_uri_scheme.get(uri_scheme) def find_exact(self, **query): """ diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 721bc2a8..74f4bebd 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -77,16 +77,7 @@ class PlaybackController(object): def __init__(self, audio, backends, core): self.audio = audio - self.backends = backends - uri_schemes_by_backend = { - backend: backend.uri_schemes.get() - for backend in backends} - self.backends_by_uri_scheme = { - uri_scheme: backend - for backend, uri_schemes in uri_schemes_by_backend.items() - for uri_scheme in uri_schemes} - self.core = core self._state = PlaybackState.STOPPED @@ -99,7 +90,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.by_uri_scheme[uri_scheme] def _get_cpid(self, cp_track): if cp_track is None: From 44186c1a03d8504aad8a68f1261538801f689c03 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 10:14:43 +0100 Subject: [PATCH 18/21] Make sure backends is a fully functional list --- mopidy/core/actor.py | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 0af8c3b2..e2eeb746 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -54,20 +54,12 @@ class Core(pykka.ThreadingActor, AudioListener): self.playback.on_end_of_track() -class Backends(object): +class Backends(list): def __init__(self, backends): - self._backends = backends + super(Backends, self).__init__(backends) - uri_schemes_by_backend = { - backend: backend.uri_schemes.get() - for backend in backends} - self.by_uri_scheme = { - uri_scheme: backend - for backend, uri_schemes in uri_schemes_by_backend.items() - for uri_scheme in uri_schemes} - - def __len__(self): - return len(self._backends) - - def __getitem__(self, key): - return self._backends[key] + self.by_uri_scheme = {} + for backend in backends: + uri_schemes = backend.uri_schemes.get() + for uri_scheme in uri_schemes: + self.by_uri_scheme[uri_scheme] = backend From 7ee43dd20893283d4cdc9fe4effeab86c227a492 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 10:24:07 +0100 Subject: [PATCH 19/21] Be explicit about returning None for unknown URI schemes --- mopidy/core/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index bf14f5d3..f7514fd8 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -15,7 +15,7 @@ class LibraryController(object): def _get_backend(self, uri): uri_scheme = urlparse.urlparse(uri).scheme - return self.backends.by_uri_scheme.get(uri_scheme) + return self.backends.by_uri_scheme.get(uri_scheme, None) def find_exact(self, **query): """ From 4a79b559d547f0602d632c8bd4b2f1b2344d485b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 10:31:35 +0100 Subject: [PATCH 20/21] Fail if two backends claims to handle the same URI schema --- mopidy/core/actor.py | 3 +++ tests/core/actor_test.py | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index e2eeb746..7fdaeb71 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -62,4 +62,7 @@ class Backends(list): for backend in backends: uri_schemes = backend.uri_schemes.get() for uri_scheme in uri_schemes: + assert uri_scheme not in self.by_uri_scheme, ( + 'URI scheme %s is already handled by %s' + % (uri_scheme, backend.__class__.__name__)) self.by_uri_scheme[uri_scheme] = backend diff --git a/tests/core/actor_test.py b/tests/core/actor_test.py index 95639cf8..9feddbd0 100644 --- a/tests/core/actor_test.py +++ b/tests/core/actor_test.py @@ -24,3 +24,9 @@ class CoreActorTest(unittest.TestCase): self.assertIn('dummy1', result) self.assertIn('dummy2', result) + + def test_backends_with_colliding_uri_schemes_fails(self): + self.backend2.uri_schemes.get.return_value = ['dummy1', 'dummy2'] + self.assertRaisesRegexp( + AssertionError, 'URI scheme dummy1 is already handled by Mock', + Core, audio=None, backends=[self.backend1, self.backend2]) From 1014c6e373313c642c4d72ad884a1ebbc69e8de1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Oct 2012 10:50:18 +0100 Subject: [PATCH 21/21] Include both involved backends in the error message --- mopidy/core/actor.py | 7 +++++-- tests/core/actor_test.py | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 7fdaeb71..482868ad 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -63,6 +63,9 @@ class Backends(list): uri_schemes = backend.uri_schemes.get() for uri_scheme in uri_schemes: assert uri_scheme not in self.by_uri_scheme, ( - 'URI scheme %s is already handled by %s' - % (uri_scheme, backend.__class__.__name__)) + 'Cannot add URI scheme %s for %s, ' + 'it is already handled by %s' + ) % ( + uri_scheme, backend.__class__.__name__, + self.by_uri_scheme[uri_scheme].__class__.__name__) self.by_uri_scheme[uri_scheme] = backend diff --git a/tests/core/actor_test.py b/tests/core/actor_test.py index 9feddbd0..8212c1da 100644 --- a/tests/core/actor_test.py +++ b/tests/core/actor_test.py @@ -26,7 +26,10 @@ class CoreActorTest(unittest.TestCase): self.assertIn('dummy2', result) def test_backends_with_colliding_uri_schemes_fails(self): + self.backend1.__class__.__name__ = 'B1' + self.backend2.__class__.__name__ = 'B2' self.backend2.uri_schemes.get.return_value = ['dummy1', 'dummy2'] self.assertRaisesRegexp( - AssertionError, 'URI scheme dummy1 is already handled by Mock', + AssertionError, + 'Cannot add URI scheme dummy1 for B2, it is already handled by B1', Core, audio=None, backends=[self.backend1, self.backend2])