diff --git a/docs/changelog.rst b/docs/changelog.rst index 71ae42cf..651b22ba 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -119,6 +119,14 @@ MPD frontend - Implement protocol extensions to output Album URIs and Album Images when outputting track data to clients. (PR: :issue:`1230`) +- The MPD commands ``lsinfo`` and ``listplaylists`` are now implemented using + the :meth:`~mopidy.core.PlaylistsProvider.as_list` method, which retrieves a + lot less data and is thus much faster than the deprecated + :meth:`~mopidy.core.PlaylistsProvider.get_playlists`. The drawback is that + the ``Last-Modified`` timestamp is not available through this method, and the + timestamps in the MPD command responses are now always set to the current + time. + Stream backend -------------- diff --git a/mopidy/mpd/protocol/stored_playlists.py b/mopidy/mpd/protocol/stored_playlists.py index a5d4b180..bf31fa10 100644 --- a/mopidy/mpd/protocol/stored_playlists.py +++ b/mopidy/mpd/protocol/stored_playlists.py @@ -75,29 +75,29 @@ def listplaylists(context): - ncmpcpp 0.5.10 segfaults if we return 'playlist: ' on a line, so we must ignore playlists without names, which isn't very useful anyway. """ + last_modified = _get_last_modified() result = [] - for playlist in context.core.playlists.get_playlists().get(): - if not playlist.name: + for playlist_ref in context.core.playlists.as_list().get(): + if not playlist_ref.name: continue - name = context.lookup_playlist_name_from_uri(playlist.uri) + name = context.lookup_playlist_name_from_uri(playlist_ref.uri) result.append(('playlist', name)) - result.append(('Last-Modified', _get_last_modified(playlist))) + result.append(('Last-Modified', last_modified)) return result # TODO: move to translators? -def _get_last_modified(playlist): +def _get_last_modified(last_modified=None): """Formats last modified timestamp of a playlist for MPD. Time in UTC with second precision, formatted in the ISO 8601 format, with the "Z" time zone marker for UTC. For example, "1970-01-01T00:00:00Z". """ - if playlist.last_modified is None: + if last_modified is None: # If unknown, assume the playlist is modified dt = datetime.datetime.utcnow() else: - dt = datetime.datetime.utcfromtimestamp( - playlist.last_modified / 1000.0) + dt = datetime.datetime.utcfromtimestamp(last_modified / 1000.0) dt = dt.replace(microsecond=0) return '%sZ' % dt.isoformat() diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index 5fe40e0d..ea944b7a 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -2,8 +2,10 @@ from __future__ import absolute_import, unicode_literals import unittest +import mock + from mopidy.models import Album, Artist, Playlist, Ref, SearchResult, Track -from mopidy.mpd.protocol import music_db +from mopidy.mpd.protocol import music_db, stored_playlists from tests.mpd import protocol @@ -299,33 +301,37 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.send_request('listfiles') self.assertEqualResponse('ACK [0@0] {listfiles} Not implemented') - def test_lsinfo_without_path_returns_same_as_for_root(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_without_path_returns_same_as_for_root( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) response1 = self.send_request('lsinfo') response2 = self.send_request('lsinfo "/"') self.assertEqual(response1, response2) - def test_lsinfo_with_empty_path_returns_same_as_for_root(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_with_empty_path_returns_same_as_for_root( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) response1 = self.send_request('lsinfo ""') response2 = self.send_request('lsinfo "/"') self.assertEqual(response1, response2) - def test_lsinfo_for_root_includes_playlists(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_for_root_includes_playlists(self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) self.send_request('lsinfo "/"') self.assertInResponse('playlist: a') - # Date without milliseconds and with time zone information - self.assertInResponse('Last-Modified: 2014-01-28T21:01:13Z') + self.assertInResponse('Last-Modified: 2015-08-05T22:51:06Z') self.assertInResponse('OK') def test_lsinfo_for_root_includes_dirs_for_each_lib_with_content(self): @@ -337,7 +343,10 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('directory: dummy') self.assertInResponse('OK') - def test_lsinfo_for_dir_with_and_without_leading_slash_is_the_same(self): + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_for_dir_with_and_without_leading_slash_is_the_same( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.library.dummy_browse_result = { 'dummy:/': [Ref.track(uri='dummy:/a', name='a'), Ref.directory(uri='dummy:/foo', name='foo')]} @@ -346,7 +355,10 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): response2 = self.send_request('lsinfo "/dummy"') self.assertEqual(response1, response2) - def test_lsinfo_for_dir_with_and_without_trailing_slash_is_the_same(self): + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_lsinfo_for_dir_with_and_without_trailing_slash_is_the_same( + self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.library.dummy_browse_result = { 'dummy:/': [Ref.track(uri='dummy:/a', name='a'), Ref.directory(uri='dummy:/foo', name='foo')]} @@ -404,12 +416,11 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_lsinfo_for_root_returns_browse_result_before_playlists(self): - last_modified = 1390942873222 self.backend.library.dummy_browse_result = { 'dummy:/': [Ref.track(uri='dummy:/a', name='a'), Ref.directory(uri='dummy:/foo', name='foo')]} self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:/a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:/a')]) response = self.send_request('lsinfo "/"') self.assertLess(response.index('directory: dummy'), diff --git a/tests/mpd/protocol/test_stored_playlists.py b/tests/mpd/protocol/test_stored_playlists.py index 2899a126..90c325ff 100644 --- a/tests/mpd/protocol/test_stored_playlists.py +++ b/tests/mpd/protocol/test_stored_playlists.py @@ -1,6 +1,9 @@ from __future__ import absolute_import, unicode_literals +import mock + from mopidy.models import Playlist, Track +from mopidy.mpd.protocol import stored_playlists from tests.mpd import protocol @@ -76,15 +79,16 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertNotInResponse('Pos: 0') self.assertInResponse('OK') - def test_listplaylists(self): - last_modified = 1390942873222 + @mock.patch.object(stored_playlists, '_get_last_modified') + def test_listplaylists(self, last_modified_mock): + last_modified_mock.return_value = '2015-08-05T22:51:06Z' self.backend.playlists.set_dummy_playlists([ - Playlist(name='a', uri='dummy:a', last_modified=last_modified)]) + Playlist(name='a', uri='dummy:a')]) self.send_request('listplaylists') self.assertInResponse('playlist: a') # Date without milliseconds and with time zone information - self.assertInResponse('Last-Modified: 2014-01-28T21:01:13Z') + self.assertInResponse('Last-Modified: 2015-08-05T22:51:06Z') self.assertInResponse('OK') def test_listplaylists_duplicate(self):