From afbff12ccc62dadce4e4d552361e056bc4b66f5c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 15 Jan 2014 00:51:56 +0100 Subject: [PATCH 1/4] mpd: Implement "listall" command --- mopidy/mpd/protocol/music_db.py | 23 +++++++++++++++++-- tests/mpd/protocol/test_music_db.py | 35 ++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index b31d295b..302d37eb 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -6,7 +6,8 @@ import re from mopidy.models import Ref, Track from mopidy.mpd import translator -from mopidy.mpd.exceptions import MpdArgError, MpdNotImplemented +from mopidy.mpd.exceptions import ( + MpdArgError, MpdNoExistError, MpdNotImplemented) from mopidy.mpd.protocol import handle_request, stored_playlists @@ -417,7 +418,25 @@ def listall(context, uri=None): Lists all songs and directories in ``URI``. """ - raise MpdNotImplemented # TODO + if uri is None: + uri = '/' + if not uri.startswith('/'): + uri = '/%s' % uri + + result = [] + browse_futures = [context.core.library.browse(uri)] + while browse_futures: + for ref in browse_futures.pop().get(): + if ref.type == Ref.DIRECTORY: + result.append(('directory', ref.uri)) + browse_futures.append(context.core.library.browse(ref.uri)) + elif ref.type == Ref.TRACK: + result.append(('file', ref.uri)) + + if not result: + raise MpdNoExistError('Not found', command='listall') + + return [('directory', uri)] + result @handle_request(r'listallinfo$') diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index 163ccf88..78a94a78 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -123,12 +123,41 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_listall_without_uri(self): + tracks = [Track(uri='dummy:/a', name='a'), + Track(uri='dummy:/b', name='b')] + self.backend.library.dummy_library = tracks + self.backend.library.dummy_browse_result = { + '/': [Ref.track(uri='dummy:/a', name='a'), + Ref.directory(uri='/foo')], + '/foo': [Ref.track(uri='dummy:/b', name='b')]} + self.sendRequest('listall') - self.assertEqualResponse('ACK [0@0] {} Not implemented') + + self.assertInResponse('file: dummy:/a') + self.assertInResponse('directory: /dummy/foo') + self.assertInResponse('file: dummy:/b') + self.assertInResponse('OK') def test_listall_with_uri(self): - self.sendRequest('listall "file:///dev/urandom"') - self.assertEqualResponse('ACK [0@0] {} Not implemented') + tracks = [Track(uri='dummy:/a', name='a'), + Track(uri='dummy:/b', name='b')] + self.backend.library.dummy_library = tracks + self.backend.library.dummy_browse_result = { + '/': [Ref.track(uri='dummy:/a', name='a'), + Ref.directory(uri='/foo')], + '/foo': [Ref.track(uri='dummy:/b', name='b')]} + + self.sendRequest('listall "/dummy/foo"') + + self.assertNotInResponse('file: dummy:/a') + self.assertInResponse('directory: /dummy/foo') + self.assertInResponse('file: dummy:/b') + self.assertInResponse('OK') + + def test_listall_with_unknown_uri(self): + self.sendRequest('listall "/unknown"') + + self.assertEqualResponse('ACK [50@0] {listall} Not found') def test_listallinfo_without_uri(self): self.sendRequest('listallinfo') From d698653d83b0be1cd3272ebd0e433e5cf8e40002 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 15 Jan 2014 01:08:17 +0100 Subject: [PATCH 2/4] mpd: Implement "listallinfo" command (fixes #145) --- mopidy/mpd/protocol/music_db.py | 32 ++++++++++++++++++++--- tests/mpd/protocol/test_music_db.py | 39 ++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 302d37eb..7ef11111 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -6,8 +6,7 @@ import re from mopidy.models import Ref, Track from mopidy.mpd import translator -from mopidy.mpd.exceptions import ( - MpdArgError, MpdNoExistError, MpdNotImplemented) +from mopidy.mpd.exceptions import MpdArgError, MpdNoExistError from mopidy.mpd.protocol import handle_request, stored_playlists @@ -450,7 +449,34 @@ def listallinfo(context, uri=None): Same as ``listall``, except it also returns metadata info in the same format as ``lsinfo``. """ - raise MpdNotImplemented # TODO + if uri is None: + uri = '/' + if not uri.startswith('/'): + uri = '/%s' % uri + + dirs_and_futures = [] + browse_futures = [context.core.library.browse(uri)] + while browse_futures: + for ref in browse_futures.pop().get(): + if ref.type == Ref.DIRECTORY: + dirs_and_futures.append(('directory', ref.uri)) + browse_futures.append(context.core.library.browse(ref.uri)) + elif ref.type == Ref.TRACK: + # TODO Lookup tracks in batch for better performance + dirs_and_futures.append(context.core.library.lookup(ref.uri)) + + result = [] + for obj in dirs_and_futures: + if hasattr(obj, 'get'): + for track in obj.get(): + result.extend(translator.track_to_mpd_format(track)) + else: + result.append(obj) + + if not result: + raise MpdNoExistError('Not found', command='listallinfo') + + return [('directory', uri)] + result @handle_request(r'lsinfo$') diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index 78a94a78..d2ecd66c 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -160,12 +160,45 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertEqualResponse('ACK [50@0] {listall} Not found') def test_listallinfo_without_uri(self): + tracks = [Track(uri='dummy:/a', name='a'), + Track(uri='dummy:/b', name='b')] + self.backend.library.dummy_library = tracks + self.backend.library.dummy_browse_result = { + '/': [Ref.track(uri='dummy:/a', name='a'), + Ref.directory(uri='/foo')], + '/foo': [Ref.track(uri='dummy:/b', name='b')]} + self.sendRequest('listallinfo') - self.assertEqualResponse('ACK [0@0] {} Not implemented') + + self.assertInResponse('file: dummy:/a') + self.assertInResponse('Title: a') + self.assertInResponse('directory: /dummy/foo') + self.assertInResponse('file: dummy:/b') + self.assertInResponse('Title: b') + self.assertInResponse('OK') def test_listallinfo_with_uri(self): - self.sendRequest('listallinfo "file:///dev/urandom"') - self.assertEqualResponse('ACK [0@0] {} Not implemented') + tracks = [Track(uri='dummy:/a', name='a'), + Track(uri='dummy:/b', name='b')] + self.backend.library.dummy_library = tracks + self.backend.library.dummy_browse_result = { + '/': [Ref.track(uri='dummy:/a', name='a'), + Ref.directory(uri='/foo')], + '/foo': [Ref.track(uri='dummy:/b', name='b')]} + + self.sendRequest('listallinfo "/dummy/foo"') + + self.assertNotInResponse('file: dummy:/a') + self.assertNotInResponse('Title: a') + self.assertInResponse('directory: /dummy/foo') + self.assertInResponse('file: dummy:/b') + self.assertInResponse('Title: b') + self.assertInResponse('OK') + + def test_listallinfo_with_unknown_uri(self): + self.sendRequest('listallinfo "/unknown"') + + self.assertEqualResponse('ACK [50@0] {listallinfo} Not found') def test_lsinfo_without_path_returns_same_as_for_root(self): last_modified = datetime.datetime(2001, 3, 17, 13, 41, 17, 12345) From 07d9a15ff36c98d1c28ac586321807fbce65221e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 15 Jan 2014 01:10:44 +0100 Subject: [PATCH 3/4] docs: Update changelog --- docs/changelog.rst | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 3e86ab9e..a3f127b9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,7 +18,7 @@ v0.18.0 (UNRELEASED) - Add :meth:`mopidy.core.LibraryController.browse` method for browsing a virtual file system of tracks. Backends can implement support for this by - implementing :meth:`mopidy.backends.base.BaseLibraryController.browse`. + implementing :meth:`mopidy.backend.LibraryProvider.browse`. - Events emitted on play/stop, pause/resume, next/previous and on end of track has been cleaned up to work consistenly. See the message of @@ -46,6 +46,10 @@ v0.18.0 (UNRELEASED) Imports from the old locations still works, but are deprecated. +- Add :meth:`mopidy.backend.LibraryProvider.browse`, which can be implemented + by backends that wants to expose directories of tracks in Mopidy's virtual + file system. + **Commands** - Reduce amount of logging from dependencies when using :option:`mopidy -v`. @@ -85,7 +89,7 @@ v0.18.0 (UNRELEASED) **Local backend** -- Added support for browsing local directories. +- Added support for browsing local directories in Mopidy's virtual file system. - Finished the work on creating pluggable libraries. Users can now reconfigure Mopidy to use alternate library providers of their choosing for @@ -125,9 +129,8 @@ v0.18.0 (UNRELEASED) **MPD frontend** -- Make the ``lsinfo`` command support browsing of Mopidy's virtual file - system. Note that the related ``listall`` and ``listallinfo`` commands are - still not implemented. +- Make the ``lsinfo``, ``listall``, and ``listallinfo`` commands support + browsing of Mopidy's virtual file system. (Fixes: :issue:`145`) - Empty commands now return a ``ACK [5@0] {} No command given`` error instead of ``OK``. This is consistent with the original MPD server implementation. From c4ab4b150af383e59409d247fd692eb75d5cdef7 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 15 Jan 2014 01:34:43 +0100 Subject: [PATCH 4/4] docs: Remove browsing from list of MPD limitations --- docs/ext/mpd.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/ext/mpd.rst b/docs/ext/mpd.rst index 5b82dce2..10ecdb24 100644 --- a/docs/ext/mpd.rst +++ b/docs/ext/mpd.rst @@ -47,7 +47,6 @@ near future: - Modifying stored playlists is not supported - ``tagtypes`` is not supported -- Browsing the file system is not supported - Live update of the music database is not supported