From 455f0145e71e77121502556b05c2c98bc33b179c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 23:08:18 +0100 Subject: [PATCH 1/5] mpd: Include artists and albums in search results --- docs/changes.rst | 5 +++ mopidy/frontends/mpd/protocol/music_db.py | 36 ++++++++++++++++--- tests/frontends/mpd/protocol/music_db_test.py | 34 ++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 8f887ed5..3bc68948 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -58,6 +58,11 @@ v0.11.0 (in development) - Add support for search by date. +- Include fake tracks representing albums and artists in the search results. + When these are added to the tracklist, they expand to either all tracks in + the album or all tracks by the artist. This makes it easy to play full albums + in proper order, which is a feature that have been frequently requested. + **Internal changes** *Models:* diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index f9149a50..ca5ef730 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -1,7 +1,9 @@ from __future__ import unicode_literals +import functools import itertools +from mopidy.models import Track from mopidy.frontends.mpd import translator from mopidy.frontends.mpd.exceptions import MpdNotImplemented from mopidy.frontends.mpd.protocol import handle_request, stored_playlists @@ -12,8 +14,28 @@ QUERY_RE = ( r'[Tt]itle|[Aa]ny)"? "[^"]*"\s?)+)$') -def _get_tracks(search_results): - return list(itertools.chain(*[r.tracks for r in search_results])) +def _get_field(field, search_results): + return list(itertools.chain(*[getattr(r, field) for r in search_results])) + + +_get_albums = functools.partial(_get_field, 'albums') +_get_artists = functools.partial(_get_field, 'artists') +_get_tracks = functools.partial(_get_field, 'tracks') + + +def _album_as_track(album): + return Track( + uri=album.uri, + name='Album: ' + album.name, + album=album, + artists=album.artists) + + +def _artist_as_track(artist): + return Track( + uri=artist.uri, + name='Artist: ' + artist.name, + artists=[artist]) @handle_request(r'^count "(?P[^"]+)" "(?P[^"]*)"$') @@ -62,7 +84,10 @@ def find(context, mpd_query): except ValueError: return results = context.core.library.find_exact(**query).get() - return translator.tracks_to_mpd_format(_get_tracks(results)) + albums = [_album_as_track(a) for a in _get_albums(results)] + artists = [_artist_as_track(a) for a in _get_artists(results)] + tracks = _get_tracks(results) + return translator.tracks_to_mpd_format(artists + albums + tracks) @handle_request(r'^findadd ' + QUERY_RE) @@ -304,7 +329,10 @@ def search(context, mpd_query): except ValueError: return results = context.core.library.search(**query).get() - return translator.tracks_to_mpd_format(_get_tracks(results)) + albums = [_album_as_track(a) for a in _get_albums(results)] + artists = [_artist_as_track(a) for a in _get_artists(results)] + tracks = _get_tracks(results) + return translator.tracks_to_mpd_format(artists + albums + tracks) @handle_request(r'^searchadd ' + QUERY_RE) diff --git a/tests/frontends/mpd/protocol/music_db_test.py b/tests/frontends/mpd/protocol/music_db_test.py index 86fd8ad7..fedc34a1 100644 --- a/tests/frontends/mpd/protocol/music_db_test.py +++ b/tests/frontends/mpd/protocol/music_db_test.py @@ -115,6 +115,23 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): class MusicDatabaseFindTest(protocol.BaseTestCase): + def test_find(self): + self.backend.library.dummy_find_exact_result = SearchResult( + albums=[Album(uri='dummy:album:a', name='A')], + artists=[Artist(uri='dummy:artist:b', name='B')], + tracks=[Track(uri='dummy:track:c', name='C')]) + + self.sendRequest('find "any" "foo"') + + self.assertInResponse('file: dummy:album:a') + self.assertInResponse('Title: Album: A') + self.assertInResponse('file: dummy:artist:b') + self.assertInResponse('Title: Artist: B') + self.assertInResponse('file: dummy:track:c') + self.assertInResponse('Title: C') + + self.assertInResponse('OK') + def test_find_album(self): self.sendRequest('find "album" "what"') self.assertInResponse('OK') @@ -409,6 +426,23 @@ class MusicDatabaseListTest(protocol.BaseTestCase): class MusicDatabaseSearchTest(protocol.BaseTestCase): + def test_search(self): + self.backend.library.dummy_search_result = SearchResult( + albums=[Album(uri='dummy:album:a', name='A')], + artists=[Artist(uri='dummy:artist:b', name='B')], + tracks=[Track(uri='dummy:track:c', name='C')]) + + self.sendRequest('search "any" "foo"') + + self.assertInResponse('file: dummy:album:a') + self.assertInResponse('Title: Album: A') + self.assertInResponse('file: dummy:artist:b') + self.assertInResponse('Title: Artist: B') + self.assertInResponse('file: dummy:track:c') + self.assertInResponse('Title: C') + + self.assertInResponse('OK') + def test_search_album(self): self.sendRequest('search "album" "analbum"') self.assertInResponse('OK') From e5c0bcd110781c0d81bf178d5d5eb8fc066a8749 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 23:28:59 +0100 Subject: [PATCH 2/5] docs: Add issue references --- docs/changes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changes.rst b/docs/changes.rst index 3bc68948..f373832d 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -62,6 +62,7 @@ v0.11.0 (in development) When these are added to the tracklist, they expand to either all tracks in the album or all tracks by the artist. This makes it easy to play full albums in proper order, which is a feature that have been frequently requested. + (Fixes: :issue:`67`, :issue:`148`) **Internal changes** From 5060db48f2ccf04f3c7c5f9908840f2891faa7e6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 23:51:40 +0100 Subject: [PATCH 3/5] mpd: Refactor search result to (fake) tracks functionality --- mopidy/frontends/mpd/protocol/music_db.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index ca5ef730..b346d714 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -38,6 +38,13 @@ def _artist_as_track(artist): artists=[artist]) +def _search_results_as_tracks(results): + albums = [_album_as_track(a) for a in _get_albums(results)] + artists = [_artist_as_track(a) for a in _get_artists(results)] + tracks = _get_tracks(results) + return artists + albums + tracks + + @handle_request(r'^count "(?P[^"]+)" "(?P[^"]*)"$') def count(context, tag, needle): """ @@ -84,10 +91,7 @@ def find(context, mpd_query): except ValueError: return results = context.core.library.find_exact(**query).get() - albums = [_album_as_track(a) for a in _get_albums(results)] - artists = [_artist_as_track(a) for a in _get_artists(results)] - tracks = _get_tracks(results) - return translator.tracks_to_mpd_format(artists + albums + tracks) + return translator.tracks_to_mpd_format(_search_results_as_tracks(results)) @handle_request(r'^findadd ' + QUERY_RE) @@ -329,10 +333,7 @@ def search(context, mpd_query): except ValueError: return results = context.core.library.search(**query).get() - albums = [_album_as_track(a) for a in _get_albums(results)] - artists = [_artist_as_track(a) for a in _get_artists(results)] - tracks = _get_tracks(results) - return translator.tracks_to_mpd_format(artists + albums + tracks) + return translator.tracks_to_mpd_format(_search_results_as_tracks(results)) @handle_request(r'^searchadd ' + QUERY_RE) From 04be75ed97ed74cdb70d0aec165a547bf5f4a355 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Dec 2012 02:12:07 +0100 Subject: [PATCH 4/5] mpd: Add album date to 'fake' tracks --- mopidy/frontends/mpd/protocol/music_db.py | 3 ++- tests/frontends/mpd/protocol/music_db_test.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index b346d714..bbacaacd 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -27,8 +27,9 @@ def _album_as_track(album): return Track( uri=album.uri, name='Album: ' + album.name, + artists=album.artists, album=album, - artists=album.artists) + date=album.date) def _artist_as_track(artist): diff --git a/tests/frontends/mpd/protocol/music_db_test.py b/tests/frontends/mpd/protocol/music_db_test.py index fedc34a1..a641cb27 100644 --- a/tests/frontends/mpd/protocol/music_db_test.py +++ b/tests/frontends/mpd/protocol/music_db_test.py @@ -117,7 +117,7 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): class MusicDatabaseFindTest(protocol.BaseTestCase): def test_find(self): self.backend.library.dummy_find_exact_result = SearchResult( - albums=[Album(uri='dummy:album:a', name='A')], + albums=[Album(uri='dummy:album:a', name='A', date='2001')], artists=[Artist(uri='dummy:artist:b', name='B')], tracks=[Track(uri='dummy:track:c', name='C')]) @@ -125,8 +125,11 @@ class MusicDatabaseFindTest(protocol.BaseTestCase): self.assertInResponse('file: dummy:album:a') self.assertInResponse('Title: Album: A') + self.assertInResponse('Date: 2001') + self.assertInResponse('file: dummy:artist:b') self.assertInResponse('Title: Artist: B') + self.assertInResponse('file: dummy:track:c') self.assertInResponse('Title: C') From 54662479ef4dbd2dbbd9765c839a42316ec1d37b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Dec 2012 12:49:27 +0100 Subject: [PATCH 5/5] mpd: Limit use of fake tracks in 'find` responses If searching for exact artist, don't include fake artist tracks. If searching for exact album, don't include fake album tracks. This makes sure that ncmpcpp's media library doesn't include the magic artist-track in an artist's album listing, and that it doesn't include the magic album-track in an album's track listing. --- mopidy/frontends/mpd/protocol/music_db.py | 20 ++++---- tests/frontends/mpd/protocol/music_db_test.py | 46 +++++++++++++++++-- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index bbacaacd..c457ee02 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -39,13 +39,6 @@ def _artist_as_track(artist): artists=[artist]) -def _search_results_as_tracks(results): - albums = [_album_as_track(a) for a in _get_albums(results)] - artists = [_artist_as_track(a) for a in _get_artists(results)] - tracks = _get_tracks(results) - return artists + albums + tracks - - @handle_request(r'^count "(?P[^"]+)" "(?P[^"]*)"$') def count(context, tag, needle): """ @@ -92,7 +85,13 @@ def find(context, mpd_query): except ValueError: return results = context.core.library.find_exact(**query).get() - return translator.tracks_to_mpd_format(_search_results_as_tracks(results)) + result_tracks = [] + if 'artist' not in query: + result_tracks += [_artist_as_track(a) for a in _get_artists(results)] + if 'album' not in query: + result_tracks += [_album_as_track(a) for a in _get_albums(results)] + result_tracks += _get_tracks(results) + return translator.tracks_to_mpd_format(result_tracks) @handle_request(r'^findadd ' + QUERY_RE) @@ -334,7 +333,10 @@ def search(context, mpd_query): except ValueError: return results = context.core.library.search(**query).get() - return translator.tracks_to_mpd_format(_search_results_as_tracks(results)) + artists = [_artist_as_track(a) for a in _get_artists(results)] + albums = [_album_as_track(a) for a in _get_albums(results)] + tracks = _get_tracks(results) + return translator.tracks_to_mpd_format(artists + albums + tracks) @handle_request(r'^searchadd ' + QUERY_RE) diff --git a/tests/frontends/mpd/protocol/music_db_test.py b/tests/frontends/mpd/protocol/music_db_test.py index a641cb27..0a69b7cf 100644 --- a/tests/frontends/mpd/protocol/music_db_test.py +++ b/tests/frontends/mpd/protocol/music_db_test.py @@ -115,7 +115,7 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): class MusicDatabaseFindTest(protocol.BaseTestCase): - def test_find(self): + def test_find_includes_fake_artist_and_album_tracks(self): self.backend.library.dummy_find_exact_result = SearchResult( albums=[Album(uri='dummy:album:a', name='A', date='2001')], artists=[Artist(uri='dummy:artist:b', name='B')], @@ -123,12 +123,52 @@ class MusicDatabaseFindTest(protocol.BaseTestCase): self.sendRequest('find "any" "foo"') + self.assertInResponse('file: dummy:artist:b') + self.assertInResponse('Title: Artist: B') + self.assertInResponse('file: dummy:album:a') self.assertInResponse('Title: Album: A') self.assertInResponse('Date: 2001') - self.assertInResponse('file: dummy:artist:b') - self.assertInResponse('Title: Artist: B') + self.assertInResponse('file: dummy:track:c') + self.assertInResponse('Title: C') + + self.assertInResponse('OK') + + def test_find_artist_does_not_include_fake_artist_tracks(self): + self.backend.library.dummy_find_exact_result = SearchResult( + albums=[Album(uri='dummy:album:a', name='A', date='2001')], + artists=[Artist(uri='dummy:artist:b', name='B')], + tracks=[Track(uri='dummy:track:c', name='C')]) + + self.sendRequest('find "artist" "foo"') + + self.assertNotInResponse('file: dummy:artist:b') + self.assertNotInResponse('Title: Artist: B') + + self.assertInResponse('file: dummy:album:a') + self.assertInResponse('Title: Album: A') + self.assertInResponse('Date: 2001') + + self.assertInResponse('file: dummy:track:c') + self.assertInResponse('Title: C') + + self.assertInResponse('OK') + + def test_find_artist_and_album_does_not_include_fake_tracks(self): + self.backend.library.dummy_find_exact_result = SearchResult( + albums=[Album(uri='dummy:album:a', name='A', date='2001')], + artists=[Artist(uri='dummy:artist:b', name='B')], + tracks=[Track(uri='dummy:track:c', name='C')]) + + self.sendRequest('find "artist" "foo" "album" "bar"') + + self.assertNotInResponse('file: dummy:artist:b') + self.assertNotInResponse('Title: Artist: B') + + self.assertNotInResponse('file: dummy:album:a') + self.assertNotInResponse('Title: Album: A') + self.assertNotInResponse('Date: 2001') self.assertInResponse('file: dummy:track:c') self.assertInResponse('Title: C')