diff --git a/mopidy/backends/base/current_playlist.py b/mopidy/backends/base/current_playlist.py index c8c83a62..34a16369 100644 --- a/mopidy/backends/base/current_playlist.py +++ b/mopidy/backends/base/current_playlist.py @@ -12,13 +12,10 @@ class BaseCurrentPlaylistController(object): :type backend: :class:`BaseBackend` """ - #: The current playlist version. Integer which is increased every time the - #: current playlist is changed. Is not reset before Mopidy is restarted. - version = 0 - def __init__(self, backend): self.backend = backend self._cp_tracks = [] + self._version = 0 def destroy(self): """Cleanup after component.""" @@ -42,6 +39,19 @@ class BaseCurrentPlaylistController(object): """ return [ct[1] for ct in self._cp_tracks] + @property + def version(self): + """ + The current playlist version. Integer which is increased every time the + current playlist is changed. Is not reset before Mopidy is restarted. + """ + return self._version + + @version.setter + def version(self, version): + self._version = version + self.backend.playback.on_current_playlist_change() + def add(self, track, at_position=None): """ Add the track to the end of, or at the given position in the current @@ -71,16 +81,13 @@ class BaseCurrentPlaylistController(object): :param tracks: tracks to append :type tracks: list of :class:`mopidy.models.Track` """ - self.version += 1 for track in tracks: self.add(track) - self.backend.playback.on_current_playlist_change() def clear(self): """Clear the current playlist.""" self._cp_tracks = [] self.version += 1 - self.backend.playback.on_current_playlist_change() def get(self, **criteria): """ @@ -146,7 +153,6 @@ class BaseCurrentPlaylistController(object): to_position += 1 self._cp_tracks = new_cp_tracks self.version += 1 - self.backend.playback.on_current_playlist_change() def remove(self, **criteria): """ @@ -191,7 +197,6 @@ class BaseCurrentPlaylistController(object): random.shuffle(shuffled) self._cp_tracks = before + shuffled + after self.version += 1 - self.backend.playback.on_current_playlist_change() def mpd_format(self, *args, **kwargs): """Not a part of the generic backend API.""" diff --git a/mopidy/backends/base/playback.py b/mopidy/backends/base/playback.py index 6d888d7d..779903a7 100644 --- a/mopidy/backends/base/playback.py +++ b/mopidy/backends/base/playback.py @@ -316,8 +316,7 @@ class BasePlaybackController(object): self._trigger_stopped_playing_event() self.play(self.cp_track_at_eot) else: - self.stop() - self.current_cp_track = None + self.stop(clear_current_track=True) if self.consume: self.backend.current_playlist.remove(cpid=original_cp_track[0]) @@ -331,13 +330,10 @@ class BasePlaybackController(object): self._first_shuffle = True self._shuffled = [] - if not self.backend.current_playlist.cp_tracks: - self.stop() - self.current_cp_track = None - elif (self.current_cp_track not in + if (not self.backend.current_playlist.cp_tracks or + self.current_cp_track not in self.backend.current_playlist.cp_tracks): - self.current_cp_track = None - self.stop() + self.stop(clear_current_track=True) def next(self): """Play the next track.""" @@ -348,8 +344,7 @@ class BasePlaybackController(object): self._trigger_stopped_playing_event() self.play(self.cp_track_at_next) else: - self.stop() - self.current_cp_track = None + self.stop(clear_current_track=True) def pause(self): """Pause playback.""" @@ -475,13 +470,21 @@ class BasePlaybackController(object): """ raise NotImplementedError - def stop(self): - """Stop playing.""" + def stop(self, clear_current_track=False): + """ + Stop playing. + + :param clear_current_track: whether to clear the current track _after_ + stopping + :type clear_current_track: boolean + """ if self.state == self.STOPPED: return self._trigger_stopped_playing_event() if self._stop(): self.state = self.STOPPED + if clear_current_track: + self.current_cp_track = None def _stop(self): """ diff --git a/tests/backends/base/current_playlist.py b/tests/backends/base/current_playlist.py index 59c7b39f..05f08e18 100644 --- a/tests/backends/base/current_playlist.py +++ b/tests/backends/base/current_playlist.py @@ -128,7 +128,7 @@ class BaseCurrentPlaylistControllerTest(object): def test_append_does_not_reset_version(self): version = self.controller.version self.controller.append([]) - self.assertEqual(self.controller.version, version + 1) + self.assertEqual(self.controller.version, version) @populate_playlist def test_append_preserves_playing_state(self): @@ -249,7 +249,12 @@ class BaseCurrentPlaylistControllerTest(object): self.assertEqual(self.tracks[0], shuffled_tracks[0]) self.assertEqual(set(self.tracks), set(shuffled_tracks)) - def test_version(self): + def test_version_does_not_change_when_appending_nothing(self): version = self.controller.version self.controller.append([]) + self.assertEquals(version, self.controller.version) + + def test_version_increases_when_appending_something(self): + version = self.controller.version + self.controller.append([Track()]) self.assert_(version < self.controller.version) diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index ca4d9941..4caaf44b 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -524,7 +524,7 @@ class BasePlaybackControllerTest(object): wrapper.called = False self.playback.on_current_playlist_change = wrapper - self.backend.current_playlist.append([]) + self.backend.current_playlist.append([Track()]) self.assert_(wrapper.called) diff --git a/tests/frontends/mpd/current_playlist_test.py b/tests/frontends/mpd/current_playlist_test.py index 8e4b62f9..8a4b9ab5 100644 --- a/tests/frontends/mpd/current_playlist_test.py +++ b/tests/frontends/mpd/current_playlist_test.py @@ -135,7 +135,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_deleteid(self): self.b.current_playlist.append([Track(), Track()]) self.assertEqual(len(self.b.current_playlist.tracks), 2) - result = self.h.handle_request(u'deleteid "2"') + result = self.h.handle_request(u'deleteid "1"') self.assertEqual(len(self.b.current_playlist.tracks), 1) self.assert_(u'OK' in result) @@ -193,7 +193,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): Track(name='a'), Track(name='b'), Track(name='c'), Track(name='d'), Track(name='e'), Track(name='f'), ]) - result = self.h.handle_request(u'moveid "5" "2"') + result = self.h.handle_request(u'moveid "4" "2"') self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') self.assertEqual(self.b.current_playlist.tracks[1].name, 'b') self.assertEqual(self.b.current_playlist.tracks[2].name, 'e') @@ -229,7 +229,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): result = self.h.handle_request( u'playlistfind filename "file:///exists"') self.assert_(u'file: file:///exists' in result) - self.assert_(u'Id: 1' in result) + self.assert_(u'Id: 0' in result) self.assert_(u'Pos: 0' in result) self.assert_(u'OK' in result) @@ -242,11 +242,11 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): def test_playlistid_with_songid(self): self.b.current_playlist.append([Track(name='a'), Track(name='b')]) - result = self.h.handle_request(u'playlistid "2"') + result = self.h.handle_request(u'playlistid "1"') self.assert_(u'Title: a' not in result) - self.assert_(u'Id: 1' not in result) + self.assert_(u'Id: 0' not in result) self.assert_(u'Title: b' in result) - self.assert_(u'Id: 2' in result) + self.assert_(u'Id: 1' in result) self.assert_(u'OK' in result) def test_playlistid_with_not_existing_songid_fails(self): @@ -429,7 +429,7 @@ class CurrentPlaylistHandlerTest(unittest.TestCase): Track(name='a'), Track(name='b'), Track(name='c'), Track(name='d'), Track(name='e'), Track(name='f'), ]) - result = self.h.handle_request(u'swapid "2" "5"') + result = self.h.handle_request(u'swapid "1" "4"') self.assertEqual(self.b.current_playlist.tracks[0].name, 'a') self.assertEqual(self.b.current_playlist.tracks[1].name, 'e') self.assertEqual(self.b.current_playlist.tracks[2].name, 'c') diff --git a/tests/frontends/mpd/playback_test.py b/tests/frontends/mpd/playback_test.py index 3ba48a54..801be6d8 100644 --- a/tests/frontends/mpd/playback_test.py +++ b/tests/frontends/mpd/playback_test.py @@ -254,7 +254,7 @@ class PlaybackControlHandlerTest(unittest.TestCase): def test_playid(self): self.b.current_playlist.append([Track()]) - result = self.h.handle_request(u'playid "1"') + result = self.h.handle_request(u'playid "0"') self.assert_(u'OK' in result) self.assertEqual(self.b.playback.PLAYING, self.b.playback.state) @@ -310,7 +310,7 @@ class PlaybackControlHandlerTest(unittest.TestCase): def test_seekid(self): self.b.current_playlist.append([Track(length=40000)]) - result = self.h.handle_request(u'seekid "1" "30"') + result = self.h.handle_request(u'seekid "0" "30"') self.assert_(u'OK' in result) self.assert_(self.b.playback.time_position >= 30000) @@ -318,8 +318,8 @@ class PlaybackControlHandlerTest(unittest.TestCase): seek_track = Track(uri='2', length=40000) self.b.current_playlist.append( [Track(length=40000), seek_track]) - result = self.h.handle_request(u'seekid "2" "30"') - self.assertEqual(self.b.playback.current_cpid, 2) + result = self.h.handle_request(u'seekid "1" "30"') + self.assertEqual(self.b.playback.current_cpid, 1) self.assertEqual(self.b.playback.current_track, seek_track) def test_stop(self): diff --git a/tests/frontends/mpd/regression_test.py b/tests/frontends/mpd/regression_test.py index 29656ac4..3cfdb855 100644 --- a/tests/frontends/mpd/regression_test.py +++ b/tests/frontends/mpd/regression_test.py @@ -75,3 +75,36 @@ class IssueGH18RegressionTest(unittest.TestCase): self.assertNotEqual(cp_track_1, cp_track_2) self.assertNotEqual(cp_track_2, cp_track_3) + + +class IssueGH22RegressionTest(unittest.TestCase): + """ + The issue: http://github.com/jodal/mopidy/issues/#issue/22 + + How to reproduce: + + Play, random on, remove all tracks from the current playlist (as in + "delete" each one, not "clear"). + + Alternatively: Play, random on, remove a random track from the current + playlist, press next until it crashes. + """ + + def setUp(self): + self.backend = DummyBackend(mixer_class=DummyMixer) + self.backend.current_playlist.append([ + Track(uri='a'), Track(uri='b'), Track(uri='c'), + Track(uri='d'), Track(uri='e'), Track(uri='f')]) + self.mpd = dispatcher.MpdDispatcher(backend=self.backend) + + def test(self): + random.seed(1) + self.mpd.handle_request(u'play') + self.mpd.handle_request(u'random "1"') + self.mpd.handle_request(u'deleteid "1"') + self.mpd.handle_request(u'deleteid "2"') + self.mpd.handle_request(u'deleteid "3"') + self.mpd.handle_request(u'deleteid "4"') + self.mpd.handle_request(u'deleteid "5"') + self.mpd.handle_request(u'deleteid "6"') + self.mpd.handle_request(u'status') diff --git a/tests/frontends/mpd/status_test.py b/tests/frontends/mpd/status_test.py index fbd0ff9e..1afe6ccd 100644 --- a/tests/frontends/mpd/status_test.py +++ b/tests/frontends/mpd/status_test.py @@ -27,7 +27,7 @@ class StatusHandlerTest(unittest.TestCase): self.assert_(u'Track: 0' in result) self.assert_(u'Date: ' in result) self.assert_(u'Pos: 0' in result) - self.assert_(u'Id: 1' in result) + self.assert_(u'Id: 0' in result) self.assert_(u'OK' in result) def test_currentsong_without_song(self): @@ -166,7 +166,7 @@ class StatusHandlerTest(unittest.TestCase): self.b.playback.play() result = dict(dispatcher.status.status(self.h)) self.assert_('songid' in result) - self.assertEqual(int(result['songid']), 1) + self.assertEqual(int(result['songid']), 0) def test_status_method_when_playing_contains_time_with_no_length(self): self.b.current_playlist.append([Track(length=None)])