From d6798ac870562094916a588f962396efa330f612 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 26 Aug 2010 19:04:08 +0200 Subject: [PATCH 1/7] Fix GH-16: 'addid ""' crashes with SpotifyError --- docs/changes.rst | 12 ++++++++++++ mopidy/backends/libspotify/library.py | 15 ++++++++++----- .../frontends/mpd/protocol/current_playlist.py | 16 ++++++++++++++-- tests/frontends/mpd/current_playlist_test.py | 10 ++++++++++ 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index e84d7aa9..d061f892 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -5,6 +5,18 @@ Changes This change log is used to track all major changes to Mopidy. +0.1.1 (in development) +====================== + +No description yet. + +**Changes** + +- MPD frontend: + + - ``add ""`` and ``addid ""`` now behaves as expected. + + 0.1.0 (2010-08-23) ================== diff --git a/mopidy/backends/libspotify/library.py b/mopidy/backends/libspotify/library.py index ffb9ee57..eb1c24d9 100644 --- a/mopidy/backends/libspotify/library.py +++ b/mopidy/backends/libspotify/library.py @@ -1,7 +1,7 @@ import logging import multiprocessing -from spotify import Link +from spotify import Link, SpotifyError from mopidy.backends.base import BaseLibraryController from mopidy.backends.libspotify import ENCODING @@ -14,10 +14,15 @@ class LibspotifyLibraryController(BaseLibraryController): return self.search(**query) def lookup(self, uri): - spotify_track = Link.from_string(uri).as_track() - # TODO Block until metadata_updated callback is called. Before that the - # track will be unloaded, unless it's already in the stored playlists. - return LibspotifyTranslator.to_mopidy_track(spotify_track) + try: + spotify_track = Link.from_string(uri).as_track() + # TODO Block until metadata_updated callback is called. Before that + # the track will be unloaded, unless it's already in the stored + # playlists. + return LibspotifyTranslator.to_mopidy_track(spotify_track) + except SpotifyError as e: + logger.warning(u'Failed to lookup: %s', uri, e) + return None def refresh(self, uri=None): pass # TODO diff --git a/mopidy/frontends/mpd/protocol/current_playlist.py b/mopidy/frontends/mpd/protocol/current_playlist.py index 90a53f5f..2f0a9f8f 100644 --- a/mopidy/frontends/mpd/protocol/current_playlist.py +++ b/mopidy/frontends/mpd/protocol/current_playlist.py @@ -11,14 +11,19 @@ def add(frontend, uri): Adds the file ``URI`` to the playlist (directories add recursively). ``URI`` can also be a single file. + + *Clarifications:* + + - ``add ""`` should add all tracks in the library to the current playlist. """ + if not uri: + return for handler_prefix in frontend.backend.uri_handlers: if uri.startswith(handler_prefix): track = frontend.backend.library.lookup(uri) if track is not None: frontend.backend.current_playlist.add(track) return - raise MpdNoExistError( u'directory or file not found', command=u'add') @@ -36,7 +41,13 @@ def addid(frontend, uri, songpos=None): addid "foo.mp3" Id: 999 OK + + *Clarifications:* + + - ``addid ""`` should return an error. """ + if not uri: + raise MpdNoExistError(u'No such song', command=u'addid') if songpos is not None: songpos = int(songpos) track = frontend.backend.library.lookup(uri) @@ -44,7 +55,8 @@ def addid(frontend, uri, songpos=None): raise MpdNoExistError(u'No such song', command=u'addid') if songpos and songpos > len(frontend.backend.current_playlist.tracks): raise MpdArgError(u'Bad song index', command=u'addid') - cp_track = frontend.backend.current_playlist.add(track, at_position=songpos) + cp_track = frontend.backend.current_playlist.add(track, + at_position=songpos) return ('Id', cp_track[0]) @handle_pattern(r'^delete "(?P\d+):(?P\d+)*"$') diff --git a/tests/frontends/mpd/current_playlist_test.py b/tests/frontends/mpd/current_playlist_test.py index c53e2b8d..8e4b62f9 100644 --- a/tests/frontends/mpd/current_playlist_test.py +++ b/tests/frontends/mpd/current_playlist_test.py @@ -33,6 +33,11 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assertEqual(result[0], u'ACK [50@0] {add} directory or file not found') + def test_add_with_empty_uri_should_add_all_known_tracks_and_ok(self): + result = self.h.handle_request(u'add ""') + # TODO check that we add all tracks (we currently don't) + self.assert_(u'OK' in result) + def test_addid_without_songpos(self): needle = Track(uri='dummy://foo') self.b.library._library = [Track(), Track(), needle, Track()] @@ -46,6 +51,11 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): in result) self.assert_(u'OK' in result) + def test_addid_with_empty_uri_does_not_lookup_and_acks(self): + self.b.library.lookup = lambda uri: self.fail("Shouldn't run") + result = self.h.handle_request(u'addid ""') + self.assertEqual(result[0], u'ACK [50@0] {addid} No such song') + def test_addid_with_songpos(self): needle = Track(uri='dummy://foo') self.b.library._library = [Track(), Track(), needle, Track()] From 4ad476e1e5442e8b4a1c70960e72789b2badc419 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 12 Sep 2010 16:38:18 +0200 Subject: [PATCH 2/7] Fix '[Errno 22] Invalid argument' caused by IPv6 socket without IPv4 support --- mopidy/frontends/mpd/server.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/frontends/mpd/server.py b/mopidy/frontends/mpd/server.py index db13e516..39a0e682 100644 --- a/mopidy/frontends/mpd/server.py +++ b/mopidy/frontends/mpd/server.py @@ -24,6 +24,9 @@ class MpdServer(asyncore.dispatcher): try: if socket.has_ipv6: self.create_socket(socket.AF_INET6, socket.SOCK_STREAM) + # Explicitly configure socket to work for both IPv4 and IPv6 + self.socket.setsockopt( + socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) else: self.create_socket(socket.AF_INET, socket.SOCK_STREAM) self.set_reuse_addr() From d975945eb063fcaa6169ccba6e710b78181b5766 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 19 Oct 2010 12:47:06 +0200 Subject: [PATCH 3/7] Update changelog with GH-18 fix --- docs/changes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index d061f892..7714408f 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -16,6 +16,9 @@ No description yet. - ``add ""`` and ``addid ""`` now behaves as expected. +- Fix wrong behavior on end of track and next after random mode has been used. + (Fixes: :issue:`18`) + 0.1.0 (2010-08-23) ================== From ada3fcd72631ab08bdd3dce265fb8fff19047cab Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 21 Oct 2010 22:01:18 +0200 Subject: [PATCH 4/7] Extend DummyPlaybackController to be able to return False on _play, _next, _previous --- mopidy/backends/dummy/__init__.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mopidy/backends/dummy/__init__.py b/mopidy/backends/dummy/__init__.py index 98257f18..a9c00631 100644 --- a/mopidy/backends/dummy/__init__.py +++ b/mopidy/backends/dummy/__init__.py @@ -44,16 +44,19 @@ class DummyLibraryController(BaseLibraryController): class DummyPlaybackController(BasePlaybackController): def _next(self, track): - return True + """Pass None as track to force failure""" + return track is not None def _pause(self): return True def _play(self, track): - return True + """Pass None as track to force failure""" + return track is not None def _previous(self, track): - return True + """Pass None as track to force failure""" + return track is not None def _resume(self): return True From e2a4aaada71c64ef1ffcb48de80ae9e3ea154bce Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 21 Oct 2010 22:01:34 +0200 Subject: [PATCH 5/7] Add regression test for GH-17 --- tests/frontends/mpd/regression_test.py | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/frontends/mpd/regression_test.py b/tests/frontends/mpd/regression_test.py index 0e8ca386..29656ac4 100644 --- a/tests/frontends/mpd/regression_test.py +++ b/tests/frontends/mpd/regression_test.py @@ -6,6 +6,40 @@ from mopidy.frontends.mpd import dispatcher from mopidy.mixers.dummy import DummyMixer from mopidy.models import Track +class IssueGH17RegressionTest(unittest.TestCase): + """ + The issue: http://github.com/jodal/mopidy/issues#issue/17 + + How to reproduce: + + - Play a playlist where one track cannot be played + - Turn on random mode + - Press next until you get to the unplayable track + """ + + def setUp(self): + self.backend = DummyBackend(mixer_class=DummyMixer) + self.backend.current_playlist.append([ + Track(uri='a'), Track(uri='b'), None, + Track(uri='d'), Track(uri='e'), Track(uri='f')]) + self.mpd = dispatcher.MpdDispatcher(backend=self.backend) + + def test(self): + random.seed(1) # Playlist order: abcfde + self.mpd.handle_request(u'play') + self.assertEquals('a', self.backend.playback.current_track.uri) + self.mpd.handle_request(u'random "1"') + self.mpd.handle_request(u'next') + self.assertEquals('b', self.backend.playback.current_track.uri) + self.mpd.handle_request(u'next') + # Should now be at track 'c', but playback fails and it skips ahead + self.assertEquals('f', self.backend.playback.current_track.uri) + self.mpd.handle_request(u'next') + self.assertEquals('d', self.backend.playback.current_track.uri) + self.mpd.handle_request(u'next') + self.assertEquals('e', self.backend.playback.current_track.uri) + + class IssueGH18RegressionTest(unittest.TestCase): """ The issue: http://github.com/jodal/mopidy/issues#issue/18 From cc4abf509794010b3cbd008fe9f81f4c10d915a5 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 21 Oct 2010 22:07:27 +0200 Subject: [PATCH 6/7] Add fix for GH-17 --- mopidy/backends/base/playback.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/backends/base/playback.py b/mopidy/backends/base/playback.py index 3a32ce07..aead26af 100644 --- a/mopidy/backends/base/playback.py +++ b/mopidy/backends/base/playback.py @@ -387,6 +387,9 @@ class BasePlaybackController(object): self.current_cp_track = cp_track self.state = self.PLAYING if not self._play(cp_track[1]): + # Track is not playable + if self.random and self._shuffled: + self._shuffled.remove(cp_track) if on_error_step == 1: self.next() elif on_error_step == -1: From c457ae644b916a45a9178a306056a72d81acd5b6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 21 Oct 2010 22:24:34 +0200 Subject: [PATCH 7/7] Update changelog --- docs/changes.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 7714408f..91ac4531 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -12,12 +12,12 @@ No description yet. **Changes** -- MPD frontend: - - - ``add ""`` and ``addid ""`` now behaves as expected. - +- MPD frontend: ``add ""`` and ``addid ""`` now behaves as expected. (Fixes + :issue:`16`) - Fix wrong behavior on end of track and next after random mode has been used. (Fixes: :issue:`18`) +- Fix infinite recursion loop crash on playback of non-playable tracks when in + random mode. (Fixes :issue:`17`) 0.1.0 (2010-08-23)