From d135066b0d274724d3723ecf0187cf10256207df Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 6 Jun 2010 22:04:50 +0200 Subject: [PATCH] addid should ACK, and not crash or be silent on error conditions --- mopidy/mpd/frontend.py | 9 ++++++--- tests/mpd/frontend_test.py | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/mopidy/mpd/frontend.py b/mopidy/mpd/frontend.py index 4b1927d5..e1942a7e 100644 --- a/mopidy/mpd/frontend.py +++ b/mopidy/mpd/frontend.py @@ -256,9 +256,12 @@ class MpdFrontend(object): if songpos is not None: songpos = int(songpos) track = self.backend.library.lookup(uri) - if track is not None: - self.backend.current_playlist.add(track, at_position=songpos) - return ('Id', track.id) + if track is None: + raise MpdAckError(u'No such song') + if songpos and songpos > self.backend.current_playlist.playlist.length: + raise MpdAckError(u'Position out of bounds') + self.backend.current_playlist.add(track, at_position=songpos) + return ('Id', track.id) @handle_pattern(r'^delete "(?P\d+):(?P\d+)*"$') def _current_playlist_delete_range(self, start, end=None): diff --git a/tests/mpd/frontend_test.py b/tests/mpd/frontend_test.py index 2ead2b1b..d62a5e08 100644 --- a/tests/mpd/frontend_test.py +++ b/tests/mpd/frontend_test.py @@ -547,6 +547,20 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): self.assert_(u'Id: 137' in result) self.assert_(u'OK' in result) + def test_addid_with_songpos_out_of_bounds_should_ack(self): + needle = Track(uri='dummy://foo', id=137) + self.b.library._library = [Track(), Track(), needle, Track()] + self.b.current_playlist.playlist = Playlist( + tracks=[Track(), Track(), Track(), Track(), Track()]) + self.assertEqual(self.b.current_playlist.playlist.length, 5) + result = self.h.handle_request(u'addid "dummy://foo" "6"') + self.assert_(u'ACK Position out of bounds' in result) + + def test_addid_with_uri_not_found_in_library_should_ack(self): + self.b.library._library = [Track(), Track(), Track()] + result = self.h.handle_request(u'addid "dummy://foo"') + self.assert_(u'ACK No such song' in result) + def test_clear(self): self.b.current_playlist.playlist = Playlist( tracks=[Track(), Track(), Track(), Track(), Track()])