diff --git a/mopidy/mpd/protocol/stored_playlists.py b/mopidy/mpd/protocol/stored_playlists.py index 721b02ab..1a05a863 100644 --- a/mopidy/mpd/protocol/stored_playlists.py +++ b/mopidy/mpd/protocol/stored_playlists.py @@ -230,8 +230,7 @@ def playlistclear(context, name): The playlist will be created if it does not exist. """ _check_playlist_name(name) - uri = context.lookup_playlist_uri_from_name(name) - playlist = uri is not None and context.core.playlists.lookup(uri).get() + playlist = _get_playlist(context, name, must_exist=False) if not playlist: playlist = context.core.playlists.create(name).get() @@ -239,7 +238,7 @@ def playlistclear(context, name): playlist = playlist.replace(tracks=[]) if context.core.playlists.save(playlist).get() is None: raise exceptions.MpdFailedToSavePlaylist( - urllib.parse.urlparse(uri).scheme) + urllib.parse.urlparse(playlist.uri).scheme) @protocol.commands.add('playlistdelete', songpos=protocol.UINT) @@ -266,7 +265,7 @@ def playlistdelete(context, name, songpos): saved_playlist = context.core.playlists.save(playlist).get() if saved_playlist is None: raise exceptions.MpdFailedToSavePlaylist( - urllib.parse.urlparse(uri).scheme) + urllib.parse.urlparse(playlist.uri).scheme) @protocol.commands.add( @@ -307,7 +306,7 @@ def playlistmove(context, name, from_pos, to_pos): saved_playlist = context.core.playlists.save(playlist).get() if saved_playlist is None: raise exceptions.MpdFailedToSavePlaylist( - urllib.parse.urlparse(uri).scheme) + urllib.parse.urlparse(playlist.uri).scheme) @protocol.commands.add('rename') diff --git a/tests/dummy_backend.py b/tests/dummy_backend.py index 465aeab6..37702bc7 100644 --- a/tests/dummy_backend.py +++ b/tests/dummy_backend.py @@ -102,11 +102,15 @@ class DummyPlaylistsProvider(backend.PlaylistsProvider): def __init__(self, backend): super(DummyPlaylistsProvider, self).__init__(backend) self._playlists = [] + self._allow_save = True def set_dummy_playlists(self, playlists): """For tests using the dummy provider through an actor proxy.""" self._playlists = playlists + def set_allow_save(self, enabled): + self._allow_save = enabled + def as_list(self): return [ Ref.playlist(uri=pl.uri, name=pl.name) for pl in self._playlists] @@ -137,6 +141,9 @@ class DummyPlaylistsProvider(backend.PlaylistsProvider): self._playlists.remove(playlist) def save(self, playlist): + if not self._allow_save: + return None + old_playlist = self.lookup(playlist.uri) if old_playlist is not None: diff --git a/tests/mpd/protocol/test_stored_playlists.py b/tests/mpd/protocol/test_stored_playlists.py index 45cd7953..cb41b574 100644 --- a/tests/mpd/protocol/test_stored_playlists.py +++ b/tests/mpd/protocol/test_stored_playlists.py @@ -266,7 +266,7 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertEqual(1, len(tracks)) self.assertEqual('dummy:a', tracks[0].uri) self.assertEqual('Track A', tracks[0].name) - self.assertEqual(5000, tracks[0].length) + self.assertEqual(5000, tracks[0].length) self.assertInResponse('OK') @@ -320,6 +320,13 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + def test_playlistclear_creates_playlist_save_fails(self): + self.backend.playlists.set_allow_save(False) + self.send_request('playlistclear "name"') + + self.assertInResponse('ACK [0@0] {playlistclear} Backend with ' + 'scheme "dummy" failed to save playlist') + def test_playlistclear_invalid_name_acks(self): self.send_request('playlistclear "foo/bar"') self.assertInResponse('ACK [2@0] {playlistclear} playlist name is ' @@ -342,6 +349,22 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertEqual( 2, len(self.backend.playlists.get_items('dummy:a1').get())) + def test_playlistdelete_save_fails(self): + tracks = [ + Track(uri='dummy:a'), + Track(uri='dummy:b'), + Track(uri='dummy:c'), + ] # len() == 3 + self.backend.playlists.set_dummy_playlists([ + Playlist( + name='name', uri='dummy:a1', tracks=tracks)]) + self.backend.playlists.set_allow_save(False) + + self.send_request('playlistdelete "name" "2"') + + self.assertInResponse('ACK [0@0] {playlistdelete} Backend with ' + 'scheme "dummy" failed to save playlist') + def test_playlistdelete_invalid_name_acks(self): self.send_request('playlistdelete "foo/bar" "0"') self.assertInResponse('ACK [2@0] {playlistdelete} playlist name is ' @@ -374,6 +397,22 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): "dummy:c", self.backend.playlists.get_items('dummy:a1').get()[0].uri) + def test_playlistmove_save_falis(self): + tracks = [ + Track(uri='dummy:a'), + Track(uri='dummy:b'), + Track(uri='dummy:c') # this one is getting moved to top + ] + self.backend.playlists.set_dummy_playlists([ + Playlist( + name='name', uri='dummy:a1', tracks=tracks)]) + self.backend.playlists.set_allow_save(False) + + self.send_request('playlistmove "name" "2" "0"') + + self.assertInResponse('ACK [0@0] {playlistmove} Backend with ' + 'scheme "dummy" failed to save playlist') + def test_playlistmove_invalid_name_acks(self): self.send_request('playlistmove "foo/bar" "0" "1"') self.assertInResponse('ACK [2@0] {playlistmove} playlist name is ' @@ -421,6 +460,17 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertIsNotNone( self.backend.playlists.lookup('dummy:new_name').get()) + def test_rename_save_fails(self): + self.backend.playlists.set_dummy_playlists([ + Playlist( + name='old_name', uri='dummy:a1', tracks=[Track(uri='b')])]) + self.backend.playlists.set_allow_save(False) + + self.send_request('rename "old_name" "new_name"') + + self.assertInResponse('ACK [0@0] {rename} Backend with ' + 'scheme "dummy" failed to save playlist') + def test_rename_unknown_playlist_acks(self): self.send_request('rename "foo" "bar"') self.assertInResponse('ACK [50@0] {rename} No such playlist') @@ -472,6 +522,13 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + def test_save_fails(self): + self.backend.playlists.set_allow_save(False) + self.send_request('save "name"') + + self.assertInResponse('ACK [0@0] {save} Backend with ' + 'scheme "dummy" failed to save playlist') + def test_save_invalid_name_acks(self): self.send_request('save "foo/bar"') self.assertInResponse('ACK [2@0] {save} playlist name is invalid: '