diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index cd4ba180..3cba20db 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import collections import itertools import pykka @@ -79,34 +80,29 @@ 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_playlists = [ - b for b in backends if b.has_playlists().get()] + self.with_library = collections.OrderedDict() + self.with_playback = collections.OrderedDict() + self.with_playlists = collections.OrderedDict() - self.by_uri_scheme = {} for backend in backends: - 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' - ) % ( - uri_scheme, backend.__class__.__name__, - self.by_uri_scheme[uri_scheme].__class__.__name__) - self.by_uri_scheme[uri_scheme] = backend + has_library = backend.has_library().get() + has_playback = backend.has_playback().get() + has_playlists = backend.has_playlists().get() - self.with_library_by_uri_scheme = {} - self.with_playback_by_uri_scheme = {} - self.with_playlists_by_uri_scheme = {} + for scheme in backend.uri_schemes.get(): + self.add(self.with_library, has_library, scheme, backend) + self.add(self.with_playback, has_playback, scheme, backend) + self.add(self.with_playlists, has_playlists, scheme, backend) - 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_playlists().get(): - self.with_playlists_by_uri_scheme[uri_scheme] = backend + def add(self, registry, supported, uri_scheme, backend): + if not supported: + return + + if uri_scheme not in registry: + registry[uri_scheme] = backend + return + + get_name = lambda actor: actor.actor_ref.actor_class.__name__ + raise AssertionError( + 'Cannot add URI scheme %s for %s, it is already handled by %s' % + (uri_scheme, get_name(backend), get_name(registry[uri_scheme]))) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index cdc3f53a..2e73e0db 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from collections import defaultdict +import collections import urlparse import pykka @@ -15,18 +15,18 @@ class LibraryController(object): def _get_backend(self, uri): uri_scheme = urlparse.urlparse(uri).scheme - return self.backends.with_library_by_uri_scheme.get(uri_scheme, None) + return self.backends.with_library.get(uri_scheme, None) def _get_backends_to_uris(self, uris): if uris: - backends_to_uris = defaultdict(list) + backends_to_uris = collections.defaultdict(list) for uri in uris: backend = self._get_backend(uri) if backend is not None: backends_to_uris[backend].append(uri) else: backends_to_uris = dict([ - (b, None) for b in self.backends.with_library]) + (b, None) for b in self.backends.with_library.values()]) return backends_to_uris def find_exact(self, query=None, uris=None, **kwargs): @@ -103,8 +103,8 @@ class LibraryController(object): if backend: backend.library.refresh(uri).get() else: - futures = [ - b.library.refresh(uri) for b in self.backends.with_library] + futures = [b.library.refresh(uri) + for b in self.backends.with_library.values()] pykka.get_all(futures) def search(self, query=None, uris=None, **kwargs): diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index d127fbbe..3c0e43fa 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -28,7 +28,7 @@ class PlaybackController(object): return None uri = self.current_tl_track.track.uri uri_scheme = urlparse.urlparse(uri).scheme - return self.backends.with_playback_by_uri_scheme.get(uri_scheme, None) + return self.backends.with_playback.get(uri_scheme, None) ### Properties diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index f0187d44..d5c03bb3 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -16,8 +16,8 @@ class PlaylistsController(object): self.core = core def get_playlists(self, include_tracks=True): - futures = [ - b.playlists.playlists for b in self.backends.with_playlists] + futures = [b.playlists.playlists + for b in self.backends.with_playlists.values()] results = pykka.get_all(futures) playlists = list(itertools.chain(*results)) if not include_tracks: @@ -49,10 +49,11 @@ class PlaylistsController(object): :type uri_scheme: string :rtype: :class:`mopidy.models.Playlist` """ - if uri_scheme in self.backends.with_playlists_by_uri_scheme: - backend = self.backends.by_uri_scheme[uri_scheme] + if uri_scheme in self.backends.with_playlists: + backend = self.backends.with_playlists[uri_scheme] else: - backend = self.backends.with_playlists[0] + # TODO: this fallback looks suspicious + backend = self.backends.with_playlists.values()[0] playlist = backend.playlists.create(name).get() listener.CoreListener.send('playlist_changed', playlist=playlist) return playlist @@ -68,8 +69,7 @@ class PlaylistsController(object): :type uri: string """ uri_scheme = urlparse.urlparse(uri).scheme - backend = self.backends.with_playlists_by_uri_scheme.get( - uri_scheme, None) + backend = self.backends.with_playlists.get(uri_scheme, None) if backend: backend.playlists.delete(uri).get() @@ -111,8 +111,7 @@ class PlaylistsController(object): :rtype: :class:`mopidy.models.Playlist` or :class:`None` """ uri_scheme = urlparse.urlparse(uri).scheme - backend = self.backends.with_playlists_by_uri_scheme.get( - uri_scheme, None) + backend = self.backends.with_playlists.get(uri_scheme, None) if backend: return backend.playlists.lookup(uri).get() else: @@ -131,13 +130,12 @@ class PlaylistsController(object): :type uri_scheme: string """ if uri_scheme is None: - futures = [ - b.playlists.refresh() for b in self.backends.with_playlists] + futures = [b.playlists.refresh() + for b in self.backends.with_playlists.values()] pykka.get_all(futures) listener.CoreListener.send('playlists_loaded') else: - backend = self.backends.with_playlists_by_uri_scheme.get( - uri_scheme, None) + backend = self.backends.with_playlists.get(uri_scheme, None) if backend: backend.playlists.refresh().get() listener.CoreListener.send('playlists_loaded') @@ -167,8 +165,7 @@ class PlaylistsController(object): if playlist.uri is None: return uri_scheme = urlparse.urlparse(playlist.uri).scheme - backend = self.backends.with_playlists_by_uri_scheme.get( - uri_scheme, None) + backend = self.backends.with_playlists.get(uri_scheme, None) if backend: playlist = backend.playlists.save(playlist).get() listener.CoreListener.send('playlist_changed', playlist=playlist) diff --git a/tests/core/actor_test.py b/tests/core/actor_test.py index c4952af3..ce50d5ed 100644 --- a/tests/core/actor_test.py +++ b/tests/core/actor_test.py @@ -28,10 +28,26 @@ class CoreActorTest(unittest.TestCase): self.assertIn('dummy2', result) def test_backends_with_colliding_uri_schemes_fails(self): - self.backend1.__class__.__name__ = b'B1' - self.backend2.__class__.__name__ = b'B2' + self.backend1.actor_ref.actor_class.__name__ = b'B1' + self.backend2.actor_ref.actor_class.__name__ = b'B2' self.backend2.uri_schemes.get.return_value = ['dummy1', 'dummy2'] self.assertRaisesRegexp( AssertionError, 'Cannot add URI scheme dummy1 for B2, it is already handled by B1', Core, audio=None, backends=[self.backend1, self.backend2]) + + def test_backends_with_colliding_uri_schemes_passes(self): + # Checks that backends with overlapping schemes, but distinct sub parts + # provided can co-exist. + self.backend1.has_library().get.return_value = False + self.backend1.has_playlists().get.return_value = False + + self.backend2.uri_schemes().get.return_value = ['dummy1'] + self.backend2.has_playback().get.return_value = False + self.backend2.has_playlists().get.return_value = False + + core = Core(audio=None, backends=[self.backend1, self.backend2]) + self.assertEqual(core.backends.with_playback, + {'dummy1': self.backend1}) + self.assertEqual(core.backends.with_library, + {'dummy2': self.backend2})