From 52b20b3297557d6bebd395419fe34da74fd18fe8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 20 Dec 2012 23:36:45 +0100 Subject: [PATCH 01/26] models: Add SearchResult model --- docs/changes.rst | 3 +++ mopidy/models.py | 31 +++++++++++++++++++++++++++ tests/models_test.py | 50 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 266f73f2..8df821f1 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -64,6 +64,9 @@ v0.11.0 (in development) - Specified that :attr:`mopidy.models.Playlist.last_modified` should be in UTC. +- Added :class:`mopidy.models.SearchResult` model to encapsulate search results + consisting of more than just tracks. + *Core API:* - Change the following methods to accept either a dict with filters or kwargs. diff --git a/mopidy/models.py b/mopidy/models.py index e47ed3be..73209b6e 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -318,3 +318,34 @@ class Playlist(ImmutableObject): def length(self): """The number of tracks in the playlist. Read-only.""" return len(self.tracks) + + +class SearchResult(ImmutableObject): + """ + :param uri: search result URI + :type uri: string + :param tracks: matching tracks + :type tracks: list of :class:`Track` elements + :param artists: matching artists + :type artists: list of :class:`Artist` elements + :param albums: matching albums + :type albums: list of :class:`Album` elements + """ + + # The search result URI. Read-only. + uri = None + + # The tracks matching the search query. Read-only. + tracks = tuple() + + # The artists matching the search query. Read-only. + artists = tuple() + + # The albums matching the search query. Read-only. + albums = tuple() + + def __init__(self, *args, **kwargs): + self.__dict__['tracks'] = tuple(kwargs.pop('tracks', [])) + self.__dict__['artists'] = tuple(kwargs.pop('artists', [])) + self.__dict__['albums'] = tuple(kwargs.pop('albums', [])) + super(SearchResult, self).__init__(*args, **kwargs) diff --git a/tests/models_test.py b/tests/models_test.py index 1a4d869a..89d0b132 100644 --- a/tests/models_test.py +++ b/tests/models_test.py @@ -4,7 +4,7 @@ import datetime import json from mopidy.models import ( - Artist, Album, TlTrack, Track, Playlist, + Artist, Album, TlTrack, Track, Playlist, SearchResult, ModelJSONEncoder, model_json_decoder) from tests import unittest @@ -862,10 +862,56 @@ class PlaylistTest(unittest.TestCase): def test_ne(self): playlist1 = Playlist( - uri='uri1', name='name2', tracks=[Track(uri='uri1')], + uri='uri1', name='name1', tracks=[Track(uri='uri1')], last_modified=1) playlist2 = Playlist( uri='uri2', name='name2', tracks=[Track(uri='uri2')], last_modified=2) self.assertNotEqual(playlist1, playlist2) self.assertNotEqual(hash(playlist1), hash(playlist2)) + + +class SearchResultTest(unittest.TestCase): + def test_uri(self): + uri = 'an_uri' + result = SearchResult(uri=uri) + self.assertEqual(result.uri, uri) + self.assertRaises(AttributeError, setattr, result, 'uri', None) + + def test_tracks(self): + tracks = [Track(), Track(), Track()] + result = SearchResult(tracks=tracks) + self.assertEqual(list(result.tracks), tracks) + self.assertRaises(AttributeError, setattr, result, 'tracks', None) + + def test_artists(self): + artists = [Artist(), Artist(), Artist()] + result = SearchResult(artists=artists) + self.assertEqual(list(result.artists), artists) + self.assertRaises(AttributeError, setattr, result, 'artists', None) + + def test_albums(self): + albums = [Album(), Album(), Album()] + result = SearchResult(albums=albums) + self.assertEqual(list(result.albums), albums) + self.assertRaises(AttributeError, setattr, result, 'albums', None) + + def test_invalid_kwarg(self): + test = lambda: SearchResult(foo='baz') + self.assertRaises(TypeError, test) + + def test_repr_without_results(self): + self.assertEquals( + "SearchResult(albums=[], artists=[], tracks=[], uri=u'uri')", + repr(SearchResult(uri='uri'))) + + def test_serialize_without_results(self): + self.assertDictEqual( + {'__model__': 'SearchResult', 'uri': 'uri'}, + SearchResult(uri='uri').serialize()) + + def test_to_json_and_back(self): + result1 = SearchResult(uri='uri') + serialized = json.dumps(result1, cls=ModelJSONEncoder) + result2 = json.loads(serialized, object_hook=model_json_decoder) + self.assertEqual(result1, result2) From b0ba2040dfc387ec55163a5462d732d14ca00380 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 00:28:24 +0100 Subject: [PATCH 02/26] Return SearchResult objects from find_exact() and search() This applies to both backends and core. --- docs/changes.rst | 6 ++ mopidy/backends/dummy.py | 6 +- mopidy/backends/local/library.py | 6 +- mopidy/backends/spotify/library.py | 27 +++---- mopidy/core/library.py | 11 ++- mopidy/frontends/mpd/protocol/music_db.py | 38 +++++----- tests/backends/base/library.py | 70 +++++++++---------- tests/core/library_test.py | 46 +++++++----- tests/frontends/mpd/protocol/music_db_test.py | 33 ++++----- 9 files changed, 131 insertions(+), 112 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 8df821f1..8f887ed5 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -69,6 +69,12 @@ v0.11.0 (in development) *Core API:* +- Change the following methods to return :class:`mopidy.models.SearchResult` + objects which can include both track results and other results: + + - :meth:`mopidy.core.LibraryController.find_exact` + - :meth:`mopidy.core.LibraryController.search` + - Change the following methods to accept either a dict with filters or kwargs. Previously they only accepted kwargs, which made them impossible to use from the Mopidy.js through JSON-RPC, which doesn't support kwargs. diff --git a/mopidy/backends/dummy.py b/mopidy/backends/dummy.py index 39180bbb..c6997b12 100644 --- a/mopidy/backends/dummy.py +++ b/mopidy/backends/dummy.py @@ -19,7 +19,7 @@ from __future__ import unicode_literals import pykka from mopidy.backends import base -from mopidy.models import Playlist +from mopidy.models import Playlist, SearchResult class DummyBackend(pykka.ThreadingActor, base.Backend): @@ -37,8 +37,8 @@ class DummyLibraryProvider(base.BaseLibraryProvider): def __init__(self, *args, **kwargs): super(DummyLibraryProvider, self).__init__(*args, **kwargs) self.dummy_library = [] - self.dummy_find_exact_result = [] - self.dummy_search_result = [] + self.dummy_find_exact_result = SearchResult() + self.dummy_search_result = SearchResult() def find_exact(self, **query): return self.dummy_find_exact_result diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index 143c6d84..ad81efea 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -4,7 +4,7 @@ import logging from mopidy import settings from mopidy.backends import base -from mopidy.models import Album +from mopidy.models import Album, SearchResult from .translator import parse_mpd_tag_cache @@ -70,7 +70,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return result_tracks + return SearchResult(tracks=result_tracks) def search(self, **query): self._validate_query(query) @@ -107,7 +107,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return result_tracks + return SearchResult(tracks=result_tracks) def _validate_query(self, query): for (_, values) in query.iteritems(): diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 81587e00..0e009fd9 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -8,7 +8,7 @@ from spotify import Link, SpotifyError from mopidy import settings from mopidy.backends import base -from mopidy.models import Track +from mopidy.models import Track, SearchResult from . import translator @@ -121,11 +121,10 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): return self._get_all_tracks() if 'uri' in query.keys(): - result = [] + tracks = [] for uri in query['uri']: - tracks = self.lookup(uri) - result += tracks - return result + tracks += self.lookup(uri) + return SearchResult(tracks=tracks) spotify_query = self._translate_search_query(query) logger.debug('Spotify search query: %s' % spotify_query) @@ -133,12 +132,14 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): future = pykka.ThreadingFuture() def callback(results, userdata=None): - # TODO Include results from results.albums(), etc. too - # TODO Consider launching a second search if results.total_tracks() - # is larger than len(results.tracks()) - tracks = [ - translator.to_mopidy_track(t) for t in results.tracks()] - future.set(tracks) + search_result = SearchResult( + albums=[ + translator.to_mopidy_album(a) for a in results.albums()], + artists=[ + translator.to_mopidy_artist(a) for a in results.artists()], + tracks=[ + translator.to_mopidy_track(t) for t in results.tracks()]) + future.set(search_result) if not self.backend.spotify.connected.wait(settings.SPOTIFY_TIMEOUT): logger.debug('Not connected: Spotify search cancelled') @@ -146,7 +147,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): self.backend.spotify.session.search( spotify_query, callback, - track_count=200, album_count=0, artist_count=0) + album_count=200, artist_count=200, track_count=200) try: return future.get(timeout=settings.SPOTIFY_TIMEOUT) @@ -154,7 +155,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): logger.debug( 'Timeout: Spotify search did not return in %ds', settings.SPOTIFY_TIMEOUT) - return [] + return SearchResult(uri='spotify:search') def _get_all_tracks(self): # Since we can't search for the entire Spotify library, we return diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 3c596a3a..39a1e99c 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -import itertools import urlparse import pykka @@ -37,13 +36,12 @@ class LibraryController(object): :param query: one or more queries to search for :type query: dict - :rtype: list of :class:`mopidy.models.Track` + :rtype: list of :class:`mopidy.models.SearchResult` """ query = query or kwargs futures = [ b.library.find_exact(**query) for b in self.backends.with_library] - results = pykka.get_all(futures) - return list(itertools.chain(*results)) + return pykka.get_all(futures) def lookup(self, uri): """ @@ -98,10 +96,9 @@ class LibraryController(object): :param query: one or more queries to search for :type query: dict - :rtype: list of :class:`mopidy.models.Track` + :rtype: list of :class:`mopidy.models.SearchResult` """ query = query or kwargs futures = [ b.library.search(**query) for b in self.backends.with_library] - results = pykka.get_all(futures) - return list(itertools.chain(*results)) + return pykka.get_all(futures) diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index 393561de..f9149a50 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -1,5 +1,7 @@ from __future__ import unicode_literals +import itertools + from mopidy.frontends.mpd import translator from mopidy.frontends.mpd.exceptions import MpdNotImplemented from mopidy.frontends.mpd.protocol import handle_request, stored_playlists @@ -10,6 +12,10 @@ QUERY_RE = ( r'[Tt]itle|[Aa]ny)"? "[^"]*"\s?)+)$') +def _get_tracks(search_results): + return list(itertools.chain(*[r.tracks for r in search_results])) + + @handle_request(r'^count "(?P[^"]+)" "(?P[^"]*)"$') def count(context, tag, needle): """ @@ -55,8 +61,8 @@ def find(context, mpd_query): query = translator.query_from_mpd_search_format(mpd_query) except ValueError: return - result = context.core.library.find_exact(**query).get() - return translator.tracks_to_mpd_format(result) + results = context.core.library.find_exact(**query).get() + return translator.tracks_to_mpd_format(_get_tracks(results)) @handle_request(r'^findadd ' + QUERY_RE) @@ -73,8 +79,8 @@ def findadd(context, mpd_query): query = translator.query_from_mpd_search_format(mpd_query) except ValueError: return - result = context.core.library.find_exact(**query).get() - context.core.tracklist.add(result) + results = context.core.library.find_exact(**query).get() + context.core.tracklist.add(_get_tracks(results)) @handle_request( @@ -179,8 +185,8 @@ def list_(context, field, mpd_query=None): def _list_artist(context, query): artists = set() - tracks = context.core.library.find_exact(**query).get() - for track in tracks: + 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)) @@ -189,8 +195,8 @@ def _list_artist(context, query): def _list_album(context, query): albums = set() - tracks = context.core.library.find_exact(**query).get() - for track in tracks: + 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 @@ -198,8 +204,8 @@ def _list_album(context, query): def _list_date(context, query): dates = set() - tracks = context.core.library.find_exact(**query).get() - for track in tracks: + results = context.core.library.find_exact(**query).get() + for track in _get_tracks(results): if track.date: dates.add(('Date', track.date)) return dates @@ -297,8 +303,8 @@ def search(context, mpd_query): query = translator.query_from_mpd_search_format(mpd_query) except ValueError: return - result = context.core.library.search(**query).get() - return translator.tracks_to_mpd_format(result) + results = context.core.library.search(**query).get() + return translator.tracks_to_mpd_format(_get_tracks(results)) @handle_request(r'^searchadd ' + QUERY_RE) @@ -318,8 +324,8 @@ def searchadd(context, mpd_query): query = translator.query_from_mpd_search_format(mpd_query) except ValueError: return - result = context.core.library.search(**query).get() - context.core.tracklist.add(result) + results = context.core.library.search(**query).get() + context.core.tracklist.add(_get_tracks(results)) @handle_request(r'^searchaddpl "(?P[^"]+)" ' + QUERY_RE) @@ -341,14 +347,14 @@ def searchaddpl(context, playlist_name, mpd_query): query = translator.query_from_mpd_search_format(mpd_query) except ValueError: return - result = context.core.library.search(**query).get() + results = context.core.library.search(**query).get() playlists = context.core.playlists.filter(name=playlist_name).get() if playlists: playlist = playlists[0] else: playlist = context.core.playlists.create(playlist_name).get() - tracks = list(playlist.tracks) + result + tracks = list(playlist.tracks) + _get_tracks(results) playlist = playlist.copy(tracks=tracks) context.core.playlists.save(playlist) diff --git a/tests/backends/base/library.py b/tests/backends/base/library.py index 57aec3c6..c75bec74 100644 --- a/tests/backends/base/library.py +++ b/tests/backends/base/library.py @@ -53,53 +53,53 @@ class LibraryControllerTest(object): def test_find_exact_no_hits(self): result = self.library.find_exact(track=['unknown track']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.find_exact(artist=['unknown artist']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.find_exact(album=['unknown artist']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) def test_find_exact_uri(self): track_1_uri = 'file://' + path_to_data_dir('uri1') result = self.library.find_exact(uri=track_1_uri) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) track_2_uri = 'file://' + path_to_data_dir('uri2') result = self.library.find_exact(uri=track_2_uri) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_track(self): result = self.library.find_exact(track=['track1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.find_exact(track=['track2']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_artist(self): result = self.library.find_exact(artist=['artist1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.find_exact(artist=['artist2']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_album(self): result = self.library.find_exact(album=['album1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.find_exact(album=['album2']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_date(self): result = self.library.find_exact(date=['2001']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.find_exact(date=['2001-02-03']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.find_exact(date=['2002']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_wrong_type(self): test = lambda: self.library.find_exact(wrong=['test']) @@ -117,70 +117,70 @@ class LibraryControllerTest(object): def test_search_no_hits(self): result = self.library.search(track=['unknown track']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.search(artist=['unknown artist']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.search(album=['unknown artist']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.search(uri=['unknown']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.search(any=['unknown']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) def test_search_uri(self): result = self.library.search(uri=['RI1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(uri=['RI2']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_track(self): result = self.library.search(track=['Rack1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(track=['Rack2']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_artist(self): result = self.library.search(artist=['Tist1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(artist=['Tist2']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_album(self): result = self.library.search(album=['Bum1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(album=['Bum2']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_date(self): result = self.library.search(date=['2001']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(date=['2001-02-03']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(date=['2001-02-04']) - self.assertEqual(result, []) + self.assertEqual(list(result[0].tracks), []) result = self.library.search(date=['2002']) - self.assertEqual(result, self.tracks[1:2]) + self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_any(self): result = self.library.search(any=['Tist1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(any=['Rack1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(any=['Bum1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) result = self.library.search(any=['RI1']) - self.assertEqual(result, self.tracks[:1]) + self.assertEqual(list(result[0].tracks), self.tracks[:1]) def test_search_wrong_type(self): test = lambda: self.library.search(wrong=['test']) diff --git a/tests/core/library_test.py b/tests/core/library_test.py index a2c358d7..32e618d2 100644 --- a/tests/core/library_test.py +++ b/tests/core/library_test.py @@ -4,7 +4,7 @@ import mock from mopidy.backends import base from mopidy.core import Core -from mopidy.models import Track +from mopidy.models import SearchResult, Track from tests import unittest @@ -75,59 +75,71 @@ class CoreLibraryTest(unittest.TestCase): def test_find_exact_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') track2 = Track(uri='dummy2:a') - self.library1.find_exact().get.return_value = [track1] + result1 = SearchResult(tracks=[track1]) + result2 = SearchResult(tracks=[track2]) + + self.library1.find_exact().get.return_value = result1 self.library1.find_exact.reset_mock() - self.library2.find_exact().get.return_value = [track2] + self.library2.find_exact().get.return_value = result2 self.library2.find_exact.reset_mock() result = self.core.library.find_exact(any=['a']) - self.assertIn(track1, result) - self.assertIn(track2, result) + self.assertIn(result1, result) + self.assertIn(result2, result) self.library1.find_exact.assert_called_once_with(any=['a']) self.library2.find_exact.assert_called_once_with(any=['a']) def test_find_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') track2 = Track(uri='dummy2:a') - self.library1.find_exact().get.return_value = [track1] + result1 = SearchResult(tracks=[track1]) + result2 = SearchResult(tracks=[track2]) + + self.library1.find_exact().get.return_value = result1 self.library1.find_exact.reset_mock() - self.library2.find_exact().get.return_value = [track2] + self.library2.find_exact().get.return_value = result2 self.library2.find_exact.reset_mock() result = self.core.library.find_exact(dict(any=['a'])) - self.assertIn(track1, result) - self.assertIn(track2, result) + self.assertIn(result1, result) + self.assertIn(result2, result) self.library1.find_exact.assert_called_once_with(any=['a']) self.library2.find_exact.assert_called_once_with(any=['a']) def test_search_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') track2 = Track(uri='dummy2:a') - self.library1.search().get.return_value = [track1] + result1 = SearchResult(tracks=[track1]) + result2 = SearchResult(tracks=[track2]) + + self.library1.search().get.return_value = result1 self.library1.search.reset_mock() - self.library2.search().get.return_value = [track2] + self.library2.search().get.return_value = result2 self.library2.search.reset_mock() result = self.core.library.search(any=['a']) - self.assertIn(track1, result) - self.assertIn(track2, result) + self.assertIn(result1, result) + self.assertIn(result2, result) self.library1.search.assert_called_once_with(any=['a']) self.library2.search.assert_called_once_with(any=['a']) def test_search_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') track2 = Track(uri='dummy2:a') - self.library1.search().get.return_value = [track1] + result1 = SearchResult(tracks=[track1]) + result2 = SearchResult(tracks=[track2]) + + self.library1.search().get.return_value = result1 self.library1.search.reset_mock() - self.library2.search().get.return_value = [track2] + self.library2.search().get.return_value = result2 self.library2.search.reset_mock() result = self.core.library.search(dict(any=['a'])) - self.assertIn(track1, result) - self.assertIn(track2, result) + self.assertIn(result1, result) + self.assertIn(result2, result) self.library1.search.assert_called_once_with(any=['a']) self.library2.search.assert_called_once_with(any=['a']) diff --git a/tests/frontends/mpd/protocol/music_db_test.py b/tests/frontends/mpd/protocol/music_db_test.py index 5c887958..86fd8ad7 100644 --- a/tests/frontends/mpd/protocol/music_db_test.py +++ b/tests/frontends/mpd/protocol/music_db_test.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from mopidy.models import Album, Artist, Track +from mopidy.models import Album, Artist, SearchResult, Track from tests.frontends.mpd import protocol @@ -13,9 +13,8 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_findadd(self): - self.backend.library.dummy_find_exact_result = [ - Track(uri='dummy:a', name='A'), - ] + self.backend.library.dummy_find_exact_result = SearchResult( + tracks=[Track(uri='dummy:a', name='A')]) self.assertEqual(self.core.tracklist.length.get(), 0) self.sendRequest('findadd "title" "A"') @@ -25,9 +24,8 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_searchadd(self): - self.backend.library.dummy_search_result = [ - Track(uri='dummy:a', name='A'), - ] + self.backend.library.dummy_search_result = SearchResult( + tracks=[Track(uri='dummy:a', name='A')]) self.assertEqual(self.core.tracklist.length.get(), 0) self.sendRequest('searchadd "title" "a"') @@ -43,9 +41,8 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): Track(uri='dummy:y', name='y'), ]) self.core.playlists.save(playlist) - self.backend.library.dummy_search_result = [ - Track(uri='dummy:a', name='A'), - ] + self.backend.library.dummy_search_result = SearchResult( + tracks=[Track(uri='dummy:a', name='A')]) playlists = self.core.playlists.filter(name='my favs').get() self.assertEqual(len(playlists), 1) self.assertEqual(len(playlists[0].tracks), 2) @@ -61,9 +58,8 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_searchaddpl_creates_missing_playlist(self): - self.backend.library.dummy_search_result = [ - Track(uri='dummy:a', name='A'), - ] + self.backend.library.dummy_search_result = SearchResult( + tracks=[Track(uri='dummy:a', name='A')]) self.assertEqual( len(self.core.playlists.filter(name='my favs').get()), 0) @@ -242,8 +238,8 @@ class MusicDatabaseListTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_list_artist_should_not_return_artists_without_names(self): - self.backend.library.dummy_find_exact_result = [ - Track(artists=[Artist(name='')])] + self.backend.library.dummy_find_exact_result = SearchResult( + tracks=[Track(artists=[Artist(name='')])]) self.sendRequest('list "artist"') self.assertNotInResponse('Artist: ') @@ -301,8 +297,8 @@ class MusicDatabaseListTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_list_album_should_not_return_albums_without_names(self): - self.backend.library.dummy_find_exact_result = [ - Track(album=Album(name=''))] + self.backend.library.dummy_find_exact_result = SearchResult( + tracks=[Track(album=Album(name=''))]) self.sendRequest('list "album"') self.assertNotInResponse('Album: ') @@ -356,7 +352,8 @@ class MusicDatabaseListTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_list_date_should_not_return_blank_dates(self): - self.backend.library.dummy_find_exact_result = [Track(date='')] + self.backend.library.dummy_find_exact_result = SearchResult( + tracks=[Track(date='')]) self.sendRequest('list "date"') self.assertNotInResponse('Date: ') From 71f27d5625328205e4d1158c20d56a22fce89fb3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 11:13:25 +0100 Subject: [PATCH 03/26] local: Add uri to SearchResults --- mopidy/backends/local/actor.py | 2 +- mopidy/backends/local/library.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index 75baeab2..c664fb99 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -20,4 +20,4 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): self.playback = base.BasePlaybackProvider(audio=audio, backend=self) self.playlists = LocalPlaylistsProvider(backend=self) - self.uri_schemes = ['file'] + self.uri_schemes = ['file', 'local'] diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index ad81efea..2295dfb5 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -70,7 +70,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return SearchResult(tracks=result_tracks) + return SearchResult(uri='local:search', tracks=result_tracks) def search(self, **query): self._validate_query(query) @@ -107,7 +107,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return SearchResult(tracks=result_tracks) + return SearchResult(uri='local:search', tracks=result_tracks) def _validate_query(self, query): for (_, values) in query.iteritems(): From e804333897b53379ac7f6cbd6f62fbf78fe4f92a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 21:57:20 +0100 Subject: [PATCH 04/26] spotify: Add uri to SearchResult --- mopidy/backends/spotify/library.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 0e009fd9..55f704f7 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals import logging import time +import urllib import pykka from spotify import Link, SpotifyError @@ -124,7 +125,11 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): tracks = [] for uri in query['uri']: tracks += self.lookup(uri) - return SearchResult(tracks=tracks) + if len(query['uri']) == 1: + uri = query['uri'] + else: + uri = 'spotify:search' + return SearchResult(uri=uri, tracks=tracks) spotify_query = self._translate_search_query(query) logger.debug('Spotify search query: %s' % spotify_query) @@ -133,6 +138,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): def callback(results, userdata=None): search_result = SearchResult( + uri='spotify:search:' + urllib.quote(results.query()), albums=[ translator.to_mopidy_album(a) for a in results.albums()], artists=[ From a8c0f6baa808dea2d13a5387dc887ef1082f806c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 22:25:21 +0100 Subject: [PATCH 05/26] spotify: Make query a bytestring before urlencoding it --- mopidy/backends/spotify/library.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 55f704f7..5dccc25e 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -138,7 +138,8 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): def callback(results, userdata=None): search_result = SearchResult( - uri='spotify:search:' + urllib.quote(results.query()), + uri='spotify:search:%s' % ( + urllib.quote(results.query().encode('utf-8'))), albums=[ translator.to_mopidy_album(a) for a in results.albums()], artists=[ From 455f0145e71e77121502556b05c2c98bc33b179c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 23:08:18 +0100 Subject: [PATCH 06/26] 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 07/26] 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 08/26] 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 357a26d7f9c39fe83c418a634b2f86fe858bb8b9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Dec 2012 01:40:19 +0100 Subject: [PATCH 09/26] spotify: Fix improper search() return value --- mopidy/backends/spotify/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 5dccc25e..0af76e4b 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -170,7 +170,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): tracks = [] for playlist in self.backend.playlists.playlists: tracks += playlist.tracks - return tracks + return SearchResult(uri='spotify:search', tracks=tracks) def _translate_search_query(self, mopidy_query): spotify_query = [] From 4f4754c57396d183ab8d94c36d57f279121d6c6e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Dec 2012 01:40:51 +0100 Subject: [PATCH 10/26] mpd: Test 'list' response content --- tests/frontends/mpd/protocol/music_db_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/frontends/mpd/protocol/music_db_test.py b/tests/frontends/mpd/protocol/music_db_test.py index 86fd8ad7..58bb33e8 100644 --- a/tests/frontends/mpd/protocol/music_db_test.py +++ b/tests/frontends/mpd/protocol/music_db_test.py @@ -181,6 +181,17 @@ 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.sendRequest('list "artist" "artist" "foo"') + + self.assertInResponse('Artist: A Artist') + self.assertInResponse('OK') + def test_list_foo_returns_ack(self): self.sendRequest('list "foo"') self.assertEqualResponse('ACK [2@0] {list} incorrect arguments') From 04be75ed97ed74cdb70d0aec165a547bf5f4a355 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Dec 2012 02:12:07 +0100 Subject: [PATCH 11/26] 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 12/26] 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') From 5d707e39186d60ee1db1468ecc4ba40ea45feded Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 15:42:49 +0100 Subject: [PATCH 13/26] settings: Fail if BACKENDS/FRONTENDS setting isn't iterable (fixes #278) --- docs/changes.rst | 9 +++++++++ mopidy/utils/settings.py | 4 ++++ tests/utils/settings_test.py | 8 ++++++++ 3 files changed, 21 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index 2dd6d940..1da3dacc 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -8,6 +8,15 @@ This change log is used to track all major changes to Mopidy. v0.11.0 (in development) ======================== +**Settings** + +- The settings validator now complains if a setting which expects a tuple of + values (e.g. :attr:`mopidy.settings.BACKENDS`, + :attr:`mopidy.settings.FRONTENDS`) has a non-iterable value. This typically + happens because the setting value contains a single value and one has + forgotten to add a comma after the string, making the value a tuple. (Fixes: + :issue:`278`) + **Spotify backend** - Add :attr:`mopidy.settings.SPOTIFY_TIMEOUT` setting which allows you to diff --git a/mopidy/utils/settings.py b/mopidy/utils/settings.py index fee5252d..6eb462ce 100644 --- a/mopidy/utils/settings.py +++ b/mopidy/utils/settings.py @@ -172,6 +172,10 @@ def validate_settings(defaults, settings): 'bin in OUTPUT.') elif setting in list_of_one_or_more: + if not hasattr(value, '__iter__'): + errors[setting] = ( + 'Must be a tuple. ' + "Remember the comma after single values: (u'value',)") if not value: errors[setting] = 'Must contain at least one value.' diff --git a/tests/utils/settings_test.py b/tests/utils/settings_test.py index 0ecbb90f..1dcac1bb 100644 --- a/tests/utils/settings_test.py +++ b/tests/utils/settings_test.py @@ -87,6 +87,14 @@ class ValidateSettingsTest(unittest.TestCase): self.assertEqual( result['BACKENDS'], 'Must contain at least one value.') + def test_noniterable_multivalue_setting_returns_error(self): + result = setting_utils.validate_settings( + self.defaults, {'FRONTENDS': ('this is not a tuple')}) + self.assertEqual( + result['FRONTENDS'], + 'Must be a tuple. ' + "Remember the comma after single values: (u'value',)") + class SettingsProxyTest(unittest.TestCase): def setUp(self): From c81d1d77bff2cb660d59c8d937b54307c8f49146 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 16:30:59 +0100 Subject: [PATCH 14/26] fab: Make 'test' and 'autotest' able to run a subset of the tests --- fabfile.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/fabfile.py b/fabfile.py index 267bdc23..370c81be 100644 --- a/fabfile.py +++ b/fabfile.py @@ -1,14 +1,15 @@ from fabric.api import local -def test(): - local('nosetests tests/') +def test(path=None): + path = path or 'tests/' + local('nosetests ' + path) -def autotest(): +def autotest(path=None): while True: local('clear') - test() + test(path) local( 'inotifywait -q -e create -e modify -e delete ' '--exclude ".*\.(pyc|sw.)" -r mopidy/ tests/') From 524bfc931797c35e5371e37f4e699ee5ffbca51d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 18:32:52 +0100 Subject: [PATCH 15/26] local: Use 'file:search' as uri for search results for now --- mopidy/backends/local/actor.py | 2 +- mopidy/backends/local/library.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/backends/local/actor.py b/mopidy/backends/local/actor.py index c664fb99..75baeab2 100644 --- a/mopidy/backends/local/actor.py +++ b/mopidy/backends/local/actor.py @@ -20,4 +20,4 @@ class LocalBackend(pykka.ThreadingActor, base.Backend): self.playback = base.BasePlaybackProvider(audio=audio, backend=self) self.playlists = LocalPlaylistsProvider(backend=self) - self.uri_schemes = ['file', 'local'] + self.uri_schemes = ['file'] diff --git a/mopidy/backends/local/library.py b/mopidy/backends/local/library.py index 2295dfb5..eb328ce2 100644 --- a/mopidy/backends/local/library.py +++ b/mopidy/backends/local/library.py @@ -70,7 +70,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return SearchResult(uri='local:search', tracks=result_tracks) + return SearchResult(uri='file:search', tracks=result_tracks) def search(self, **query): self._validate_query(query) @@ -107,7 +107,7 @@ class LocalLibraryProvider(base.BaseLibraryProvider): result_tracks = filter(any_filter, result_tracks) else: raise LookupError('Invalid lookup field: %s' % field) - return SearchResult(uri='local:search', tracks=result_tracks) + return SearchResult(uri='file:search', tracks=result_tracks) def _validate_query(self, query): for (_, values) in query.iteritems(): From eec6c271c2b2c9ab3ff46c3952e66cbe1b68f234 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 18:41:07 +0100 Subject: [PATCH 16/26] spotify: Refactor URI lookup --- mopidy/backends/spotify/library.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 0af76e4b..45ec0563 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -121,12 +121,13 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): if not query: return self._get_all_tracks() - if 'uri' in query.keys(): + uris = query.get('uri', []) + if uris: tracks = [] - for uri in query['uri']: + for uri in uris: tracks += self.lookup(uri) - if len(query['uri']) == 1: - uri = query['uri'] + if len(uris) == 1: + uri = uris[0] else: uri = 'spotify:search' return SearchResult(uri=uri, tracks=tracks) From 8da2495e833ebb1115a1d0f59a660cd1d80f2f98 Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Mon, 24 Dec 2012 00:29:37 +0100 Subject: [PATCH 17/26] spotify: Only return available tracks from lookups --- mopidy/backends/spotify/library.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index a42fc21f..8e8e47f9 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -84,7 +84,10 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): track = Link.from_string(uri).as_track() self._wait_for_object_to_load(track) if track.is_loaded(): - return [SpotifyTrack(track=track)] + if track.availability() == 1: + return [SpotifyTrack(track=track)] + else: + return None else: return [SpotifyTrack(uri=uri)] @@ -92,18 +95,18 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): album = Link.from_string(uri).as_album() album_browser = self.backend.spotify.session.browse_album(album) self._wait_for_object_to_load(album_browser) - return [SpotifyTrack(track=t) for t in album_browser] + return [SpotifyTrack(track=t) for t in album_browser if t.availability() == 1] def _lookup_artist(self, uri): artist = Link.from_string(uri).as_artist() artist_browser = self.backend.spotify.session.browse_artist(artist) self._wait_for_object_to_load(artist_browser) - return [SpotifyTrack(track=t) for t in artist_browser] + return [SpotifyTrack(track=t) for t in artist_browser if t.availability() == 1] def _lookup_playlist(self, uri): playlist = Link.from_string(uri).as_playlist() self._wait_for_object_to_load(playlist) - return [SpotifyTrack(track=t) for t in playlist] + return [SpotifyTrack(track=t) for t in playlist if t.availability() == 1] def _wait_for_object_to_load( self, spotify_obj, timeout=settings.SPOTIFY_TIMEOUT): From 30a78ba84b1fd819c8b03fd2fe2b3bf57ce49b4d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 20:56:49 +0100 Subject: [PATCH 18/26] mpd: Minor refactoring --- mopidy/frontends/mpd/protocol/playback.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index b8153dc9..8e08585f 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -329,8 +329,7 @@ def seek(context, songpos, seconds): - issues ``seek 1 120`` without quotes around the arguments. """ - songpos = int(songpos) - if context.core.playback.tracklist_position.get() != songpos: + if context.core.playback.tracklist_position.get() != int(songpos): playpos(context, songpos) context.core.playback.seek(int(seconds) * 1000).get() @@ -344,9 +343,8 @@ def seekid(context, tlid, seconds): Seeks to the position ``TIME`` (in seconds) of song ``SONGID``. """ - tlid = int(tlid) tl_track = context.core.playback.current_tl_track.get() - if not tl_track or tl_track.tlid != tlid: + if not tl_track or tl_track.tlid != int(tlid): playid(context, tlid) context.core.playback.seek(int(seconds) * 1000).get() From fdd4ac19ae630d679f6f5e7e22eab56415f5c3da Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 01:21:08 +0100 Subject: [PATCH 19/26] spotify: Fix wrong search return type --- mopidy/backends/spotify/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index a42fc21f..45835d04 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -154,7 +154,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): if not self.backend.spotify.connected.wait(settings.SPOTIFY_TIMEOUT): logger.debug('Not connected: Spotify search cancelled') - return [] + return SearchResult(uri='spotify:search') self.backend.spotify.session.search( spotify_query, callback, From 2a487ecd303e09f4471a887ed02801c5f29d901b Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Mon, 24 Dec 2012 01:39:56 +0100 Subject: [PATCH 20/26] spotify: Fix flake8 warnings --- mopidy/backends/spotify/library.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 8e8e47f9..f3406821 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -95,18 +95,24 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): album = Link.from_string(uri).as_album() album_browser = self.backend.spotify.session.browse_album(album) self._wait_for_object_to_load(album_browser) - return [SpotifyTrack(track=t) for t in album_browser if t.availability() == 1] + return [ + SpotifyTrack(track=t) + for t in album_browser if t.availability() == 1] def _lookup_artist(self, uri): artist = Link.from_string(uri).as_artist() artist_browser = self.backend.spotify.session.browse_artist(artist) self._wait_for_object_to_load(artist_browser) - return [SpotifyTrack(track=t) for t in artist_browser if t.availability() == 1] + return [ + SpotifyTrack(track=t) + for t in artist_browser if t.availability() == 1] def _lookup_playlist(self, uri): playlist = Link.from_string(uri).as_playlist() self._wait_for_object_to_load(playlist) - return [SpotifyTrack(track=t) for t in playlist if t.availability() == 1] + return [ + SpotifyTrack(track=t) + for t in playlist if t.availability() == 1] def _wait_for_object_to_load( self, spotify_obj, timeout=settings.SPOTIFY_TIMEOUT): From 75279721fb41d88bf27fe3dd8629f49a8485cb57 Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Mon, 24 Dec 2012 01:41:08 +0100 Subject: [PATCH 21/26] spotify: Return [] instead of None in _lookup_track --- mopidy/backends/spotify/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index f3406821..a39d674a 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -87,7 +87,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): if track.availability() == 1: return [SpotifyTrack(track=track)] else: - return None + return [] else: return [SpotifyTrack(uri=uri)] From 31ddbbc0171d2daea85d891d81de762f1dcec59e Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Mon, 24 Dec 2012 02:07:34 +0100 Subject: [PATCH 22/26] spotify: Use TRACK_AVAILABLE constant --- mopidy/backends/spotify/library.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index a39d674a..044e51d9 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -15,6 +15,8 @@ from . import translator logger = logging.getLogger('mopidy.backends.spotify') +TRACK_AVAILABLE = 1 + class SpotifyTrack(Track): """Proxy object for unloaded Spotify tracks.""" @@ -84,7 +86,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): track = Link.from_string(uri).as_track() self._wait_for_object_to_load(track) if track.is_loaded(): - if track.availability() == 1: + if track.availability() == TRACK_AVAILABLE: return [SpotifyTrack(track=track)] else: return [] @@ -97,7 +99,7 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): self._wait_for_object_to_load(album_browser) return [ SpotifyTrack(track=t) - for t in album_browser if t.availability() == 1] + for t in album_browser if t.availability() == TRACK_AVAILABLE] def _lookup_artist(self, uri): artist = Link.from_string(uri).as_artist() @@ -105,14 +107,14 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): self._wait_for_object_to_load(artist_browser) return [ SpotifyTrack(track=t) - for t in artist_browser if t.availability() == 1] + for t in artist_browser if t.availability() == TRACK_AVAILABLE] def _lookup_playlist(self, uri): playlist = Link.from_string(uri).as_playlist() self._wait_for_object_to_load(playlist) return [ SpotifyTrack(track=t) - for t in playlist if t.availability() == 1] + for t in playlist if t.availability() == TRACK_AVAILABLE] def _wait_for_object_to_load( self, spotify_obj, timeout=settings.SPOTIFY_TIMEOUT): From f0ba9dd31c615448a74a8d96d5101af8a07b5e85 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 10:48:29 +0100 Subject: [PATCH 23/26] Bump version number to 0.11.0 --- mopidy/__init__.py | 2 +- tests/version_test.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/mopidy/__init__.py b/mopidy/__init__.py index 049db682..2e5aeeba 100644 --- a/mopidy/__init__.py +++ b/mopidy/__init__.py @@ -23,7 +23,7 @@ if (isinstance(pykka.__version__, basestring) warnings.filterwarnings('ignore', 'could not open display') -__version__ = '0.10.0' +__version__ = '0.11.0' from mopidy import settings as default_settings_module diff --git a/tests/version_test.py b/tests/version_test.py index 271f004a..f353f201 100644 --- a/tests/version_test.py +++ b/tests/version_test.py @@ -32,5 +32,6 @@ class VersionTest(unittest.TestCase): self.assertLess(SV('0.7.3'), SV('0.8.0')) self.assertLess(SV('0.8.0'), SV('0.8.1')) self.assertLess(SV('0.8.1'), SV('0.9.0')) - self.assertLess(SV('0.9.0'), SV(__version__)) - self.assertLess(SV(__version__), SV('0.10.1')) + self.assertLess(SV('0.9.0'), SV('0.10.0')) + self.assertLess(SV('0.10.0'), SV(__version__)) + self.assertLess(SV(__version__), SV('0.11.1')) From e7d9a1bcdb9e5acd588a70024b111da7d9f48ac6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 10:54:50 +0100 Subject: [PATCH 24/26] docs: Update changelog for v0.11.0 --- docs/changes.rst | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 2357590d..e705444b 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -5,8 +5,13 @@ Changes This change log is used to track all major changes to Mopidy. -v0.11.0 (in development) -======================== +v0.11.0 (2012-12-24) +==================== + +In celebration of Mopidy's three year anniversary December 23, we're releasing +Mopidy 0.11. This release brings several improvements, most notably better +search which now includes matching artists and albums from Spotify in the +search results. **Settings** From 01bb71107465849f1a64371cf2a46992d491b5c6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 11:11:39 +0100 Subject: [PATCH 25/26] docs: Add v0.12 section to changelog --- docs/changes.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index e705444b..23e53be0 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -5,6 +5,12 @@ Changes This change log is used to track all major changes to Mopidy. +v0.12.0 (in development) +======================== + +(in development) + + v0.11.0 (2012-12-24) ==================== From 81b9d1116dd4096077b5cb48abded1e2fe2208ad Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 24 Dec 2012 12:13:00 +0100 Subject: [PATCH 26/26] docs: Update changelog --- docs/changes.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index 23e53be0..89707b6a 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -10,6 +10,10 @@ v0.12.0 (in development) (in development) +**Spotify** + +- Let GStreamer handle time position tracking and seeks. (Fixes: :issue:`191`) + v0.11.0 (2012-12-24) ====================