mpd: Switch list command to using list_distinct

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.

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')

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
 

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')