From 55b1eb73835d67cf08a7401344440df55fcac0a5 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 22 Mar 2015 22:16:03 +0100 Subject: [PATCH 01/11] backend: Add playlists.as_list() and playlists.get_items(uri) --- mopidy/backend.py | 30 ++++++++++++++++++++++++++++++ tests/backend/test_backend.py | 19 +++++++++++++++---- tests/dummy_backend.py | 11 +++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/mopidy/backend.py b/mopidy/backend.py index 0dc656ad..c1554c7f 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -318,6 +318,36 @@ class PlaylistsProvider(object): def playlists(self, playlists): raise NotImplementedError + def as_list(self): + """ + Get a list of the currently available playlists. + + Returns a list of :class:`~mopidy.models.Ref` objects referring to the + playlists. In other words, no information about the playlists' content + is given. + + :rtype: list of :class:`mopidy.models.Ref` + + .. versionadded:: 1.0 + """ + raise NotImplementedError + + def get_items(self, uri): + """ + Get the items in a playlist specified by ``uri``. + + Returns a list of :class:`~mopidy.models.Ref` objects referring to the + playlist's items. + + If a playlist with the given ``uri`` doesn't exist, it returns + :class:`None`. + + :rtype: list of :class:`mopidy.models.Ref`, or :class:`None` + + .. versionadded:: 1.0 + """ + raise NotImplementedError + def create(self, name): """ Create a new empty playlist with the given name. diff --git a/tests/backend/test_backend.py b/tests/backend/test_backend.py index c72633fb..23cfedd5 100644 --- a/tests/backend/test_backend.py +++ b/tests/backend/test_backend.py @@ -8,6 +8,7 @@ from tests import dummy_backend class LibraryTest(unittest.TestCase): + def test_default_get_images_impl_falls_back_to_album_image(self): album = models.Album(images=['imageuri']) track = models.Track(uri='trackuri', album=album) @@ -31,10 +32,20 @@ class LibraryTest(unittest.TestCase): class PlaylistsTest(unittest.TestCase): - def test_playlists_default_impl(self): - playlists = backend.PlaylistsProvider(backend=None) - self.assertEqual(playlists.playlists, []) + def setUp(self): # noqa: N802 + self.provider = backend.PlaylistsProvider(backend=None) + + def test_playlists_default_impl(self): + self.assertEqual(self.provider.playlists, []) with self.assertRaises(NotImplementedError): - playlists.playlists = [] + self.provider.playlists = [] + + def test_as_list_default_impl(self): + with self.assertRaises(NotImplementedError): + self.provider.as_list() + + def test_get_items_default_impl(self): + with self.assertRaises(NotImplementedError): + self.provider.get_items('some uri') diff --git a/tests/dummy_backend.py b/tests/dummy_backend.py index d4441673..9f4a0986 100644 --- a/tests/dummy_backend.py +++ b/tests/dummy_backend.py @@ -100,6 +100,17 @@ class DummyPlaylistsProvider(backend.PlaylistsProvider): super(DummyPlaylistsProvider, self).__init__(backend) self._playlists = [] + def as_list(self): + return [ + Ref.playlist(uri=pl.uri, name=pl.name) for pl in self._playlists] + + def get_items(self, uri): + playlist = self._playlists.get(uri) + if playlist is None: + return + return [ + Ref.track(uri=t.uri, name=t.name) for t in playlist.tracks] + @property def playlists(self): return copy.copy(self._playlists) From 4f3a0839b33221124657e8e82d81190d900fa0fc Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 22 Mar 2015 22:51:55 +0100 Subject: [PATCH 02/11] core: Add playlists.as_list() and playlists.get_items(uri) --- docs/changelog.rst | 11 +++++++ mopidy/core/playlists.py | 50 +++++++++++++++++++++++++++---- tests/core/test_playlists.py | 57 +++++++++++++++++++++++++++++------- 3 files changed, 102 insertions(+), 16 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5155fc79..0fdcaa16 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -77,6 +77,17 @@ v1.0.0 (UNRELEASED) :meth:`mopidy.core.LibraryController.find_exact` to normalize and warn about bad queries from clients. (Fixes: :issue:`1067`, PR: :issue:`1073`) +- Add :meth:`mopidy.core.PlaylistsController.as_list`. (Fixes: :issue:`1057`, + PR: :issue:`1075`) + +- Add :meth:`mopidy.core.PlaylistsController.get_items`. (Fixes: :issue:`1057`, + PR: :issue:`1075`) + +- **Deprecated:** :meth:`mopidy.core.PlaylistsController.get_playlists`. Use + :meth:`~mopidy.core.PlaylistsController.as_list` and + :meth:`~mopidy.core.PlaylistsController.get_items` instead. (Fixes: + :issue:`1057`, PR: :issue:`1075`) + **Backend API** - Remove default implementation of diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 15d35aa9..146b8058 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -16,12 +16,52 @@ class PlaylistsController(object): self.backends = backends self.core = core - """ - Get the available playlists. + def as_list(self): + """ + Get a list of the currently available playlists. + + Returns a list of :class:`~mopidy.models.Ref` objects referring to the + playlists. In other words, no information about the playlists' content + is given. + + :rtype: list of :class:`mopidy.models.Ref` + + .. versionadded:: 1.0 + """ + futures = [ + b.playlists.as_list() + for b in self.backends.with_playlists.values()] + results = pykka.get_all(futures) + return list(itertools.chain(*results)) + + def get_items(self, uri): + """ + Get the items in a playlist specified by ``uri``. + + Returns a list of :class:`~mopidy.models.Ref` objects referring to the + playlist's items. + + If a playlist with the given ``uri`` doesn't exist, it returns + :class:`None`. + + :rtype: list of :class:`mopidy.models.Ref`, or :class:`None` + + .. versionadded:: 1.0 + """ + uri_scheme = urlparse.urlparse(uri).scheme + backend = self.backends.with_playlists.get(uri_scheme, None) + if backend: + return backend.playlists.get_items(uri).get() - Returns a list of :class:`mopidy.models.Playlist`. - """ def get_playlists(self, include_tracks=True): + """ + Get the available playlists. + + :rtype: list of :class:`mopidy.models.Playlist` + + .. deprecated:: 1.0 + Use :meth:`as_list` and :meth:`get_items` instead. + """ futures = [b.playlists.playlists for b in self.backends.with_playlists.values()] results = pykka.get_all(futures) @@ -33,7 +73,7 @@ class PlaylistsController(object): playlists = deprecated_property(get_playlists) """ .. deprecated:: 1.0 - Use :meth:`get_playlists` instead. + Use :meth:`as_list` and :meth:`get_items` instead. """ def create(self, name, uri_scheme=None): diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index 55a75767..232631d7 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -5,19 +5,37 @@ import unittest import mock from mopidy import backend, core -from mopidy.models import Playlist, Track +from mopidy.models import Playlist, Ref, Track class PlaylistsTest(unittest.TestCase): def setUp(self): # noqa: N802 + self.plr1a = Ref.playlist(name='A', uri='dummy1:pl:a') + self.plr1b = Ref.playlist(name='B', uri='dummy1:pl:b') + self.plr2a = Ref.playlist(name='A', uri='dummy2:pl:a') + self.plr2b = Ref.playlist(name='B', uri='dummy2:pl:b') + + self.pl1a = Playlist(name='A', tracks=[Track(uri='dummy1:t:a')]) + self.pl1b = Playlist(name='B', tracks=[Track(uri='dummy1:t:b')]) + self.pl2a = Playlist(name='A', tracks=[Track(uri='dummy2:t:a')]) + self.pl2b = Playlist(name='B', tracks=[Track(uri='dummy2:t:b')]) + + self.sp1 = mock.Mock(spec=backend.PlaylistsProvider) + self.sp1.as_list.return_value.get.return_value = [ + self.plr1a, self.plr1b] + self.sp1.playlists.get.return_value = [self.pl1a, self.pl1b] + + self.sp2 = mock.Mock(spec=backend.PlaylistsProvider) + self.sp2.as_list.return_value.get.return_value = [ + self.plr2a, self.plr2b] + self.sp2.playlists.get.return_value = [self.pl2a, self.pl2b] + self.backend1 = mock.Mock() self.backend1.uri_schemes.get.return_value = ['dummy1'] - self.sp1 = mock.Mock(spec=backend.PlaylistsProvider) self.backend1.playlists = self.sp1 self.backend2 = mock.Mock() self.backend2.uri_schemes.get.return_value = ['dummy2'] - self.sp2 = mock.Mock(spec=backend.PlaylistsProvider) self.backend2.playlists = self.sp2 # A backend without the optional playlists provider @@ -26,17 +44,34 @@ class PlaylistsTest(unittest.TestCase): self.backend3.has_playlists().get.return_value = False self.backend3.playlists = None - self.pl1a = Playlist(name='A', tracks=[Track(uri='dummy1:a')]) - self.pl1b = Playlist(name='B', tracks=[Track(uri='dummy1:b')]) - self.sp1.playlists.get.return_value = [self.pl1a, self.pl1b] - - self.pl2a = Playlist(name='A', tracks=[Track(uri='dummy2:a')]) - self.pl2b = Playlist(name='B', tracks=[Track(uri='dummy2:b')]) - self.sp2.playlists.get.return_value = [self.pl2a, self.pl2b] - self.core = core.Core(mixer=None, backends=[ self.backend3, self.backend1, self.backend2]) + def test_as_list_combines_result_from_backends(self): + result = self.core.playlists.as_list() + + self.assertIn(self.plr1a, result) + self.assertIn(self.plr1b, result) + self.assertIn(self.plr2a, result) + self.assertIn(self.plr2b, result) + + def test_get_items_selects_the_matching_backend(self): + ref = Ref.track() + self.sp2.get_items.return_value.get.return_value = [ref] + + result = self.core.playlists.get_items('dummy2:pl:a') + + self.assertEqual([ref], result) + self.assertFalse(self.sp1.get_items.called) + self.sp2.get_items.assert_called_once_with('dummy2:pl:a') + + def test_get_items_with_unknown_uri_scheme_does_nothing(self): + result = self.core.playlists.get_items('unknown:a') + + self.assertIsNone(result) + self.assertFalse(self.sp1.delete.called) + self.assertFalse(self.sp2.delete.called) + def test_get_playlists_combines_result_from_backends(self): result = self.core.playlists.playlists From bd2e4f7af0cabeaa0d271aaca1cd6a8b0e7077fa Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 22 Mar 2015 23:43:57 +0100 Subject: [PATCH 03/11] core: Reimplement get_playlists() using new backend API --- mopidy/core/playlists.py | 16 +++++++++------- tests/core/test_playlists.py | 6 +++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 146b8058..715e5870 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -6,6 +6,7 @@ import urlparse import pykka from mopidy.core import listener +from mopidy.models import Playlist from mopidy.utils.deprecation import deprecated_property @@ -62,13 +63,14 @@ class PlaylistsController(object): .. deprecated:: 1.0 Use :meth:`as_list` and :meth:`get_items` instead. """ - 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: - playlists = [p.copy(tracks=[]) for p in playlists] - return playlists + playlist_refs = self.as_list() + + if include_tracks: + playlists = [self.lookup(r.uri) for r in playlist_refs] + return [pl for pl in playlists if pl is not None] + else: + return [ + Playlist(uri=r.uri, name=r.name) for r in playlist_refs] playlists = deprecated_property(get_playlists) """ diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index 232631d7..fecbbdcb 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -23,12 +23,12 @@ class PlaylistsTest(unittest.TestCase): self.sp1 = mock.Mock(spec=backend.PlaylistsProvider) self.sp1.as_list.return_value.get.return_value = [ self.plr1a, self.plr1b] - self.sp1.playlists.get.return_value = [self.pl1a, self.pl1b] + self.sp1.lookup.return_value.get.side_effect = [self.pl1a, self.pl1b] self.sp2 = mock.Mock(spec=backend.PlaylistsProvider) self.sp2.as_list.return_value.get.return_value = [ self.plr2a, self.plr2b] - self.sp2.playlists.get.return_value = [self.pl2a, self.pl2b] + self.sp2.lookup.return_value.get.side_effect = [self.pl2a, self.pl2b] self.backend1 = mock.Mock() self.backend1.uri_schemes.get.return_value = ['dummy1'] @@ -73,7 +73,7 @@ class PlaylistsTest(unittest.TestCase): self.assertFalse(self.sp2.delete.called) def test_get_playlists_combines_result_from_backends(self): - result = self.core.playlists.playlists + result = self.core.playlists.get_playlists() self.assertIn(self.pl1a, result) self.assertIn(self.pl1b, result) From 5693b454eefbcb7cf44c71de8ccae1072756c7c4 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 00:15:34 +0100 Subject: [PATCH 04/11] m3u: Use lookup() instead of playlists prop in tests --- tests/m3u/test_playlists.py | 44 ++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index fd77348c..443cfb9e 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -120,12 +120,10 @@ class M3UPlaylistsProviderTest(unittest.TestCase): backend = self.backend_class(config=self.config, audio=self.audio) self.assert_(backend.playlists.playlists) - self.assertEqual( - playlist.uri, backend.playlists.playlists[0].uri) - self.assertEqual( - playlist.name, backend.playlists.playlists[0].name) - self.assertEqual( - track.uri, backend.playlists.playlists[0].tracks[0].uri) + result = backend.playlists.lookup(playlist.uri) + self.assertEqual(playlist.uri, result.uri) + self.assertEqual(playlist.name, result.name) + self.assertEqual(track.uri, result.tracks[0].uri) @unittest.SkipTest def test_santitising_of_playlist_filenames(self): @@ -156,15 +154,15 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_delete_playlist_removes_it_from_the_collection(self): playlist = self.core.playlists.create('test') - self.assertIn(playlist, self.core.playlists.playlists) + self.assertEqual(playlist, self.core.playlists.lookup(playlist.uri)) self.core.playlists.delete(playlist.uri) - self.assertNotIn(playlist, self.core.playlists.playlists) + self.assertIsNone(self.core.playlists.lookup(playlist.uri)) def test_delete_playlist_without_file(self): playlist = self.core.playlists.create('test') - self.assertIn(playlist, self.core.playlists.playlists) + self.assertEqual(playlist, self.core.playlists.lookup(playlist.uri)) path = playlist_uri_to_path(playlist.uri, self.playlists_dir) self.assertTrue(os.path.exists(path)) @@ -173,11 +171,11 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assertFalse(os.path.exists(path)) self.core.playlists.delete(playlist.uri) - self.assertNotIn(playlist, self.core.playlists.playlists) + self.assertIsNone(self.core.playlists.lookup(playlist.uri)) def test_filter_without_criteria(self): self.assertEqual( - self.core.playlists.playlists, self.core.playlists.filter()) + self.core.playlists.get_playlists(), self.core.playlists.filter()) def test_filter_with_wrong_criteria(self): self.assertEqual([], self.core.playlists.filter(name='foo')) @@ -188,13 +186,14 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assertEqual([playlist], playlists) def test_filter_by_name_returns_single_match(self): - playlist = Playlist(name='b') - self.backend.playlists.playlists = [Playlist(name='a'), playlist] + playlist = Playlist(uri='m3u:b', name='b') + self.backend.playlists.playlists = [ + Playlist(uri='m3u:a', name='a'), playlist] self.assertEqual([playlist], self.core.playlists.filter(name='b')) def test_filter_by_name_returns_no_matches(self): self.backend.playlists.playlists = [ - Playlist(name='a'), Playlist(name='b')] + Playlist(uri='m3u:a', name='a'), Playlist(uri='m3u:b', name='b')] self.assertEqual([], self.core.playlists.filter(name='c')) def test_lookup_finds_playlist_by_uri(self): @@ -206,31 +205,32 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_refresh(self): playlist = self.core.playlists.create('test') - self.assertIn(playlist, self.core.playlists.playlists) + self.assertEqual(playlist, self.core.playlists.lookup(playlist.uri)) self.core.playlists.refresh() - self.assertIn(playlist, self.core.playlists.playlists) + self.assertEqual(playlist, self.core.playlists.lookup(playlist.uri)) def test_save_replaces_existing_playlist_with_updated_playlist(self): playlist1 = self.core.playlists.create('test1') - self.assertIn(playlist1, self.core.playlists.playlists) + self.assertEqual(playlist1, self.core.playlists.lookup(playlist1.uri)) playlist2 = playlist1.copy(name='test2') playlist2 = self.core.playlists.save(playlist2) - self.assertNotIn(playlist1, self.core.playlists.playlists) - self.assertIn(playlist2, self.core.playlists.playlists) + self.assertIsNone(self.core.playlists.lookup(playlist1.uri)) + self.assertEqual(playlist2, self.core.playlists.lookup(playlist2.uri)) def test_create_replaces_existing_playlist_with_updated_playlist(self): track = Track(uri=generate_song(1)) playlist1 = self.core.playlists.create('test') playlist1 = self.core.playlists.save(playlist1.copy(tracks=[track])) - self.assertIn(playlist1, self.core.playlists.playlists) + self.assertEqual(playlist1, self.core.playlists.lookup(playlist1.uri)) playlist2 = self.core.playlists.create('test') self.assertEqual(playlist1.uri, playlist2.uri) - self.assertNotIn(playlist1, self.core.playlists.playlists) - self.assertIn(playlist2, self.core.playlists.playlists) + self.assertNotEqual( + playlist1, self.core.playlists.lookup(playlist1.uri)) + self.assertEqual(playlist2, self.core.playlists.lookup(playlist1.uri)) def test_save_playlist_with_new_uri(self): uri = 'm3u:test.m3u' From 4bae9c874c44dedaae64f26dbe19ec165636bab8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 00:16:13 +0100 Subject: [PATCH 05/11] m3u: Add playlists.as_list() --- mopidy/m3u/playlists.py | 8 +++++++- tests/m3u/test_playlists.py | 23 +++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index a753f00a..d9eb341e 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -8,7 +8,7 @@ import sys from mopidy import backend from mopidy.m3u import translator -from mopidy.models import Playlist +from mopidy.models import Playlist, Ref logger = logging.getLogger(__name__) @@ -22,6 +22,12 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): self._playlists = {} self.refresh() + def as_list(self): + refs = [ + Ref.playlist(uri=pl.uri, name=pl.name) + for pl in self._playlists.values()] + return sorted(refs, key=operator.attrgetter('name')) + @property def playlists(self): return sorted( diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index 443cfb9e..83dec321 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -146,8 +146,8 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assert_(self.core.playlists.playlists) self.assertIn(playlist, self.core.playlists.playlists) - def test_playlists_empty_to_start_with(self): - self.assert_(not self.core.playlists.playlists) + def test_as_list_empty_to_start_with(self): + self.assertEqual(len(self.core.playlists.as_list()), 0) def test_delete_non_existant_playlist(self): self.core.playlists.delete('m3u:unknown') @@ -249,12 +249,11 @@ class M3UPlaylistsProviderTest(unittest.TestCase): backend = self.backend_class(config=self.config, audio=self.audio) - self.assert_(backend.playlists.playlists) - self.assertEqual('m3u:test.m3u', backend.playlists.playlists[0].uri) - self.assertEqual( - playlist.name, backend.playlists.playlists[0].name) - self.assertEqual( - track.uri, backend.playlists.playlists[0].tracks[0].uri) + self.assertEqual(len(backend.playlists.as_list()), 1) + result = backend.playlists.lookup('m3u:test.m3u') + self.assertEqual('m3u:test.m3u', result.uri) + self.assertEqual(playlist.name, result.name) + self.assertEqual(track.uri, result.tracks[0].uri) def test_playlist_sort_order(self): def check_order(playlists, names): @@ -264,18 +263,18 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.core.playlists.create('a') self.core.playlists.create('b') - check_order(self.core.playlists.playlists, ['a', 'b', 'c']) + check_order(self.core.playlists.as_list(), ['a', 'b', 'c']) self.core.playlists.refresh() - check_order(self.core.playlists.playlists, ['a', 'b', 'c']) + check_order(self.core.playlists.as_list(), ['a', 'b', 'c']) playlist = self.core.playlists.lookup('m3u:a.m3u') playlist = playlist.copy(name='d') playlist = self.core.playlists.save(playlist) - check_order(self.core.playlists.playlists, ['b', 'c', 'd']) + check_order(self.core.playlists.as_list(), ['b', 'c', 'd']) self.core.playlists.delete('m3u:c.m3u') - check_order(self.core.playlists.playlists, ['b', 'd']) + check_order(self.core.playlists.as_list(), ['b', 'd']) From e3f2e368c7ebca2cc05d324b3bc360075ffe2b29 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 00:28:49 +0100 Subject: [PATCH 06/11] m3u: Add playlists.get_items() --- mopidy/m3u/playlists.py | 6 ++++++ tests/m3u/test_playlists.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index d9eb341e..d5f2b1e9 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -28,6 +28,12 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): for pl in self._playlists.values()] return sorted(refs, key=operator.attrgetter('name')) + def get_items(self, uri): + playlist = self._playlists.get(uri) + if playlist is None: + return None + return [Ref.track(uri=t.uri, name=t.name) for t in playlist.tracks] + @property def playlists(self): return sorted( diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index 83dec321..be94ed2f 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -278,3 +278,20 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.core.playlists.delete('m3u:c.m3u') check_order(self.core.playlists.as_list(), ['b', 'd']) + + def test_get_items_returns_item_refs(self): + track = Track(uri='dummy:a', name='A', length=60000) + playlist = self.core.playlists.create('test') + playlist = self.core.playlists.save(playlist.copy(tracks=[track])) + + item_refs = self.core.playlists.get_items(playlist.uri) + + self.assertEqual(len(item_refs), 1) + self.assertEqual(item_refs[0].type, 'track') + self.assertEqual(item_refs[0].uri, 'dummy:a') + self.assertEqual(item_refs[0].name, 'A') + + def test_get_items_of_unknown_playlist_returns_none(self): + item_refs = self.core.playlists.get_items('dummy:unknown') + + self.assertIsNone(item_refs) From d37bd62bb1a921360370eb6bb9b2a2b475fc0b55 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 00:32:19 +0100 Subject: [PATCH 07/11] backend: Remove playlists.playlists property --- docs/changelog.rst | 12 +++++++++++- mopidy/backend.py | 18 ------------------ tests/backend/test_backend.py | 6 ------ 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0fdcaa16..5d68143d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -94,7 +94,7 @@ v1.0.0 (UNRELEASED) :attr:`mopidy.backend.PlaylistsProvider.playlists`. This is potentially backwards incompatible. (PR: :issue:`1046`) -- Changed the API for :class:`mopidy.backend.PlaybackProvider`, note that this +- Changed the API for :class:`mopidy.backend.PlaybackProvider`. Note that this change is **not** backwards compatible for certain backends. These changes are crucial to adding gapless in one of the upcoming releases. (Fixes: :issue:`1052`, PR: :issue:`1064`) @@ -113,6 +113,16 @@ v1.0.0 (UNRELEASED) - :meth:`mopidy.backend.PlaybackProvider.prepare_change` has been added. +- Changed the API for :class:`mopidy.backend.PlaylistsProvider`. Note that this + change is **not** backwards compatible. These changes are important to reduce + the Mopidy startup time. (Fixes: :issue:`1057`, PR: :issue:`1075`) + + - Add :meth:`mopidy.backend.PlaylistsProvider.as_list`. + + - Add :meth:`mopidy.backend.PlaylistsProvider.get_items`. + + - Remove :attr:`mopidy.backend.PlaylistsProvider.playlists` property. + **Commands** - Make the ``mopidy`` command print a friendly error message if the diff --git a/mopidy/backend.py b/mopidy/backend.py index c1554c7f..02a624d9 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -300,24 +300,6 @@ class PlaylistsProvider(object): def __init__(self, backend): self.backend = backend - # TODO Replace playlists property with a get_playlists() method which - # returns playlist Ref's instead of the gigantic data structures we - # currently make available. lookup() should be used for getting full - # playlists with all details. - - @property - def playlists(self): - """ - Currently available playlists. - - Read/write. List of :class:`mopidy.models.Playlist`. - """ - return [] - - @playlists.setter # noqa - def playlists(self, playlists): - raise NotImplementedError - def as_list(self): """ Get a list of the currently available playlists. diff --git a/tests/backend/test_backend.py b/tests/backend/test_backend.py index 23cfedd5..e6aac76f 100644 --- a/tests/backend/test_backend.py +++ b/tests/backend/test_backend.py @@ -36,12 +36,6 @@ class PlaylistsTest(unittest.TestCase): def setUp(self): # noqa: N802 self.provider = backend.PlaylistsProvider(backend=None) - def test_playlists_default_impl(self): - self.assertEqual(self.provider.playlists, []) - - with self.assertRaises(NotImplementedError): - self.provider.playlists = [] - def test_as_list_default_impl(self): with self.assertRaises(NotImplementedError): self.provider.as_list() From df604bb3e54edb5de5a770775cc03b032e09801e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 00:49:56 +0100 Subject: [PATCH 08/11] core: Deprecated playlists.filter() --- docs/changelog.rst | 3 +++ mopidy/core/playlists.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5d68143d..4a8464c6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -88,6 +88,9 @@ v1.0.0 (UNRELEASED) :meth:`~mopidy.core.PlaylistsController.get_items` instead. (Fixes: :issue:`1057`, PR: :issue:`1075`) +- **Deprecated:** :meth:`mopidy.core.PlaylistsController.filter`. Use + :meth:`~mopidy.core.PlaylistsController.as_list` and filter yourself. + **Backend API** - Remove default implementation of diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 715e5870..0262deaa 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -141,6 +141,9 @@ class PlaylistsController(object): :param criteria: one or more criteria to match by :type criteria: dict :rtype: list of :class:`mopidy.models.Playlist` + + .. deprecated:: 1.0 + Use :meth:`as_list` and filter yourself. """ criteria = criteria or kwargs matches = self.playlists From 6815868e241400e9ae2144fa434ae7b3a507de1e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 13:22:50 +0100 Subject: [PATCH 09/11] core: Doc Playlist.last_modified not being set ...if get_playlists() is called with include_tracks=False --- mopidy/core/playlists.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 0262deaa..54797abe 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -60,6 +60,11 @@ class PlaylistsController(object): :rtype: list of :class:`mopidy.models.Playlist` + .. versionchanged:: 1.0 + If you call the method with ``include_tracks=False``, the + :attr:`~mopidy.models.Playlist.last_modified` field of the returned + playlists is no longer set. + .. deprecated:: 1.0 Use :meth:`as_list` and :meth:`get_items` instead. """ From dbe4165a0f3663bfe9b77faa6f47edff5c1563df Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 13:31:25 +0100 Subject: [PATCH 10/11] m3u: Only test through core actor --- tests/m3u/test_playlists.py | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index be94ed2f..07ffc0a3 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -28,10 +28,10 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.config['m3u']['playlists_dir'] = tempfile.mkdtemp() self.playlists_dir = self.config['m3u']['playlists_dir'] - self.audio = dummy_audio.create_proxy() - self.backend = actor.M3UBackend.start( - config=self.config, audio=self.audio).proxy() - self.core = core.Core(backends=[self.backend]) + audio = dummy_audio.create_proxy() + backend = actor.M3UBackend.start( + config=self.config, audio=audio).proxy() + self.core = core.Core(backends=[backend]) def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() @@ -117,10 +117,8 @@ class M3UPlaylistsProviderTest(unittest.TestCase): playlist = playlist.copy(tracks=[track]) playlist = self.core.playlists.save(playlist) - backend = self.backend_class(config=self.config, audio=self.audio) - - self.assert_(backend.playlists.playlists) - result = backend.playlists.lookup(playlist.uri) + self.assertEqual(len(self.core.playlists.as_list()), 1) + result = self.core.playlists.lookup(playlist.uri) self.assertEqual(playlist.uri, result.uri) self.assertEqual(playlist.name, result.name) self.assertEqual(track.uri, result.tracks[0].uri) @@ -186,14 +184,15 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.assertEqual([playlist], playlists) def test_filter_by_name_returns_single_match(self): - playlist = Playlist(uri='m3u:b', name='b') - self.backend.playlists.playlists = [ - Playlist(uri='m3u:a', name='a'), playlist] + self.core.playlists.create('a') + playlist = self.core.playlists.create('b') + self.assertEqual([playlist], self.core.playlists.filter(name='b')) def test_filter_by_name_returns_no_matches(self): - self.backend.playlists.playlists = [ - Playlist(uri='m3u:a', name='a'), Playlist(uri='m3u:b', name='b')] + self.core.playlists.create('a') + self.core.playlists.create('b') + self.assertEqual([], self.core.playlists.filter(name='c')) def test_lookup_finds_playlist_by_uri(self): @@ -247,10 +246,8 @@ class M3UPlaylistsProviderTest(unittest.TestCase): playlist = playlist.copy(tracks=[track]) playlist = self.core.playlists.save(playlist) - backend = self.backend_class(config=self.config, audio=self.audio) - - self.assertEqual(len(backend.playlists.as_list()), 1) - result = backend.playlists.lookup('m3u:test.m3u') + self.assertEqual(len(self.core.playlists.as_list()), 1) + result = self.core.playlists.lookup('m3u:test.m3u') self.assertEqual('m3u:test.m3u', result.uri) self.assertEqual(playlist.name, result.name) self.assertEqual(track.uri, result.tracks[0].uri) From c0f99466c3d6aa1bea3c1e98e2dbd72fe89078b2 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 23 Mar 2015 13:31:42 +0100 Subject: [PATCH 11/11] m3u: Remove playlists property --- mopidy/m3u/playlists.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index d5f2b1e9..1fc5b4c3 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -34,15 +34,6 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): return None return [Ref.track(uri=t.uri, name=t.name) for t in playlist.tracks] - @property - def playlists(self): - return sorted( - self._playlists.values(), key=operator.attrgetter('name')) - - @playlists.setter - def playlists(self, playlists): - self._playlists = {playlist.uri: playlist for playlist in playlists} - def create(self, name): playlist = self._save_m3u(Playlist(name=name)) self._playlists[playlist.uri] = playlist