From 5fd2afa7ca6d65624396fe2f96466542161df449 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 1 Mar 2015 22:48:31 +0100 Subject: [PATCH] mpd: Switch list command to using list_distinct --- docs/changelog.rst | 3 + mopidy/mpd/protocol/music_db.py | 120 ++++++---------------------- tests/dummy_backend.py | 4 + tests/mpd/protocol/test_music_db.py | 13 ++- 4 files changed, 37 insertions(+), 103 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 03b25897..0cc145fc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -101,6 +101,9 @@ v0.20.0 (UNRELEASED) "database". If you insist on using a client that needs these commands change :confval:`mpd/command_blacklist`. +- Switch the ``list`` command over to using + :meth:`mopidy.core.LibraryController.list_distinct`. (Fixes: :issue:`913`) + **HTTP frontend** - Prevent race condition in webservice broadcast from breaking the server. diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index f08e51f2..04ad7d85 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -30,6 +30,15 @@ _LIST_MAPPING = { 'genre': 'genre', 'performer': 'performer'} +_LIST_NAME_MAPPING = { + 'album': 'Album', + 'albumartist': 'AlbumArtist', + 'artist': 'Artist', + 'composer': 'Composer', + 'date': 'Date', + 'genre': 'Genre', + 'performer': 'Performer'} + def _query_from_mpd_search_parameters(parameters, mapping): query = {} @@ -246,109 +255,30 @@ def list_(context, *args): - does not add quotes around the field argument. - capitalizes the field argument. """ - parameters = list(args) - if not parameters: + params = list(args) + if not params: raise exceptions.MpdArgError('incorrect arguments') - field = parameters.pop(0).lower() + field = params.pop(0).lower() if field not in _LIST_MAPPING: raise exceptions.MpdArgError('incorrect arguments') - if len(parameters) == 1: + if len(params) == 1: if field != 'album': raise exceptions.MpdArgError('should be "Album" for 3 arguments') - return _list_album(context, {'artist': parameters}) + query = {'artist': params} + else: + try: + query = _query_from_mpd_search_parameters(params, _LIST_MAPPING) + except exceptions.MpdArgError as e: + e.message = 'not able to parse args' + raise + except ValueError: + return - try: - query = _query_from_mpd_search_parameters(parameters, _LIST_MAPPING) - except exceptions.MpdArgError as e: - e.message = 'not able to parse args' - raise - except ValueError: - return - - if field == 'artist': - return _list_artist(context, query) - if field == 'albumartist': - return _list_albumartist(context, query) - elif field == 'album': - return _list_album(context, query) - elif field == 'composer': - return _list_composer(context, query) - elif field == 'performer': - return _list_performer(context, query) - elif field == 'date': - return _list_date(context, query) - elif field == 'genre': - return _list_genre(context, query) - - -def _list_artist(context, query): - artists = set() - results = context.core.library.find_exact(**query).get() - for track in _get_tracks(results): - for artist in track.artists: - if artist.name: - artists.add(('Artist', artist.name)) - return artists - - -def _list_albumartist(context, query): - albumartists = set() - results = context.core.library.find_exact(**query).get() - for track in _get_tracks(results): - if track.album: - for artist in track.album.artists: - if artist.name: - albumartists.add(('AlbumArtist', artist.name)) - return albumartists - - -def _list_album(context, query): - albums = set() - results = context.core.library.find_exact(**query).get() - for track in _get_tracks(results): - if track.album and track.album.name: - albums.add(('Album', track.album.name)) - return albums - - -def _list_composer(context, query): - composers = set() - results = context.core.library.find_exact(**query).get() - for track in _get_tracks(results): - for composer in track.composers: - if composer.name: - composers.add(('Composer', composer.name)) - return composers - - -def _list_performer(context, query): - performers = set() - results = context.core.library.find_exact(**query).get() - for track in _get_tracks(results): - for performer in track.performers: - if performer.name: - performers.add(('Performer', performer.name)) - return performers - - -def _list_date(context, query): - dates = set() - results = context.core.library.find_exact(**query).get() - for track in _get_tracks(results): - if track.date: - dates.add(('Date', track.date)) - return dates - - -def _list_genre(context, query): - genres = set() - results = context.core.library.find_exact(**query).get() - for track in _get_tracks(results): - if track.genre: - genres.add(('Genre', track.genre)) - return genres + name = _LIST_NAME_MAPPING[field] + result = context.core.library.list_distinct(field, query) + return [(name, value) for value in result.get()] @protocol.commands.add('listall') diff --git a/tests/dummy_backend.py b/tests/dummy_backend.py index 05b0fbff..a20c5686 100644 --- a/tests/dummy_backend.py +++ b/tests/dummy_backend.py @@ -33,6 +33,7 @@ class DummyLibraryProvider(backend.LibraryProvider): def __init__(self, *args, **kwargs): super(DummyLibraryProvider, self).__init__(*args, **kwargs) self.dummy_library = [] + self.dummy_list_distinct_result = {} self.dummy_browse_result = {} self.dummy_find_exact_result = SearchResult() self.dummy_search_result = SearchResult() @@ -40,6 +41,9 @@ class DummyLibraryProvider(backend.LibraryProvider): def browse(self, path): return self.dummy_browse_result.get(path, []) + def list_distinct(self, field, query=None): + return self.dummy_list_distinct_result.get(field, set()) + def find_exact(self, **query): return self.dummy_find_exact_result diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index 9f3b7348..30ecf27c 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -55,7 +55,7 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): # Count the lone track self.backend.library.dummy_find_exact_result = SearchResult( tracks=[ - Track(uri='dummy:a', name="foo", date="2001", length=4000), + Track(uri='dummy:a', name='foo', date='2001', length=4000), ]) self.send_request('count "title" "foo"') self.assertInResponse('songs: 1') @@ -613,11 +613,8 @@ class MusicDatabaseFindTest(protocol.BaseTestCase): class MusicDatabaseListTest(protocol.BaseTestCase): def test_list(self): - self.backend.library.dummy_find_exact_result = SearchResult( - tracks=[ - Track(uri='dummy:a', name='A', artists=[ - Artist(name='A Artist')])]) - + self.backend.library.dummy_list_distinct_result = { + 'artist': set(['A Artist'])} self.send_request('list "artist" "artist" "foo"') self.assertInResponse('Artist: A Artist') @@ -891,8 +888,8 @@ class MusicDatabaseListTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_list_album_with_artist_name(self): - self.backend.library.dummy_find_exact_result = SearchResult( - tracks=[Track(album=Album(name='foo'))]) + self.backend.library.dummy_list_distinct_result = { + 'album': set(['foo'])} self.send_request('list "album" "anartist"') self.assertInResponse('Album: foo')