From 3d05f3c65f26e04fda43e57406ab761de2b896f3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 31 Oct 2012 16:37:07 +0100 Subject: [PATCH] Change stored_playlists.delete() to accepting an URI (#217) --- docs/changes.rst | 7 ++++++- mopidy/backends/base.py | 2 +- mopidy/backends/local/stored_playlists.py | 15 +++++++++------ mopidy/core/stored_playlists.py | 17 +++++++++++------ tests/backends/base/stored_playlists.py | 11 +++++++---- tests/backends/local/stored_playlists_test.py | 5 +---- tests/core/stored_playlists_test.py | 18 ++++++++++++++++++ 7 files changed, 53 insertions(+), 22 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 9c2bea62..1427c867 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -56,13 +56,18 @@ backends: dummy/mocked lower layers easier than with the old variant, where dependencies where looked up in Pykka's actor registry. -- The stored playlists part of the core API have been revised a bit: +- The stored playlists part of the core API have been revised to be more + focused around the playlist URI, and some redundant functionality have been + removed: - :attr:`mopidy.core.StoredPlaylistsController.playlists` no longer supports assignment to it. The `playlists` property on the backend layer still does, and all functionality is maintained by assigning to the playlists collections at the backend level. + - :meth:`mopidy.core.StoredPlaylistsController.delete` now accepts an URI, + and not a playlist object. + - :meth:`mopidy.core.StoredPlaylistsController.save` now returns the saved playlist. The returned playlist may differ from the saved playlist, and should thus be used instead of the playlist passed to ``save()``. diff --git a/mopidy/backends/base.py b/mopidy/backends/base.py index e4c40a92..95cd93c4 100644 --- a/mopidy/backends/base.py +++ b/mopidy/backends/base.py @@ -180,7 +180,7 @@ class BaseStoredPlaylistsProvider(object): """ raise NotImplementedError - def delete(self, playlist): + def delete(self, uri): """ See :meth:`mopidy.core.StoredPlaylistsController.delete`. diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py index 6d12dd46..139cfac8 100644 --- a/mopidy/backends/local/stored_playlists.py +++ b/mopidy/backends/local/stored_playlists.py @@ -29,15 +29,13 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): self.save(playlist) return playlist - def delete(self, playlist): - if playlist not in self._playlists: + def delete(self, uri): + playlist = self.lookup(uri) + if not playlist: return self._playlists.remove(playlist) - filename = os.path.join(self._folder, playlist.name + '.m3u') - - if os.path.exists(filename): - os.remove(filename) + self._delete_m3u(playlist) def lookup(self, uri): for playlist in self._playlists: @@ -94,3 +92,8 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): else: uri = track.uri file_handle.write(uri + '\n') + + def _delete_m3u(self, playlist): + file_path = playlist.uri[len('file://'):] + if os.path.exists(file_path): + os.remove(file_path) diff --git a/mopidy/core/stored_playlists.py b/mopidy/core/stored_playlists.py index af88d86b..16dffdb0 100644 --- a/mopidy/core/stored_playlists.py +++ b/mopidy/core/stored_playlists.py @@ -43,15 +43,20 @@ class StoredPlaylistsController(object): backend = self.backends[0] return backend.stored_playlists.create(name).get() - def delete(self, playlist): + def delete(self, uri): """ - Delete playlist. + Delete playlist identified by the URI. - :param playlist: the playlist to delete - :type playlist: :class:`mopidy.models.Playlist` + If the URI doesn't match the URI schemes handled by the current + backends, nothing happens. + + :param uri: URI of the playlist to delete + :type uri: string """ - # TODO Support multiple backends - return self.backends[0].stored_playlists.delete(playlist).get() + uri_scheme = urlparse.urlparse(uri).scheme + backend = self.backends.by_uri_scheme.get(uri_scheme, None) + if backend: + return backend.stored_playlists.delete(uri).get() def get(self, **criteria): """ diff --git a/tests/backends/base/stored_playlists.py b/tests/backends/base/stored_playlists.py index ba907b13..2b5469ac 100644 --- a/tests/backends/base/stored_playlists.py +++ b/tests/backends/base/stored_playlists.py @@ -47,12 +47,15 @@ class StoredPlaylistsControllerTest(object): self.assert_(not self.stored.playlists) def test_delete_non_existant_playlist(self): - self.stored.delete(Playlist()) + self.stored.delete('file:///unknown/playlist') - def test_delete_playlist(self): + def test_delete_playlist_removes_it_from_the_collection(self): playlist = self.stored.create('test') - self.stored.delete(playlist) - self.assert_(not self.stored.playlists) + self.assertIn(playlist, self.stored.playlists) + + self.stored.delete(playlist.uri) + + self.assertNotIn(playlist, self.stored.playlists) def test_get_without_criteria(self): test = self.stored.get diff --git a/tests/backends/local/stored_playlists_test.py b/tests/backends/local/stored_playlists_test.py index 7025c402..987c0788 100644 --- a/tests/backends/local/stored_playlists_test.py +++ b/tests/backends/local/stored_playlists_test.py @@ -39,15 +39,12 @@ class LocalStoredPlaylistsControllerTest( def test_deleted_playlist_is_removed(self): path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u') - self.assertFalse(os.path.exists(path)) playlist = self.stored.create('test') - self.assertTrue(os.path.exists(path)) - self.stored.delete(playlist) - + self.stored.delete(playlist.uri) self.assertFalse(os.path.exists(path)) def test_playlist_contents_is_written_to_disk(self): diff --git a/tests/core/stored_playlists_test.py b/tests/core/stored_playlists_test.py index 2e90416e..39914766 100644 --- a/tests/core/stored_playlists_test.py +++ b/tests/core/stored_playlists_test.py @@ -59,6 +59,24 @@ class StoredPlaylistsTest(unittest.TestCase): self.assertFalse(self.sp1.create.called) self.sp2.create.assert_called_once_with('foo') + def test_delete_selects_the_dummy1_backend(self): + self.core.stored_playlists.delete('dummy1:a') + + self.sp1.delete.assert_called_once_with('dummy1:a') + self.assertFalse(self.sp2.delete.called) + + def test_delete_selects_the_dummy2_backend(self): + self.core.stored_playlists.delete('dummy2:a') + + self.assertFalse(self.sp1.delete.called) + self.sp2.delete.assert_called_once_with('dummy2:a') + + def test_delete_with_unknown_uri_scheme_does_nothing(self): + self.core.stored_playlists.delete('unknown:a') + + self.assertFalse(self.sp1.delete.called) + self.assertFalse(self.sp2.delete.called) + def test_lookup_selects_the_dummy1_backend(self): self.core.stored_playlists.lookup('dummy1:a')