From b0ba2040dfc387ec55163a5462d732d14ca00380 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 21 Dec 2012 00:28:24 +0100 Subject: [PATCH] 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: ')