Merge pull request #1022 from adamcik/fix/913

Add list_distinct command for more effective library lookups.

docs/changelog.rst

@@ -36,6 +36,9 @@ v0.20.0 (UNRELEASED)
 - When seeking in paused state, do not change to playing state. (Fixed
   :issue:`939`)
 
+- Add :meth:`mopidy.core.LibraryController.get_distinct` for getting unique
+  values for a given field. (Fixes: :issue:`913`)
+
 **Commands**
 
 - Make the ``mopidy`` command print a friendly error message if the
@@ -98,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.get_distinct`. (Fixes: :issue:`913`)
+
 **HTTP frontend**
 
 - Prevent race condition in webservice broadcast from breaking the server.

mopidy/backend.py

@@ -92,6 +92,16 @@ class LibraryProvider(object):
         """
         return []
 
+    def get_distinct(self, field, query=None):
+        """
+        See :meth:`mopidy.core.LibraryController.get_distinct`.
+
+        *MAY be implemented by subclass.*
+
+        Default implementation will simply return an empty set.
+        """
+        return set()
+
     def get_images(self, uris):
         """
         See :meth:`mopidy.core.LibraryController.get_images`.

mopidy/core/library.py

@@ -72,6 +72,27 @@ class LibraryController(object):
             return []
         return backend.library.browse(uri).get()
 
+    def get_distinct(self, field, query=None):
+        """
+        List distinct values for a given field from the library.
+
+        This has mainly been added to support the list commands the MPD
+        protocol supports in a more sane fashion. Other frontends are not
+        recommended to use this method.
+
+        :param string field: One of ``artist``, ``albumartist``, ``album``,
+            ``composer``, ``performer``, ``date``or ``genre``.
+        :param dict query: Query to use for limiting results, see
+            :method:`search` for details about the query format.
+        :rtype: set of values corresponding to the requested field type.
+        """
+        futures = [b.library.get_distinct(field, query)
+                   for b in self.backends.with_library.values()]
+        result = set()
+        for r in pykka.get_all(futures):
+            result.update(r)
+        return result
+
     def get_images(self, uris):
         """Lookup the images for the given URIs
 

mopidy/local/__init__.py

@@ -89,6 +89,18 @@ class Library(object):
         """
         raise NotImplementedError
 
+    def get_distinct(self, field, query=None):
+        """
+        List distinct values for a given field from the library.
+
+        :param string field: One of ``artist``, ``albumartist``, ``album``,
+            ``composer``, ``performer``, ``date``or ``genre``.
+        :param dict query: Query to use for limiting results, see
+            :method:`search` for details about the query format.
+        :rtype: set of values corresponding to the requested field type.
+        """
+        return set()
+
     def load(self):
         """
         (Re)load any tracks stored in memory, if any, otherwise just return

mopidy/local/json.py

@@ -155,6 +155,38 @@ class JsonLibrary(local.Library):
         except KeyError:
             return []
 
+    def get_distinct(self, field, query=None):
+        if field == 'artist':
+            def distinct(track):
+                return {a.name for a in track.artists}
+        elif field == 'albumartist':
+            def distinct(track):
+                album = track.album or models.Album()
+                return {a.name for a in album.artists}
+        elif field == 'album':
+            def distinct(track):
+                album = track.album or models.Album()
+                return {album.name}
+        elif field == 'composer':
+            def distinct(track):
+                return {a.name for a in track.composers}
+        elif field == 'performer':
+            def distinct(track):
+                return {a.name for a in track.performers}
+        elif field == 'date':
+            def distinct(track):
+                return {track.date}
+        elif field == 'genre':
+            def distinct(track):
+                return {track.genre}
+        else:
+            return set()
+
+        result = set()
+        for track in search.search(self._tracks.values(), query).tracks:
+            result.update(distinct(track))
+        return result
+
     def search(self, query=None, limit=100, offset=0, uris=None, exact=False):
         tracks = self._tracks.values()
         # TODO: pass limit and offset into search helpers

mopidy/local/library.py

@@ -23,6 +23,11 @@ class LocalLibraryProvider(backend.LibraryProvider):
             return []
         return self._library.browse(uri)
 
+    def get_distinct(self, field, query=None):
+        if not self._library:
+            return set()
+        return self._library.get_distinct(field, query)
+
     def refresh(self, uri=None):
         if not self._library:
             return 0

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)
+    params = list(args)
-    if not parameters:
+    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:
+    name = _LIST_NAME_MAPPING[field]
-        query = _query_from_mpd_search_parameters(parameters, _LIST_MAPPING)
+    result = context.core.library.get_distinct(field, query)
-    except exceptions.MpdArgError as e:
+    return [(name, value) for value in result.get()]
-        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
 
 
 @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_get_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 get_distinct(self, field, query=None):
+        return self.dummy_get_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(
+        self.backend.library.dummy_get_distinct_result = {
-            tracks=[
+            'artist': set(['A Artist'])}
-                Track(uri='dummy:a', name='A', artists=[
-                    Artist(name='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(
+        self.backend.library.dummy_get_distinct_result = {
-            tracks=[Track(album=Album(name='foo'))])
+            'album': set(['foo'])}
 
         self.send_request('list "album" "anartist"')
         self.assertInResponse('Album: foo')