From 00b2b9538e6a1d3dc9c1b938eed603f10127bbc8 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 1 Mar 2015 22:46:18 +0100 Subject: [PATCH 1/4] core: Add library.list_distinct for getting distinct field values --- docs/changelog.rst | 3 +++ mopidy/backend.py | 10 ++++++++++ mopidy/core/library.py | 21 +++++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 733d122c..03b25897 100644 --- a/docs/changelog.rst +++ b/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.list_distinct` for getting unique + values for a given field. (Fixes: :issue:`913`) + **Commands** - Make the ``mopidy`` command print a friendly error message if the diff --git a/mopidy/backend.py b/mopidy/backend.py index c713d083..38d4c5db 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -92,6 +92,16 @@ class LibraryProvider(object): """ return [] + def list_distinct(self, field, query=None): + """ + See :meth:`mopidy.core.LibraryController.list_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`. diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 822836a6..4ccfd657 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -72,6 +72,27 @@ class LibraryController(object): return [] return backend.library.browse(uri).get() + def list_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.list_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 From ba8fc51f860861c5bfebc1db2ca4f5f8b7be0ddf Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 1 Mar 2015 22:48:16 +0100 Subject: [PATCH 2/4] local: Add support for list_distinct and implement for json backend --- mopidy/local/__init__.py | 12 ++++++++++++ mopidy/local/json.py | 32 ++++++++++++++++++++++++++++++++ mopidy/local/library.py | 5 +++++ 3 files changed, 49 insertions(+) diff --git a/mopidy/local/__init__.py b/mopidy/local/__init__.py index 31ec6426..3099e240 100644 --- a/mopidy/local/__init__.py +++ b/mopidy/local/__init__.py @@ -89,6 +89,18 @@ class Library(object): """ raise NotImplementedError + def list_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 diff --git a/mopidy/local/json.py b/mopidy/local/json.py index 38e1bf6c..dcf8ff9b 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -155,6 +155,38 @@ class JsonLibrary(local.Library): except KeyError: return [] + def list_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 diff --git a/mopidy/local/library.py b/mopidy/local/library.py index f3828f1b..8a6c2a8a 100644 --- a/mopidy/local/library.py +++ b/mopidy/local/library.py @@ -23,6 +23,11 @@ class LocalLibraryProvider(backend.LibraryProvider): return [] return self._library.browse(uri) + def list_distinct(self, field, query=None): + if not self._library: + return set() + return self._library.list_distinct(field, query) + def refresh(self, uri=None): if not self._library: return 0 From 5fd2afa7ca6d65624396fe2f96466542161df449 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 1 Mar 2015 22:48:31 +0100 Subject: [PATCH 3/4] 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') From 8cc9c9bbc032888cde31919bbe360cfdf5e801f5 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 2 Mar 2015 22:41:09 +0100 Subject: [PATCH 4/4] core: Rename list_distinct to get_distinct --- docs/changelog.rst | 4 ++-- mopidy/backend.py | 4 ++-- mopidy/core/library.py | 4 ++-- mopidy/local/__init__.py | 2 +- mopidy/local/json.py | 2 +- mopidy/local/library.py | 4 ++-- mopidy/mpd/protocol/music_db.py | 2 +- tests/dummy_backend.py | 6 +++--- tests/mpd/protocol/test_music_db.py | 4 ++-- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0cc145fc..36dbcc1e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -36,7 +36,7 @@ v0.20.0 (UNRELEASED) - When seeking in paused state, do not change to playing state. (Fixed :issue:`939`) -- Add :meth:`mopidy.core.LibraryController.list_distinct` for getting unique +- Add :meth:`mopidy.core.LibraryController.get_distinct` for getting unique values for a given field. (Fixes: :issue:`913`) **Commands** @@ -102,7 +102,7 @@ v0.20.0 (UNRELEASED) :confval:`mpd/command_blacklist`. - Switch the ``list`` command over to using - :meth:`mopidy.core.LibraryController.list_distinct`. (Fixes: :issue:`913`) + :meth:`mopidy.core.LibraryController.get_distinct`. (Fixes: :issue:`913`) **HTTP frontend** diff --git a/mopidy/backend.py b/mopidy/backend.py index 38d4c5db..f7808ac8 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -92,9 +92,9 @@ class LibraryProvider(object): """ return [] - def list_distinct(self, field, query=None): + def get_distinct(self, field, query=None): """ - See :meth:`mopidy.core.LibraryController.list_distinct`. + See :meth:`mopidy.core.LibraryController.get_distinct`. *MAY be implemented by subclass.* diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 4ccfd657..5937b2c0 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -72,7 +72,7 @@ class LibraryController(object): return [] return backend.library.browse(uri).get() - def list_distinct(self, field, query=None): + def get_distinct(self, field, query=None): """ List distinct values for a given field from the library. @@ -86,7 +86,7 @@ class LibraryController(object): :method:`search` for details about the query format. :rtype: set of values corresponding to the requested field type. """ - futures = [b.library.list_distinct(field, query) + futures = [b.library.get_distinct(field, query) for b in self.backends.with_library.values()] result = set() for r in pykka.get_all(futures): diff --git a/mopidy/local/__init__.py b/mopidy/local/__init__.py index 3099e240..1587b63a 100644 --- a/mopidy/local/__init__.py +++ b/mopidy/local/__init__.py @@ -89,7 +89,7 @@ class Library(object): """ raise NotImplementedError - def list_distinct(self, field, query=None): + def get_distinct(self, field, query=None): """ List distinct values for a given field from the library. diff --git a/mopidy/local/json.py b/mopidy/local/json.py index dcf8ff9b..aa16a6df 100644 --- a/mopidy/local/json.py +++ b/mopidy/local/json.py @@ -155,7 +155,7 @@ class JsonLibrary(local.Library): except KeyError: return [] - def list_distinct(self, field, query=None): + def get_distinct(self, field, query=None): if field == 'artist': def distinct(track): return {a.name for a in track.artists} diff --git a/mopidy/local/library.py b/mopidy/local/library.py index 8a6c2a8a..90a54770 100644 --- a/mopidy/local/library.py +++ b/mopidy/local/library.py @@ -23,10 +23,10 @@ class LocalLibraryProvider(backend.LibraryProvider): return [] return self._library.browse(uri) - def list_distinct(self, field, query=None): + def get_distinct(self, field, query=None): if not self._library: return set() - return self._library.list_distinct(field, query) + return self._library.get_distinct(field, query) def refresh(self, uri=None): if not self._library: diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 04ad7d85..62147b7d 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -277,7 +277,7 @@ def list_(context, *args): return name = _LIST_NAME_MAPPING[field] - result = context.core.library.list_distinct(field, query) + result = context.core.library.get_distinct(field, query) return [(name, value) for value in result.get()] diff --git a/tests/dummy_backend.py b/tests/dummy_backend.py index a20c5686..9c5a8c0c 100644 --- a/tests/dummy_backend.py +++ b/tests/dummy_backend.py @@ -33,7 +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_get_distinct_result = {} self.dummy_browse_result = {} self.dummy_find_exact_result = SearchResult() self.dummy_search_result = SearchResult() @@ -41,8 +41,8 @@ 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 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 diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index 30ecf27c..613467ed 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -613,7 +613,7 @@ class MusicDatabaseFindTest(protocol.BaseTestCase): class MusicDatabaseListTest(protocol.BaseTestCase): def test_list(self): - self.backend.library.dummy_list_distinct_result = { + self.backend.library.dummy_get_distinct_result = { 'artist': set(['A Artist'])} self.send_request('list "artist" "artist" "foo"') @@ -888,7 +888,7 @@ class MusicDatabaseListTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_list_album_with_artist_name(self): - self.backend.library.dummy_list_distinct_result = { + self.backend.library.dummy_get_distinct_result = { 'album': set(['foo'])} self.send_request('list "album" "anartist"')