From 357a26d7f9c39fe83c418a634b2f86fe858bb8b9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 22 Dec 2012 01:40:19 +0100 Subject: [PATCH 1/8] 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 2/8] 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 ce318316a3d74a83a0aa9f40e9de21849afd3dc5 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 12:20:30 +0100 Subject: [PATCH 3/8] mpd: Don't restart current track before seek --- docs/changes.rst | 3 ++ mopidy/frontends/mpd/protocol/playback.py | 2 ++ tests/frontends/mpd/protocol/playback_test.py | 29 ++++++++++++++----- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 266f73f2..2dd6d940 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -58,6 +58,9 @@ v0.11.0 (in development) - Add support for search by date. +- Make ``seek`` and ``seekid`` not restart the current track before seeking in + it. + **Internal changes** *Models:* diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index 68c49ca0..b8153dc9 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -329,6 +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: playpos(context, songpos) context.core.playback.seek(int(seconds) * 1000).get() @@ -343,6 +344,7 @@ 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: playid(context, tlid) diff --git a/tests/frontends/mpd/protocol/playback_test.py b/tests/frontends/mpd/protocol/playback_test.py index 063493ec..cc49a8cd 100644 --- a/tests/frontends/mpd/protocol/playback_test.py +++ b/tests/frontends/mpd/protocol/playback_test.py @@ -371,45 +371,58 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.sendRequest('previous') self.assertInResponse('OK') - def test_seek(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) + def test_seek_in_current_track(self): + seek_track = Track(uri='dummy:a', length=40000) + self.core.tracklist.add([seek_track]) + self.core.playback.play() - self.sendRequest('seek "0"') self.sendRequest('seek "0" "30"') + + self.assertEqual(self.core.playback.current_track.get(), seek_track) self.assertGreaterEqual(self.core.playback.time_position, 30000) self.assertInResponse('OK') - def test_seek_with_songpos(self): + def test_seek_in_another_track(self): seek_track = Track(uri='dummy:b', length=40000) self.core.tracklist.add( [Track(uri='dummy:a', length=40000), seek_track]) + self.core.playback.play() + self.assertNotEqual(self.core.playback.current_track.get(), seek_track) self.sendRequest('seek "1" "30"') + self.assertEqual(self.core.playback.current_track.get(), seek_track) self.assertInResponse('OK') def test_seek_without_quotes(self): self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) + self.core.playback.play() - self.sendRequest('seek 0') self.sendRequest('seek 0 30') self.assertGreaterEqual( self.core.playback.time_position.get(), 30000) self.assertInResponse('OK') - def test_seekid(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) + def test_seekid_in_current_track(self): + seek_track = Track(uri='dummy:a', length=40000) + self.core.tracklist.add([seek_track]) + self.core.playback.play() + self.sendRequest('seekid "0" "30"') + + self.assertEqual(self.core.playback.current_track.get(), seek_track) self.assertGreaterEqual( self.core.playback.time_position.get(), 30000) self.assertInResponse('OK') - def test_seekid_with_tlid(self): + def test_seekid_in_another_track(self): seek_track = Track(uri='dummy:b', length=40000) self.core.tracklist.add( [Track(uri='dummy:a', length=40000), seek_track]) + self.core.playback.play() self.sendRequest('seekid "1" "30"') + self.assertEqual(1, self.core.playback.current_tl_track.get().tlid) self.assertEqual(seek_track, self.core.playback.current_track.get()) self.assertInResponse('OK') From 8fcc7966b2222a44a22053801ed9f50112da64fb Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 12:25:20 +0100 Subject: [PATCH 4/8] spotify: Create SpotifyTrack with uri if lookup track isn't loaded --- mopidy/backends/spotify/library.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/library.py b/mopidy/backends/spotify/library.py index 81587e00..db4c5d7e 100644 --- a/mopidy/backends/spotify/library.py +++ b/mopidy/backends/spotify/library.py @@ -82,7 +82,10 @@ class SpotifyLibraryProvider(base.BaseLibraryProvider): def _lookup_track(self, uri): track = Link.from_string(uri).as_track() self._wait_for_object_to_load(track) - return [SpotifyTrack(track=track)] + if track.is_loaded(): + return [SpotifyTrack(track=track)] + else: + return [SpotifyTrack(uri=uri)] def _lookup_album(self, uri): album = Link.from_string(uri).as_album() From 5d707e39186d60ee1db1468ecc4ba40ea45feded Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 23 Dec 2012 15:42:49 +0100 Subject: [PATCH 5/8] 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 6/8] 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 7/8] 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 8/8] 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)